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

chrome/browser/extensions/extension_context_menu_model.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/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/extensions/active_script_controller.h"
@@ skipping 15 matching lines @@
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/context_menu_params.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/management_policy.h"
 #include "extensions/browser/uninstall_reason.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/feature_switch.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using content::OpenURLParams;
 using content::Referrer;
 using content::WebContents;
 using extensions::Extension;
+using extensions::ExtensionActionAPI;
+using extensions::ExtensionPrefs;
 using extensions::MenuItem;
 using extensions::MenuManager;
 
 namespace {
 
 // Returns true if the given |item| is of the given |type|.
 bool MenuItemMatchesAction(ExtensionContextMenuModel::ActionType type,
                            const MenuItem* item) {
   if (type == ExtensionContextMenuModel::NO_ACTION)
     return false;
 
   const MenuItem::ContextList& contexts = item->contexts();
 
   if (contexts.Contains(MenuItem::ALL))
     return true;
   if (contexts.Contains(MenuItem::PAGE_ACTION) &&
       (type == ExtensionContextMenuModel::PAGE_ACTION))
     return true;
   if (contexts.Contains(MenuItem::BROWSER_ACTION) &&
       (type == ExtensionContextMenuModel::BROWSER_ACTION))
     return true;
 
   return false;
 }
 
+// Returns the id for the visibility command for the given |extension|, or -1
+// if none should be shown.
+int GetVisibilityStringId(Profile* profile, const Extension* extension) {

[Finnur, 2014/08/19 10:18:04]

nit: const Profile&

[Devlin, 2014/08/19 16:54:39]

Sadly... can't.

ExtensionActionAPI takes a non-const BrowserContext* for Get(), which hooks into
the whole BrowserContextKeyedService implementation (as does ExtensionPrefs). 
Could pass in a non-const Profile&, but we really don't do that much without
compelling reason.

But added in a DCHECK at least, for good measure.

+  int string_id = -1;
+  if (!extensions::FeatureSwitch::extension_action_redesign()->IsEnabled()) {
+    // Without the toolbar redesign, we only show the visibility toggle for
+    // browser actions, and only give the option to hide.
+    if (extensions::ExtensionActionManager::Get(profile)->GetBrowserAction(
+            *extension)) {
+      string_id = IDS_EXTENSIONS_HIDE_BUTTON;
+    }
+  } else {
+    // With the redesign, we display "show" or "hide" based on the icon's
+    // visibility.
+    bool visible = ExtensionActionAPI::GetBrowserActionVisibility(
+                       ExtensionPrefs::Get(profile), extension->id());
+    string_id =
+        visible ? IDS_EXTENSIONS_HIDE_BUTTON : IDS_EXTENSIONS_SHOW_BUTTON;
+  }
+  return string_id;
+}
+
 }  // namespace
 
 ExtensionContextMenuModel::ExtensionContextMenuModel(const Extension* extension,
                                                      Browser* browser,
                                                      PopupDelegate* delegate)
     : SimpleMenuModel(this),
       extension_id_(extension->id()),
       browser_(browser),
       profile_(browser->profile()),
       delegate_(delegate),
@@ skipping 91 matching lines @@
       if (web_contents) {
         extensions::ActiveScriptController::GetForWebContents(web_contents)
             ->AlwaysRunOnVisibleOrigin(extension);
       }
       break;
     }
     case CONFIGURE:
       DCHECK(!extensions::ManifestURL::GetOptionsPage(extension).is_empty());
       extensions::ExtensionTabUtil::OpenOptionsPage(extension, browser_);
       break;
-    case HIDE: {
-      extensions::ExtensionActionAPI::SetBrowserActionVisibility(
-          extensions::ExtensionPrefs::Get(profile_), extension->id(), false);
+    case TOGGLE_VISIBILITY: {
+      ExtensionPrefs* prefs = ExtensionPrefs::Get(profile_);
+      bool visible = ExtensionActionAPI::GetBrowserActionVisibility(
+                         prefs, extension->id());
+      ExtensionActionAPI::SetBrowserActionVisibility(
+          prefs, extension->id(), !visible);
       break;
     }
     case UNINSTALL: {
       AddRef();  // Balanced in Accepted() and Canceled()
       extension_uninstall_dialog_.reset(
           extensions::ExtensionUninstallDialog::Create(
               profile_, browser_->window()->GetNativeWindow(), this));
       extension_uninstall_dialog_->ConfirmUninstall(extension);
       break;
     }
@@ skipping 60 matching lines @@
   // attached to the script injection request icon, but that's okay.
   WebContents* web_contents = GetActiveWebContents();
   if (web_contents &&
       extensions::ActiveScriptController::GetForWebContents(web_contents)
           ->HasActiveScriptAction(extension)) {
     AddItemWithStringId(ALWAYS_RUN, IDS_EXTENSIONS_ALWAYS_RUN);
   }
 
   AddItemWithStringId(CONFIGURE, IDS_EXTENSIONS_OPTIONS_MENU_ITEM);
   AddItem(UNINSTALL, l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNINSTALL));
-  if (extension_action_manager->GetBrowserAction(*extension))
-    AddItemWithStringId(HIDE, IDS_EXTENSIONS_HIDE_BUTTON);
+
+  // Add a toggle visibility (show/hide) if the extension icon is shown on the
+  // toolbar.
+  int visibility_string_id = GetVisibilityStringId(profile_, extension);
+  if (visibility_string_id != -1)
+    AddItemWithStringId(TOGGLE_VISIBILITY, visibility_string_id);
+
   AddSeparator(ui::NORMAL_SEPARATOR);
   AddItemWithStringId(MANAGE, IDS_MANAGE_EXTENSION);
 }
 
 const Extension* ExtensionContextMenuModel::GetExtension() const {
   return extensions::ExtensionRegistry::Get(profile_)
       ->enabled_extensions()
       .GetByID(extension_id_);
 }
 
@@ skipping 10 matching lines @@
   extension_items_count_ = 0;
   extension_items_->AppendExtensionItems(MenuItem::ExtensionKey(extension_id_),
                                          base::string16(),
                                          &extension_items_count_,
                                          true);  // is_action_menu
 }
 
 content::WebContents* ExtensionContextMenuModel::GetActiveWebContents() const {
   return browser_->tab_strip_model()->GetActiveWebContents();
 }