[Extensions] Fix hiding browser actions without the toolbar redesign

chrome/browser/extensions/extension_context_menu_model_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_context_menu_model.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/extension_action_test_util.h"
 #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/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/test_browser_window.h"
@@ skipping 270 matching lines @@
   // We start at 1, so this should try to add the limit + 1.
   for (int i = 0; i < api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT; ++i)
     builder.AddContextItem(MenuItem::PAGE_ACTION);
 
   // We shouldn't go above the limit of top-level items.
   EXPECT_EQ(api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT,
             CountExtensionItems(*builder.BuildMenu()));
 }
 
 // Test that the "show" and "hide" menu items appear correctly in the extension
-// context menu.
-TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
+// context menu without the toolbar redesign.
+TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHideLegacy) {
+  // Start with the toolbar redesign disabled.
+  scoped_ptr<FeatureSwitch::ScopedOverride> toolbar_redesign_override(
+      new FeatureSwitch::ScopedOverride(
+          FeatureSwitch::extension_action_redesign(), false));
+
   InitializeEmptyExtensionService();
+  Browser* browser = GetBrowser();
+  extension_action_test_util::CreateToolbarModelForProfile(profile());
   scoped_refptr<const Extension> page_action =
       BuildExtension("page_action_extension",
                      manifest_keys::kPageAction,
                      Manifest::INTERNAL);
   ASSERT_TRUE(page_action.get());
   scoped_refptr<const Extension> browser_action =
       BuildExtension("browser_action_extension",
                      manifest_keys::kBrowserAction,
                      Manifest::INTERNAL);
   ASSERT_TRUE(browser_action.get());
 
   service()->AddExtension(page_action.get());
   service()->AddExtension(browser_action.get());
 
   // For laziness.
   const ExtensionContextMenuModel::MenuEntries visibility_command =
       ExtensionContextMenuModel::TOGGLE_VISIBILITY;
   base::string16 hide_string =
       l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_BUTTON);
