Integrate browser actions with the wrench menu. Browser

actions always show up in a submenu of the wrench menu, and
if they have an icon, they also show up in the toolbar area.

BUG=23380,22883
TEST=Added new automated tests for the command handling, but we need to test that the menu items show up manually. To do that, run with no extension installed, you should see "extensions" in the wrench menu. Add an extension that adds a browser action, you should now see an "extensions" submenu with "manage extensions" and the browser action(s) in the submenu.

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

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

4 files changed, 35 insertions, 20 deletions

diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 8aaefa4..07fa897 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -320,29 +320,34 @@ ExtensionAction* Extension::LoadExtensionActionHelper(
   result->set_extension_id(id());
   result->set_type(action_type);
 
-  ListValue* icons;
+  ListValue* icons = NULL;
   // Read the page action |icons|.
   if (!page_action->HasKey(keys::kPageActionIcons) ||
       !page_action->GetList(keys::kPageActionIcons, &icons) ||
       icons->GetSize() == 0) {
-    *error = ExtensionErrorUtils::FormatErrorMessage(
-        errors::kInvalidPageActionIconPaths, IntToString(definition_index));
-    return NULL;
-  }
-
-  int icon_count = 0;
-  for (ListValue::const_iterator iter = icons->begin();
-       iter != icons->end(); ++iter) {
-    std::string path;
-    if (!(*iter)->GetAsString(&path) || path.empty()) {
+    // Icons are only required for page actions.
+    if (action_type == ExtensionAction::PAGE_ACTION) {
       *error = ExtensionErrorUtils::FormatErrorMessage(
-          errors::kInvalidPageActionIconPath,
-          IntToString(definition_index), IntToString(icon_count));
+          errors::kInvalidPageActionIconPaths, IntToString(definition_index));
       return NULL;
     }
+  }
 
-    result->AddIconPath(path);
-    ++icon_count;
+  int icon_count = 0;
+  if (icons) {
+    for (ListValue::const_iterator iter = icons->begin();
+         iter != icons->end(); ++iter) {
+      std::string path;
+      if (!(*iter)->GetAsString(&path) || path.empty()) {
+        *error = ExtensionErrorUtils::FormatErrorMessage(
+            errors::kInvalidPageActionIconPath,
+            IntToString(definition_index), IntToString(icon_count));
+        return NULL;
+      }
+
+      result->AddIconPath(path);
+      ++icon_count;
+    }
   }
 
   if (action_type == ExtensionAction::BROWSER_ACTION) {
diff --git a/chrome/common/extensions/extension_action.cc b/chrome/common/extensions/extension_action.cc
index 2d3291e..dee9eed 100644
--- a/chrome/common/extensions/extension_action.cc
+++ b/chrome/common/extensions/extension_action.cc
@@ -2,10 +2,13 @@
 // 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) {
+  : type_(PAGE_ACTION), command_id_(next_command_id_++) {
 }
 
 ExtensionAction::~ExtensionAction() {
diff --git a/chrome/common/extensions/extension_action.h b/chrome/common/extensions/extension_action.h
index 445f98e..53f6124 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -21,6 +21,8 @@ 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; }
 
@@ -41,6 +43,8 @@ class ExtensionAction {
   }
 
  private:
+  static int next_command_id_;
+
   // The id for the ExtensionAction, for example: "RssPageAction".
   // For BrowserActions this is blank.
   std::string id_;
@@ -57,6 +61,10 @@ 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_;
 };
 
 typedef std::map<std::string, ExtensionAction*> ExtensionActionMap;
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 43e116f..fc024ab 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -403,14 +403,13 @@ TEST(ExtensionTest, LoadPageActionHelper) {
                            errors::kInvalidPageActionIconPaths));
   error_msg = "";
 
-  // Same test (name key), but with browser action.
+  // Same test (name key), but with browser action (icons are not required for
+  // browser actions).
   copy.reset(static_cast<DictionaryValue*>(input.DeepCopy()));
   copy->Remove(keys::kPageActionIcons, NULL);
   action.reset(extension.LoadExtensionActionHelper(
       copy.get(), 0, &error_msg, ExtensionAction::BROWSER_ACTION));
-  ASSERT_TRUE(NULL == action.get());
-  ASSERT_TRUE(MatchPattern(error_msg.c_str(),
-                           errors::kInvalidPageActionIconPaths));
+  ASSERT_TRUE(NULL != action.get());
 }
 
 TEST(ExtensionTest, IdIsValid) {