Revert 288418 "Extend contextMenus API to support browser/page a..."

> Extend contextMenus API to support browser/page actions
> 
> Screenshot:
> http://i.imgur.com/RILqQqe.png
> 
> BUG=234425
> 
> Review URL: https://codereview.chromium.org/359493005

TBR=gpdavis.chromium@gmail.com

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

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

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

@@ -9,8 +9,6 @@
 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,7 +38,6 @@ scoped_ptr<extensions::MenuItem::Id> GetParentId(
 
 }  // namespace
 
-extern const char kActionNotAllowedError[];
 extern const char kCannotFindItemError[];
 extern const char kCheckedError[];
 extern const char kDuplicateIDError[];
@@ -90,14 +89,6 @@ 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();
     }
@@ -160,15 +151,6 @@ 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,21 +14,10 @@
 #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;
@@ -64,8 +53,7 @@ ContextMenuMatcher::ContextMenuMatcher(
 void ContextMenuMatcher::AppendExtensionItems(
     const MenuItem::ExtensionKey& extension_key,
     const base::string16& selection_text,
-    int* index,
-    bool is_action_menu) {
+    int* index) {
   DCHECK_GE(*index, 0);
   int max_index =
       extensions_context_custom_last - extensions_context_custom_first;
@@ -89,18 +77,11 @@ 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), 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);
+  // 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);
   } else {
     int menu_id = ConvertToExtensionsCustomCommandId(*index);
     (*index)++;
@@ -126,15 +107,10 @@ 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,
-                                      false);  // is_action_menu_top_level
+      RecursivelyAppendExtensionItems(submenu_items, can_cross_incognito,
+                                      selection_text, submenu, index);
     }
-    if (!is_action_menu)
-      SetExtensionIcon(extension_key.extension_id);
+    SetExtensionIcon(extension_key.extension_id);
   }
 }
 
@@ -239,11 +215,10 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
     bool can_cross_incognito,
     const base::string16& selection_text,
     ui::SimpleMenuModel* menu_model,
-    int* index,
-    bool is_action_menu_top_level) {
+    int* index)
+{
   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) {
@@ -258,15 +233,9 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems(
     }
 
     int menu_id = ConvertToExtensionsCustomCommandId(*index);
-    ++(*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))
+    (*index)++;
+    if (menu_id >= extensions_context_custom_last)
       return;
-
     extension_item_map_[menu_id] = item->id();
     base::string16 title = item->TitleWithReplacement(selection_text,
                                                 kMaxExtensionItemTitleLength);
@@ -279,12 +248,8 @@ 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,
-                                        false);  // is_action_menu_top_level
+        RecursivelyAppendExtensionItems(children, can_cross_incognito,
+                                        selection_text, submenu, index);
       }
     } else if (item->type() == MenuItem::CHECKBOX) {
       menu_model->AddCheckItem(menu_id, title);

chrome/browser/extensions/context_menu_matcher.h

@@ -43,13 +43,10 @@ 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. |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.
+  // each item actually added.
   void AppendExtensionItems(const MenuItem::ExtensionKey& extension_key,
                             const base::string16& selection_text,
-                            int* index,
-                            bool is_action_menu);
+                            int* index);
 
   void Clear();
 
@@ -83,8 +80,7 @@ class ContextMenuMatcher {
                                        bool can_cross_incognito,
                                        const base::string16& selection_text,
                                        ui::SimpleMenuModel* menu_model,
-                                       int* index,
-                                       bool is_action_menu_top_level);
+                                       int* index);
 
   // 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,14 +6,11 @@
 
 #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"
@@ -24,9 +21,7 @@
 #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"
