[Extensions UI] Remove unnecessary extensions menu switch logic in tests

Remove test logic that checks for whether the ExtensionsToolbarMenu
feature is enabled. Since the feature has launched, these branches are
no longer necessary or useful.

This CL should have no functional behavior change.

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

chrome/browser/extensions/extension_context_menu_model.h

@@ -71,6 +71,7 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
 
   // The current visibility of the button; this can affect the "hide"/"show"
   // strings in the menu.
+  // TODO(devlin): Update these to be more appropriate for the extensions menu.
   enum ButtonVisibility {
     // The button is visible on the toolbar.
     VISIBLE,

chrome/browser/extensions/extension_context_menu_model_unittest.cc

@@ -7,7 +7,6 @@
 #include <utility>
 
 #include "base/bind.h"
-#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
@@ -29,7 +28,6 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/common/extensions/api/context_menus.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
@@ -547,9 +545,8 @@ TEST_F(ExtensionContextMenuModelTest,
       ExtensionContextMenuModel::PAGE_ACCESS_RUN_ON_ALL_SITES));
 }
 
-// Test that the "show" and "hide" menu items appear correctly in the extension
-// context menu. When kExtensionsToolbarMenu is enabled, the "hide" is instead
-// an "unpin" menu item.
+// Test that the "pin" and "unpin" menu items appear correctly in the extension
+// context menu.
 TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
   InitializeEmptyExtensionService();
   Browser* browser = GetBrowser();
@@ -566,18 +563,10 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
   // For laziness.
   const ExtensionContextMenuModel::MenuEntries visibility_command =
       ExtensionContextMenuModel::TOGGLE_VISIBILITY;
-  base::string16 hide_string = l10n_util::GetStringUTF16(
-      base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)
-          ? IDS_EXTENSIONS_UNPIN_FROM_TOOLBAR
-          : IDS_EXTENSIONS_HIDE_BUTTON_IN_MENU);
-  base::string16 show_string = l10n_util::GetStringUTF16(
-      base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)
-          ? IDS_EXTENSIONS_PIN_TO_TOOLBAR
-          : IDS_EXTENSIONS_SHOW_BUTTON_IN_TOOLBAR);
-  base::string16 keep_string = l10n_util::GetStringUTF16(
-      base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)
-          ? IDS_EXTENSIONS_PIN_TO_TOOLBAR
-          : IDS_EXTENSIONS_KEEP_BUTTON_IN_TOOLBAR);
+  const base::string16 pin_string =
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_PIN_TO_TOOLBAR);
+  const base::string16 unpin_string =
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNPIN_FROM_TOOLBAR);
 
   {
     // Even page actions should have a visibility option.
@@ -586,7 +575,7 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
                                    true);
     int index = menu.GetIndexOfCommandId(visibility_command);
     EXPECT_NE(-1, index);
-    EXPECT_EQ(hide_string, menu.GetLabelAt(index));
+    EXPECT_EQ(unpin_string, menu.GetLabelAt(index));
   }
 
   {
@@ -595,36 +584,33 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
                                    true);
     int index = menu.GetIndexOfCommandId(visibility_command);
     EXPECT_NE(-1, index);
-    EXPECT_EQ(hide_string, menu.GetLabelAt(index));
+    EXPECT_EQ(unpin_string, menu.GetLabelAt(index));
 
-    if (base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)) {
-      // Pin before unpinning.
-      ToolbarActionsModel::Get(profile())->SetActionVisibility(
-          browser_action->id(), true);
-    }
+    // Pin before unpinning.
+    ToolbarActionsModel::Get(profile())->SetActionVisibility(
+        browser_action->id(), true);
     menu.ExecuteCommand(visibility_command, 0);
   }
 
   {
-    // If the action is overflowed, it should have the "Show button in toolbar"
-    // string.
+    // If the action is unpinned, it should have the "Pin" string.
     ExtensionContextMenuModel menu(browser_action, browser,
                                    ExtensionContextMenuModel::OVERFLOWED,
                                    nullptr, true);
     int index = menu.GetIndexOfCommandId(visibility_command);
     EXPECT_NE(-1, index);
-    EXPECT_EQ(show_string, menu.GetLabelAt(index));
+    EXPECT_EQ(pin_string, menu.GetLabelAt(index));
   }
 
   {
     // If the action is transitively visible, as happens when it is showing a
-    // popup, we should use a "Keep button in toolbar" string.
+    // popup, we should use the same "Pin" string.
     ExtensionContextMenuModel menu(
         browser_action, browser,
         ExtensionContextMenuModel::TRANSITIVELY_VISIBLE, nullptr, true);
     int index = menu.GetIndexOfCommandId(visibility_command);
     EXPECT_NE(-1, index);
-    EXPECT_EQ(keep_string, menu.GetLabelAt(index));
+    EXPECT_EQ(pin_string, menu.GetLabelAt(index));
   }
 }
 

