diff options
15 files changed, 362 insertions, 66 deletions
diff --git a/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.cc b/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.cc index 45ab85e..2b99485 100644 --- a/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.cc +++ b/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"; diff --git a/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h b/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h index 773bd62..bcd3a4b 100644 --- a/chrome/browser/extensions/api/context_menus/context_menus_api_helpers.h +++ b/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()) diff --git a/chrome/browser/extensions/context_menu_matcher.cc b/chrome/browser/extensions/context_menu_matcher.cc index 1fd4e63..5825ea7 100644 --- a/chrome/browser/extensions/context_menu_matcher.cc +++ b/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); diff --git a/chrome/browser/extensions/context_menu_matcher.h b/chrome/browser/extensions/context_menu_matcher.h index f3f631f..ef44dac 100644 --- a/chrome/browser/extensions/context_menu_matcher.h +++ b/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; diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc index 3334fd9..14a39b8 100644 --- a/chrome/browser/extensions/extension_context_menu_model.cc +++ b/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 } diff --git a/chrome/browser/extensions/extension_context_menu_model.h b/chrome/browser/extensions/extension_context_menu_model.h index a07d9d5..0ec50f7 100644 --- a/chrome/browser/extensions/extension_context_menu_model.h +++ b/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); }; diff --git a/chrome/browser/extensions/extension_context_menu_model_unittest.cc b/chrome/browser/extensions/extension_context_menu_model_unittest.cc index f02d6e3..cd15d52 100644 --- a/chrome/browser/extensions/extension_context_menu_model_unittest.cc +++ b/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 diff --git a/chrome/browser/extensions/menu_manager.h b/chrome/browser/extensions/menu_manager.h index 23a922a..25feb8f 100644 --- a/chrome/browser/extensions/menu_manager.h +++ b/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_; } diff --git a/chrome/browser/extensions/menu_manager_factory.cc b/chrome/browser/extensions/menu_manager_factory.cc index 60fa844..33c3f74 100644 --- a/chrome/browser/extensions/menu_manager_factory.cc +++ b/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( diff --git a/chrome/browser/extensions/menu_manager_factory.h b/chrome/browser/extensions/menu_manager_factory.h index cdfdfa4..7dee9b7 100644 --- a/chrome/browser/extensions/menu_manager_factory.h +++ b/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>; diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc index 2d61b52..7c03e47 100644 --- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc +++ b/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 } } diff --git a/chrome/browser/ui/app_list/app_context_menu.cc b/chrome/browser/ui/app_list/app_context_menu.cc index ede0599..6af8638 100644 --- a/chrome/browser/ui/app_list/app_context_menu.cc +++ b/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) diff --git a/chrome/browser/ui/ash/launcher/launcher_context_menu.cc b/chrome/browser/ui/ash/launcher/launcher_context_menu.cc index c63743b..50a5975 100644 --- a/chrome/browser/ui/ash/launcher/launcher_context_menu.cc +++ b/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); } } diff --git a/chrome/common/extensions/api/context_menus.json b/chrome/common/extensions/api/context_menus.json index 7c757e0..3a8c281 100644 --- a/chrome/common/extensions/api/context_menus.json +++ b/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 diff --git a/chrome/common/extensions/api/web_view_internal.json b/chrome/common/extensions/api/web_view_internal.json index 94bf180..c09c126 100644 --- a/chrome/common/extensions/api/web_view_internal.json +++ b/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, |