@@ -39,32 +34,6 @@ 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,
@@ -73,9 +42,7 @@ ExtensionContextMenuModel::ExtensionContextMenuModel(const Extension* extension,
       extension_id_(extension->id()),
       browser_(browser),
       profile_(browser->profile()),
-      delegate_(delegate),
-      action_type_(NO_ACTION),
-      extension_items_count_(0) {
+      delegate_(delegate) {
   InitMenu(extension);
 
   if (profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode) &&
@@ -91,28 +58,20 @@ ExtensionContextMenuModel::ExtensionContextMenuModel(const Extension* extension,
       extension_id_(extension->id()),
       browser_(browser),
       profile_(browser->profile()),
-      delegate_(NULL),
-      action_type_(NO_ACTION),
-      extension_items_count_(0) {
+      delegate_(NULL) {
   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 = GetExtension();
+  const Extension* extension = this->GetExtension();
   if (!extension)
     return false;
 
-  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) {
+  if (command_id == CONFIGURE) {
     return
         extensions::ManifestURL::GetOptionsPage(extension).spec().length() > 0;
   } else if (command_id == NAME) {
@@ -146,16 +105,6 @@ 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),
@@ -219,26 +168,14 @@ 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));
@@ -249,24 +186,7 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension) {
 }
 
 const Extension* ExtensionContextMenuModel::GetExtension() const {
-  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
+  ExtensionService* extension_service =
+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+  return extension_service->GetExtensionById(extension_id_, false);
 }

chrome/browser/extensions/extension_context_menu_model.h

@@ -17,8 +17,6 @@ class Profile;
 
 namespace extensions {
 class Extension;
-class ContextMenuMatcher;
-class ExtensionContextMenuModelTest;
 }
 
 // The context menu model for extension icons.
@@ -37,9 +35,6 @@ 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:
@@ -79,8 +74,6 @@ class ExtensionContextMenuModel
 
  private:
   friend class base::RefCounted<ExtensionContextMenuModel>;
-  friend class extensions::ExtensionContextMenuModelTest;
-
   virtual ~ExtensionContextMenuModel();
 
   void InitMenu(const extensions::Extension* extension);
@@ -89,9 +82,6 @@ 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_;
 
@@ -106,18 +96,9 @@ 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,11 +6,8 @@
 
 #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"
@@ -20,83 +17,25 @@
 #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();
-  scoped_refptr<Extension> extension = BuildExtension();
+  // 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();
   ASSERT_TRUE(extension.get());
   service_->AddExtension(extension.get());
 
@@ -107,7 +46,7 @@ TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) {
   Browser browser(params);
 
   scoped_refptr<ExtensionContextMenuModel> menu(
-      new ExtensionContextMenuModel(extension.get(), &browser));
+      new ExtensionContextMenuModel(extension.get(), &browser, NULL));
 
   extensions::ExtensionSystem* system =
       extensions::ExtensionSystem::Get(profile_.get());
@@ -127,64 +66,5 @@ 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,9 +94,7 @@ class MenuItem {
     VIDEO = 64,
     AUDIO = 128,
     FRAME = 256,
-    LAUNCHER = 512,
-    BROWSER_ACTION = 1024,
-    PAGE_ACTION = 2048
+    LAUNCHER = 512
   };
 
   // An item can be only one of these types.
@@ -169,7 +167,7 @@ class MenuItem {
   const Id& id() const { return id_; }
   Id* parent_id() const { return parent_id_.get(); }
   int child_count() const { return children_.size(); }
-  const ContextList& contexts() const { return contexts_; }
+  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,12 +25,6 @@ MenuManagerFactory* MenuManagerFactory::GetInstance() {
   return Singleton<MenuManagerFactory>::get();
 }
 
-// static
-KeyedService* MenuManagerFactory::BuildServiceInstanceForTesting(
-      content::BrowserContext* context) {
-  return GetInstance()->BuildServiceInstanceFor(context);
-}
-
 MenuManagerFactory::MenuManagerFactory()
     : BrowserContextKeyedServiceFactory(
         "MenuManager",
@@ -43,7 +37,9 @@ 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,9 +22,6 @@ 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,6 +18,7 @@
 #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"
@@ -417,17 +418,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 iter = ids.begin();
-       iter != ids.end();
-       ++iter) {
+  for (std::set<MenuItem::ExtensionKey>::iterator i = ids.begin();
+       i != ids.end();
+       ++i) {
     const Extension* extension =
-        service->GetExtensionById(iter->extension_id, false);
+        service->GetExtensionById(i->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(
-          *iter, printable_selection_text);
-      map_ids[menu_title] = iter->extension_id;
+          *i, printable_selection_text);
+      map_ids[menu_title] = i->extension_id;
       sorted_menu_titles.push_back(menu_title);
     }
   }
@@ -438,14 +439,17 @@ 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,
-                                          false);  // is_action_menu
+    extension_items_.AppendExtensionItems(
+        extension_key, printable_selection_text, &index);
   }
+
+  UMA_HISTOGRAM_TIMES("Extensions.ContextMenus_BuildTime",
+                      base::TimeTicks::Now() - begin);
+  UMA_HISTOGRAM_COUNTS("Extensions.ContextMenus_ItemCount", index);
 }
 
 void RenderViewContextMenu::AppendCurrentExtensionItems() {
@@ -458,10 +462,8 @@ void RenderViewContextMenu::AppendCurrentExtensionItems() {
     int index = 0;
     const MenuItem::ExtensionKey key(
         extension->id(), WebViewGuest::GetViewInstanceId(source_web_contents_));
-    extension_items_.AppendExtensionItems(key,
-                                          PrintableSelectionText(),
-                                          &index,
-                                          false);  // is_action_menu
+    extension_items_.AppendExtensionItems(
+        key, PrintableSelectionText(), &index);
   }
 }
 

chrome/browser/ui/app_list/app_context_menu.cc

@@ -152,10 +152,7 @@ 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,
-        false);  // is_action_menu
+        extensions::MenuItem::ExtensionKey(app_id_), base::string16(), &index);
 
     // 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,10 +161,8 @@ void LauncherContextMenu::Init() {
           controller_->GetAppIDForShelfID(item_.id));
       if (!app_key.empty()) {
         int index = 0;
-        extension_items_->AppendExtensionItems(app_key,
-                                               base::string16(),
-                                               &index,
-                                               false);  // is_action_menu
+        extension_items_->AppendExtensionItems(
+            app_key, base::string16(), &index);
         AddSeparator(ui::NORMAL_SEPARATOR);
       }
     }

chrome/common/extensions/api/context_menus.json

@@ -6,12 +6,6 @@
   {
     "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",
@@ -54,7 +48,7 @@
                 "type": "array",
                 "items": {
                   "type": "string",
-                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher", "browser_action", "page_action"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
                 },
                 "minItems": 1,
                 "optional": true,
@@ -149,7 +143,7 @@
                 "type": "array",
                 "items": {
                   "type": "string",
-                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher", "browser_action", "page_action"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
                 },
                 "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", "browser_action", "page_action"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
                 },
                 "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", "browser_action", "page_action"]
+                  "enum": ["all", "page", "frame", "selection", "link", "editable", "image", "video", "audio", "launcher"]
                 },
                 "minItems": 1,
                 "optional": true,