Default browser action icon to the puzzle piece.

If an extension has a browser action but does not specify an
icon, it is now set the puzzle piece icon.

*I do not have a Mac for testing, so the cocoa changes will need to be verified*

BUG=116626
TEST=BrowserActionApiTest.DynamicBrowserAction,
Make an extension with a browser action but no icon. The
icon should default to the puzzle piece.


Review URL: http://codereview.chromium.org/9665038

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127245 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 41 insertions, 21 deletions

diff --git a/chrome/browser/extensions/browser_action_apitest.cc b/chrome/browser/extensions/browser_action_apitest.cc
index 84dfb04..e42af47 100644
--- a/chrome/browser/extensions/browser_action_apitest.cc
+++ b/chrome/browser/extensions/browser_action_apitest.cc
@@ -98,7 +98,11 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
 
   // Test that there is a browser action in the toolbar.
   ASSERT_EQ(1, GetBrowserActionsBar().NumberOfBrowserActions());
-  EXPECT_FALSE(GetBrowserActionsBar().HasIcon(0));
+  EXPECT_TRUE(GetBrowserActionsBar().HasIcon(0));
+
+  // Set prev_id which holds the id of the previous image, and use it in the
+  // next test to see if the image changes.
+  uint32_t prev_id = extension->browser_action()->GetIcon(0).getGenerationID();
 
   // Tell the extension to update the icon using setIcon({imageData:...}).
   ResultCatcher catcher;
@@ -108,18 +112,15 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DynamicBrowserAction) {
 
   // Test that we received the changes.
   EXPECT_TRUE(GetBrowserActionsBar().HasIcon(0));
-
-  // Set prev_id which holds the id of the previous image, and use it in the
-  // next test to see if the image changes.
-  uint32_t prev_id = extension->browser_action()->GetIcon(0).getGenerationID();
+  EXPECT_NE(prev_id, extension->browser_action()->GetIcon(0).getGenerationID());
+  prev_id = extension->browser_action()->GetIcon(0).getGenerationID();
 
   // Tell the extension to update the icon using setIcon({path:...}).
   ui_test_utils::NavigateToURL(browser(),
       GURL(extension->GetResourceURL("update2.html")));
   ASSERT_TRUE(catcher.GetNextResult());
   EXPECT_TRUE(GetBrowserActionsBar().HasIcon(0));
-  EXPECT_TRUE(prev_id !=
-      extension->browser_action()->GetIcon(0).getGenerationID());
+  EXPECT_NE(prev_id, extension->browser_action()->GetIcon(0).getGenerationID());
 }
 
 // This test is flaky as per http://crbug.com/74557.
diff --git a/chrome/browser/ui/cocoa/extensions/browser_action_button.mm b/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
index f90ae1d..3c5617d 100644
--- a/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
+++ b/chrome/browser/ui/cocoa/extensions/browser_action_button.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -19,8 +19,10 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
+#include "grit/theme_resources.h"
 #include "skia/ext/skia_utils_mac.h"
 #import "third_party/GTM/AppKit/GTMNSAnimation+Duration.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas_skia_paint.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/rect.h"
@@ -58,6 +60,12 @@ 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];
     }
     registrar_.Add(
         this, chrome::NOTIFICATION_EXTENSION_BROWSER_ACTION_UPDATED,
diff --git a/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc
index 457df5f..c5b40e0 100644
--- a/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc
@@ -40,6 +40,7 @@
 #include "grit/ui_resources.h"
 #include "ui/base/accelerators/accelerator_gtk.h"
 #include "ui/base/gtk/gtk_compat.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas_skia_paint.h"
 #include "ui/gfx/gtk_util.h"
 #include "ui/gfx/image/image.h"
@@ -113,8 +114,6 @@ class BrowserActionButton : public content::NotificationObserver,
 
     DCHECK(extension_->browser_action());
 
-    UpdateState();
-
     // 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();
@@ -123,8 +122,16 @@ class BrowserActionButton : public content::NotificationObserver,
                          gfx::Size(Extension::kBrowserActionIconMaxSize,
                                    Extension::kBrowserActionIconMaxSize),
                          ImageLoadingTracker::DONT_CACHE);
+    } else {
+      const SkBitmap* bm =
+          ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+              IDR_EXTENSIONS_FAVICON).ToSkBitmap();
+      default_skbitmap_ = *bm;
+      default_icon_ = gfx::GdkPixbufFromSkBitmap(bm);
     }
 
+    UpdateState();
+
     signals_.Connect(button(), "button-press-event",
                      G_CALLBACK(OnButtonPress), this);
     signals_.Connect(button(), "clicked",
diff --git a/chrome/browser/ui/views/browser_actions_container.cc b/chrome/browser/ui/views/browser_actions_container.cc
index fbd7ba49..12f0d78 100644
--- a/chrome/browser/ui/views/browser_actions_container.cc
+++ b/chrome/browser/ui/views/browser_actions_container.cc
@@ -100,17 +100,21 @@ void BrowserActionButton::ViewHierarchyChanged(
     // 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 relative_path = browser_action_->default_icon_path();
-    if (relative_path.empty())
-      return;
-
-    // LoadImage is not guaranteed to be synchronous, so we might see the
-    // callback OnImageLoaded execute immediately. It (through UpdateState)
-    // expects parent() to return the owner for this button, so this
-    // function is as early as we can start this request.
-    tracker_.LoadImage(extension_, extension_->GetResource(relative_path),
-                       gfx::Size(Extension::kBrowserActionIconMaxSize,
-                                 Extension::kBrowserActionIconMaxSize),
-                       ImageLoadingTracker::DONT_CACHE);
+    if (!relative_path.empty()) {
+      // LoadImage is not guaranteed to be synchronous, so we might see the
+      // callback OnImageLoaded execute immediately. It (through UpdateState)
+      // expects parent() to return the owner for this button, so this
+      // function is as early as we can start this request.
+      tracker_.LoadImage(extension_, extension_->GetResource(relative_path),
+                         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();
+    }
 
     // Iterate through all the keybindings and see if one is assigned to the
     // browserAction.