-  base::string16 redesign_hide_string =
-      l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_BUTTON_IN_MENU);
-  base::string16 redesign_show_string =
-      l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_BUTTON_IN_TOOLBAR);
-  base::string16 redesign_keep_string =
-      l10n_util::GetStringUTF16(IDS_EXTENSIONS_KEEP_BUTTON_IN_TOOLBAR);
 
   {
-    ExtensionContextMenuModel menu(page_action.get(), GetBrowser(),
+    ExtensionContextMenuModel menu(page_action.get(), browser,
                                    ExtensionContextMenuModel::VISIBLE, nullptr);
 
     int index = GetCommandIndex(menu, visibility_command);
     // Without the toolbar redesign switch, page action menus shouldn't have a
     // visibility option.
     EXPECT_EQ(-1, index);
   }
 
   {
-    ExtensionContextMenuModel menu(browser_action.get(), GetBrowser(),
+    ExtensionContextMenuModel menu(browser_action.get(), browser,
                                    ExtensionContextMenuModel::VISIBLE, nullptr);
     int index = GetCommandIndex(menu, visibility_command);
     // Browser actions should have the visibility option.
     EXPECT_NE(-1, index);
     // Since the action is currently visible, it should have the option to hide
     // it.
     EXPECT_EQ(hide_string, menu.GetLabelAt(index));
   }
 
-  // Enabling the toolbar redesign switch should give page actions the button.
-  FeatureSwitch::ScopedOverride enable_toolbar_redesign(
-      FeatureSwitch::extension_action_redesign(), true);
   {
-    ExtensionContextMenuModel menu(page_action.get(), GetBrowser(),
+    ExtensionContextMenuModel menu(browser_action.get(), browser,
+                                   ExtensionContextMenuModel::OVERFLOWED,
+                                   nullptr);
+    int index = GetCommandIndex(menu, visibility_command);
+    EXPECT_NE(-1, index);
+    // Without the redesign, 'hiding' refers to removing the action from the
+    // toolbar entirely, so even with the action overflowed, the string should
+    // be 'Hide action'.
+    EXPECT_EQ(hide_string, menu.GetLabelAt(index));
+
+    ExtensionActionAPI* action_api = ExtensionActionAPI::Get(profile());
+    // At the start, the action should be visible.
+    EXPECT_TRUE(action_api->GetBrowserActionVisibility(browser_action->id()));
+    menu.ExecuteCommand(visibility_command, 0);
+    // After execution, it should be hidden.
+    EXPECT_FALSE(action_api->GetBrowserActionVisibility(browser_action->id()));
+
+    // Cleanup - make the action visible again.
+    action_api->SetBrowserActionVisibility(browser_action->id(), true);
+  }
+}
+
+// Test that the "show" and "hide" menu items appear correctly in the extension
+// context menu with the toolbar redesign.
+TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHideRedesign) {
+  // Start with the toolbar redesign disabled.
+  scoped_ptr<FeatureSwitch::ScopedOverride> toolbar_redesign_override(
+      new FeatureSwitch::ScopedOverride(
+          FeatureSwitch::extension_action_redesign(), true));
+
+  InitializeEmptyExtensionService();
+  Browser* browser = GetBrowser();
+  extension_action_test_util::CreateToolbarModelForProfile(profile());
+  scoped_refptr<const Extension> page_action =
+      BuildExtension("page_action_extension",
+                     manifest_keys::kPageAction,
+                     Manifest::INTERNAL);
+  ASSERT_TRUE(page_action.get());
+  scoped_refptr<const Extension> browser_action =
+      BuildExtension("browser_action_extension",
+                     manifest_keys::kBrowserAction,
+                     Manifest::INTERNAL);
+  ASSERT_TRUE(browser_action.get());
+
+  service()->AddExtension(page_action.get());
+  service()->AddExtension(browser_action.get());
+
+  // For laziness.
+  const ExtensionContextMenuModel::MenuEntries visibility_command =
+      ExtensionContextMenuModel::TOGGLE_VISIBILITY;
+  base::string16 redesign_hide_string =
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_BUTTON_IN_MENU);
+  base::string16 redesign_show_string =
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_BUTTON_IN_TOOLBAR);
+  base::string16 redesign_keep_string =
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_KEEP_BUTTON_IN_TOOLBAR);
+
+  {
+    // Even page actions should have a visibility option with the redesign on.
+    ExtensionContextMenuModel menu(page_action.get(), browser,
                                    ExtensionContextMenuModel::VISIBLE, nullptr);
     int index = GetCommandIndex(menu, visibility_command);
     EXPECT_NE(-1, index);
     EXPECT_EQ(redesign_hide_string, menu.GetLabelAt(index));
   }
 
   {
+    ExtensionContextMenuModel menu(browser_action.get(), browser,
+                                   ExtensionContextMenuModel::VISIBLE, nullptr);
+    int index = GetCommandIndex(menu, visibility_command);
+    EXPECT_NE(-1, index);
+    EXPECT_EQ(redesign_hide_string, menu.GetLabelAt(index));
+
+    ExtensionActionAPI* action_api = ExtensionActionAPI::Get(profile());
+    EXPECT_TRUE(action_api->GetBrowserActionVisibility(browser_action->id()));
+    // Executing the 'hide' command shouldn't modify the prefs with the redesign
+    // turned on (the ordering behavior is tested in ToolbarActionsModel tests).
+    menu.ExecuteCommand(visibility_command, 0);
+    EXPECT_TRUE(action_api->GetBrowserActionVisibility(browser_action->id()));
+  }
+
+  {
     // If the action is overflowed, it should have the "Show button in toolbar"
     // string.
-    ExtensionContextMenuModel menu(browser_action.get(), GetBrowser(),
+    ExtensionContextMenuModel menu(browser_action.get(), browser,
                                    ExtensionContextMenuModel::OVERFLOWED,
                                    nullptr);
     int index = GetCommandIndex(menu, visibility_command);
     EXPECT_NE(-1, index);
     EXPECT_EQ(redesign_show_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.
     ExtensionContextMenuModel menu(
-        browser_action.get(), GetBrowser(),
+        browser_action.get(), browser,
         ExtensionContextMenuModel::TRANSITIVELY_VISIBLE, nullptr);
     int index = GetCommandIndex(menu, visibility_command);
     EXPECT_NE(-1, index);
     EXPECT_EQ(redesign_keep_string, menu.GetLabelAt(index));
   }
 }
 
 TEST_F(ExtensionContextMenuModelTest, ExtensionContextUninstall) {
   InitializeEmptyExtensionService();
 
@@ skipping 13 matching lines @@
     ExtensionContextMenuModel menu(extension.get(), GetBrowser(),
                                    ExtensionContextMenuModel::VISIBLE, nullptr);
     menu.ExecuteCommand(ExtensionContextMenuModel::UNINSTALL, 0);
   }
   uninstalled_observer.WaitForExtensionUninstalled();
   EXPECT_FALSE(registry()->GetExtensionById(extension_id,
                                             ExtensionRegistry::EVERYTHING));
 }
 
 }  // namespace extensions