Move icon fallbacks into ExtensionAction.

chrome/browser/ui/views/browser_action_view.cc

Index: chrome/browser/ui/views/browser_action_view.cc
diff --git a/chrome/browser/ui/views/browser_action_view.cc b/chrome/browser/ui/views/browser_action_view.cc
index bd751e487752e665faef607b46221ed05c0a8e5a..98d6cda91adfb3e95672e6b64c50e557f595a24f 100644
--- a/chrome/browser/ui/views/browser_action_view.cc
+++ b/chrome/browser/ui/views/browser_action_view.cc
@@ -95,11 +95,6 @@ void BrowserActionButton::ViewHierarchyChanged(
                          gfx::Size(Extension::kBrowserActionIconMaxSize,
                                    Extension::kBrowserActionIconMaxSize),
                          ImageLoadingTracker::DONT_CACHE);
-    } else {
-      // Set the icon to be the default extensions icon.
-      default_icon_ = *ui::ResourceBundle::GetSharedInstance().GetImageNamed(
-          IDR_EXTENSIONS_FAVICON).ToSkBitmap();
-      UpdateState();

[sky, 2012/07/31 16:25:19]

Are you sure you don't still need the call to UpdateState here?

[Jeffrey Yasskin, 2012/07/31 19:53:20]

Pretty sure, but not 100%. I believe the only time the call to UpdateState()
matters is when this is called indirectly from
BrowserActionView::BrowserActionView(), which calls UpdateState() itself. Is
there some other time I've missed?

     }
 
     MaybeRegisterExtensionCommand();
@@ -153,8 +148,7 @@ void BrowserActionButton::ShowContextMenuForView(View* source,
 void BrowserActionButton::OnImageLoaded(const gfx::Image& image,
                                         const std::string& extension_id,
                                         int index) {
-  if (!image.IsEmpty())
-    default_icon_ = *image.ToSkBitmap();
+  browser_action_->CacheIcon(browser_action_->default_icon_path(), image);
 
   // Call back to UpdateState() because a more specific icon might have been set
   // while the load was outstanding.
@@ -174,9 +168,7 @@ void BrowserActionButton::UpdateState() {
              views::CustomButton::BS_NORMAL);
   }
 
-  SkBitmap icon(browser_action()->GetIcon(tab_id));
-  if (icon.isNull())
-    icon = default_icon_;
+  SkBitmap icon(*browser_action()->GetIcon(tab_id).ToSkBitmap());

[sky, 2012/07/31 16:25:19]

I believe all this code should be using ImageSkia.

[Jeffrey Yasskin, 2012/07/31 19:53:20]

I believe you're right. However, I don't think this is the right CL to do that
migration because it's primarily focused on something else.

By the way, is there a porting guide for migrating SkBitmap to gfx::Image? I
tried to avoid Image->SkBitmap->Image conversions in this CL, but I was stumped
by things like finding the analog of SkDevice. Also, is there a guideline for
picking a scale factor when building an SkCanvas out of an ImageSkia?

   if (!icon.isNull()) {
     if (!browser_action()->GetIsVisible(tab_id))
       icon = MakeTransparent(icon);
@@ -392,9 +384,8 @@ BrowserActionView::~BrowserActionView() {
 gfx::Canvas* BrowserActionView::GetIconWithBadge() {
   int tab_id = panel_->GetCurrentTabId();
 
-  SkBitmap icon = button_->extension()->browser_action()->GetIcon(tab_id);
-  if (icon.isNull())
-    icon = button_->default_icon();
+  SkBitmap icon = *button_->extension()->browser_action()->GetIcon(
+      tab_id).ToSkBitmap();
 
   // Dim the icon if our button is disabled.
   if (!button_->IsEnabled(tab_id))