Animate the script badges.

chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm

Index: chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
diff --git a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
index 778ce2e133de9981c0c348684018a0b66a264ffa..dc6439286807022a7f1a24e344661928cb8e5ede 100644
--- a/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
+++ b/chrome/browser/ui/cocoa/location_bar/page_action_decoration.mm
@@ -42,13 +42,16 @@ const CGFloat kBubblePointYOffset = 2.0;
 PageActionDecoration::PageActionDecoration(
     LocationBarViewMac* owner,
     Profile* profile,
-    ExtensionAction* page_action)
+    ExtensionAction* page_action,
+    const base::WeakPtr<extensions::LocationBarController::IconAnimation>&
+        icon_animation)
     : owner_(NULL),
       profile_(profile),
       page_action_(page_action),
       tracker_(this),
       current_tab_id_(-1),
-      preview_enabled_(false) {
+      preview_enabled_(false),
+      icon_animation_(icon_animation) {
   DCHECK(profile);
   const Extension* extension = profile->GetExtensionService()->
       GetExtensionById(page_action->extension_id(), false);
@@ -68,6 +71,9 @@ PageActionDecoration::PageActionDecoration(
                        ImageLoadingTracker::DONT_CACHE);
   }
 
+  if (icon_animation.get())
+    icon_animation->AddObserver(this);
+
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE,
       content::Source<Profile>(profile_));
 
@@ -76,7 +82,10 @@ PageActionDecoration::PageActionDecoration(
   owner_ = owner;
 }
 
-PageActionDecoration::~PageActionDecoration() {}
+PageActionDecoration::~PageActionDecoration() {
+  if (icon_animation_.get())
+    icon_animation_->RemoveObserver(this);
+}
 
 // Always |kPageActionIconMaxSize| wide.  |ImageDecoration| draws the
 // image centered.
@@ -153,11 +162,12 @@ void PageActionDecoration::OnImageLoaded(const gfx::Image& image,
       page_action_icons_[page_action_->default_icon_path()] = *bitmap;
   }
 
-  // If we have no owner, that means this class is still being constructed and
-  // we should not UpdatePageActions, since it leads to the PageActions being
-  // destroyed again and new ones recreated (causing an infinite loop).
-  if (owner_)
-    owner_->UpdatePageActions();
+  // If we have no owner, that means this class is still being constructed.
+  TabContents* tab_contents = owner_ ? owner_->GetTabContents() : NULL;
+  if (tab_contents) {
+    UpdateVisibility(tab_contents->web_contents(), current_url_);
+    owner_->RedrawDecoration(this);

[not at google - send to devlin, 2012/06/20 22:43:25]

Minor cleanup. It's unnecessary to relayout/repaint the entire page action area
since it's only the icon for this page action which needs to be updated.

There's the potential for a small cleanup here with the owner_ not existing
thing, but eh.

+  }
 }
 
 void PageActionDecoration::UpdateVisibility(WebContents* contents,
@@ -195,6 +205,8 @@ void PageActionDecoration::UpdateVisibility(WebContents* contents,
       }
     }
     if (!skia_icon.isNull()) {
+      if (icon_animation_.get())
+        skia_icon = icon_animation_->Apply(skia_icon);
       SetImage(gfx::SkBitmapToNSImage(skia_icon));
     } else if (!GetImage()) {
       // During install the action can be displayed before the icons
@@ -263,6 +275,21 @@ NSMenu* PageActionDecoration::GetMenu() {
   return menu_.get();
 }
 
+void PageActionDecoration::OnIconChanged(
+    const LocationBarController::IconAnimation& animation,
+    LocationBarController* controller) {
+  TabContents* tab_contents = owner_->GetTabContents();
+
+  // Animation notification might be for another tab.

[Yoyo Zhou, 2012/06/22 00:50:29]

How can this happen?

Anyway, if it could, since the icon's owner doesn't ever change, couldn't we
check that the owner is correct at AddObserver time? i.e. it seems like we don't
need the controller parameter to OnIconChanged.

[not at google - send to devlin, 2012/06/26 07:30:47]

Done.

This check was because there might be multiple animations in flight at once, and
when the same icons are present on tab1 and tab2 then switching between them
doesn't re-create the tabs.

However, what I had wasn't enough to support this anyway; we'd need to add code
to location_bar_view_mac.mm which goes though and adds observers, or something
similar.

I don't think this is worth it for a 100ms animation though. A fine
approximation is to just assume that there's only ever a single tab during the
lifetime (and not crash if there isn't), which is what I've simplified this to
(basically what you suggested).

+  LocationBarController* current_controller = tab_contents ?
+      tab_contents->extension_tab_helper()->location_bar_controller() : NULL;
+  if (controller != current_controller)
+    return;
+
+  UpdateVisibility(tab_contents->web_contents(), current_url_);
+  owner_->RedrawDecoration(this);
+}
+
 void PageActionDecoration::Observe(
     int type,
     const content::NotificationSource& source,