Implement chrome.browserAction.setIcon({path:...});

BUG=22575

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

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

14 files changed, 245 insertions, 105 deletions

diff --git a/chrome/browser/extensions/browser_action_apitest.cc b/chrome/browser/extensions/browser_action_apitest.cc
index e5a4650..13b73d8 100644
--- a/chrome/browser/extensions/browser_action_apitest.cc
+++ b/chrome/browser/extensions/browser_action_apitest.cc
@@ -65,13 +65,15 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, BrowserAction) {
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DynamicBrowserAction) {
   ASSERT_TRUE(RunExtensionTest("browser_action_no_icon")) << message_;
 
-  // Test that there is a browser action in the toolbar.
+  // Test that there is a browser action in the toolbar and that it has no icon.
   BrowserActionsContainer* browser_actions =
       browser()->window()->GetBrowserWindowTesting()->GetToolbarView()->
       browser_actions();
   ASSERT_EQ(1, browser_actions->num_browser_actions());
+  ASSERT_TRUE(browser_actions->GetBrowserActionViewAt(0)->button()->icon()
+                  .empty());
 
-  // Tell the extension to update the browser action state.
+  // Tell the extension to update the icon using setIcon({imageData:...}).
   ResultCatcher catcher;
   ExtensionsService* service = browser()->profile()->GetExtensionsService();
   Extension* extension = service->extensions()->at(0);
@@ -80,8 +82,19 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DynamicBrowserAction) {
   ASSERT_TRUE(catcher.GetNextResult());
 
   // Test that we received the changes.
-  ExtensionActionState* action_state = extension->browser_action_state();
-  ASSERT_TRUE(action_state->icon());
+  ASSERT_FALSE(browser_actions->GetBrowserActionViewAt(0)->button()->icon()
+                   .empty());
+
+  // Tell the extension to update using setIcon({path:...});
+  ui_test_utils::NavigateToURL(browser(),
+      GURL(extension->GetResourceURL("update2.html")));
+  ASSERT_TRUE(catcher.GetNextResult());
+
+  // Test that we received the changes.
+  ASSERT_FALSE(browser_actions->GetBrowserActionViewAt(0)->button()->icon()
+                   .empty());
+
+  // TODO(aa): Would be nice here to actually compare that the pixels change.
 }
 
 IN_PROC_BROWSER_TEST_F(ExtensionApiTest, BrowserActionPopup) {
diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index 1d4f825f..52cc137 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -11,7 +11,6 @@
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/extensions/extension_tabs_module.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/view_ids.h"
 #include "chrome/browser/views/extensions/extension_popup.h"
@@ -23,7 +22,6 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkTypeface.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
-#include "views/controls/button/menu_button.h"
 #include "views/controls/button/text_button.h"
 
 // The size (both dimensions) of the buttons for page actions.
@@ -41,85 +39,10 @@ static const int kControlVertOffset = 6;
 // not enough space to fit all the browser actions in the toolbar.
 static const int kMinimumNumberOfVisibleBrowserActions = 2;
 
+
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionButton
 
-// The BrowserActionButton is a specialization of the MenuButton class.
-// It acts on a ExtensionAction, in this case a BrowserAction and handles
-// loading the image for the button asynchronously on the file thread to
-class BrowserActionButton : public views::MenuButton,
-                            public views::ButtonListener,
-                            public ImageLoadingTracker::Observer,
-                            public NotificationObserver {
- public:
-  BrowserActionButton(ExtensionAction* browser_action,
-                      Extension* extension,
-                      BrowserActionsContainer* panel);
-  ~BrowserActionButton();
-
-  const ExtensionAction& browser_action() const { return *browser_action_; }
-  ExtensionActionState* browser_action_state() { return browser_action_state_; }
-
-  // Overriden from views::View. Return a 0-inset so the icon can draw all the
-  // way to the edge of the view if it wants.
-  virtual gfx::Insets GetInsets() const;
-
-  // Overridden from views::ButtonListener:
-  virtual void ButtonPressed(views::Button* sender, const views::Event& event);
-
-  // Overridden from ImageLoadingTracker.
-  virtual void OnImageLoaded(SkBitmap* image, size_t index);
-
-  // Overridden from NotificationObserver:
-  virtual void Observe(NotificationType type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
-  // MenuButton behavior overrides.  These methods all default to TextButton
-  // behavior unless this button is a popup.  In that case, it uses MenuButton
-  // behavior.  MenuButton has the notion of a child popup being shown where the
-  // button will stay in the pushed state until the "menu" (a popup in this
-  // case) is dismissed.
-  virtual bool Activate();
-  virtual bool OnMousePressed(const views::MouseEvent& e);
-  virtual void OnMouseReleased(const views::MouseEvent& e, bool canceled);
-  virtual bool OnKeyReleased(const views::KeyEvent& e);
-  virtual void OnMouseExited(const views::MouseEvent& event);
-
-  // Does this button's action have a popup?
-  virtual bool IsPopup();
-
-  // Notifications when the popup is hidden or shown by the container.
-  virtual void PopupDidShow();
-  virtual void PopupDidHide();
-
- private:
-  // Called to update the display to match the browser action's state.
-  void OnStateUpdated();
-
-  // The browser action this view represents. The ExtensionAction is not owned
-  // by this class.
-  ExtensionAction* browser_action_;
-
-  // The state of our browser action. Not owned by this class.
-  ExtensionActionState* browser_action_state_;
-
-  // The icons representing different states for the browser action.
-  std::vector<SkBitmap> browser_action_icons_;
-
-  // The object that is waiting for the image loading to complete
-  // asynchronously. This object can potentially outlive the BrowserActionView,
-  // and takes care of deleting itself.
-  ImageLoadingTracker* tracker_;
-
-  // The browser action shelf.
-  BrowserActionsContainer* panel_;
-
-  NotificationRegistrar registrar_;
-
-  DISALLOW_COPY_AND_ASSIGN(BrowserActionButton);
-};
-
 BrowserActionButton::BrowserActionButton(
     ExtensionAction* browser_action, Extension* extension,
     BrowserActionsContainer* panel)
@@ -266,25 +189,6 @@ void BrowserActionButton::PopupDidHide() {
 
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionView
-// A single section in the browser action container. This contains the actual
-// BrowserActionButton, as well as the logic to paint the badge.
-
-class BrowserActionView : public views::View {
- public:
-  BrowserActionView(ExtensionAction* browser_action, Extension* extension,
-                    BrowserActionsContainer* panel);
-
-  BrowserActionButton* button() { return button_; }
-
- private:
-  virtual void Layout();
-
-  // Override PaintChildren so that we can paint the badge on top of children.
-  virtual void PaintChildren(gfx::Canvas* canvas);
-
-  // The button this view contains.
-  BrowserActionButton* button_;
-};
 
 BrowserActionView::BrowserActionView(ExtensionAction* browser_action,
                                      Extension* extension,
diff --git a/chrome/browser/views/browser_actions_container.h b/chrome/browser/views/browser_actions_container.h
index c849d44..e365bcd 100644
--- a/chrome/browser/views/browser_actions_container.h
+++ b/chrome/browser/views/browser_actions_container.h
@@ -8,19 +8,125 @@
 #include <vector>
 
 #include "base/task.h"
+#include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/views/browser_bubble.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
+#include "views/controls/button/menu_button.h"
 #include "views/view.h"
 
-class BrowserActionButton;
-class BrowserActionView;
+class BrowserActionsContainer;
+class Extension;
 class ExtensionAction;
+class ExtensionActionState;
 class ExtensionPopup;
 class Profile;
 class ToolbarView;
 
 ////////////////////////////////////////////////////////////////////////////////
+// BrowserActionButton
+
+// The BrowserActionButton is a specialization of the MenuButton class.
+// It acts on a ExtensionAction, in this case a BrowserAction and handles
+// loading the image for the button asynchronously on the file thread to
+class BrowserActionButton : public views::MenuButton,
+                            public views::ButtonListener,
+                            public ImageLoadingTracker::Observer,
+                            public NotificationObserver {
+ public:
+  BrowserActionButton(ExtensionAction* browser_action,
+                      Extension* extension,
+                      BrowserActionsContainer* panel);
+  ~BrowserActionButton();
+
+  const ExtensionAction& browser_action() const { return *browser_action_; }
+  ExtensionActionState* browser_action_state() { return browser_action_state_; }
+
+  // Overriden from views::View. Return a 0-inset so the icon can draw all the
+  // way to the edge of the view if it wants.
+  virtual gfx::Insets GetInsets() const;
+
+  // Overridden from views::ButtonListener:
+  virtual void ButtonPressed(views::Button* sender, const views::Event& event);
+
+  // Overridden from ImageLoadingTracker.
+  virtual void OnImageLoaded(SkBitmap* image, size_t index);
+
+  // Overridden from NotificationObserver:
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+  // MenuButton behavior overrides.  These methods all default to TextButton
+  // behavior unless this button is a popup.  In that case, it uses MenuButton
+  // behavior.  MenuButton has the notion of a child popup being shown where the
+  // button will stay in the pushed state until the "menu" (a popup in this
+  // case) is dismissed.
+  virtual bool Activate();
+  virtual bool OnMousePressed(const views::MouseEvent& e);
+  virtual void OnMouseReleased(const views::MouseEvent& e, bool canceled);
+  virtual bool OnKeyReleased(const views::KeyEvent& e);
+  virtual void OnMouseExited(const views::MouseEvent& event);
+
+  // Does this button's action have a popup?
+  virtual bool IsPopup();
+
+  // Notifications when the popup is hidden or shown by the container.
+  virtual void PopupDidShow();
+  virtual void PopupDidHide();
+
+ private:
+  // Called to update the display to match the browser action's state.
+  void OnStateUpdated();
+
+  // The browser action this view represents. The ExtensionAction is not owned
+  // by this class.
+  ExtensionAction* browser_action_;
+
+  // The state of our browser action. Not owned by this class.
+  ExtensionActionState* browser_action_state_;
+
+  // The icons representing different states for the browser action.
+  std::vector<SkBitmap> browser_action_icons_;
+
+  // The object that is waiting for the image loading to complete
+  // asynchronously. This object can potentially outlive the BrowserActionView,
+  // and takes care of deleting itself.
+  ImageLoadingTracker* tracker_;
+
+  // The browser action shelf.
+  BrowserActionsContainer* panel_;
+
+  NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserActionButton);
+};
+
+
+////////////////////////////////////////////////////////////////////////////////
+// BrowserActionView
+// A single section in the browser action container. This contains the actual
+// BrowserActionButton, as well as the logic to paint the badge.
+
+class BrowserActionView : public views::View {
+ public:
+  BrowserActionView(ExtensionAction* browser_action, Extension* extension,
+                    BrowserActionsContainer* panel);
+
+  BrowserActionButton* button() { return button_; }
+
+ private:
+  virtual void Layout();
+
+  // Override PaintChildren so that we can paint the badge on top of children.
+  virtual void PaintChildren(gfx::Canvas* canvas);
+
+  // The button this view contains.
+  BrowserActionButton* button_;
+};
+
+
+////////////////////////////////////////////////////////////////////////////////
 //
 // The BrowserActionsContainer is a container view, responsible for drawing the
 // icons that represent browser actions (extensions that add icons to the
@@ -34,8 +140,14 @@ class BrowserActionsContainer : public views::View,
   BrowserActionsContainer(Profile* profile, ToolbarView* toolbar);
   virtual ~BrowserActionsContainer();
 
+  // Get the number of browser actions being displayed.
   int num_browser_actions() { return browser_action_views_.size(); }
 
+  // Get a particular browser action view.
+  BrowserActionView* GetBrowserActionViewAt(int index) {
+    return browser_action_views_[index];
+  }
+
   // Update the views to reflect the state of the browser action icons.
   void RefreshBrowserActionViews();
 
diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json
index 53480d98..5d866b0 100755
--- a/chrome/common/extensions/api/extension_api.json
+++ b/chrome/common/extensions/api/extension_api.json
@@ -999,6 +999,11 @@
                 "minimum": 0,
                 "description": "The zero-based index into the |icons| vector specified in the manifest.",
                 "optional": true
+              },
+              "path": {
+                "type": "string",
+                "description": "Relative path to an image in the extension to show in the browser action.",
+                "optional": true
               }
             }
           }
