[Extensions cleanup] Remove ButtonVisibility in context menu model
ButtonVisibility was used to determine whether the extension was pinned
or not on the toolbar. The enum was in the extension context menu,
but its computation was made in the extensions menu. This was
problematic, because it's not a property that belongs to either of them.
Instead we can directly retrieve whether the extension is pinned from
the toolbar actions model and remove the enum.
Bug: 1416359
Change-Id: If4c2da83eb36a471fb80e07f78480837bda7bcde
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4573221
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Emilia Paz <emiliapaz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1153967}
diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc
index 76d266a8..02573042 100644
--- a/chrome/browser/extensions/extension_context_menu_model.cc
+++ b/chrome/browser/extensions/extension_context_menu_model.cc
@@ -97,15 +97,14 @@
}
// Returns the id for the visibility command for the given |extension|.
-int GetVisibilityStringId(
- Profile* profile,
- const Extension* extension,
- ExtensionContextMenuModel::ButtonVisibility button_visibility) {
- if (IsExtensionForcePinned(*extension, profile))
+int GetVisibilityStringId(Profile* profile,
+ const Extension* extension,
+ bool is_pinned) {
+ if (IsExtensionForcePinned(*extension, profile)) {
return IDS_EXTENSIONS_PINNED_BY_ADMIN;
- if (button_visibility == ExtensionContextMenuModel::PINNED)
- return IDS_EXTENSIONS_UNPIN_FROM_TOOLBAR;
- return IDS_EXTENSIONS_PIN_TO_TOOLBAR;
+ }
+ return is_pinned ? IDS_EXTENSIONS_UNPIN_FROM_TOOLBAR
+ : IDS_EXTENSIONS_PIN_TO_TOOLBAR;
}
// Returns true if the given |extension| is required to remain installed by
@@ -256,7 +255,7 @@
ExtensionContextMenuModel::ExtensionContextMenuModel(
const Extension* extension,
Browser* browser,
- ButtonVisibility button_visibility,
+ bool is_pinned,
PopupDelegate* delegate,
bool can_show_icon_in_toolbar,
ContextMenuSource source)
@@ -266,7 +265,7 @@
browser_(browser),
profile_(browser->profile()),
delegate_(delegate),
- button_visibility_(button_visibility),
+ is_pinned_(is_pinned),
source_(source) {
if (base::FeatureList::IsEnabled(
extensions_features::kExtensionsMenuAccessControl)) {
@@ -400,9 +399,8 @@
ExtensionTabUtil::OpenOptionsPage(extension, browser_);
break;
case TOGGLE_VISIBILITY: {
- bool currently_visible = button_visibility_ == PINNED;
ToolbarActionsModel::Get(browser_->profile())
- ->SetActionVisibility(extension->id(), !currently_visible);
+ ->SetActionVisibility(extension->id(), !is_pinned_);
break;
}
case UNINSTALL: {
@@ -581,7 +579,7 @@
ui::ImageModel::FromVectorIcon(vector_icons::kBusinessIcon,
ui::kColorIcon, 16));
} else {
- int message_id = button_visibility_ == ExtensionContextMenuModel::PINNED
+ int message_id = is_pinned_
? IDS_EXTENSIONS_CONTEXT_MENU_UNPIN_FROM_TOOLBAR
: IDS_EXTENSIONS_CONTEXT_MENU_PIN_TO_TOOLBAR;
AddItemWithStringId(TOGGLE_VISIBILITY, message_id);
@@ -664,7 +662,7 @@
if (can_show_icon_in_toolbar &&
source_ == ContextMenuSource::kToolbarAction) {
int visibility_string_id =
- GetVisibilityStringId(profile_, extension, button_visibility_);
+ GetVisibilityStringId(profile_, extension, is_pinned_);
DCHECK_NE(-1, visibility_string_id);
AddItemWithStringId(TOGGLE_VISIBILITY, visibility_string_id);
if (IsExtensionForcePinned(*extension, profile_)) {
diff --git a/chrome/browser/extensions/extension_context_menu_model.h b/chrome/browser/extensions/extension_context_menu_model.h
index 5a692e90..fbc0ac0 100644
--- a/chrome/browser/extensions/extension_context_menu_model.h
+++ b/chrome/browser/extensions/extension_context_menu_model.h
@@ -80,21 +80,6 @@
// Location where the context menu is open from.
enum class ContextMenuSource { kToolbarAction = 0, kMenuItem = 1 };
- // The current visibility of the extension; this affects the "pin" / "unpin"
- // strings in the menu.
- // TODO(crbug.com/1416359): Rename this "PinState" when we finish removing the
- // old UI bits and move outside this class (pin state is not tied to the
- // context menu).
- enum ButtonVisibility {
- // The extension is pinned on the toolbar.
- PINNED,
- // The extension is temporarily visible on the toolbar, as for showing a
- // popup.
- TRANSITIVELY_VISIBLE,
- // The extension is not pinned (and is shown in the extensions menu).
- UNPINNED,
- };
-
// Delegate to handle showing an ExtensionAction popup.
class PopupDelegate {
public:
@@ -113,7 +98,7 @@
// ShowPopupForDevToolsWindow() to be called on |delegate|.
ExtensionContextMenuModel(const Extension* extension,
Browser* browser,
- ButtonVisibility visibility,
+ bool is_pinned,
PopupDelegate* delegate,
bool can_show_icon_in_toolbar,
ContextMenuSource source);
@@ -182,8 +167,8 @@
// The delegate which handles the 'inspect popup' menu command (or NULL).
raw_ptr<PopupDelegate> delegate_;
- // The visibility of the button at the time the menu opened.
- ButtonVisibility button_visibility_;
+ // Whether the extension icon is pinned at the time the menu opened.
+ bool is_pinned_;
// Menu matcher for context menu items specified by the extension.
std::unique_ptr<ContextMenuMatcher> extension_items_;
diff --git a/chrome/browser/extensions/extension_context_menu_model_unittest.cc b/chrome/browser/extensions/extension_context_menu_model_unittest.cc
index 7f8f073d..632c3f1 100644
--- a/chrome/browser/extensions/extension_context_menu_model_unittest.cc
+++ b/chrome/browser/extensions/extension_context_menu_model_unittest.cc
@@ -152,7 +152,7 @@
std::unique_ptr<ExtensionContextMenuModel> BuildMenu() {
return std::make_unique<ExtensionContextMenuModel>(
- extension_.get(), browser_, ExtensionContextMenuModel::PINNED, nullptr,
+ extension_.get(), browser_, /*is_pinned=*/true, nullptr,
/* can_show_icon_in_toolbar=*/true, ContextMenuSource::kToolbarAction);
}
@@ -476,8 +476,8 @@
{
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// Uninstallation should be enabled when all policy provider were
// unregistered.
@@ -491,8 +491,8 @@
{
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// If there's a policy provider that requires the extension stay enabled,
// the uninstall item should be hidden and instead should display the
@@ -526,8 +526,8 @@
service()->AddExtension(extension.get());
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// A component extension's context menu should not include options for
// managing extensions or removing it, and should only include an option for
@@ -562,8 +562,8 @@
.SetLocation(ManifestLocation::kComponent)
.Build();
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
service()->AddExtension(extension.get());
EXPECT_TRUE(extensions::OptionsPageInfo::HasOptionsPage(extension.get()));
EXPECT_EQ(GetCommandState(menu, ExtensionContextMenuModel::OPTIONS),
@@ -586,8 +586,7 @@
for (auto source : sources) {
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, source);
+ /*is_pinned=*/true, nullptr, true, source);
EXPECT_NE(GetCommandState(menu, ExtensionContextMenuModel::HOME_PAGE),
CommandState::kAbsent);
EXPECT_NE(GetCommandState(menu, ExtensionContextMenuModel::UNINSTALL),
@@ -612,7 +611,7 @@
// Verify the "toggle visibility" entry is absent if the context menu
// source is a menu item.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
+ /*is_pinned=*/true, nullptr,
/* can_show_icon_in_toolbar=*/true,
ContextMenuSource::kMenuItem);
EXPECT_FALSE(
@@ -627,7 +626,7 @@
// Verify the "toggle visibility" entry is absent if the context menu
// source is a toolbar action and the icon cannot be shown in the toolbar.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
+ /*is_pinned=*/true, nullptr,
/* can_show_icon_in_toolbar=*/false,
ContextMenuSource::kToolbarAction);
@@ -641,7 +640,7 @@
// context menu source is a toolbar action and the icon can be shown in the
// toolbar.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
+ /*is_pinned=*/true, nullptr,
/* can_show_icon_in_toolbar=*/true,
ContextMenuSource::kToolbarAction);
EXPECT_EQ(
@@ -665,8 +664,8 @@
// Verify the "options" entry is absent if the extension doesn't have
// an options page.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(GetCommandState(menu, ExtensionContextMenuModel::OPTIONS),
CommandState::kAbsent);
}
@@ -683,8 +682,8 @@
// Verify the "options" entry is enabled if and only if the
// extension has an options page.
ExtensionContextMenuModel menu(extension_with_options.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(GetCommandState(menu, ExtensionContextMenuModel::OPTIONS),
CommandState::kEnabled);
}
@@ -702,8 +701,8 @@
"page_action", manifest_keys::kPageAction, ManifestLocation::kInternal);
ASSERT_TRUE(page_action);
ExtensionContextMenuModel menu(page_action, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(GetCommandState(menu, ExtensionContextMenuModel::INSPECT_POPUP),
CommandState::kAbsent);
}
@@ -713,8 +712,8 @@
AddExtension("browser_action", manifest_keys::kBrowserAction,
ManifestLocation::kInternal);
ExtensionContextMenuModel menu(browser_action, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(GetCommandState(menu, ExtensionContextMenuModel::INSPECT_POPUP),
CommandState::kAbsent);
}
@@ -725,8 +724,8 @@
const Extension* no_action =
AddExtension("no_action", nullptr, ManifestLocation::kInternal);
ExtensionContextMenuModel menu(no_action, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(GetCommandState(menu, ExtensionContextMenuModel::INSPECT_POPUP),
CommandState::kAbsent);
}
@@ -756,8 +755,8 @@
{
// Even page actions should have a visibility option.
ExtensionContextMenuModel menu(page_action, browser,
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
absl::optional<size_t> index = menu.GetIndexOfCommandId(visibility_command);
ASSERT_TRUE(index.has_value());
EXPECT_EQ(unpin_string, menu.GetLabelAt(index.value()));
@@ -765,8 +764,8 @@
{
ExtensionContextMenuModel menu(browser_action, browser,
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
absl::optional<size_t> index = menu.GetIndexOfCommandId(visibility_command);
ASSERT_TRUE(index.has_value());
EXPECT_EQ(unpin_string, menu.GetLabelAt(index.value()));
@@ -780,20 +779,8 @@
{
// If the action is unpinned, it should have the "Pin" string.
ExtensionContextMenuModel menu(browser_action, browser,
- ExtensionContextMenuModel::UNPINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
- absl::optional<size_t> index = menu.GetIndexOfCommandId(visibility_command);
- ASSERT_TRUE(index.has_value());
- EXPECT_EQ(pin_string, menu.GetLabelAt(index.value()));
- }
-
- {
- // If the action is transitively visible, as happens when it is showing a
- // popup, we should use the same "Pin" string.
- ExtensionContextMenuModel menu(
- browser_action, browser,
- ExtensionContextMenuModel::TRANSITIVELY_VISIBLE, nullptr, true,
- ContextMenuSource::kToolbarAction);
+ /*is_pinned,=*/false, nullptr, true,
+ ContextMenuSource::kToolbarAction);
absl::optional<size_t> index = menu.GetIndexOfCommandId(visibility_command);
ASSERT_TRUE(index.has_value());
EXPECT_EQ(pin_string, menu.GetLabelAt(index.value()));
@@ -837,8 +824,8 @@
{
// Not force-pinned.
ExtensionContextMenuModel menu(extension, browser,
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
absl::optional<size_t> index = menu.GetIndexOfCommandId(visibility_command);
ASSERT_TRUE(index.has_value());
EXPECT_TRUE(menu.IsEnabledAt(index.value()));
@@ -848,8 +835,8 @@
{
// Force-pinned.
ExtensionContextMenuModel menu(force_pinned_extension, browser,
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
absl::optional<size_t> index = menu.GetIndexOfCommandId(visibility_command);
ASSERT_TRUE(index.has_value());
EXPECT_FALSE(menu.IsEnabledAt(index.value()));
@@ -872,8 +859,8 @@
// reflects what normally happens (Chrome closes the menu when the uninstall
// dialog shows up).
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
menu.ExecuteCommand(ExtensionContextMenuModel::UNINSTALL, 0);
}
uninstalled_observer.WaitForExtensionUninstalled();
@@ -917,8 +904,8 @@
std::move(increment_run_count_1));
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// Since we want to test the page access submenu, verify the site permission
// is set to "customize by extension" by default and the page access submenu
@@ -1057,8 +1044,8 @@
"single_host_extension", manifest_keys::kBrowserAction,
ManifestLocation::kInternal, "http://www.example.com/*");
ExtensionContextMenuModel single_host_menu(
- single_host_extension, GetBrowser(), ExtensionContextMenuModel::PINNED,
- nullptr, true, ContextMenuSource::kToolbarAction);
+ single_host_extension, GetBrowser(), /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_TRUE(
single_host_menu
.GetIndexOfCommandId(ExtensionContextMenuModel::PAGE_ACCESS_SUBMENU)
@@ -1255,8 +1242,8 @@
PermissionsManager::UserSiteSetting::kCustomizeByExtension);
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(test_case.selected_entry.has_value(),
!test_case.expected_entries.empty())
@@ -1352,8 +1339,8 @@
// Verify the extension can run on all sites for the active url, and has
// access to both urls.
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_TRUE(HasPageAccessSubmenu(menu));
EXPECT_FALSE(menu.IsCommandIdChecked(kOnClick));
EXPECT_FALSE(menu.IsCommandIdChecked(kOnSite));
@@ -1424,8 +1411,8 @@
// Verify the extension can run on all sites even though it
// can't access the active url.
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_TRUE(HasPageAccessSubmenu(menu));
EXPECT_FALSE(menu.IsCommandIdChecked(kOnClick));
EXPECT_FALSE(menu.IsCommandIdChecked(kOnSite));
@@ -1476,8 +1463,8 @@
PermissionsManager::UserSiteSetting::kCustomizeByExtension);
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_TRUE(HasPageAccessSubmenu(menu));
EXPECT_EQ(CommandState::kEnabled, GetPageAccessCommandState(menu, kOnClick));
EXPECT_EQ(CommandState::kDisabled, GetPageAccessCommandState(menu, kOnSite));
@@ -1502,8 +1489,8 @@
content::WebContents* web_contents = AddTab(kOriginalUrl);
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// By default, extension is granted access to all sites.
PermissionsManager* permissions_manager = PermissionsManager::Get(profile());
@@ -1552,8 +1539,8 @@
{
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// Without withholding host permissions, the menu should be visible on
// a.com...
@@ -1581,8 +1568,8 @@
{
// ... but not on b.com, where it doesn't want to run.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_FALSE(HasPageAccessSubmenu(menu));
EXPECT_TRUE(HasCantAccessPageEntry(menu));
}
@@ -1597,8 +1584,8 @@
{
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_TRUE(HasPageAccessSubmenu(menu));
EXPECT_FALSE(HasCantAccessPageEntry(menu));
EXPECT_EQ(CommandState::kEnabled,
@@ -1630,8 +1617,8 @@
}
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// Somewhat strangely, this also removes the access controls, because we don't
// show it for sites the extension doesn't want to run on.
EXPECT_FALSE(HasPageAccessSubmenu(menu));
@@ -1656,8 +1643,8 @@
content::WebContents* web_contents = AddTab(a_com);
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(CommandState::kEnabled, GetPageAccessCommandState(menu, kOnClick));
EXPECT_EQ(CommandState::kEnabled, GetPageAccessCommandState(menu, kOnSite));
@@ -1707,8 +1694,8 @@
Browser* browser = GetBrowser();
ExtensionContextMenuModel menu(extension.get(), browser,
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(0, user_action_tester.GetActionCount(kLearnMoreAction));
EXPECT_EQ(CommandState::kEnabled,
@@ -1739,8 +1726,8 @@
{
// The menu is constructed, but never shown.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
}
tester.ExpectTotalCount(kHistogramName, 0);
}
@@ -1750,8 +1737,8 @@
{
// The menu is constructed and shown, but no action is taken.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
menu.OnMenuWillShow(&menu);
menu.MenuClosed(&menu);
}
@@ -1765,8 +1752,8 @@
{
// The menu is constructed, shown, and an action taken.
ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
menu.OnMenuWillShow(&menu);
menu.ExecuteCommand(ExtensionContextMenuModel::MANAGE_EXTENSIONS, 0);
menu.MenuClosed(&menu);
@@ -1995,8 +1982,8 @@
// Verify "page access" submenu item and "permissions page" item are visible
// and enabled.
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(GetCommandState(menu, kGrantAllExtensions),
CommandState::kAbsent);
EXPECT_EQ(GetCommandState(menu, kBlockAllExtensions),
@@ -2028,8 +2015,8 @@
manager_waiter.WaitForUserPermissionsSettingsChange();
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
if (is_feature_enabled) {
// Verify "block all extensions" item is visible and disabled, and
@@ -2081,8 +2068,8 @@
AddTab(url);
ExtensionContextMenuModel menu(enterprise_extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
// By default, user can customize site access by extension and the 'grant all
// extensions' and 'block all extensions' are not visible.
@@ -2139,8 +2126,8 @@
Browser* browser = GetBrowser();
ExtensionContextMenuModel menu(extension.get(), browser,
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(user_action_tester.GetActionCount(kPermissionsPageAction), 0);
// "permissions page" button is not visible when the enhanced host controls
@@ -2204,8 +2191,8 @@
// By default, the site permission is set to "customize by extension".
// Verify "page access" submenu item is visible and enabled.
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
EXPECT_EQ(GetCommandState(menu, kGrantAllExtensions),
CommandState::kAbsent);
EXPECT_EQ(GetCommandState(menu, kBlockAllExtensions),
@@ -2237,8 +2224,8 @@
manager_waiter.WaitForUserPermissionsSettingsChange();
ExtensionContextMenuModel menu(extension, GetBrowser(),
- ExtensionContextMenuModel::PINNED, nullptr,
- true, ContextMenuSource::kToolbarAction);
+ /*is_pinned=*/true, nullptr, true,
+ ContextMenuSource::kToolbarAction);
if (is_feature_enabled) {
// Verify "grant all extensions" item is visible and disabled, and
diff --git a/chrome/browser/ui/extensions/extension_action_view_controller.cc b/chrome/browser/ui/extensions/extension_action_view_controller.cc
index 5b6c264..ef427a4 100644
--- a/chrome/browser/ui/extensions/extension_action_view_controller.cc
+++ b/chrome/browser/ui/extensions/extension_action_view_controller.cc
@@ -331,12 +331,12 @@
if (!ExtensionIsValid())
return nullptr;
- extensions::ExtensionContextMenuModel::ButtonVisibility visibility =
- extensions_container_->GetActionVisibility(GetId());
+ bool is_pinned =
+ ToolbarActionsModel::Get(browser_->profile())->IsActionPinned(GetId());
// Reconstruct the menu every time because the menu's contents are dynamic.
context_menu_model_ = std::make_unique<extensions::ExtensionContextMenuModel>(
- extension(), browser_, visibility, this,
+ extension(), browser_, is_pinned, this,
extensions_container_->CanShowActionsInToolbar(), context_menu_source);
return context_menu_model_.get();
}
diff --git a/chrome/browser/ui/extensions/extensions_container.h b/chrome/browser/ui/extensions/extensions_container.h
index 1efbcf7a..992c8be0 100644
--- a/chrome/browser/ui/extensions/extensions_container.h
+++ b/chrome/browser/ui/extensions/extensions_container.h
@@ -43,10 +43,6 @@
// toolbar.
virtual bool IsActionVisibleOnToolbar(const std::string& action_id) const = 0;
- // Returns the action's toolbar button visibility.
- virtual extensions::ExtensionContextMenuModel::ButtonVisibility
- GetActionVisibility(const std::string& action_id) const = 0;
-
// Undoes the current "pop out"; i.e., moves the popped out action back into
// overflow.
virtual void UndoPopOut() = 0;
diff --git a/chrome/browser/ui/views/extensions/extensions_menu_view_interactive_uitest.cc b/chrome/browser/ui/views/extensions/extensions_menu_view_interactive_uitest.cc
index 335e776..5ee17dbc 100644
--- a/chrome/browser/ui/views/extensions/extensions_menu_view_interactive_uitest.cc
+++ b/chrome/browser/ui/views/extensions/extensions_menu_view_interactive_uitest.cc
@@ -91,7 +91,7 @@
"ExtensionUninstallDialog");
extensions::ExtensionContextMenuModel menu_model(
extensions()[0].get(), browser(),
- extensions::ExtensionContextMenuModel::PINNED, nullptr,
+ /*is_pinned=*/true, nullptr,
/*can_show_icon_in_toolbar=*/false,
extensions::ExtensionContextMenuModel::ContextMenuSource::kMenuItem);
menu_model.ExecuteCommand(
@@ -489,7 +489,7 @@
// browser.
extensions::ExtensionContextMenuModel menu(
extensions()[0].get(), incognito_browser(),
- extensions::ExtensionContextMenuModel::PINNED, nullptr,
+ /*is_pinned=*/true, nullptr,
/* can_show_icon_in_toolbar=*/true,
extensions::ExtensionContextMenuModel::ContextMenuSource::kMenuItem);
EXPECT_FALSE(menu.IsCommandIdEnabled(
diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc
index cfa7924..22b0904 100644
--- a/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc
+++ b/chrome/browser/ui/views/extensions/extensions_toolbar_container.cc
@@ -410,22 +410,7 @@
bool ExtensionsToolbarContainer::IsActionVisibleOnToolbar(
const std::string& action_id) const {
- return GetActionVisibility(action_id) !=
- extensions::ExtensionContextMenuModel::UNPINNED;
-}
-
-extensions::ExtensionContextMenuModel::ButtonVisibility
-ExtensionsToolbarContainer::GetActionVisibility(
- const std::string& action_id) const {
- if (model_->IsActionPinned(action_id)) {
- return extensions::ExtensionContextMenuModel::PINNED;
- }
-
- if (ShouldForceVisibility(action_id)) {
- return extensions::ExtensionContextMenuModel::TRANSITIVELY_VISIBLE;
- }
-
- return extensions::ExtensionContextMenuModel::UNPINNED;
+ return model_->IsActionPinned(action_id) || ShouldForceVisibility(action_id);
}
void ExtensionsToolbarContainer::UndoPopOut() {
diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_container.h b/chrome/browser/ui/views/extensions/extensions_toolbar_container.h
index 298ab6f2..65e2e62 100644
--- a/chrome/browser/ui/views/extensions/extensions_toolbar_container.h
+++ b/chrome/browser/ui/views/extensions/extensions_toolbar_container.h
@@ -83,7 +83,6 @@
delete;
~ExtensionsToolbarContainer() override;
- DisplayMode display_mode() const { return display_mode_; }
const ToolbarIcons& icons_for_testing() const { return icons_; }
ToolbarActionViewController* popup_owner_for_testing() {
return popup_owner_;
@@ -153,8 +152,6 @@
void OnContextMenuClosed() override;
bool CanShowActionsInToolbar() const override;
bool IsActionVisibleOnToolbar(const std::string& action_id) const override;
- extensions::ExtensionContextMenuModel::ButtonVisibility GetActionVisibility(
- const std::string& action_id) const override;
void UndoPopOut() override;
void SetPopupOwner(ToolbarActionViewController* popup_owner) override;
void HideActivePopup() override;
diff --git a/chrome/browser/ui/views/extensions/extensions_toolbar_controls_unittest.cc b/chrome/browser/ui/views/extensions/extensions_toolbar_controls_unittest.cc
index 50c89aed..513b817 100644
--- a/chrome/browser/ui/views/extensions/extensions_toolbar_controls_unittest.cc
+++ b/chrome/browser/ui/views/extensions/extensions_toolbar_controls_unittest.cc
@@ -182,8 +182,8 @@
EXPECT_FALSE(IsRequestAccessButtonVisible());
extensions::ExtensionContextMenuModel context_menu(
- extension.get(), browser(), extensions::ExtensionContextMenuModel::PINNED,
- nullptr, true,
+ extension.get(), browser(), /*is_pinned=*/true, /*delegate=*/nullptr,
+ /*can_show_icon_in_toolbar=*/true,
extensions::ExtensionContextMenuModel::ContextMenuSource::kToolbarAction);
// Changing the context menu may trigger the reload page bubble. Accept it so
