Reland r29095 (removes wrench integration for

browser actions).

Test failure was a fluke. I forgot that changing
resources always makes the first bot run fail.

TBR=mpcomplete@chromium.org
BUG=24379,24671

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

25 files changed, 265 insertions, 276 deletions

diff --git a/chrome/app/chrome_dll_resource.h b/chrome/app/chrome_dll_resource.h
index 92e7487..d83cf8a 100644
--- a/chrome/app/chrome_dll_resource.h
+++ b/chrome/app/chrome_dll_resource.h
@@ -179,6 +179,7 @@
 #define IDC_SHOW_APP_MENU               40020
 #define IDC_SHOW_PAGE_MENU              40021
 #define IDC_SHOW_EXTENSION_SHELF        40022
+#define IDC_MANAGE_EXTENSIONS           40023
 
 // Spell-check
 // Insert any additional suggestions before _LAST; these have to be consecutive.
@@ -216,11 +217,3 @@
 #define IDC_HISTORY_MENU                46000     // OSX only
 #define IDC_HISTORY_MENU_VISITED        46100     // OSX only
 #define IDC_HISTORY_MENU_CLOSED         46200     // OSX only
-
-// Extensions menu
-// Dynamic items from extensions are filled in between _FIRST and _LAST. If we
-// end up with more than 997 browser actions registered, we have other problems.
-#define IDC_SHOW_EXTENSIONS_SUBMENU     47000
-#define IDC_MANAGE_EXTENSIONS           47001
-#define IDC_BROWSER_ACTION_FIRST        47002
-#define IDC_BROWSER_ACTION_LAST         47999
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index ebc1ed7..e89a8202 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -946,9 +946,6 @@ each locale. -->
         <message name="IDS_SHOW_EXTENSIONS" desc="The show extensions menu in the app menu">
           &amp;Extensions
         </message>
-        <message name="IDS_MANAGE_EXTENSIONS" desc="The manage extensions menu item in the extensions submenu">
-          &amp;Manage extensions
-        </message>
         <message name="IDS_OPTIONS" desc="The text label of the Options menu item">
           &amp;Options
         </message>
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 60a78ce..3dcd0fb 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -137,8 +137,6 @@ Browser::Browser(Type type, Profile* profile)
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_UPDATE_DISABLED,
                  NotificationService::AllSources());