diff --git a/chrome/common/extensions/docs/browserAction.html b/chrome/common/extensions/docs/browserAction.html
index c9ca19e..050f127 100755
--- a/chrome/common/extensions/docs/browserAction.html
+++ b/chrome/common/extensions/docs/browserAction.html
@@ -595,7 +595,7 @@
             </dl>
           </dd>
         </div>
-              </div><div jsinstance="*1">
+              </div><div jsinstance="1">
                 <div>
           <dt>
             <var>iconIndex</var>
@@ -636,6 +636,47 @@
             </dl>
           </dd>
         </div>
+              </div><div jsinstance="*2">
+                <div>
+          <dt>
+            <var>path</var>
+              <em>
+
+                <!-- TYPE -->
+                <div style="display:inline">
+                  (
+                    <span class="optional">optional</span>
+                    <span id="typeTemplate">
+                      <span style="display: none; ">
+                        <a> Type</a>
+                      </span>
+                      <span>
+                        <span style="display: none; ">
+                          array of <span><span></span></span>
+                        </span>
+                        <span>string</span>
+                      </span>
+                    </span>
+                  )
+                </div>
+
+              </em>
+          </dt>
+          <dd class="todo" style="display: none; ">
+            Undocumented.
+          </dd>
+          <dd>Relative path to an image in the extension to show in the browser action.</dd>
+
+          <!-- OBJECT PROPERTIES -->
+          <dd style="display: none; ">
+            <dl>
+              <div>
+                <div>
+                </div>
+              </div>
+            </dl>
+          </dd>
+        </div>
               </div>
             </dl>
           </dd>
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index dc81f67..ab5fee3 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -333,6 +333,7 @@ var chrome = chrome || {};
       return GetL10nMessage(message_name, placeholders);
     }
 
