Extend contextMenus API to support browser/page actions

Screenshot:
http://i.imgur.com/RILqQqe.png

BUG=234425

Review URL: https://codereview.chromium.org/359493005

Cr-Commit-Position: refs/heads/master@{#288418}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288418 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/context_menus/context_menus_api_helpers.cc

@@ -9,6 +9,8 @@
 namespace extensions {
 namespace context_menus_api_helpers {
 
+const char kActionNotAllowedError[] =
+    "Only extensions are allowed to use action contexts";
 const char kCannotFindItemError[] = "Cannot find menu item with id *";
 const char kCheckedError[] =
     "Only items with type \"radio\" or \"checkbox\" can be checked";

chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h

@@ -38,6 +38,7 @@ scoped_ptr<extensions::MenuItem::Id> GetParentId(
 
 }  // namespace
 
+extern const char kActionNotAllowedError[];
 extern const char kCannotFindItemError[];
 extern const char kCheckedError[];
 extern const char kDuplicateIDError[];
@@ -89,6 +90,14 @@ MenuItem::ContextList GetContexts(const PropertyWithEnumT& property) {
         // Not available for <webview>.
         contexts.Add(extensions::MenuItem::LAUNCHER);
         break;
+      case PropertyWithEnumT::CONTEXTS_TYPE_BROWSER_ACTION:
+        // Not available for <webview>.
+        contexts.Add(extensions::MenuItem::BROWSER_ACTION);
+        break;
+      case PropertyWithEnumT::CONTEXTS_TYPE_PAGE_ACTION:
+        // Not available for <webview>.
+        contexts.Add(extensions::MenuItem::PAGE_ACTION);
+        break;
       case PropertyWithEnumT::CONTEXTS_TYPE_NONE:
         NOTREACHED();
     }
@@ -151,6 +160,15 @@ bool CreateMenuItem(const PropertyWithEnumT& create_properties,
     }
   }
 
+  if (contexts.Contains(MenuItem::BROWSER_ACTION) ||
+      contexts.Contains(MenuItem::PAGE_ACTION)) {
+    // Action items are not allowed for <webview>.
+    if (!extension->is_extension() || is_webview) {
+      *error = kActionNotAllowedError;
+      return false;
+    }
+  }
+
   // Title.
   std::string title;
   if (create_properties.title.get())

chrome/browser/extensions/context_menu_matcher.cc

@@ -14,10 +14,21 @@
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image.h"
 
+#if defined(ENABLE_EXTENSIONS)
+#include "chrome/common/extensions/api/context_menus.h"
+#endif
+
 namespace extensions {
 
 namespace {
 
+#if defined(ENABLE_EXTENSIONS)
+int action_menu_top_level_limit =
+    api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT;
+#else
+int action_menu_top_level_limit = 0;
+#endif
+
 // The range of command IDs reserved for extension's custom menus.
 // TODO(oshima): These values will be injected by embedders.
 int extensions_context_custom_first = IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST;
@@ -53,7 +64,8 @@ ContextMenuMatcher::ContextMenuMatcher(
 void ContextMenuMatcher::AppendExtensionItems(
     const MenuItem::ExtensionKey& extension_key,
     const base::string16& selection_text,
-    int* index) {
+    int* index,
+    bool is_action_menu) {
   DCHECK_GE(*index, 0);
   int max_index =
       extensions_context_custom_last - extensions_context_custom_first;
@@ -77,11 +89,18 @@ void ContextMenuMatcher::AppendExtensionItems(
 
   // Extensions (other than platform apps) are only allowed one top-level slot
   // (and it can't be a radio or checkbox item because we are going to put the
-  // extension icon next to it).
-  // If they have more than that, we automatically push them into a submenu.
-  if (extension->is_platform_app()) {
-    RecursivelyAppendExtensionItems(items, can_cross_incognito, selection_text,
-                                    menu_model_, index);
+  // extension icon next to it), unless the context menu is an an action menu.
+  // Action menus do not include the extension action, and they only include
+  // items from one extension, so they are not placed within a submenu.
+  // Otherwise, we automatically push them into a submenu if there is more than
+  // one top-level item.
+  if (extension->is_platform_app() || is_action_menu) {
+    RecursivelyAppendExtensionItems(items,
+                                    can_cross_incognito,
+                                    selection_text,
+                                    menu_model_,
+                                    index,
+                                    is_action_menu);
   } else {
     int menu_id = ConvertToExtensionsCustomCommandId(*index);
     (*index)++;
@@ -107,10 +126,15 @@ void ContextMenuMatcher::AppendExtensionItems(
       ui::SimpleMenuModel* submenu = new ui::SimpleMenuModel(delegate_);
       extension_menu_models_.push_back(submenu);
       menu_model_->AddSubMenu(menu_id, title, submenu);
-      RecursivelyAppendExtensionItems(submenu_items, can_cross_incognito,
-                                      selection_text, submenu, index);
+      RecursivelyAppendExtensionItems(submenu_items,
+                                      can_cross_incognito,
+                                      selection_text,
+                                      submenu,
+                                      index,
+                                      false);  // is_action_menu_top_level
     }
-    SetExtensionIcon(extension_key.extension_id);
+    if (!is_action_menu)
+      SetExtensionIcon(extension_key.extension_id);
   }
 }
 
@@ -215,10 +239,11 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
     bool can_cross_incognito,
     const base::string16& selection_text,
     ui::SimpleMenuModel* menu_model,
-    int* index)
-{
+    int* index,
+    bool is_action_menu_top_level) {
   MenuItem::Type last_type = MenuItem::NORMAL;
   int radio_group_id = 1;
+  int num_items = 0;
 
   for (MenuItem::List::const_iterator i = items.begin();
        i != items.end(); ++i) {
@@ -233,9 +258,15 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
     }
 
     int menu_id = ConvertToExtensionsCustomCommandId(*index);
-    (*index)++;
-    if (menu_id >= extensions_context_custom_last)
+    ++(*index);
+    ++num_items;
+    // Action context menus have a limit for top level extension items to
+    // prevent control items from being pushed off the screen, since extension
+    // items will not be placed in a submenu.
+    if (menu_id >= extensions_context_custom_last ||
+        (is_action_menu_top_level && num_items >= action_menu_top_level_limit))
       return;
+
     extension_item_map_[menu_id] = item->id();
     base::string16 title = item->TitleWithReplacement(selection_text,
                                                 kMaxExtensionItemTitleLength);
@@ -248,8 +279,12 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
         ui::SimpleMenuModel* submenu = new ui::SimpleMenuModel(delegate_);
         extension_menu_models_.push_back(submenu);
         menu_model->AddSubMenu(menu_id, title, submenu);
-        RecursivelyAppendExtensionItems(children, can_cross_incognito,
-                                        selection_text, submenu, index);
+        RecursivelyAppendExtensionItems(children,
+                                        can_cross_incognito,
+                                        selection_text,
+                                        submenu,
+                                        index,
+                                        false);  // is_action_menu_top_level
       }
     } else if (item->type() == MenuItem::CHECKBOX) {
       menu_model->AddCheckItem(menu_id, title);

chrome/browser/extensions/context_menu_matcher.h

@@ -43,10 +43,13 @@ class ContextMenuMatcher {
 
   // This is a helper function to append items for one particular extension.
   // The |index| parameter is used for assigning id's, and is incremented for
-  // each item actually added.
+  // each item actually added. |is_action_menu| is used for browser and page
+  // action context menus, in which menu items are not placed in submenus
+  // and the extension's icon is not shown.
   void AppendExtensionItems(const MenuItem::ExtensionKey& extension_key,
                             const base::string16& selection_text,
-                            int* index);
+                            int* index,
+                            bool is_action_menu);
 
   void Clear();
 
@@ -80,7 +83,8 @@ class ContextMenuMatcher {
                                        bool can_cross_incognito,
                                        const base::string16& selection_text,
                                        ui::SimpleMenuModel* menu_model,
-                                       int* index);
+                                       int* index,
+                                       bool is_action_menu_top_level);
 
   // Attempts to get an MenuItem given the id of a context menu item.
   extensions::MenuItem* GetExtensionMenuItem(int id) const;

chrome/browser/extensions/extension_context_menu_model.cc

@@ -6,11 +6,14 @@
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
+#include "chrome/browser/extensions/context_menu_matcher.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/extensions/menu_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -21,7 +24,9 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/context_menu_params.h"
 #include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/management_policy.h"
 #include "extensions/browser/uninstall_reason.h"
@@ -34,6 +39,32 @@ using content::OpenURLParams;
 using content::Referrer;
 using content::WebContents;
 using extensions::Extension;
+using extensions::MenuItem;
+using extensions::MenuManager;
+
+namespace {
+
+// Returns true if the given |item| is of the given |type|.
+bool MenuItemMatchesAction(ExtensionContextMenuModel::ActionType type,
+                           const MenuItem* item) {
+  if (type == ExtensionContextMenuModel::NO_ACTION)
+    return false;
+
+  const MenuItem::ContextList& contexts = item->contexts();
+
+  if (contexts.Contains(MenuItem::ALL))
+    return true;
+  if (contexts.Contains(MenuItem::PAGE_ACTION) &&
+      (type == ExtensionContextMenuModel::PAGE_ACTION))
+    return true;
+  if (contexts.Contains(MenuItem::BROWSER_ACTION) &&
+      (type == ExtensionContextMenuModel::BROWSER_ACTION))
+    return true;
+
+  return false;
+}
+
+}  // namespace
 
 ExtensionContextMenuModel::ExtensionContextMenuModel(const Extension* extension,
                                                      Browser* browser,
@@ -42,7 +73,9 @@ ExtensionContextMenuModel::ExtensionContextMenuModel(const Extension* extension,
       extension_id_(extension->id()),
       browser_(browser),
       profile_(browser->profile()),
-      delegate_(delegate) {
+      delegate_(delegate),
+      action_type_(NO_ACTION),
+      extension_items_count_(0) {
   InitMenu(extension);
 
   if (profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode) &&
@@ -58,20 +91,28 @@ ExtensionContextMenuModel::ExtensionContextMenuModel(const Extension* extension,
       extension_id_(extension->id()),
       browser_(browser),
       profile_(browser->profile()),
-      delegate_(NULL) {
+      delegate_(NULL),
+      action_type_(NO_ACTION),
+      extension_items_count_(0) {
   InitMenu(extension);
 }
 
 bool ExtensionContextMenuModel::IsCommandIdChecked(int command_id) const {
+  if (command_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
+      command_id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST)
+    return extension_items_->IsCommandIdChecked(command_id);
   return false;
 }
 
 bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const {
-  const Extension* extension = this->GetExtension();
+  const Extension* extension = GetExtension();
   if (!extension)
     return false;
 
-  if (command_id == CONFIGURE) {
+  if (command_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
+      command_id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST) {
+    return extension_items_->IsCommandIdEnabled(command_id);
+  } else if (command_id == CONFIGURE) {
     return
         extensions::ManifestURL::GetOptionsPage(extension).spec().length() > 0;
   } else if (command_id == NAME) {
@@ -105,6 +146,16 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
   if (!extension)
     return;
 
+  if (command_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
+      command_id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST) {
+    WebContents* web_contents =
+        browser_->tab_strip_model()->GetActiveWebContents();
+    DCHECK(extension_items_);
+    extension_items_->ExecuteCommand(
+        command_id, web_contents, content::ContextMenuParams());
+    return;
+  }
+
   switch (command_id) {
     case NAME: {
       OpenURLParams params(extensions::ManifestURL::GetHomepageURL(extension),
@@ -168,14 +219,26 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension) {
   extensions::ExtensionActionManager* extension_action_manager =
       extensions::ExtensionActionManager::Get(profile_);
   extension_action_ = extension_action_manager->GetBrowserAction(*extension);
-  if (!extension_action_)
+  if (!extension_action_) {
     extension_action_ = extension_action_manager->GetPageAction(*extension);
+    if (extension_action_)
+      action_type_ = PAGE_ACTION;
+  } else {
+    action_type_ = BROWSER_ACTION;
+  }
+
+  extension_items_.reset(new extensions::ContextMenuMatcher(
+      profile_,
+      this,
+      this,
+      base::Bind(MenuItemMatchesAction, action_type_)));
 
   std::string extension_name = extension->name();
   // Ampersands need to be escaped to avoid being treated like
   // mnemonics in the menu.
   base::ReplaceChars(extension_name, "&", "&&", &extension_name);
   AddItem(NAME, base::UTF8ToUTF16(extension_name));
+  AppendExtensionItems();
   AddSeparator(ui::NORMAL_SEPARATOR);
   AddItemWithStringId(CONFIGURE, IDS_EXTENSIONS_OPTIONS_MENU_ITEM);
   AddItem(UNINSTALL, l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNINSTALL));
@@ -186,7 +249,24 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension) {
 }
 
 const Extension* ExtensionContextMenuModel::GetExtension() const {
-  ExtensionService* extension_service =
-      extensions::ExtensionSystem::Get(profile_)->extension_service();
-  return extension_service->GetExtensionById(extension_id_, false);
+  return extensions::ExtensionRegistry::Get(profile_)
+      ->enabled_extensions()
+      .GetByID(extension_id_);
+}
+
+void ExtensionContextMenuModel::AppendExtensionItems() {
+  extension_items_->Clear();
+
+  MenuManager* menu_manager = MenuManager::Get(profile_);
+  if (!menu_manager ||
+      !menu_manager->MenuItems(MenuItem::ExtensionKey(extension_id_)))
+    return;
+
+  AddSeparator(ui::NORMAL_SEPARATOR);
+
+  extension_items_count_ = 0;
+  extension_items_->AppendExtensionItems(MenuItem::ExtensionKey(extension_id_),
+                                         base::string16(),
+                                         &extension_items_count_,
+                                         true);  // is_action_menu
 }

chrome/browser/extensions/extension_context_menu_model.h

@@ -17,6 +17,8 @@ class Profile;
 
 namespace extensions {
 class Extension;
+class ContextMenuMatcher;
+class ExtensionContextMenuModelTest;
 }
 
 // The context menu model for extension icons.
@@ -35,6 +37,9 @@ class ExtensionContextMenuModel
     INSPECT_POPUP
   };
 
+  // Type of action the extension icon represents.
+  enum ActionType { NO_ACTION = 0, BROWSER_ACTION, PAGE_ACTION };
+
   // Delegate to handle showing an ExtensionAction popup.
   class PopupDelegate {
    public:
@@ -74,6 +79,8 @@ class ExtensionContextMenuModel
 
  private:
   friend class base::RefCounted<ExtensionContextMenuModel>;
+  friend class extensions::ExtensionContextMenuModelTest;
+
   virtual ~ExtensionContextMenuModel();
 
   void InitMenu(const extensions::Extension* extension);
@@ -82,6 +89,9 @@ class ExtensionContextMenuModel
   // extension has been uninstalled and no longer exists.
   const extensions::Extension* GetExtension() const;
 
+  // Appends the extension's context menu items.
+  void AppendExtensionItems();
+
   // A copy of the extension's id.
   std::string extension_id_;
 
@@ -96,9 +106,18 @@ class ExtensionContextMenuModel
   // The delegate which handles the 'inspect popup' menu command (or NULL).
   PopupDelegate* delegate_;
 
+  // The type of extension action to which this context menu is attached.
+  ActionType action_type_;
+
   // Keeps track of the extension uninstall dialog.
   scoped_ptr<extensions::ExtensionUninstallDialog> extension_uninstall_dialog_;
 
+  // Menu matcher for context menu items specified by the extension.
+  scoped_ptr<extensions::ContextMenuMatcher> extension_items_;
+
+  // Number of extension items in this menu. Used for testing.
+  int extension_items_count_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionContextMenuModel);
 };
 

chrome/browser/extensions/extension_context_menu_model_unittest.cc

@@ -6,8 +6,11 @@
 
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_service_test_base.h"
+#include "chrome/browser/extensions/menu_manager.h"
+#include "chrome/browser/extensions/menu_manager_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/host_desktop.h"
+#include "chrome/common/extensions/api/context_menus.h"
 #include "chrome/test/base/test_browser_window.h"
 #include "chrome/test/base/testing_profile.h"
 #include "extensions/browser/extension_system.h"
@@ -17,25 +20,83 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace extensions {
-namespace {
 
 class ExtensionContextMenuModelTest : public ExtensionServiceTestBase {
+ public:
+  ExtensionContextMenuModelTest();
+
+  // Build an extension to pass to the menu constructor.  It needs an
+  // ExtensionAction.
+  scoped_refptr<Extension> BuildExtension();
+
+  // Creates an extension menu item for |extension| with the given |context|
+  // and adds it to |manager|. Refreshes |model| to show new item.
+  void AddContextItemAndRefreshModel(MenuManager* manager,
+                                     Extension* extension,
+                                     MenuItem::Context context,
+                                     ExtensionContextMenuModel* model);
+
+  // Reinitializes the given |model|.
+  void RefreshMenu(ExtensionContextMenuModel* model);
+
+  // Returns the number of extension menu items that show up in |model|.
+  int CountExtensionItems(ExtensionContextMenuModel* model);
+
+ private:
+  int cur_id_;
 };
 
+ExtensionContextMenuModelTest::ExtensionContextMenuModelTest()
+    : cur_id_(0) {}
+
+scoped_refptr<Extension> ExtensionContextMenuModelTest::BuildExtension() {
+  return ExtensionBuilder()
+      .SetManifest(
+           DictionaryBuilder()
+               .Set("name", "Page Action Extension")
+               .Set("version", "1")
+               .Set("manifest_version", 2)
+               .Set("page_action",
+                    DictionaryBuilder().Set("default_title", "Hello")))
+      .Build();
+}
+
+void ExtensionContextMenuModelTest::AddContextItemAndRefreshModel(
+    MenuManager* manager,
+    Extension* extension,
+    MenuItem::Context context,
+    ExtensionContextMenuModel* model) {
+  MenuItem::Type type = MenuItem::NORMAL;
+  MenuItem::ContextList contexts(context);
+  const MenuItem::ExtensionKey key(extension->id());
+  MenuItem::Id id(false, key);
+  id.uid = ++cur_id_;
+  manager->AddContextItem(extension, new MenuItem(id,
+                                                  "test",
+                                                  false,  // checked
+                                                  true,  // enabled
+                                                  type,
+                                                  contexts));
+  RefreshMenu(model);
+}
+
+void ExtensionContextMenuModelTest::RefreshMenu(
+    ExtensionContextMenuModel* model) {
+  model->InitMenu(model->GetExtension());
+}
+
+int ExtensionContextMenuModelTest::CountExtensionItems(
+    ExtensionContextMenuModel* model) {
+  return model->extension_items_count_;
+}
+
+namespace {
+
 // Tests that applicable menu items are disabled when a ManagementPolicy
 // prohibits them.
 TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) {
   InitializeEmptyExtensionService();
-  // Build an extension to pass to the menu constructor.  It needs an
-  // ExtensionAction.
-  scoped_refptr<Extension> extension = ExtensionBuilder()
-      .SetManifest(DictionaryBuilder()
-                   .Set("name", "Page Action Extension")
-                   .Set("version", "1")
-                   .Set("manifest_version", 2)
-                   .Set("page_action", DictionaryBuilder()
-                        .Set("default_title", "Hello")))
-      .Build();
+  scoped_refptr<Extension> extension = BuildExtension();
   ASSERT_TRUE(extension.get());
   service_->AddExtension(extension.get());
 
@@ -46,7 +107,7 @@ TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) {
   Browser browser(params);
 
   scoped_refptr<ExtensionContextMenuModel> menu(
-      new ExtensionContextMenuModel(extension.get(), &browser, NULL));
+      new ExtensionContextMenuModel(extension.get(), &browser));
 
   extensions::ExtensionSystem* system =
       extensions::ExtensionSystem::Get(profile_.get());
@@ -66,5 +127,64 @@ TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) {
   system->management_policy()->UnregisterAllProviders();
 }
 
+TEST_F(ExtensionContextMenuModelTest, ExtensionItemTest) {
+  InitializeEmptyExtensionService();
+  scoped_refptr<Extension> extension = BuildExtension();
+  ASSERT_TRUE(extension.get());
+  service_->AddExtension(extension.get());
+
+  // Create a Browser for the ExtensionContextMenuModel to use.
+  Browser::CreateParams params(profile_.get(), chrome::GetActiveDesktop());
+  TestBrowserWindow test_window;
+  params.window = &test_window;
+  Browser browser(params);
+
+  // Create a MenuManager for adding context items.
+  MenuManager* manager = static_cast<MenuManager*>(
+      (MenuManagerFactory::GetInstance()->SetTestingFactoryAndUse(
+      profile_.get(), &MenuManagerFactory::BuildServiceInstanceForTesting)));
+  ASSERT_TRUE(manager);
+
+  scoped_refptr<ExtensionContextMenuModel> menu(
+      new ExtensionContextMenuModel(extension.get(), &browser));
+
+  // There should be no extension items yet.
+  EXPECT_EQ(0, CountExtensionItems(menu));
+
+  // Add a browser action menu item for |extension| to |manager|.
+  AddContextItemAndRefreshModel(
+      manager, extension.get(), MenuItem::BROWSER_ACTION, menu);
+
+  // Since |extension| has a page action, the browser action menu item should
+  // not be present.
+  EXPECT_EQ(0, CountExtensionItems(menu));
+
+  // Add a page action menu item and reset the context menu.
+  AddContextItemAndRefreshModel(
+      manager, extension.get(), MenuItem::PAGE_ACTION, menu);
+
+  // The page action item should be present because |extension| has a page
+  // action.
+  EXPECT_EQ(1, CountExtensionItems(menu));
+
+  // Create more page action items to test top level menu item limitations.
+  for (int i = 0; i < api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT; ++i)
+    AddContextItemAndRefreshModel(
+        manager, extension.get(), MenuItem::PAGE_ACTION, menu);
+
+  // The menu should only have a limited number of extension items, since they
+  // are all top level items, and we limit the number of top level extension
+  // items.
+  EXPECT_EQ(api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT,
+            CountExtensionItems(menu));
+
+  AddContextItemAndRefreshModel(
+      manager, extension.get(), MenuItem::PAGE_ACTION, menu);
+
+  // Adding another top level item should not increase the count.
+  EXPECT_EQ(api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT,
+            CountExtensionItems(menu));
+}
+
 }  // namespace
 }  // namespace extensions

chrome/browser/extensions/menu_manager.h

@@ -94,7 +94,9 @@ class MenuItem {
     VIDEO = 64,
     AUDIO = 128,
     FRAME = 256,
-    LAUNCHER = 512
+    LAUNCHER = 512,
+    BROWSER_ACTION = 1024,
+    PAGE_ACTION = 2048
   };
 
   // An item can be only one of these types.
@@ -167,7 +169,7 @@ class MenuItem {
   const Id& id() const { return id_; }
   Id* parent_id() const { return parent_id_.get(); }
   int child_count() const { return children_.size(); }
-  ContextList contexts() const { return contexts_; }
+  const ContextList& contexts() const { return contexts_; }
   Type type() const { return type_; }
   bool checked() const { return checked_; }
   bool enabled() const { return enabled_; }

chrome/browser/extensions/menu_manager_factory.cc

@@ -25,6 +25,12 @@ MenuManagerFactory* MenuManagerFactory::GetInstance() {
   return Singleton<MenuManagerFactory>::get();
 }
 
+// static
+KeyedService* MenuManagerFactory::BuildServiceInstanceForTesting(
+      content::BrowserContext* context) {
+  return GetInstance()->BuildServiceInstanceFor(context);
+}
+
 MenuManagerFactory::MenuManagerFactory()
     : BrowserContextKeyedServiceFactory(
         "MenuManager",
@@ -37,9 +43,7 @@ MenuManagerFactory::~MenuManagerFactory() {}
 KeyedService* MenuManagerFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = Profile::FromBrowserContext(context);
-  return new MenuManager(
-      profile,
-      ExtensionSystem::Get(profile)->state_store());
+  return new MenuManager(profile, ExtensionSystem::Get(profile)->state_store());
 }
 
 content::BrowserContext* MenuManagerFactory::GetBrowserContextToUse(

chrome/browser/extensions/menu_manager_factory.h

@@ -22,6 +22,9 @@ class MenuManagerFactory : public BrowserContextKeyedServiceFactory {
 
   static MenuManagerFactory* GetInstance();
 
+  static KeyedService* BuildServiceInstanceForTesting(
+      content::BrowserContext* context);
+
  private:
   friend struct DefaultSingletonTraits<MenuManagerFactory>;
 

chrome/browser/renderer_context_menu/render_view_context_menu.cc

@@ -18,7 +18,6 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/time/time.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
@@ -418,17 +417,17 @@ void RenderViewContextMenu::AppendAllExtensionItems() {
   std::set<MenuItem::ExtensionKey> ids = menu_manager->ExtensionIds();
   std::vector<base::string16> sorted_menu_titles;
   std::map<base::string16, std::string> map_ids;
-  for (std::set<MenuItem::ExtensionKey>::iterator i = ids.begin();
-       i != ids.end();
-       ++i) {
+  for (std::set<MenuItem::ExtensionKey>::iterator iter = ids.begin();
+       iter != ids.end();
+       ++iter) {
     const Extension* extension =
-        service->GetExtensionById(i->extension_id, false);
+        service->GetExtensionById(iter->extension_id, false);
     // Platform apps have their context menus created directly in
     // AppendPlatformAppItems.
     if (extension && !extension->is_platform_app()) {
       base::string16 menu_title = extension_items_.GetTopLevelContextMenuTitle(
-          *i, printable_selection_text);
-      map_ids[menu_title] = i->extension_id;
+          *iter, printable_selection_text);
+      map_ids[menu_title] = iter->extension_id;
       sorted_menu_titles.push_back(menu_title);
     }
   }
@@ -439,17 +438,14 @@ void RenderViewContextMenu::AppendAllExtensionItems() {
   l10n_util::SortStrings16(app_locale, &sorted_menu_titles);
 
   int index = 0;
-  base::TimeTicks begin = base::TimeTicks::Now();
   for (size_t i = 0; i < sorted_menu_titles.size(); ++i) {
     const std::string& id = map_ids[sorted_menu_titles[i]];
     const MenuItem::ExtensionKey extension_key(id);
-    extension_items_.AppendExtensionItems(
-        extension_key, printable_selection_text, &index);
+    extension_items_.AppendExtensionItems(extension_key,
+                                          printable_selection_text,
+                                          &index,
+                                          false);  // is_action_menu
   }
-
-  UMA_HISTOGRAM_TIMES("Extensions.ContextMenus_BuildTime",
-                      base::TimeTicks::Now() - begin);
-  UMA_HISTOGRAM_COUNTS("Extensions.ContextMenus_ItemCount", index);
 }
 
 void RenderViewContextMenu::AppendCurrentExtensionItems() {
@@ -462,8 +458,10 @@ void RenderViewContextMenu::AppendCurrentExtensionItems() {
     int index = 0;
     const MenuItem::ExtensionKey key(
         extension->id(), WebViewGuest::GetViewInstanceId(source_web_contents_));
-    extension_items_.AppendExtensionItems(
-        key, PrintableSelectionText(), &index);
+    extension_items_.AppendExtensionItems(key,
+                                          PrintableSelectionText(),
+                                          &index,
+                                          false);  // is_action_menu
   }
 }
 

chrome/browser/ui/app_list/app_context_menu.cc

@@ -152,7 +152,10 @@ ui::MenuModel* AppContextMenu::GetMenuModel() {
     // Assign unique IDs to commands added by the app itself.
     int index = USE_LAUNCH_TYPE_COMMAND_END;
     extension_menu_items_->AppendExtensionItems(
-        extensions::MenuItem::ExtensionKey(app_id_), base::string16(), &index);
+        extensions::MenuItem::ExtensionKey(app_id_),
+        base::string16(),
+        &index,
+        false);  // is_action_menu
 
     // If at least 1 item was added, add another separator after the list.
     if (index > USE_LAUNCH_TYPE_COMMAND_END)

chrome/browser/ui/ash/launcher/launcher_context_menu.cc

@@ -161,8 +161,10 @@ void LauncherContextMenu::Init() {
           controller_->GetAppIDForShelfID(item_.id));
       if (!app_key.empty()) {
         int index = 0;
-        extension_items_->AppendExtensionItems(
-            app_key, base::string16(), &index);
+        extension_items_->AppendExtensionItems(app_key,
+                                               base::string16(),
+                                               &index,
+                                               false);  // is_action_menu
         AddSeparator(ui::NORMAL_SEPARATOR);
       }
     }

chrome/common/extensions/api/context_menus.json

@@ -6,6 +6,12 @@
   {
     "namespace": "contextMenus",
     "description": "Use the <code>chrome.contextMenus</code> API to add items to Google Chrome's context menu. You can choose what types of objects your context menu additions apply to, such as images, hyperlinks, and pages.",
+    "properties": {
+      "ACTION_MENU_TOP_LEVEL_LIMIT": {
+        "value": 6,
+        "description": "The maximum number of top level extension items that can be added to an extension action context menu. Any items beyond this limit will be ignored."
+      }
+    },
     "functions": [
       {
         "name": "create",
@@ -48,7 +54,7 @@
                 "type": "array",
                 "items": {
                   "type": "string",
-                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher", "browser_action", "page_action"]
                 },
                 "minItems": 1,
                 "optional": true,
@@ -143,7 +149,7 @@
                 "type": "array",
                 "items": {
                   "type": "string",
-                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher", "browser_action", "page_action"]
                 },
                 "minItems": 1,
                 "optional": true

chrome/common/extensions/api/web_view_internal.json

@@ -124,7 +124,7 @@
                 "items": {
                   "type": "string",
                   // |launcher| isn't actually supported, this is listed here so that we can build |contexts| using the same code from chrome.contextMenus API.
-                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher", "browser_action", "page_action"]
                 },
                 "minItems": 1,
                 "optional": true,
@@ -218,7 +218,7 @@
                 "items": {
                   "type": "string",
                   // |launcher| isn't actually supported, this is listed here so that we can build |contexts| using the same code from chrome.contextMenus API.
-                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher", "browser_action", "page_action"]
                 },
                 "minItems": 1,
                 "optional": true,