-  registrar_.Add(this, NotificationType::EXTENSION_LOADED,
-                 NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_PROCESS_CRASHED,
@@ -1469,27 +1467,6 @@ void Browser::ExecuteCommandWithDisposition(
 // Browser, CommandUpdater::CommandUpdaterDelegate implementation:
 
 void Browser::ExecuteCommand(int id) {
-  if (id >= IDC_BROWSER_ACTION_FIRST && id <= IDC_BROWSER_ACTION_LAST) {
-    ExtensionsService* service = profile_->GetExtensionsService();
-    DCHECK(service);  // No browser action command should have been created
-                      // in this window.
-
-    // Go find the browser action in question.
-    std::vector<ExtensionAction*> browser_actions =
-        service->GetBrowserActions(false);  // false means no popup actions.
-    for (size_t i = 0; i < browser_actions.size(); ++i) {
-      if (browser_actions[i]->command_id() == id) {
-        ExtensionBrowserEventRouter::GetInstance()->BrowserActionExecuted(
-            profile_, browser_actions[i]->extension_id(), this);
-        return;
-      }
-    }
-
-    // Could not find the command in question. Perhaps it went away while the
-    // menu was open? More likely, it is a bug.
-    LOG(WARNING) << "Unknown browser action executed: " << id;
-  }
-
   ExecuteCommandWithDisposition(id, CURRENT_TAB);
 }
 
@@ -2190,16 +2167,6 @@ void Browser::Observe(NotificationType type,
       break;
     }
 
-    case NotificationType::EXTENSION_LOADED: {
-      // Enable the browser action for the extension, if it has one.
-      Extension* extension = Details<Extension>(details).ptr();
-      if (extension->browser_action()) {
-        command_updater_.UpdateCommandEnabled(
-            extension->browser_action()->command_id(), true);
-      }
-      break;
-    }
-
     case NotificationType::EXTENSION_UNLOADED: {
       window()->GetLocationBar()->InvalidatePageActions();
 
@@ -2214,12 +2181,6 @@ void Browser::Observe(NotificationType type,
         }
       }
 
-      // Disable the browser action for the extension, if it has one.
-      if (extension->browser_action()) {
-        command_updater_.UpdateCommandEnabled(
-            extension->browser_action()->command_id(), false);
-      }
-
       break;
     }
 
@@ -2377,17 +2338,6 @@ void Browser::InitCommandState() {
   command_updater_.UpdateCommandEnabled(IDC_CONTROL_PANEL, true);
 #endif
 
-  // Set up any browser action commands that are installed.
-  ExtensionsService* service = profile()->GetExtensionsService();
-  if (service) {
-    std::vector<ExtensionAction*> browser_actions =
-        service->GetBrowserActions(false);  // false means no popup actions.
-    for (size_t i = 0; i < browser_actions.size(); ++i) {
-      command_updater_.UpdateCommandEnabled(browser_actions[i]->command_id(),
-                                            true);
-    }
-  }
-
   // Initialize other commands based on the window type.
   {
     bool normal_window = type() == TYPE_NORMAL;
diff --git a/chrome/browser/extensions/browser_action_apitest.cc b/chrome/browser/extensions/browser_action_apitest.cc
new file mode 100644
index 0000000..692e74e
--- /dev/null
+++ b/chrome/browser/extensions/browser_action_apitest.cc
@@ -0,0 +1,86 @@
+// Copyright (c) 2009 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.
+
+#include "chrome/browser/browser.h"
+#include "chrome/browser/browser_window.h"
+#include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/extension_browser_event_router.h"
+#include "chrome/browser/extensions/extension_tabs_module.h"
+#include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/views/browser_actions_container.h"
+#include "chrome/browser/views/toolbar_view.h"
+#include "chrome/common/extensions/extension_action.h"
+#include "chrome/test/ui_test_utils.h"
+
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, BrowserAction) {
+  StartHTTPServer();
+  ASSERT_TRUE(RunExtensionTest("browser_action")) << message_;
+
+  // Test that there is a browser action in the toolbar.
+  BrowserActionsContainer* browser_actions =
+      browser()->window()->GetBrowserWindowTesting()->GetToolbarView()->
+      browser_actions();
+  ASSERT_EQ(1, browser_actions->num_browser_actions());
+
+  // Tell the extension to update the browser action state.
+  ResultCatcher catcher;
+  ExtensionsService* service = browser()->profile()->GetExtensionsService();
+  Extension* extension = service->extensions()->at(0);
+  ui_test_utils::NavigateToURL(browser(),
+      GURL(extension->GetResourceURL("update.html")));
+  ASSERT_TRUE(catcher.GetNextResult());
+
+  // Test that we received the changes.
+  ExtensionActionState* action_state = extension->browser_action_state();
+  ASSERT_EQ("Modified", action_state->title());
+  ASSERT_EQ(1, action_state->icon_index());
+  ASSERT_EQ("badge", action_state->badge_text());
+  ASSERT_EQ(SkColorSetARGB(255, 255, 255, 255),
+            action_state->badge_background_color());
+
+  // Simulate the browser action being clicked.
+  ui_test_utils::NavigateToURL(browser(),
+      GURL("http://localhost:1337/files/extensions/test_file.txt"));
+
+  ExtensionAction* browser_action = service->GetBrowserActions(false)[0];
+  int window_id = ExtensionTabUtil::GetWindowId(browser());
+  ExtensionBrowserEventRouter::GetInstance()->BrowserActionExecuted(
+      browser()->profile(), browser_action->extension_id(), browser());
+
+  // Verify the command worked.
+  TabContents* tab = browser()->GetSelectedTabContents();
+  bool result = false;
+  ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      tab->render_view_host(), L"",
+      L"setInterval(function(){"
+      L"  if(document.body.bgColor == 'red'){"
+      L"    window.domAutomationController.send(true)}}, 100)",
+      &result);
+  ASSERT_TRUE(result);
+}
+
+
+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.
+  BrowserActionsContainer* browser_actions =
+      browser()->window()->GetBrowserWindowTesting()->GetToolbarView()->
+      browser_actions();
+  ASSERT_EQ(1, browser_actions->num_browser_actions());
+
+  // Tell the extension to update the browser action state.
+  ResultCatcher catcher;
+  ExtensionsService* service = browser()->profile()->GetExtensionsService();
+  Extension* extension = service->extensions()->at(0);
+  ui_test_utils::NavigateToURL(browser(),
+      GURL(extension->GetResourceURL("update.html")));
+  ASSERT_TRUE(catcher.GetNextResult());
+
+  // Test that we received the changes.
+  ExtensionActionState* action_state = extension->browser_action_state();
+  ASSERT_TRUE(action_state->icon());
+}
diff --git a/chrome/browser/extensions/browser_action_test.cc b/chrome/browser/extensions/browser_action_test.cc
deleted file mode 100644
index dca093c..0000000
--- a/chrome/browser/extensions/browser_action_test.cc
+++ /dev/null
@@ -1,86 +0,0 @@
-// Copyright (c) 2009 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.
-
-#include "chrome/browser/browser.h"
-#include "chrome/browser/browser_window.h"
-#include "chrome/browser/extensions/extension_browsertest.h"
-#include "chrome/browser/extensions/extensions_service.h"
-#include "chrome/browser/profile.h"
-#include "chrome/browser/tab_contents/tab_contents.h"
-#include "chrome/browser/views/browser_actions_container.h"
-#include "chrome/browser/views/toolbar_view.h"
-#include "chrome/common/extensions/extension_action.h"
-#include "chrome/test/ui_test_utils.h"
-
-#if defined(OS_LINUX)
-#include "chrome/browser/gtk/view_id_util.h"
-#endif
-
-static void CheckButtonCount(Browser* browser, const int expected_count) {
-#if defined(OS_WIN)
-  BrowserActionsContainer* browser_actions =
-      browser->window()->GetBrowserWindowTesting()->GetToolbarView()->
-      browser_actions();
-  int num_buttons = browser_actions->num_browser_actions();
-#elif defined(OS_LINUX)
-  GtkWidget* widget = ViewIDUtil::GetWidget(
-      GTK_WIDGET(browser->window()->GetNativeHandle()),
-      VIEW_ID_BROWSER_ACTION_TOOLBAR);
-  ASSERT_TRUE(widget);
-  GList* children = gtk_container_get_children(GTK_CONTAINER(widget));
-  int num_buttons = g_list_length(children);
-  g_list_free(children);
-#endif
-
-  EXPECT_EQ(expected_count, num_buttons);
-}
-
-static void TestAction(Browser* browser) {
-  // Navigate to a page we have permission to modify.
-  ui_test_utils::NavigateToURL(browser,
-      GURL("http://localhost:1337/files/extensions/test_file.txt"));
-
-  // Send the command.  Note that this only works for non-popup actions, so
-  // we specify |false|.
-  ExtensionsService* service = browser->profile()->GetExtensionsService();
-  browser->ExecuteCommand(service->GetBrowserActions(false)[0]->command_id());
-
-  // Verify the command worked.
-  TabContents* tab = browser->GetSelectedTabContents();
-  bool result = false;
-  ui_test_utils::ExecuteJavaScriptAndExtractBool(
-      tab->render_view_host(), L"",
-      L"setInterval(function(){"
-      L"  if(document.body.bgColor == 'red'){"
-      L"    window.domAutomationController.send(true)}}, 100)",
-      &result);
-  ASSERT_TRUE(result);
-}
-
-// Crashes frequently on Linux. See http://crbug.com/24802.
-IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, DISABLED_BrowserAction) {
-  StartHTTPServer();
-
-  ASSERT_TRUE(LoadExtension(
-      test_data_dir_.AppendASCII("samples")
-                    .AppendASCII("make_page_red")));
-
-  // Test that there is a browser action in the toolbar.
-  CheckButtonCount(browser(), 1);
-
-  TestAction(browser());
-}
-
-IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, BrowserActionNoIcon) {
-  StartHTTPServer();
-
-  ASSERT_TRUE(LoadExtension(
-      test_data_dir_.AppendASCII("samples")
-                    .AppendASCII("make_page_red_no_icon")));
-
-  // Test that there is a *not* a browser action in the toolbar.
-  CheckButtonCount(browser(), 0);
-
-  TestAction(browser());
-}
diff --git a/chrome/browser/extensions/extension_apitest.cc b/chrome/browser/extensions/extension_apitest.cc
index 0a8875d..65f1058 100644
--- a/chrome/browser/extensions/extension_apitest.cc
+++ b/chrome/browser/extensions/extension_apitest.cc
@@ -12,35 +12,14 @@ namespace {
 static const int kTimeoutMs = 60 * 1000;  // 1 minute
 };
 
