Move icon fallbacks into ExtensionAction.

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/extensions/browser_action_button.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/logging.h"
@@ skipping 32 matching lines @@
 }  // anonymous namespace
 
 // A helper class to bridge the asynchronous Skia bitmap loading mechanism to
 // the extension's button.
 class ExtensionImageTrackerBridge : public content::NotificationObserver,
                                     public ImageLoadingTracker::Observer {
  public:
   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();
     if (!path.empty()) {
       tracker_.LoadImage(extension, extension->GetResource(path),
                          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,
         content::Source<ExtensionAction>(extension->browser_action()));
   }
 
   ~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];
   }
 
   // Overridden from content::NotificationObserver.
   void Observe(int type,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) {
     if (type == chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED)
       [owner_ updateState];
     else
       NOTREACHED();
   }
 
  private:
   // Weak. Owns us.
   BrowserActionButton* owner_;
 
   // 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_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionImageTrackerBridge);
 };
 
 @interface BrowserActionCell (Internals)
 - (void)drawBadgeWithinFrame:(NSRect)frame;
 @end
 
@@ skipping 122 matching lines @@
             self, NSViewAnimationTargetKey,
             [NSValue valueWithRect:[self frame]], NSViewAnimationStartFrameKey,
             [NSValue valueWithRect:frameRect], NSViewAnimationEndFrameKey,
             nil];
     [moveAnimation_ setViewAnimations:
         [NSArray arrayWithObject:animationDictionary]];
     [moveAnimation_ startAnimation];
   }
 }
 
-- (void)setDefaultIcon:(NSImage*)image {
-  defaultIcon_.reset([image retain]);
-}
-
-- (void)setTabSpecificIcon:(NSImage*)image {
-  tabSpecificIcon_.reset([image retain]);
-}
-
 - (void)updateState {
   if (tabId_ < 0)
     return;
 
   std::string tooltip = extension_->browser_action()->GetTitle(tabId_);
   if (tooltip.empty()) {
     [self setToolTip:nil];
   } else {
     [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_];
 
   bool enabled = extension_->browser_action()->GetIsVisible(tabId_);
   [self setEnabled:enabled ? YES : NO];
 
   [self setNeedsDisplay:YES];
 }
 
@@ skipping 43 matching lines @@
 }
 
 - (void)drawInteriorWithFrame:(NSRect)cellFrame inView:(NSView*)controlView {
   gfx::ScopedNSGraphicsContextSaveGState scopedGState;
   [super drawInteriorWithFrame:cellFrame inView:controlView];
   cellFrame.origin.y += kBrowserActionBadgeOriginYOffset;
   [self drawBadgeWithinFrame:cellFrame];
 }
 
 @end