+    var canvas_context;
     function setIconCommon(details, name, parameters) {
       if ("iconIndex" in details) {
         sendRequest(name, [details], parameters);
@@ -350,9 +351,32 @@ var chrome = chrome || {};
               "The imageData property must contain an ImageData object.");
         }
         sendCustomRequest(SetExtensionActionIcon, name, [details], parameters);
+      } else if ("path" in details) {
+        if (!canvas_context) {
+          var canvas = document.createElement("canvas");
+          canvas.width = 19;
+          canvas.height = 19;
+          canvas_context = canvas.getContext('2d');
+        }
+
+        var img = new Image();
+        var self = this;
+        img.onerror = function() {
+          console.error("Could not load browser action icon '" + details.path +
+                        "'.");
+        }
+        img.onload = function() {
+          canvas_context.clearRect(0, 0, canvas.width, canvas.height);
+          canvas_context.drawImage(img, 0, 0, canvas.width, canvas.height);
+          delete details.path;
+          details.imageData = canvas_context.getImageData(0, 0, canvas.width,
+                                                          canvas.height);
+          sendCustomRequest(SetExtensionActionIcon, name, [details], parameters);
+        }
+        img.src = details.path;
       } else {
         throw new Error(
-            "Either the iconIndex or imageData property must be specified.");
+            "Either the path or imageData property must be specified.");
       }
     }
 
