Commit 94aa4fbb authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions] Update ExtensionContextMenuModel::MenuEntries names

A few of the name values for the menu entries in the
ExtensionContextMenuModel were a little misleading. Remap the
following (<old> -> <new>):
- NAME -> HOME_PAGE (opens the extension's home page, or the webstore
  if a separate home page was not specified).
- CONFIGURE - OPTIONS (opens the extension's options page)
- MANAGE -> MANAGE_EXTENSIONS (opens the chrome://extensions page)

No functional change is intended; this is purely a renaming of code
values.

Bug: None

Change-Id: Iabac73bf3bd7d85ea02b23379a9a19eeef02cf21
Reviewed-on: https://chromium-review.googlesource.com/1229774Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593082}
parent bef96cf0
...@@ -199,14 +199,14 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const { ...@@ -199,14 +199,14 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const {
return extension_items_->IsCommandIdEnabled(command_id); return extension_items_->IsCommandIdEnabled(command_id);
switch (command_id) { switch (command_id) {
case NAME: case HOME_PAGE:
// The NAME links to the Homepage URL. If the extension doesn't have a // The HOME_PAGE links to the Homepage URL. If the extension doesn't have
// homepage, we just disable this menu item. We also disable for component // a homepage, we just disable this menu item. We also disable for
// extensions, because it doesn't make sense to link to a webstore page or // component extensions, because it doesn't make sense to link to a
// chrome://extensions. // webstore page or chrome://extensions.
return ManifestURL::GetHomepageURL(extension).is_valid() && return ManifestURL::GetHomepageURL(extension).is_valid() &&
!is_component_; !is_component_;
case CONFIGURE: case OPTIONS:
return OptionsPageInfo::HasOptionsPage(extension); return OptionsPageInfo::HasOptionsPage(extension);
case INSPECT_POPUP: { case INSPECT_POPUP: {
content::WebContents* web_contents = GetActiveWebContents(); content::WebContents* web_contents = GetActiveWebContents();
...@@ -218,7 +218,7 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const { ...@@ -218,7 +218,7 @@ bool ExtensionContextMenuModel::IsCommandIdEnabled(int command_id) const {
return !IsExtensionRequiredByPolicy(extension, profile_); return !IsExtensionRequiredByPolicy(extension, profile_);
// The following, if they are present, are always enabled. // The following, if they are present, are always enabled.
case TOGGLE_VISIBILITY: case TOGGLE_VISIBILITY:
case MANAGE: case MANAGE_EXTENSIONS:
case PAGE_ACCESS_SUBMENU: case PAGE_ACCESS_SUBMENU:
case PAGE_ACCESS_RUN_ON_CLICK: case PAGE_ACCESS_RUN_ON_CLICK:
case PAGE_ACCESS_RUN_ON_SITE: case PAGE_ACCESS_RUN_ON_SITE:
...@@ -245,7 +245,7 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id, ...@@ -245,7 +245,7 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
} }
switch (command_id) { switch (command_id) {
case NAME: { case HOME_PAGE: {
content::OpenURLParams params(ManifestURL::GetHomepageURL(extension), content::OpenURLParams params(ManifestURL::GetHomepageURL(extension),
content::Referrer(), content::Referrer(),
WindowOpenDisposition::NEW_FOREGROUND_TAB, WindowOpenDisposition::NEW_FOREGROUND_TAB,
...@@ -253,7 +253,7 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id, ...@@ -253,7 +253,7 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
browser_->OpenURL(params); browser_->OpenURL(params);
break; break;
} }
case CONFIGURE: case OPTIONS:
DCHECK(OptionsPageInfo::HasOptionsPage(extension)); DCHECK(OptionsPageInfo::HasOptionsPage(extension));
ExtensionTabUtil::OpenOptionsPage(extension, browser_); ExtensionTabUtil::OpenOptionsPage(extension, browser_);
break; break;
...@@ -267,7 +267,7 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id, ...@@ -267,7 +267,7 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
UninstallDialogHelper::UninstallExtension(browser_, extension); UninstallDialogHelper::UninstallExtension(browser_, extension);
break; break;
} }
case MANAGE: { case MANAGE_EXTENSIONS: {
chrome::ShowExtensions(browser_, extension->id()); chrome::ShowExtensions(browser_, extension->id());
break; break;
} }
...@@ -308,14 +308,14 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension, ...@@ -308,14 +308,14 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension,
// Ampersands need to be escaped to avoid being treated like // Ampersands need to be escaped to avoid being treated like
// mnemonics in the menu. // mnemonics in the menu.
base::ReplaceChars(extension_name, "&", "&&", &extension_name); base::ReplaceChars(extension_name, "&", "&&", &extension_name);
AddItem(NAME, base::UTF8ToUTF16(extension_name)); AddItem(HOME_PAGE, base::UTF8ToUTF16(extension_name));
AppendExtensionItems(); AppendExtensionItems();
AddSeparator(ui::NORMAL_SEPARATOR); AddSeparator(ui::NORMAL_SEPARATOR);
CreatePageAccessSubmenu(extension); CreatePageAccessSubmenu(extension);
if (!is_component_ || OptionsPageInfo::HasOptionsPage(extension)) if (!is_component_ || OptionsPageInfo::HasOptionsPage(extension))
AddItemWithStringId(CONFIGURE, IDS_EXTENSIONS_OPTIONS_MENU_ITEM); AddItemWithStringId(OPTIONS, IDS_EXTENSIONS_OPTIONS_MENU_ITEM);
if (!is_component_) { if (!is_component_) {
bool is_required_by_policy = bool is_required_by_policy =
...@@ -340,7 +340,7 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension, ...@@ -340,7 +340,7 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension,
if (!is_component_) { if (!is_component_) {
AddSeparator(ui::NORMAL_SEPARATOR); AddSeparator(ui::NORMAL_SEPARATOR);
AddItemWithStringId(MANAGE, IDS_MANAGE_EXTENSION); AddItemWithStringId(MANAGE_EXTENSIONS, IDS_MANAGE_EXTENSION);
} }
const ActionInfo* action_info = ActionInfo::GetPageActionInfo(extension); const ActionInfo* action_info = ActionInfo::GetPageActionInfo(extension);
......
...@@ -28,11 +28,11 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel, ...@@ -28,11 +28,11 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
public ui::SimpleMenuModel::Delegate { public ui::SimpleMenuModel::Delegate {
public: public:
enum MenuEntries { enum MenuEntries {
NAME = 0, HOME_PAGE = 0,
CONFIGURE, OPTIONS,
TOGGLE_VISIBILITY, TOGGLE_VISIBILITY,
UNINSTALL, UNINSTALL,
MANAGE, MANAGE_EXTENSIONS,
INSPECT_POPUP, INSPECT_POPUP,
PAGE_ACCESS_SUBMENU, PAGE_ACCESS_SUBMENU,
PAGE_ACCESS_RUN_ON_CLICK, PAGE_ACCESS_RUN_ON_CLICK,
......
...@@ -406,15 +406,16 @@ TEST_F(ExtensionContextMenuModelTest, ComponentExtensionContextMenu) { ...@@ -406,15 +406,16 @@ TEST_F(ExtensionContextMenuModelTest, ComponentExtensionContextMenu) {
// A component extension's context menu should not include options for // A component extension's context menu should not include options for
// managing extensions or removing it, and should only include an option for // managing extensions or removing it, and should only include an option for
// the options page if the extension has one (which this one doesn't). // the options page if the extension has one (which this one doesn't).
EXPECT_EQ(-1, EXPECT_EQ(-1, menu.GetIndexOfCommandId(ExtensionContextMenuModel::OPTIONS));
menu.GetIndexOfCommandId(ExtensionContextMenuModel::CONFIGURE));
EXPECT_EQ(-1, EXPECT_EQ(-1,
menu.GetIndexOfCommandId(ExtensionContextMenuModel::UNINSTALL)); menu.GetIndexOfCommandId(ExtensionContextMenuModel::UNINSTALL));
EXPECT_EQ(-1, menu.GetIndexOfCommandId(ExtensionContextMenuModel::MANAGE)); EXPECT_EQ(-1, menu.GetIndexOfCommandId(
ExtensionContextMenuModel::MANAGE_EXTENSIONS));
// The "name" option should be present, but not enabled for component // The "name" option should be present, but not enabled for component
// extensions. // extensions.
EXPECT_NE(-1, menu.GetIndexOfCommandId(ExtensionContextMenuModel::NAME)); EXPECT_NE(-1,
EXPECT_FALSE(menu.IsCommandIdEnabled(ExtensionContextMenuModel::NAME)); menu.GetIndexOfCommandId(ExtensionContextMenuModel::HOME_PAGE));
EXPECT_FALSE(menu.IsCommandIdEnabled(ExtensionContextMenuModel::HOME_PAGE));
} }
{ {
...@@ -432,9 +433,8 @@ TEST_F(ExtensionContextMenuModelTest, ComponentExtensionContextMenu) { ...@@ -432,9 +433,8 @@ TEST_F(ExtensionContextMenuModelTest, ComponentExtensionContextMenu) {
ExtensionContextMenuModel::VISIBLE, nullptr); ExtensionContextMenuModel::VISIBLE, nullptr);
service()->AddExtension(extension.get()); service()->AddExtension(extension.get());
EXPECT_TRUE(extensions::OptionsPageInfo::HasOptionsPage(extension.get())); EXPECT_TRUE(extensions::OptionsPageInfo::HasOptionsPage(extension.get()));
EXPECT_NE(-1, EXPECT_NE(-1, menu.GetIndexOfCommandId(ExtensionContextMenuModel::OPTIONS));
menu.GetIndexOfCommandId(ExtensionContextMenuModel::CONFIGURE)); EXPECT_TRUE(menu.IsCommandIdEnabled(ExtensionContextMenuModel::OPTIONS));
EXPECT_TRUE(menu.IsCommandIdEnabled(ExtensionContextMenuModel::CONFIGURE));
} }
} }
...@@ -581,12 +581,13 @@ TEST_F(ExtensionContextMenuModelTest, ...@@ -581,12 +581,13 @@ TEST_F(ExtensionContextMenuModelTest,
ExtensionContextMenuModel menu(extension, GetBrowser(), ExtensionContextMenuModel menu(extension, GetBrowser(),
ExtensionContextMenuModel::VISIBLE, nullptr); ExtensionContextMenuModel::VISIBLE, nullptr);
EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::NAME)); EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::HOME_PAGE));
EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::CONFIGURE)); EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::OPTIONS));
EXPECT_TRUE( EXPECT_TRUE(
menu.IsCommandIdVisible(ExtensionContextMenuModel::TOGGLE_VISIBILITY)); menu.IsCommandIdVisible(ExtensionContextMenuModel::TOGGLE_VISIBILITY));
EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::UNINSTALL)); EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::UNINSTALL));
EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::MANAGE)); EXPECT_TRUE(
menu.IsCommandIdVisible(ExtensionContextMenuModel::MANAGE_EXTENSIONS));
EXPECT_TRUE( EXPECT_TRUE(
menu.IsCommandIdVisible(ExtensionContextMenuModel::INSPECT_POPUP)); menu.IsCommandIdVisible(ExtensionContextMenuModel::INSPECT_POPUP));
EXPECT_TRUE(menu.IsCommandIdVisible( EXPECT_TRUE(menu.IsCommandIdVisible(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment