Move icon fallbacks into ExtensionAction.

chrome/browser/ui/cocoa/extensions/browser_action_button.mm

Index: chrome/browser/ui/cocoa/extensions/browser_action_button.mm
diff --git a/chrome/browser/ui/cocoa/extensions/browser_action_button.mm b/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
index 0ab34116a3837320678197f9b9a3d2f60f996fcf..f1ae3b495c65b189fd012a6e1f585075f7961353 100644
--- a/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
+++ b/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
@@ -50,7 +50,8 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
   ExtensionImageTrackerBridge(BrowserActionButton* owner,
                               const Extension* extension)
       : owner_(owner),
-        tracker_(this) {
+        tracker_(this),
+        extension_(extension) {
     // The Browser Action API does not allow the default icon path to be
     // changed at runtime, so we can load this now and cache it.
     std::string path = extension->browser_action()->default_icon_path();
@@ -59,12 +60,6 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
                          gfx::Size(Extension::kBrowserActionIconMaxSize,
                                    Extension::kBrowserActionIconMaxSize),
                          ImageLoadingTracker::DONT_CACHE);
-    } else {
-      // Set the icon to be the default extensions icon.
-      SkBitmap bm = *ResourceBundle::GetSharedInstance().GetBitmapNamed(
-          IDR_EXTENSIONS_FAVICON);
-      [owner_ setDefaultIcon:gfx::SkBitmapToNSImage(bm)];
-      [owner_ updateState];

[Jeffrey Yasskin, 2012/07/25 19:45:26]

This updateState call is provided by the function that creates the
ExtensionImageTrackerBridge, and similarly for the Windows version.

     }
     registrar_.Add(
         this, chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED,
@@ -73,12 +68,16 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
 
   ~ExtensionImageTrackerBridge() {}
 
+  const ExtensionAction::PathToIconCache& loaded_icons() const {
+    return loaded_icons_;
+  }
+
   // ImageLoadingTracker::Observer implementation.
   void OnImageLoaded(const gfx::Image& image,
                      const std::string& extension_id,
                      int index) OVERRIDE {
     if (!image.IsEmpty())
-      [owner_ setDefaultIcon:image.ToNSImage()];
+      loaded_icons_[extension_->browser_action()->default_icon_path()] = image;
     [owner_ updateState];
   }
 
@@ -99,6 +98,12 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
   // Loads the button's icons for us on the file thread.
   ImageLoadingTracker tracker_;
 
+  // The extension whose images we're loading.
+  const Extension* const extension_;

[not at google - send to devlin, 2012/07/26 02:29:44]

We only ever access extension_->browser_action(), doesn't seem necessary to hold
onto the whole extension, particularly since it's weak?

[Jeffrey Yasskin, 2012/07/26 21:11:47]

Good point. Turned this into the browser action.

+
+  // Icons loaded from paths.  Currently just the browser action's default icon.
+  ExtensionAction::PathToIconCache loaded_icons_;
+
   // Used for registering to receive notifications and automatic clean up.
   content::NotificationRegistrar registrar_;
 
@@ -241,14 +246,6 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
   }
 }
 
-- (void)setDefaultIcon:(NSImage*)image {
-  defaultIcon_.reset([image retain]);
-}
-
-- (void)setTabSpecificIcon:(NSImage*)image {
-  tabSpecificIcon_.reset([image retain]);
-}
-
 - (void)updateState {
   if (tabId_ < 0)
     return;
@@ -260,12 +257,10 @@ class ExtensionImageTrackerBridge : public content::NotificationObserver,
     [self setToolTip:base::SysUTF8ToNSString(tooltip)];
   }
 
-  SkBitmap image = extension_->browser_action()->GetIcon(tabId_);
-  if (!image.isNull()) {
-    [self setTabSpecificIcon:gfx::SkBitmapToNSImage(image)];
-    [self setImage:tabSpecificIcon_];
-  } else if (defaultIcon_) {
-    [self setImage:defaultIcon_];
+  gfx::Image image = extension_->browser_action()->GetIcon(
+      tabId_, imageLoadingBridge_->loaded_icons());
+  if (!image.IsEmpty()) {
+    [self setImage:image.ToNSImage()];

[not at google - send to devlin, 2012/07/26 02:29:44]

nit: no {}

[Jeffrey Yasskin, 2012/07/26 21:11:47]

Done.

   }
 
   [[self cell] setTabId:tabId_];