Make hiding an extension action cause it to go to the overflow menu

chrome/browser/extensions/extension_context_menu_model_unittest.cc

Index: chrome/browser/extensions/extension_context_menu_model_unittest.cc
diff --git a/chrome/browser/extensions/extension_context_menu_model_unittest.cc b/chrome/browser/extensions/extension_context_menu_model_unittest.cc
index 3fe636ba9f81226e5e7efa18cac1c647215e9882..51d645c40d7514ac7854eb1af2df5c0edb375fb8 100644
--- a/chrome/browser/extensions/extension_context_menu_model_unittest.cc
+++ b/chrome/browser/extensions/extension_context_menu_model_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/extension_context_menu_model.h"
 
+#include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_service_test_base.h"
 #include "chrome/browser/extensions/menu_manager.h"
@@ -13,26 +14,66 @@
 #include "chrome/common/extensions/api/context_menus.h"
 #include "chrome/test/base/test_browser_window.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/crx_file/id_util.h"
+#include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/test_management_policy.h"
 #include "extensions/common/extension_builder.h"
+#include "extensions/common/feature_switch.h"
+#include "extensions/common/manifest_constants.h"
 #include "extensions/common/value_builder.h"
+#include "grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace extensions {
 
+namespace {
+
+// Build an extension to pass to the menu constructor, with the an action
+// specified by |action_key|.
+scoped_refptr<const Extension> BuildExtension(const std::string& name,
+                                              const char* action_key) {
+  return ExtensionBuilder()
+      .SetManifest(DictionaryBuilder()
+                       .Set("name", name)
+                       .Set("version", "1")
+                       .Set("manifest_version", 2)
+                       .Set(action_key, DictionaryBuilder().Pass()))
+      .SetID(crx_file::id_util::GenerateId(name))
+      .Build();
+}
+
+// Create a Browser for the ExtensionContextMenuModel to use.
+scoped_ptr<Browser> CreateBrowser(Profile* profile) {
+  Browser::CreateParams params(profile, chrome::GetActiveDesktop());
+  TestBrowserWindow test_window;
+  params.window = &test_window;
+  return scoped_ptr<Browser>(new Browser(params));
+}
+
+// Returns the index of the given |command_id| in the given |menu|, or -1 if it
+// is not found.
+int GetCommandIndex(const scoped_refptr<ExtensionContextMenuModel> menu,
+                         int command_id) {
+  int item_count = menu->GetItemCount();
+  for (int i = 0; i < item_count; ++i) {
+    if (menu->GetCommandIdAt(i) == command_id)
+      return i;
+  }
+  return -1;
+}
+
+}  // namespace
+
 class ExtensionContextMenuModelTest : public ExtensionServiceTestBase {
  public:
   ExtensionContextMenuModelTest();
 
-  // Build an extension to pass to the menu constructor.  It needs an
-  // ExtensionAction.
-  scoped_refptr<Extension> BuildExtension();
-
   // Creates an extension menu item for |extension| with the given |context|
   // and adds it to |manager|. Refreshes |model| to show new item.
   void AddContextItemAndRefreshModel(MenuManager* manager,
-                                     Extension* extension,
+                                     const Extension* extension,
                                      MenuItem::Context context,
                                      ExtensionContextMenuModel* model);
 
@@ -49,20 +90,10 @@ class ExtensionContextMenuModelTest : public ExtensionServiceTestBase {
 ExtensionContextMenuModelTest::ExtensionContextMenuModelTest() : cur_id_(0) {
 }
 
-scoped_refptr<Extension> ExtensionContextMenuModelTest::BuildExtension() {
-  return ExtensionBuilder()
-      .SetManifest(DictionaryBuilder()
-                       .Set("name", "Page Action Extension")
-                       .Set("version", "1")
-                       .Set("manifest_version", 2)
-                       .Set("page_action",
-                            DictionaryBuilder().Set("default_title", "Hello")))
-      .Build();
-}
 
 void ExtensionContextMenuModelTest::AddContextItemAndRefreshModel(
     MenuManager* manager,
-    Extension* extension,
+    const Extension* extension,
     MenuItem::Context context,
     ExtensionContextMenuModel* model) {
   MenuItem::Type type = MenuItem::NORMAL;
@@ -90,34 +121,28 @@ int ExtensionContextMenuModelTest::CountExtensionItems(
   return model->extension_items_count_;
 }
 
-namespace {
-
 // Tests that applicable menu items are disabled when a ManagementPolicy
 // prohibits them.
 TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) {
   InitializeEmptyExtensionService();
-  scoped_refptr<Extension> extension = BuildExtension();
+  scoped_refptr<const Extension> extension =
+      BuildExtension("extension", manifest_keys::kPageAction);
   ASSERT_TRUE(extension.get());
-  service_->AddExtension(extension.get());
+  service()->AddExtension(extension.get());
 
-  // Create a Browser for the ExtensionContextMenuModel to use.
-  Browser::CreateParams params(profile_.get(), chrome::GetActiveDesktop());
-  TestBrowserWindow test_window;
-  params.window = &test_window;
-  Browser browser(params);
+  scoped_ptr<Browser> browser = CreateBrowser(profile());
 
   scoped_refptr<ExtensionContextMenuModel> menu(
-      new ExtensionContextMenuModel(extension.get(), &browser));
+      new ExtensionContextMenuModel(extension.get(), browser.get()));
 
-  extensions::ExtensionSystem* system =
-      extensions::ExtensionSystem::Get(profile_.get());
+  ExtensionSystem* system = ExtensionSystem::Get(profile());
   system->management_policy()->UnregisterAllProviders();
 
   // Actions should be enabled.
   ASSERT_TRUE(menu->IsCommandIdEnabled(ExtensionContextMenuModel::UNINSTALL));
 
-  extensions::TestManagementPolicyProvider policy_provider(
-    extensions::TestManagementPolicyProvider::PROHIBIT_MODIFY_STATUS);
+  TestManagementPolicyProvider policy_provider(
+      TestManagementPolicyProvider::PROHIBIT_MODIFY_STATUS);
   system->management_policy()->RegisterProvider(&policy_provider);
 
   // Now the actions are disabled.
@@ -129,25 +154,22 @@ TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) {
 
 TEST_F(ExtensionContextMenuModelTest, ExtensionItemTest) {
   InitializeEmptyExtensionService();
-  scoped_refptr<Extension> extension = BuildExtension();
+  scoped_refptr<const Extension> extension =
+      BuildExtension("extension", manifest_keys::kPageAction);
   ASSERT_TRUE(extension.get());
-  service_->AddExtension(extension.get());
+  service()->AddExtension(extension.get());
 
-  // Create a Browser for the ExtensionContextMenuModel to use.
-  Browser::CreateParams params(profile_.get(), chrome::GetActiveDesktop());
-  TestBrowserWindow test_window;
-  params.window = &test_window;
-  Browser browser(params);
+  scoped_ptr<Browser> browser = CreateBrowser(profile());
 
   // Create a MenuManager for adding context items.
   MenuManager* manager = static_cast<MenuManager*>(
       (MenuManagerFactory::GetInstance()->SetTestingFactoryAndUse(
-          profile_.get(),
+          profile(),
           &MenuManagerFactory::BuildServiceInstanceForTesting)));
   ASSERT_TRUE(manager);
 
   scoped_refptr<ExtensionContextMenuModel> menu(
-      new ExtensionContextMenuModel(extension.get(), &browser));
+      new ExtensionContextMenuModel(extension.get(), browser.get()));
 
   // There should be no extension items yet.
   EXPECT_EQ(0, CountExtensionItems(menu));
@@ -187,5 +209,67 @@ TEST_F(ExtensionContextMenuModelTest, ExtensionItemTest) {
             CountExtensionItems(menu));
 }
 
-}  // namespace
+// Test that the "show" and "hide" menu items appear correctly in the extension
+// context menu.
+TEST_F(ExtensionContextMenuModelTest, ExtensionContextMenuShowAndHide) {
+  InitializeEmptyExtensionService();
+  scoped_refptr<const Extension> page_action =
+      BuildExtension("page_action_extension", manifest_keys::kPageAction);
+  ASSERT_TRUE(page_action.get());
+  scoped_refptr<const Extension> browser_action =
+      BuildExtension("browser_action_extension", manifest_keys::kBrowserAction);
+  ASSERT_TRUE(browser_action.get());
+
+  service()->AddExtension(page_action.get());
+  service()->AddExtension(browser_action.get());
+
+  scoped_ptr<Browser> browser = CreateBrowser(profile());
+
+  scoped_refptr<ExtensionContextMenuModel> menu(
+      new ExtensionContextMenuModel(page_action.get(), browser.get()));
+
+  // For laziness.
+  const ExtensionContextMenuModel::MenuEntries visibility_command =
+      ExtensionContextMenuModel::TOGGLE_VISIBILITY;
+  base::string16 hide_string =
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_BUTTON);
+  base::string16 show_string =
+      l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_BUTTON);
+  ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
+
+  int index = GetCommandIndex(menu, visibility_command);
+  // Without the toolbar redesign switch, page action menus shouldn't have a
+  // visibility option.
+  EXPECT_EQ(-1, index);
+
+  menu = new ExtensionContextMenuModel(browser_action.get(), browser.get());
+  index = GetCommandIndex(menu, visibility_command);
+  // Browser actions should have the visibility option.
+  EXPECT_NE(-1, index);
+
+  // Enabling the toolbar redesign switch should give page actions the button.
+  FeatureSwitch::ScopedOverride enable_toolbar_redesign(
+      FeatureSwitch::extension_action_redesign(), true);
+  menu = new ExtensionContextMenuModel(page_action.get(), browser.get());
+  index = GetCommandIndex(menu, visibility_command);
+  EXPECT_NE(-1, index);
+
+  // Next, we test the command label.
+  menu = new ExtensionContextMenuModel(browser_action.get(), browser.get());
+  index = GetCommandIndex(menu, visibility_command);
+  // By default, browser actions should be visible (and therefore the button
+  // should be to hide).
+  EXPECT_TRUE(ExtensionActionAPI::GetBrowserActionVisibility(
+                  prefs, browser_action->id()));
+  EXPECT_EQ(hide_string, menu->GetLabelAt(index));
+
+  // Hide the browser action. This should mean the string is "show".
+  ExtensionActionAPI::SetBrowserActionVisibility(
+      prefs, browser_action->id(), false);
+  menu = new ExtensionContextMenuModel(browser_action.get(), browser.get());
+  index = GetCommandIndex(menu, visibility_command);
+  EXPECT_NE(-1, index);
+  EXPECT_EQ(show_string, menu->GetLabelAt(index));
+}
+
 }  // namespace extensions