[Extensions UI] Rename some "overflow" references to "unpinned"

The "overflow" menu referred to the old extensions toolbar UI; now we
use the terminology of "pinned" and "unpinned". Update a few of these
sites - most notably in the ExtensionContextMenuModel.

Note: This deliberately does not update any references in old UI code,
since those still (conceptually) refer to the overflow menu.

Bug: 1100412
Change-Id: I2d3f968f548f43e9aaa1e4d7fa01b99c1ef4f02c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2387137
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803441}

chrome/browser/extensions/extension_context_menu_model.cc

@@ -90,7 +90,7 @@ int GetVisibilityStringId(
     const Extension* extension,
     ExtensionContextMenuModel::ButtonVisibility button_visibility) {
   if (base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)) {
-    return button_visibility == ExtensionContextMenuModel::VISIBLE
+    return button_visibility == ExtensionContextMenuModel::PINNED
                ? IDS_EXTENSIONS_UNPIN_FROM_TOOLBAR
                : IDS_EXTENSIONS_PIN_TO_TOOLBAR;
   }
@@ -100,13 +100,13 @@ int GetVisibilityStringId(
   // "transitively shown" buttons that are shown only while the button has a
   // popup or menu visible.
   switch (button_visibility) {
-    case (ExtensionContextMenuModel::VISIBLE):
+    case (ExtensionContextMenuModel::PINNED):
       string_id = IDS_EXTENSIONS_HIDE_BUTTON_IN_MENU;
       break;
     case (ExtensionContextMenuModel::TRANSITIVELY_VISIBLE):
       string_id = IDS_EXTENSIONS_KEEP_BUTTON_IN_TOOLBAR;
       break;
-    case (ExtensionContextMenuModel::OVERFLOWED):
+    case (ExtensionContextMenuModel::UNPINNED):
       string_id = IDS_EXTENSIONS_SHOW_BUTTON_IN_TOOLBAR;
       break;
   }
@@ -338,7 +338,7 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id,
       ExtensionTabUtil::OpenOptionsPage(extension, browser_);
       break;
     case TOGGLE_VISIBILITY: {
-      bool currently_visible = button_visibility_ == VISIBLE;
+      bool currently_visible = button_visibility_ == PINNED;
       ToolbarActionsModel::Get(browser_->profile())
           ->SetActionVisibility(extension->id(), !currently_visible);
       break;

chrome/browser/extensions/extension_context_menu_model.h

@@ -69,16 +69,18 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
     kMaxValue = kPageAccessLearnMore,
   };
 
-  // The current visibility of the button; this can affect the "hide"/"show"
+  // The current visibility of the extension; this affects the "pin" / "unpin"
   // strings in the menu.
-  // TODO(devlin): Update these to be more appropriate for the extensions menu.
+  // TODO(devlin): Rename this "PinState" when we finish removing the old UI
+  // bits.
   enum ButtonVisibility {
-    // The button is visible on the toolbar.
-    VISIBLE,
-    // The button is temporarily visible on the toolbar, as for showign a popup.
+    // The extension is pinned on the toolbar.
+    PINNED,
+    // The extension is temporarily visible on the toolbar, as for showing a
+    // popup.
     TRANSITIVELY_VISIBLE,
-    // The button is showed in the overflow menu.
-    OVERFLOWED
+    // The extension is not pinned (and is shown in the extensions menu).
+    UNPINNED,
   };
 
   // Delegate to handle showing an ExtensionAction popup.

chrome/browser/extensions/extension_context_menu_model_unittest.cc

@@ -129,7 +129,7 @@ class MenuBuilder {
 
   std::unique_ptr<ExtensionContextMenuModel> BuildMenu() {
     return std::make_unique<ExtensionContextMenuModel>(
-        extension_.get(), browser_, ExtensionContextMenuModel::VISIBLE, nullptr,
+        extension_.get(), browser_, ExtensionContextMenuModel::PINNED, nullptr,
         true /* can_show_icon_in_toolbar */);
   }
 
@@ -426,7 +426,7 @@ TEST_F(ExtensionContextMenuModelTest, RequiredInstallationsDisablesItems) {
       "extension", manifest_keys::kPageAction, Manifest::EXTERNAL_POLICY);
 
   ExtensionContextMenuModel menu(extension, GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
 
   ExtensionSystem* system = ExtensionSystem::Get(profile());
@@ -478,7 +478,7 @@ TEST_F(ExtensionContextMenuModelTest, ComponentExtensionContextMenu) {
     service()->AddExtension(extension.get());
 
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
 
     // A component extension's context menu should not include options for
@@ -508,7 +508,7 @@ TEST_F(ExtensionContextMenuModelTest, ComponentExtensionContextMenu) {
             .SetLocation(Manifest::COMPONENT)
             .Build();
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     service()->AddExtension(extension.get());
     EXPECT_TRUE(extensions::OptionsPageInfo::HasOptionsPage(extension.get()));
@@ -526,7 +526,7 @@ TEST_F(ExtensionContextMenuModelTest,
       AddExtension("extension", manifest_keys::kPageAction, Manifest::INTERNAL);
 
   ExtensionContextMenuModel menu(extension, GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
   EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::HOME_PAGE));
   EXPECT_TRUE(menu.IsCommandIdVisible(ExtensionContextMenuModel::OPTIONS));
@@ -570,9 +570,8 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
 
   {
     // Even page actions should have a visibility option.
-    ExtensionContextMenuModel menu(page_action, browser,
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
-                                   true);
+    ExtensionContextMenuModel menu(
+        page_action, browser, ExtensionContextMenuModel::PINNED, nullptr, true);
     int index = menu.GetIndexOfCommandId(visibility_command);
     EXPECT_NE(-1, index);
     EXPECT_EQ(unpin_string, menu.GetLabelAt(index));
@@ -580,7 +579,7 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
 
   {
     ExtensionContextMenuModel menu(browser_action, browser,
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     int index = menu.GetIndexOfCommandId(visibility_command);
     EXPECT_NE(-1, index);
@@ -595,8 +594,8 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
   {
     // If the action is unpinned, it should have the "Pin" string.
     ExtensionContextMenuModel menu(browser_action, browser,
-                                   ExtensionContextMenuModel::OVERFLOWED,
-                                   nullptr, true);
+                                   ExtensionContextMenuModel::UNPINNED, nullptr,
+                                   true);
     int index = menu.GetIndexOfCommandId(visibility_command);
     EXPECT_NE(-1, index);
     EXPECT_EQ(pin_string, menu.GetLabelAt(index));
@@ -629,7 +628,7 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionContextUninstall) {
     // reflects what normally happens (Chrome closes the menu when the uninstall
     // dialog shows up).
     ExtensionContextMenuModel menu(extension, GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     menu.ExecuteCommand(ExtensionContextMenuModel::UNINSTALL, 0);
   }
@@ -673,7 +672,7 @@ TEST_F(ExtensionContextMenuModelTest, TestPageAccessSubmenu) {
       extension, UserScript::DOCUMENT_IDLE, increment_run_count);
 
   ExtensionContextMenuModel menu(extension, GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
 
   EXPECT_NE(-1, menu.GetIndexOfCommandId(
@@ -799,7 +798,7 @@ TEST_F(ExtensionContextMenuModelTest, TestPageAccessSubmenu) {
       "single_host_extension", manifest_keys::kBrowserAction,
       Manifest::INTERNAL, "http://www.example.com/*");
   ExtensionContextMenuModel single_host_menu(
-      single_host_extension, GetBrowser(), ExtensionContextMenuModel::VISIBLE,
+      single_host_extension, GetBrowser(), ExtensionContextMenuModel::PINNED,
       nullptr, true);
   EXPECT_NE(-1, single_host_menu.GetIndexOfCommandId(
                     ExtensionContextMenuModel::PAGE_ACCESS_SUBMENU));
@@ -812,7 +811,7 @@ TEST_F(ExtensionContextMenuModelTest, TestInspectPopupPresence) {
         "page_action", manifest_keys::kPageAction, Manifest::INTERNAL);
     ASSERT_TRUE(page_action);
     ExtensionContextMenuModel menu(page_action, GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     int inspect_popup_index =
         menu.GetIndexOfCommandId(ExtensionContextMenuModel::INSPECT_POPUP);
@@ -822,7 +821,7 @@ TEST_F(ExtensionContextMenuModelTest, TestInspectPopupPresence) {
     const Extension* browser_action = AddExtension(
         "browser_action", manifest_keys::kBrowserAction, Manifest::INTERNAL);
     ExtensionContextMenuModel menu(browser_action, GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     int inspect_popup_index =
         menu.GetIndexOfCommandId(ExtensionContextMenuModel::INSPECT_POPUP);
@@ -834,7 +833,7 @@ TEST_F(ExtensionContextMenuModelTest, TestInspectPopupPresence) {
     const Extension* no_action = AddExtension(
         "no_action", nullptr, Manifest::INTERNAL);
     ExtensionContextMenuModel menu(no_action, GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     int inspect_popup_index =
         menu.GetIndexOfCommandId(ExtensionContextMenuModel::INSPECT_POPUP);
@@ -1033,7 +1032,7 @@ TEST_F(ExtensionContextMenuModelTest, PageAccessMenuOptions) {
     web_contents_tester->NavigateAndCommit(test_case.current_url);
 
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
 
     EXPECT_EQ(test_case.selected_entry.has_value(),
@@ -1132,7 +1131,7 @@ TEST_F(ExtensionContextMenuModelTest, PageAccessSubmenu_OnSiteWithAllURLs) {
   // urls.
   AddTab(kActiveUrl);
   ExtensionContextMenuModel menu(extension, GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
   EXPECT_TRUE(HasPageAccessSubmenu(menu));
   EXPECT_FALSE(menu.IsCommandIdChecked(kOnClick));
@@ -1199,7 +1198,7 @@ TEST_F(ExtensionContextMenuModelTest,
   // can't access the active url.
   AddTab(kActiveUrl);
   ExtensionContextMenuModel menu(extension, GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
   EXPECT_TRUE(HasPageAccessSubmenu(menu));
   EXPECT_FALSE(menu.IsCommandIdChecked(kOnClick));
@@ -1246,7 +1245,7 @@ TEST_F(ExtensionContextMenuModelTest, PageAccessWithActiveTab) {
   AddTab(GURL("https://a.com"));
 
   ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
   EXPECT_EQ(CommandState::kEnabled, GetPageAccessCommandState(menu, kOnClick));
   EXPECT_EQ(CommandState::kDisabled, GetPageAccessCommandState(menu, kOnSite));
@@ -1289,7 +1288,7 @@ TEST_F(ExtensionContextMenuModelTest,
 
   {
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
 
     // Without withholding host permissions, the menu should be visible on
@@ -1318,7 +1317,7 @@ TEST_F(ExtensionContextMenuModelTest,
   {
     // ... but not on b.com, where it doesn't want to run.
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     EXPECT_FALSE(HasPageAccessSubmenu(menu));
     EXPECT_TRUE(HasCantAccessPageEntry(menu));
@@ -1333,7 +1332,7 @@ TEST_F(ExtensionContextMenuModelTest,
 
   {
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true);
     EXPECT_TRUE(HasPageAccessSubmenu(menu));
     EXPECT_FALSE(HasCantAccessPageEntry(menu));
@@ -1360,7 +1359,7 @@ TEST_F(ExtensionContextMenuModelTest,
   }
 
   ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
   // Somewhat strangely, this also removes the access controls, because we don't
   // show it for sites the extension doesn't want to run on.
@@ -1392,7 +1391,7 @@ TEST_F(ExtensionContextMenuModelTest,
   AddTab(a_com);
 
   ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
 
   EXPECT_EQ(CommandState::kEnabled, GetPageAccessCommandState(menu, kOnClick));
@@ -1437,7 +1436,7 @@ TEST_F(ExtensionContextMenuModelTest, TestClickingPageAccessLearnMore) {
 
   Browser* browser = GetBrowser();
   ExtensionContextMenuModel menu(extension.get(), browser,
-                                 ExtensionContextMenuModel::VISIBLE, nullptr,
+                                 ExtensionContextMenuModel::PINNED, nullptr,
                                  true);
   EXPECT_EQ(0, user_action_tester.GetActionCount(kLearnMoreAction));
 
@@ -1471,8 +1470,8 @@ TEST_F(ExtensionContextMenuModelTest, HistogramTest_Basic) {
     {
       // The menu is constructed, but never shown.
       ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                     ExtensionContextMenuModel::VISIBLE,
-                                     nullptr, true);
+                                     ExtensionContextMenuModel::PINNED, nullptr,
+                                     true);
     }
     tester.ExpectTotalCount(kHistogramName, 0);
   }
@@ -1482,8 +1481,8 @@ TEST_F(ExtensionContextMenuModelTest, HistogramTest_Basic) {
     {
       // The menu is constructed and shown, but no action is taken.
       ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                     ExtensionContextMenuModel::VISIBLE,
-                                     nullptr, true);
+                                     ExtensionContextMenuModel::PINNED, nullptr,
+                                     true);
       menu.OnMenuWillShow(&menu);
       menu.MenuClosed(&menu);
     }
@@ -1497,8 +1496,8 @@ TEST_F(ExtensionContextMenuModelTest, HistogramTest_Basic) {
     {
       // The menu is constructed, shown, and an action taken.
       ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                     ExtensionContextMenuModel::VISIBLE,
-                                     nullptr, true);
+                                     ExtensionContextMenuModel::PINNED, nullptr,
+                                     true);
       menu.OnMenuWillShow(&menu);
       menu.ExecuteCommand(ExtensionContextMenuModel::MANAGE_EXTENSIONS, 0);
       menu.MenuClosed(&menu);
@@ -1548,14 +1547,14 @@ TEST_F(ExtensionContextMenuModelTest, HideToggleVisibility) {
   InitializeAndAddExtension(*extension);
   {
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    true /* can_show_icon_in_toolbar */);
     EXPECT_TRUE(
         menu.IsCommandIdVisible(ExtensionContextMenuModel::TOGGLE_VISIBILITY));
   }
   {
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
-                                   ExtensionContextMenuModel::VISIBLE, nullptr,
+                                   ExtensionContextMenuModel::PINNED, nullptr,
                                    false /* can_show_icon_in_toolbar */);
     EXPECT_FALSE(
         menu.IsCommandIdVisible(ExtensionContextMenuModel::TOGGLE_VISIBILITY));

chrome/browser/extensions/extension_keybinding_apitest.cc

@@ -381,9 +381,9 @@ IN_PROC_BROWSER_TEST_F(CommandsApiTest, InactivePageActionDoesntTrigger) {
                        "pageAction.onClicked", expect_dispatch);
 }
 
-// Tests that a page action that is overflowed will still properly trigger when
-// the keybinding is used.
-IN_PROC_BROWSER_TEST_F(CommandsApiTest, OverflowedPageActionTriggers) {
+// Tests that a page action that is unpinned and only shown within the
+// extensions menu will still properly trigger when the keybinding is used.
+IN_PROC_BROWSER_TEST_F(CommandsApiTest, UnpinnedPageActionTriggers) {
   base::AutoReset<bool> disable_toolbar_animations(
       &ToolbarActionsBar::disable_animations_for_testing_, true);
 

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

@@ -503,12 +503,12 @@ extensions::ExtensionContextMenuModel::ButtonVisibility
 ToolbarActionsBar::GetActionVisibility(
     const ToolbarActionViewController* action) const {
   extensions::ExtensionContextMenuModel::ButtonVisibility visibility =
-      extensions::ExtensionContextMenuModel::VISIBLE;
+      extensions::ExtensionContextMenuModel::PINNED;
 
   if (GetPoppedOutAction() == action) {
     visibility = extensions::ExtensionContextMenuModel::TRANSITIVELY_VISIBLE;
   } else if (!IsActionVisibleOnToolbar(action)) {
-    visibility = extensions::ExtensionContextMenuModel::OVERFLOWED;
+    visibility = extensions::ExtensionContextMenuModel::UNPINNED;
   }
   return visibility;
 }

chrome/browser/ui/views/extensions/extensions_menu_view_browsertest.cc

@@ -91,7 +91,7 @@ class ExtensionsMenuViewBrowserTest : public ExtensionsToolbarBrowserTest {
           "ExtensionUninstallDialogDelegateView");
       extensions::ExtensionContextMenuModel menu_model(
           extensions()[0].get(), browser(),
-          extensions::ExtensionContextMenuModel::VISIBLE, nullptr,
+          extensions::ExtensionContextMenuModel::PINNED, nullptr,
           false /* can_show_icon_in_toolbar */);
       menu_model.ExecuteCommand(
           extensions::ExtensionContextMenuModel::UNINSTALL, 0);
@@ -557,7 +557,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionsMenuViewBrowserTest,
   // browser.
   extensions::ExtensionContextMenuModel menu(
       extensions()[0].get(), incognito_browser(),
-      extensions::ExtensionContextMenuModel::VISIBLE, nullptr,
+      extensions::ExtensionContextMenuModel::PINNED, nullptr,
       true /* can_show_icon_in_toolbar */);
   EXPECT_FALSE(menu.IsCommandIdEnabled(
       extensions::ExtensionContextMenuModel::TOGGLE_VISIBILITY));

chrome/browser/ui/views/extensions/extensions_toolbar_container.cc

@@ -276,13 +276,13 @@ extensions::ExtensionContextMenuModel::ButtonVisibility
 ExtensionsToolbarContainer::GetActionVisibility(
     const ToolbarActionViewController* action) const {
   extensions::ExtensionContextMenuModel::ButtonVisibility visibility =
-      extensions::ExtensionContextMenuModel::VISIBLE;
+      extensions::ExtensionContextMenuModel::PINNED;
 
   if (ShouldForceVisibility(action->GetId()) &&
       !model_->IsActionPinned(action->GetId())) {
     visibility = extensions::ExtensionContextMenuModel::TRANSITIVELY_VISIBLE;
   } else if (!IsActionVisibleOnToolbar(action)) {
-    visibility = extensions::ExtensionContextMenuModel::OVERFLOWED;
+    visibility = extensions::ExtensionContextMenuModel::UNPINNED;
   }
   return visibility;
 }