chrome/browser/extensions/extension_keybinding_apitest.cc

@@ -22,7 +22,6 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/sessions/content/session_tab_helper.h"
@@ -36,7 +35,6 @@
 #include "extensions/browser/test_event_router_observer.h"
 #include "extensions/common/api/extension_action/action_info_test_util.h"
 #include "extensions/common/extension.h"
-#include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/test/extension_test_message_listener.h"
@@ -394,12 +392,6 @@ IN_PROC_BROWSER_TEST_F(CommandsApiTest, OverflowedPageActionTriggers) {
   const Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(extension) << message_;
 
-  // With the old toolbar, we need to explicitly overflow the extension.
-  if (!base::FeatureList::IsEnabled(features::kExtensionsToolbarMenu)) {
-    ToolbarActionsModel* toolbar_actions_model =
-        ToolbarActionsModel::Get(profile());
-    toolbar_actions_model->SetVisibleIconCount(0);
-  }
   std::unique_ptr<ExtensionActionTestHelper> test_helper =
       ExtensionActionTestHelper::Create(browser());
   RunScheduledLayouts();

chrome/browser/ui/toolbar/toolbar_actions_model_browsertest.cc

@@ -5,22 +5,17 @@
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 
 #include "base/test/metrics/histogram_tester.h"
-#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "content/public/test/browser_test.h"
 
 class ToolbarActionsModelBrowserTest : public extensions::ExtensionBrowserTest {
  public:
-  ToolbarActionsModelBrowserTest() {
-    scoped_feature_list_.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-  }
+  ToolbarActionsModelBrowserTest() = default;
   ~ToolbarActionsModelBrowserTest() override = default;
 
   base::HistogramTester* histogram_tester() { return &histogram_tester_; }
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
   base::HistogramTester histogram_tester_;
 };
 

chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc

@@ -1233,9 +1233,6 @@ TEST_F(ToolbarActionsModelUnitTest, AddUserScriptExtension) {
 }
 
 TEST_F(ToolbarActionsModelUnitTest, IsActionPinnedCorrespondsToPinningState) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1253,9 +1250,6 @@ TEST_F(ToolbarActionsModelUnitTest, IsActionPinnedCorrespondsToPinningState) {
 
 TEST_F(ToolbarActionsModelUnitTest,
        TogglingVisibilityAppendsToPinnedExtensions) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1290,9 +1284,6 @@ TEST_F(ToolbarActionsModelUnitTest,
 }
 
 TEST_F(ToolbarActionsModelUnitTest, ChangesToPinningNotifiesObserver) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1310,9 +1301,6 @@ TEST_F(ToolbarActionsModelUnitTest, ChangesToPinningNotifiesObserver) {
 }
 
 TEST_F(ToolbarActionsModelUnitTest, ChangesToPinningSavedInExtensionPrefs) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1346,9 +1334,6 @@ TEST_F(ToolbarActionsModelUnitTest, ChangesToPinningSavedInExtensionPrefs) {
 }
 
 TEST_F(ToolbarActionsModelUnitTest, ChangesToExtensionPrefsReflectedInModel) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1373,9 +1358,6 @@ TEST_F(ToolbarActionsModelUnitTest, ChangesToExtensionPrefsReflectedInModel) {
 
 TEST_F(ToolbarActionsModelUnitTest,
        MismatchInPinnedExtensionPreferencesNotReflectedInModel) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1405,9 +1387,6 @@ TEST_F(ToolbarActionsModelUnitTest,
 }
 
 TEST_F(ToolbarActionsModelUnitTest, PinnedExtensionsFilteredOnInitialization) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1446,9 +1425,6 @@ TEST_F(ToolbarActionsModelUnitTest, PinnedExtensionsFilteredOnInitialization) {
 }
 
 TEST_F(ToolbarActionsModelUnitTest, ChangesToPinnedOrderSavedInExtensionPrefs) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
   ASSERT_TRUE(AddBrowserActionExtensions());
 
@@ -1497,15 +1473,16 @@ TEST_F(ToolbarActionsModelUnitTest,
   // Add the three browser action extensions.
   ASSERT_TRUE(AddBrowserActionExtensions());
 
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
+  extensions::ExtensionPrefs* const extension_prefs =
+      extensions::ExtensionPrefs::Get(profile());
+  EXPECT_FALSE(extension_prefs->IsPinnedExtensionsMigrationComplete());
+
   // Initialization of the toolbar model triggers migration of the visible
   // extensions to pinned extensions.
   InitToolbarModelAndObserver();
 
   // Verify that the extensions that were visible are now the pinned extensions.
-  extensions::ExtensionPrefs* const extension_prefs =
-      extensions::ExtensionPrefs::Get(profile());
+  EXPECT_TRUE(extension_prefs->IsPinnedExtensionsMigrationComplete());
   EXPECT_THAT(
       extension_prefs->GetPinnedExtensions(),
       testing::ElementsAre(browser_action_a()->id(), browser_action_b()->id(),
@@ -1520,24 +1497,22 @@ TEST_F(ToolbarActionsModelUnitTest,
   // Add the three browser action extensions.
   ASSERT_TRUE(AddBrowserActionExtensions());
 
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
+  extensions::ExtensionPrefs* const extension_prefs =
+      extensions::ExtensionPrefs::Get(profile());
+  EXPECT_FALSE(extension_prefs->IsPinnedExtensionsMigrationComplete());
+
   // Initialization of the toolbar model triggers migration of the visible
   // extensions to pinned extensions.
   InitToolbarModelAndObserver();
 
   // Verify that the extensions that were visible are now the pinned extensions.
-  extensions::ExtensionPrefs* const extension_prefs =
-      extensions::ExtensionPrefs::Get(profile());
+  EXPECT_TRUE(extension_prefs->IsPinnedExtensionsMigrationComplete());
   EXPECT_THAT(
       extension_prefs->GetPinnedExtensions(),
       testing::ElementsAre(browser_action_a()->id(), browser_action_b()->id()));
 }
 
 TEST_F(ToolbarActionsModelUnitTest, PinStateErasedOnUninstallation) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-
   Init();
 
   scoped_refptr<const extensions::Extension> extension =

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

@@ -6,10 +6,8 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/user_action_tester.h"
-#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/toolbar/test_toolbar_action_view_controller.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/extensions/extensions_menu_button.h"
 #include "chrome/browser/ui/views/hover_button_controller.h"
 #include "chrome/browser/ui/views/native_widget_factory.h"
@@ -23,7 +21,6 @@ class ExtensionsMenuItemViewTest : public BrowserWithTestWindowTest {
       : initial_extension_name_(base::ASCIIToUTF16("Initial Extension Name")),
         initial_tooltip_(base::ASCIIToUTF16("Initial tooltip")) {}
   void SetUp() override {
-    scoped_feature_list_.InitAndEnableFeature(features::kExtensionsToolbarMenu);
     BrowserWithTestWindowTest::SetUp();
 
     widget_ = std::make_unique<views::Widget>();
@@ -64,7 +61,6 @@ class ExtensionsMenuItemViewTest : public BrowserWithTestWindowTest {
 
   ExtensionsMenuButton* primary_button() { return primary_button_on_menu_; }
 
-  base::test::ScopedFeatureList scoped_feature_list_;
   const base::string16 initial_extension_name_;
   const base::string16 initial_tooltip_;
   std::unique_ptr<views::Widget> widget_;

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

@@ -8,11 +8,11 @@
 #include <string>
 #include <vector>
 
+#include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/user_action_tester.h"
-#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/chrome_test_extension_loader.h"
 #include "chrome/browser/extensions/extension_action_test_util.h"
@@ -21,7 +21,6 @@
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/extensions/extensions_menu_button.h"
 #include "chrome/browser/ui/views/extensions/extensions_menu_item_view.h"
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_button.h"
@@ -77,9 +76,7 @@ class ExtensionsMenuViewUnitTest : public TestWithBrowserView {
  public:
   ExtensionsMenuViewUnitTest()
       : allow_extension_menu_instances_(
-            ExtensionsMenuView::AllowInstancesForTesting()) {
-    feature_list_.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-  }
+            ExtensionsMenuView::AllowInstancesForTesting()) {}
   ~ExtensionsMenuViewUnitTest() override = default;
 
   // TestWithBrowserView:
@@ -126,7 +123,6 @@ class ExtensionsMenuViewUnitTest : public TestWithBrowserView {
 
  private:
   base::AutoReset<bool> allow_extension_menu_instances_;
-  base::test::ScopedFeatureList feature_list_;
 
   extensions::ExtensionService* extension_service_ = nullptr;
 

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

@@ -3,12 +3,10 @@
 // found in the LICENSE file.
 
 #include "base/run_loop.h"
-#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/extensions/chrome_test_extension_loader.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_controller.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
-#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/extensions/extensions_toolbar_container.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
@@ -24,9 +22,7 @@
 class ExtensionsToolbarInteractiveUiTest
     : public extensions::ExtensionBrowserTest {
  public:
-  ExtensionsToolbarInteractiveUiTest() {
-    feature_list_.InitAndEnableFeature(features::kExtensionsToolbarMenu);
-  }
+  ExtensionsToolbarInteractiveUiTest() = default;
 
   ExtensionsToolbarContainer* GetExtensionsToolbarContainer() {
     return BrowserView::GetBrowserViewForBrowser(browser())
@@ -39,7 +35,6 @@ class ExtensionsToolbarInteractiveUiTest
   }
 
  private:
-  base::test::ScopedFeatureList feature_list_;
   ui::ScopedAnimationDurationScaleMode disable_animation_{
       ui::ScopedAnimationDurationScaleMode::ZERO_DURATION};
 };