diff --git a/chrome/test/data/extensions/api_test/browser_action_no_icon/update2.html b/chrome/test/data/extensions/api_test/browser_action_no_icon/update2.html
new file mode 100755
index 0000000..d5702ff
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action_no_icon/update2.html
@@ -0,0 +1,10 @@
+<html>
+<body>
+<canvas id="canvas" width="27" height="23">
+<script>
+  // Test that setting an icon using its relative path works.
+  chrome.browserAction.setIcon({path: "icon.png"});
+  chrome.test.notifyPass();
+</script>
+</body>
+</html>
diff --git a/chrome/test/data/extensions/samples/set_icon_path/background.html b/chrome/test/data/extensions/samples/set_icon_path/background.html
new file mode 100755
index 0000000..11aea5a
--- /dev/null
+++ b/chrome/test/data/extensions/samples/set_icon_path/background.html
@@ -0,0 +1,20 @@
+<html>
+<head>
+<script>
+  var min = 1;
+  var max = 5;
+  var current = min;
+
+  function updateIcon() {
+    chrome.browserAction.setIcon({path:"icon" + current + ".png"});
+    current++;
+
+    if (current > max)
+      current = min;
+  }
+
+  chrome.browserAction.onClicked.addListener(updateIcon);
+  updateIcon();
+</script>
+</head>
+</html>
diff --git a/chrome/test/data/extensions/samples/set_icon_path/icon1.png b/chrome/test/data/extensions/samples/set_icon_path/icon1.png
new file mode 100755
index 0000000..9a79a46
--- /dev/null
+++ b/chrome/test/data/extensions/samples/set_icon_path/icon1.png
diff --git a/chrome/test/data/extensions/samples/set_icon_path/icon2.png b/chrome/test/data/extensions/samples/set_icon_path/icon2.png
new file mode 100755
index 0000000..8d3f710
--- /dev/null
+++ b/chrome/test/data/extensions/samples/set_icon_path/icon2.png
diff --git a/chrome/test/data/extensions/samples/set_icon_path/icon3.png b/chrome/test/data/extensions/samples/set_icon_path/icon3.png
new file mode 100755
index 0000000..2d9dec3
--- /dev/null
+++ b/chrome/test/data/extensions/samples/set_icon_path/icon3.png
diff --git a/chrome/test/data/extensions/samples/set_icon_path/icon4.png b/chrome/test/data/extensions/samples/set_icon_path/icon4.png
new file mode 100755
index 0000000..896fc36
--- /dev/null
+++ b/chrome/test/data/extensions/samples/set_icon_path/icon4.png
diff --git a/chrome/test/data/extensions/samples/set_icon_path/icon5.png b/chrome/test/data/extensions/samples/set_icon_path/icon5.png
new file mode 100755
index 0000000..a5afa25
--- /dev/null
+++ b/chrome/test/data/extensions/samples/set_icon_path/icon5.png
diff --git a/chrome/test/data/extensions/samples/set_icon_path/manifest.json b/chrome/test/data/extensions/samples/set_icon_path/manifest.json
new file mode 100755
index 0000000..ceffb65
--- /dev/null
+++ b/chrome/test/data/extensions/samples/set_icon_path/manifest.json
@@ -0,0 +1,11 @@
+{
+  "name": "A browser action with no icon that makes the page red",
+  "version": "1.0",
+  "background_page": "background.html",
+  "permissions": [
+    "tabs", "http://*/*"
+  ],
+  "browser_action": {
+      "name": "Make this page red"
+  }
+}
\ No newline at end of file