-// Load an extension and wait for it to notify of PASSED or FAILED.
-bool ExtensionApiTest::RunExtensionTest(const char* extension_name) {
-  // Note the inner scope here. The |registrar| will fall out of scope and
-  // remove listeners *before* the call to WaitForPassFail() below.
-  {
-    LOG(INFO) << "Running ExtensionApiTest with: " << extension_name;
-    NotificationRegistrar registrar;
-    registrar.Add(this, NotificationType::EXTENSION_TEST_PASSED,
-                  NotificationService::AllSources());
-    registrar.Add(this, NotificationType::EXTENSION_TEST_FAILED,
-                  NotificationService::AllSources());
-
-    if (!LoadExtension(test_data_dir_.AppendASCII(extension_name))) {
-      message_ = "Failed to load extension.";
-      return false;
-    }
-  }
-
-  // TODO(erikkay) perhaps we shouldn't do this implicitly.
-  return WaitForPassFail();
+ExtensionApiTest::ResultCatcher::ResultCatcher() {
+  registrar_.Add(this, NotificationType::EXTENSION_TEST_PASSED,
+                 NotificationService::AllSources());
+  registrar_.Add(this, NotificationType::EXTENSION_TEST_FAILED,
+                 NotificationService::AllSources());
 }
 
-bool ExtensionApiTest::WaitForPassFail() {
-  NotificationRegistrar registrar;
-  registrar.Add(this, NotificationType::EXTENSION_TEST_PASSED,
-                NotificationService::AllSources());
-  registrar.Add(this, NotificationType::EXTENSION_TEST_FAILED,
-                NotificationService::AllSources());
-
+bool ExtensionApiTest::ResultCatcher::GetNextResult() {
   // Depending on the tests, multiple results can come in from a single call
   // to RunMessageLoop(), so we maintain a queue of results and just pull them
   // off as the test calls this, going to the run loop only when the queue is
@@ -61,14 +40,9 @@ bool ExtensionApiTest::WaitForPassFail() {
   return false;
 }
 
-void ExtensionApiTest::SetUpCommandLine(CommandLine* command_line) {
-  ExtensionBrowserTest::SetUpCommandLine(command_line);
-  test_data_dir_ = test_data_dir_.AppendASCII("api_test");
-}
-
-void ExtensionApiTest::Observe(NotificationType type,
-                               const NotificationSource& source,
-                               const NotificationDetails& details) {
+void ExtensionApiTest::ResultCatcher::Observe(
+    NotificationType type, const NotificationSource& source,
+    const NotificationDetails& details) {
   switch (type.value) {
     case NotificationType::EXTENSION_TEST_PASSED:
       std::cout << "Got EXTENSION_TEST_PASSED notification.\n";
@@ -85,6 +59,29 @@ void ExtensionApiTest::Observe(NotificationType type,
       break;
 
     default:
-      ExtensionBrowserTest::Observe(type, source, details);
+      NOTREACHED();
+  }
+}
+
+// Load an extension and wait for it to notify of PASSED or FAILED.
+bool ExtensionApiTest::RunExtensionTest(const char* extension_name) {
+  ResultCatcher catcher;
+
+  LOG(INFO) << "Running ExtensionApiTest with: " << extension_name;
+  if (!LoadExtension(test_data_dir_.AppendASCII(extension_name))) {
+    message_ = "Failed to load extension.";
+    return false;
+  }
+
+  if (!catcher.GetNextResult()) {
+    message_ = catcher.message();
+    return false;
+  } else {
+    return true;
   }
 }
+
+void ExtensionApiTest::SetUpCommandLine(CommandLine* command_line) {
+  ExtensionBrowserTest::SetUpCommandLine(command_line);
+  test_data_dir_ = test_data_dir_.AppendASCII("api_test");
+}
diff --git a/chrome/browser/extensions/extension_apitest.h b/chrome/browser/extensions/extension_apitest.h
index 466735d..01ada82 100644
--- a/chrome/browser/extensions/extension_apitest.h
+++ b/chrome/browser/extensions/extension_apitest.h
@@ -19,25 +19,42 @@
 
 class ExtensionApiTest : public ExtensionBrowserTest {
  protected:
+  // Helper class that observes tests failing or passing. Observation starts when
+  // the class is constructed. Get the next result by calling GetNextResult() and
+  // message() if GetNextResult() return false. If there are no results, this
+  // method will pump the UI message loop until one is received.
+  class ResultCatcher : public NotificationObserver {
+   public:
+    ResultCatcher();
+
+    // Pumps the UI loop until a notification is received that an API test
+    // succeeded or failed. Returns true if the test succeeded, false otherwise.
+    bool GetNextResult();
+
+    const std::string& message() { return message_; }
+
+   private:
+    virtual void Observe(NotificationType type, const NotificationSource& source,
+                         const NotificationDetails& details);
+
+    NotificationRegistrar registrar_;
+
+    // A sequential list of pass/fail notifications from the test extension(s).
+    std::deque<bool> results_;
+
+    // If it failed, what was the error message?
+    std::deque<std::string> messages_;
+    std::string message_;
+  };
+
   // Load |extension_name| and wait for pass / fail notification.
   // |extension_name| is a directory in "test/data/extensions/api_test".
   bool RunExtensionTest(const char* extension_name);
 
-  // Reset |completed_| and wait for a new pass / fail notification.
-  bool WaitForPassFail();
-
   // All extensions tested by ExtensionApiTest are in the "api_test" dir.
   virtual void SetUpCommandLine(CommandLine* command_line);
 
-  // NotificationObserver
-  void Observe(NotificationType type, const NotificationSource& source,
-               const NotificationDetails& details);
-
-  // A sequential list of pass/fail notifications from the test extension(s).
-  std::deque<bool> results_;
-
   // If it failed, what was the error message?
-  std::deque<std::string> messages_;
   std::string message_;
 };
 
diff --git a/chrome/browser/extensions/extension_browser_actions_api.cc b/chrome/browser/extensions/extension_browser_actions_api.cc
index d2cb604..e7d875e 100644
--- a/chrome/browser/extensions/extension_browser_actions_api.cc
+++ b/chrome/browser/extensions/extension_browser_actions_api.cc
@@ -42,6 +42,7 @@ bool BrowserActionSetIconFunction::RunImpl() {
     EXTENSION_FUNCTION_VALIDATE(
         static_cast<DictionaryValue*>(args_)->GetInteger(
             L"iconIndex", &icon_index));
+
     if (icon_index < 0 ||
         static_cast<size_t>(icon_index) >=
             extension->browser_action()->icon_paths().size()) {
diff --git a/chrome/browser/extensions/extension_browser_actions_api.h b/chrome/browser/extensions/extension_browser_actions_api.h
index 1becc70..8d92b8a 100755
--- a/chrome/browser/extensions/extension_browser_actions_api.h
+++ b/chrome/browser/extensions/extension_browser_actions_api.h
@@ -9,12 +9,12 @@
 
 class BrowserActionSetIconFunction : public SyncExtensionFunction {
   virtual bool RunImpl();
-  DECLARE_EXTENSION_FUNCTION_NAME("browserAction.setName")
+  DECLARE_EXTENSION_FUNCTION_NAME("browserAction.setIcon")
 };
 
 class BrowserActionSetTitleFunction : public SyncExtensionFunction {
   virtual bool RunImpl();
-  DECLARE_EXTENSION_FUNCTION_NAME("browserAction.setIcon")
+  DECLARE_EXTENSION_FUNCTION_NAME("browserAction.setTitle")
 };
 
 class BrowserActionSetBadgeTextFunction : public SyncExtensionFunction {
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index 7347876..a5f8796 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -9,6 +9,7 @@
 
 #include "app/gfx/gtk_util.h"
 #include "chrome/browser/browser.h"
+#include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/gtk/gtk_chrome_button.h"
@@ -64,8 +65,9 @@ class BrowserActionButton : public NotificationObserver,
   GtkWidget* widget() { return button_.get(); }
 
   static void OnButtonClicked(GtkWidget* widget, BrowserActionButton* action) {
-    action->browser_->ExecuteCommand(
-        action->extension_->browser_action()->command_id());
+    ExtensionBrowserEventRouter::GetInstance()->BrowserActionExecuted(
+        action->browser_->profile(), action->extension_->id(),
+        action->browser_);
   }
 
   // Called when the tooltip has changed or an image has loaded.
diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index 1c0733f..df6d905 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -133,13 +133,14 @@ BrowserActionButton::BrowserActionButton(
   // Load the images this view needs asynchronously on the file thread. We'll
   // get a call back into OnImageLoaded if the image loads successfully. If not,
   // the ImageView will have no image and will not appear in the browser chrome.
-  DCHECK(!browser_action->icon_paths().empty());
-  const std::vector<std::string>& icon_paths = browser_action->icon_paths();
-  browser_action_icons_.resize(icon_paths.size());
-  tracker_ = new ImageLoadingTracker(this, icon_paths.size());
-  for (std::vector<std::string>::const_iterator iter = icon_paths.begin();
-       iter != icon_paths.end(); ++iter) {
-    tracker_->PostLoadImageTask(extension->GetResource(*iter));
+  if (!browser_action->icon_paths().empty()) {
+    const std::vector<std::string>& icon_paths = browser_action->icon_paths();
+    browser_action_icons_.resize(icon_paths.size());
+    tracker_ = new ImageLoadingTracker(this, icon_paths.size());
+    for (std::vector<std::string>::const_iterator iter = icon_paths.begin();
+         iter != icon_paths.end(); ++iter) {
+      tracker_->PostLoadImageTask(extension->GetResource(*iter));
+    }
   }
 
   registrar_.Add(this, NotificationType::EXTENSION_BROWSER_ACTION_UPDATED,
@@ -174,9 +175,16 @@ void BrowserActionButton::OnImageLoaded(SkBitmap* image, size_t index) {
 
 void BrowserActionButton::OnStateUpdated() {
   SkBitmap* image = browser_action_state_->icon();
-  if (!image)
-    image = &browser_action_icons_[browser_action_state_->icon_index()];
-  SetIcon(*image);
+  if (!image) {
+    if (static_cast<size_t>(browser_action_state_->icon_index()) <
+        browser_action_icons_.size()) {
+      image = &browser_action_icons_[browser_action_state_->icon_index()];
+    }
+  }
+
+  if (image)
+    SetIcon(*image);
+
   SetTooltipText(ASCIIToWide(browser_action_state_->title()));
   panel_->OnBrowserActionVisibilityChanged();
   GetParent()->SchedulePaint();
@@ -414,13 +422,10 @@ void BrowserActionsContainer::RefreshBrowserActionViews() {
         browser_actions[i]->extension_id());
     DCHECK(extension);
 
-    // Only show browser actions that have an icon.
-    if (browser_actions[i]->icon_paths().size() > 0) {
-      BrowserActionView* view =
-          new BrowserActionView(browser_actions[i], extension, this);
-      browser_action_views_.push_back(view);
-      AddChildView(view);
-    }
+    BrowserActionView* view =
+        new BrowserActionView(browser_actions[i], extension, this);
+    browser_action_views_.push_back(view);
+    AddChildView(view);
   }
 }
 
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index ed8912a..072e6c8 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -1081,8 +1081,8 @@ void ToolbarView::CreateDevToolsMenuContents() {
 #endif
 
 void ToolbarView::CreateAppMenu() {
-  // We always rebuild the app menu so that we can get the current state of the
-  // extension system.
+  if (app_menu_contents_.get())
+    return;
 
   app_menu_contents_.reset(new views::SimpleMenuModel(this));
   app_menu_contents_->AddItemWithStringId(IDC_NEW_TAB, IDS_NEW_TAB);
@@ -1112,47 +1112,12 @@ void ToolbarView::CreateAppMenu() {
   app_menu_contents_->AddItemWithStringId(IDC_SHOW_DOWNLOADS,
                                           IDS_SHOW_DOWNLOADS);
 
-  // Create the extensions item or submenu.
-  // If there are any browser actions, we create an "Extensions" submenu, of
-  // which "Manage extensions" is the first entry. If there are no browser
-  // actions, we just create an "Extensions" menu item which does the same thing
-  // as "Manage extensions".
+  // Create the manage extensions menu item.
   ExtensionsService* extensions_service =
       browser_->profile()->GetExtensionsService();
   if (extensions_service && extensions_service->extensions_enabled()) {
-    // Get a count of all non-popup browser actions to decide how to layout
-    // the Extensions menu.
-    std::vector<ExtensionAction*> browser_actions =
-        browser_->profile()->GetExtensionsService()->GetBrowserActions(false);
-    if (browser_actions.size() == 0) {
-      app_menu_contents_->AddItemWithStringId(IDC_MANAGE_EXTENSIONS,
-                                              IDS_SHOW_EXTENSIONS);
-    } else {
-      extension_menu_contents_.reset(new views::SimpleMenuModel(this));
-      app_menu_contents_->AddSubMenuWithStringId(
-          IDS_SHOW_EXTENSIONS, extension_menu_contents_.get());
-
-      extension_menu_contents_->AddItemWithStringId(IDC_MANAGE_EXTENSIONS,
-                                                    IDS_MANAGE_EXTENSIONS);
-
-      // TODO(erikkay) Even though we just got the list of all browser actions,
-      // we have to enumerate the list of extensions in order to get the action
-      // state.  It seems like we should find a way to combine these.
-      const ExtensionList* extensions = extensions_service->extensions();
-      for (size_t i = 0; i < extensions->size(); ++i) {
-        Extension* extension = extensions->at(i);
-        if (!extension->browser_action()) {
-          continue;
-        } else if (extension->browser_action()->command_id() >
-                   IDC_BROWSER_ACTION_LAST) {
-          NOTREACHED() << "Too many browser actions.";
-        } else if (!extension->browser_action()->is_popup()) {
-          extension_menu_contents_->AddItem(
-              extension->browser_action()->command_id(),
-              UTF8ToUTF16(extension->browser_action_state()->title()));
-        }
-      }
-    }
+    app_menu_contents_->AddItemWithStringId(IDC_MANAGE_EXTENSIONS,
+                                            IDS_SHOW_EXTENSIONS);
   }
 
   app_menu_contents_->AddSeparator();
diff --git a/chrome/browser/views/toolbar_view.h b/chrome/browser/views/toolbar_view.h
index 514265a..97fba3b 100644
--- a/chrome/browser/views/toolbar_view.h
+++ b/chrome/browser/views/toolbar_view.h
@@ -247,7 +247,6 @@ class ToolbarView : public views::View,
   scoped_ptr<EncodingMenuModel> encoding_menu_contents_;
   scoped_ptr<views::SimpleMenuModel> devtools_menu_contents_;
   scoped_ptr<views::SimpleMenuModel> app_menu_contents_;
-  scoped_ptr<views::SimpleMenuModel> extension_menu_contents_;
 
   // TODO(beng): build these into MenuButton.
   scoped_ptr<views::Menu2> page_menu_menu_;
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index b5b206a..c084794 100755
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -58,7 +58,6 @@
       'browser/browser_init_browsertest.cc',
       'browser/crash_recovery_browsertest.cc',
       'browser/download/save_page_browsertest.cc',
-      'browser/extensions/browser_action_test.cc',
       'browser/extensions/autoupdate_interceptor.cc',
       'browser/extensions/autoupdate_interceptor.h',
       'browser/extensions/cross_origin_xhr_apitest.cc',
@@ -79,6 +78,7 @@
       'browser/ssl/ssl_browser_tests.cc',
     ],
     'browser_tests_sources_win_specific': [
+      'browser/extensions/browser_action_apitest.cc',
       'browser/extensions/extension_devtools_browsertest.cc',
       'browser/extensions/extension_devtools_browsertest.h',
       'browser/extensions/extension_devtools_browsertests.cc',
@@ -98,7 +98,6 @@
       'browser/task_manager_browsertest.cc',
     ],
     'browser_tests_sources_exclude_on_mac': [
-      'browser/extensions/browser_action_test.cc',
       'browser/extensions/cross_origin_xhr_apitest.cc',
       'browser/extensions/execute_script_apitest.cc',
       'browser/extensions/extension_apitest.cc',
diff --git a/chrome/common/extensions/extension_action.cc b/chrome/common/extensions/extension_action.cc
index dee9eed..2d3291e 100644
--- a/chrome/common/extensions/extension_action.cc
+++ b/chrome/common/extensions/extension_action.cc
@@ -2,13 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/app/chrome_dll_resource.h"
 #include "chrome/common/extensions/extension_action.h"
 
-int ExtensionAction::next_command_id_ = IDC_BROWSER_ACTION_FIRST;
-
 ExtensionAction::ExtensionAction()
-  : type_(PAGE_ACTION), command_id_(next_command_id_++) {
+  : type_(PAGE_ACTION) {
 }
 
 ExtensionAction::~ExtensionAction() {
diff --git a/chrome/common/extensions/extension_action.h b/chrome/common/extensions/extension_action.h
index 046ab63..1577224 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -25,8 +25,6 @@ class ExtensionAction {
     BROWSER_ACTION = 1,
   } ExtensionActionType;
 
-  int command_id() const { return command_id_; }
-
   std::string id() const { return id_; }
   void set_id(const std::string& id) { id_ = id; }
 
@@ -55,8 +53,6 @@ class ExtensionAction {
   bool is_popup() const { return !popup_url_.is_empty(); }
 
  private:
-  static int next_command_id_;
-
   // The id for the ExtensionAction, for example: "RssPageAction".
   // For BrowserActions this is blank.
   std::string id_;
@@ -74,10 +70,6 @@ class ExtensionAction {
   // The paths to the icons that this PageIcon can show.
   std::vector<std::string> icon_paths_;
 
-  // An integer for use with the browser's command system. These should always
-  // be in the range [IDC_BROWSER_ACTION_FIRST, IDC_BROWSER_ACTION_LAST].
-  int command_id_;
-
   // If the action has a popup, it has a URL and a height.
   GURL popup_url_;
   int popup_height_;
diff --git a/chrome/renderer/resources/extension_process_bindings.js b/chrome/renderer/resources/extension_process_bindings.js
index c0e94a1..2941644 100644
--- a/chrome/renderer/resources/extension_process_bindings.js
+++ b/chrome/renderer/resources/extension_process_bindings.js
@@ -202,12 +202,12 @@ var chrome = chrome || {};
     chrome.pageAction.onClicked = new chrome.Event(eventName);
   }
 
-  // Browser action events send {windowpId}.
-  function setupBrowserActionEvent(extensionId) {
-    var eventName = "browserAction/" + extensionId;
-    chrome.browserAction = chrome.browserAction || {};
-    chrome.browserAction.onClicked = new chrome.Event(eventName);
-  }
+  // Browser action events send {windowpId}.

+  function setupBrowserActionEvent(extensionId) {

+    var eventName = "browserAction/" + extensionId;

+    chrome.browserAction = chrome.browserAction || {};

+    chrome.browserAction.onClicked = new chrome.Event(eventName);

+  }

 
   function setupToolstripEvents(renderViewId) {
     chrome.toolstrip = chrome.toolstrip || {};
@@ -349,6 +349,7 @@ var chrome = chrome || {};
           throw new Error(
               "The imageData property must contain an ImageData object.");
         }
+
         sendCustomRequest(SetBrowserActionIcon, "browserAction.setIcon",
                           details, this.definition.parameters);
       } else {
diff --git a/chrome/test/data/extensions/api_test/browser_action/background.html b/chrome/test/data/extensions/api_test/browser_action/background.html
new file mode 100755
index 0000000..5379de0
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action/background.html
@@ -0,0 +1,12 @@
+<html>
+<head>
+<script>
+  // Called when the user clicks on the browser action.
+  chrome.browserAction.onClicked.addListener(function(windowId) {
+    chrome.tabs.executeScript(null, {code:"document.body.bgColor='red'"});
+  });
+  
+  chrome.test.notifyPass();
+</script>
+</head>
+</html>
diff --git a/chrome/test/data/extensions/api_test/browser_action/icon.png b/chrome/test/data/extensions/api_test/browser_action/icon.png
new file mode 100755
index 0000000..9a79a46
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action/icon.png
diff --git a/chrome/test/data/extensions/api_test/browser_action/icon2.png b/chrome/test/data/extensions/api_test/browser_action/icon2.png
new file mode 100755
index 0000000..9a79a46
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action/icon2.png
diff --git a/chrome/test/data/extensions/api_test/browser_action/manifest.json b/chrome/test/data/extensions/api_test/browser_action/manifest.json
new file mode 100755
index 0000000..750c5906
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action/manifest.json
@@ -0,0 +1,12 @@
+{
+  "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",
+      "icons": ["icon.png", "icon2.png"]
+  }
+}
\ No newline at end of file
diff --git a/chrome/test/data/extensions/api_test/browser_action/update.html b/chrome/test/data/extensions/api_test/browser_action/update.html
new file mode 100755
index 0000000..6d69bd3
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action/update.html
@@ -0,0 +1,14 @@
+<html>
+<head>
+<script>
+  // Test that we can change various properties of the browser action.
+  // The C++ verifies.
+  chrome.browserAction.setTitle({title: "Modified"});
+  chrome.browserAction.setIcon({iconIndex: 1});
+  chrome.browserAction.setBadgeText({text: "badge"});
+  chrome.browserAction.setBadgeBackgroundColor({color: [255,255,255,255]});
+  
+  chrome.test.notifyPass();
+</script>
+</head>
+</html>
diff --git a/chrome/test/data/extensions/api_test/browser_action_no_icon/background.html b/chrome/test/data/extensions/api_test/browser_action_no_icon/background.html
new file mode 100755
index 0000000..5379de0
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action_no_icon/background.html
@@ -0,0 +1,12 @@
+<html>
+<head>
+<script>
+  // Called when the user clicks on the browser action.
+  chrome.browserAction.onClicked.addListener(function(windowId) {
+    chrome.tabs.executeScript(null, {code:"document.body.bgColor='red'"});
+  });
+  
+  chrome.test.notifyPass();
+</script>
+</head>
+</html>
diff --git a/chrome/test/data/extensions/api_test/browser_action_no_icon/manifest.json b/chrome/test/data/extensions/api_test/browser_action_no_icon/manifest.json
new file mode 100755
index 0000000..ceffb65
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action_no_icon/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
diff --git a/chrome/test/data/extensions/api_test/browser_action_no_icon/update.html b/chrome/test/data/extensions/api_test/browser_action_no_icon/update.html
new file mode 100755
index 0000000..4aac446
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/browser_action_no_icon/update.html
@@ -0,0 +1,13 @@
+<html>
+<head>
+</head>
+<body>
+<canvas id="canvas" width="27" height="23">
+<script>
+  // Test that even if we set the icon after the extension loads, it shows up.
+  chrome.browserAction.setIcon({imageData:document.getElementById("canvas")
+      .getContext('2d').getImageData(0, 0, 16, 16)});
+  chrome.test.notifyPass();
+</script>
+</body>
+</html>