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

chrome/browser/extensions/extension_toolbar_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_toolbar_model.h"
 
 #include <string>
 
+#include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_base.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/extension_toolbar_model_factory.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
@@ skipping 89 matching lines @@
   if (!inserted) {
     DCHECK_EQ(index, static_cast<int>(toolbar_items_.size()));
     index = toolbar_items_.size();
 
     toolbar_items_.push_back(make_scoped_refptr(extension));
     last_known_positions_.push_back(extension->id());
   }
 
   FOR_EACH_OBSERVER(
       Observer, observers_, ToolbarExtensionMoved(extension, index));
-
+  MaybeUpdateVisibilityPref(extension, index);
   UpdatePrefs();
 }
 
 ExtensionAction::ShowAction ExtensionToolbarModel::ExecuteBrowserAction(
     const Extension* extension,
     Browser* browser,
     GURL* popup_url_out,
     bool should_grant) {
   content::WebContents* web_contents = NULL;
   int tab_id = 0;
@@ skipping 20 matching lines @@
   }
 
   ExtensionActionAPI::BrowserActionExecuted(
       browser->profile(), *browser_action, web_contents);
   return ExtensionAction::ACTION_NONE;
 }
 
 void ExtensionToolbarModel::SetVisibleIconCount(int count) {
   visible_icon_count_ =
       count == static_cast<int>(toolbar_items_.size()) ? -1 : count;
+
   // Only set the prefs if we're not in highlight mode. Highlight mode is
   // designed to be a transitory state, and should not persist across browser
   // restarts (though it may be re-entered).
-  if (!is_highlighting_)
+  if (!is_highlighting_) {
+    // Additionally, if we are using the new toolbar, any icons which are in the
+    // overflow menu are considered "hidden". But it so happens that the times
+    // we are likely to call SetVisibleIconCount() are also those when we are
+    // in flux. So wait for things to cool down before setting the prefs.
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&ExtensionToolbarModel::MaybeUpdateVisibilityPrefs,
+                   weak_ptr_factory_.GetWeakPtr()));
     prefs_->SetInteger(pref_names::kToolbarSize, visible_icon_count_);
+  }
 }
 
 void ExtensionToolbarModel::OnExtensionLoaded(
     content::BrowserContext* browser_context,
     const Extension* extension) {
   // We don't want to add the same extension twice. It may have already been
   // added by EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED below, if the user
   // hides the browser action and then disables and enables the extension.
   for (size_t i = 0; i < toolbar_items_.size(); i++) {
     if (toolbar_items_[i].get() == extension)
@@ skipping 23 matching lines @@
   if (pos != last_known_positions_.end()) {
     last_known_positions_.erase(pos);
     UpdatePrefs();
   }
 }
 
 void ExtensionToolbarModel::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  DCHECK_EQ(
-      extensions::NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
-      type);
+  DCHECK_EQ(NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED, type);
   const Extension* extension =
       ExtensionRegistry::Get(profile_)->GetExtensionById(
           *content::Details<const std::string>(details).ptr(),
           ExtensionRegistry::EVERYTHING);
-  if (ExtensionActionAPI::GetBrowserActionVisibility(extension_prefs_,
-                                                     extension->id()))
-    AddExtension(extension);
-  else
-    RemoveExtension(extension);
+
+  bool visible = ExtensionActionAPI::GetBrowserActionVisibility(
+                     extension_prefs_, extension->id());
+  // Hiding works differently with the new and old toolbars.
+  if (include_all_extensions_) {
+    int new_size = 0;
+    int new_index = 0;
+    if (visible) {
+      // If this action used to be hidden, we can't possible be showing all.
+      DCHECK_NE(-1, visible_icon_count_);
+      // Grow the bar by one and move the extension to the end of the visibles.
+      new_size = visible_icon_count_ + 1;
+      new_index = new_size - 1;
+    } else {
+      // If we're hiding one, we must be showing at least one.
+      DCHECK_NE(visible_icon_count_, 0);
+      // Shrink the bar by one and move the extension to the beginning of the
+      // overflow menu.
+      new_size = visible_icon_count_ == -1 ?
+                     toolbar_items_.size() - 1 : visible_icon_count_ - 1;
+      new_index = new_size;
+    }
+    SetVisibleIconCount(new_size);
+    MoveExtensionIcon(extension, new_index);
+    FOR_EACH_OBSERVER(Observer, observers_, ToolbarVisibleCountChanged());
+  } else {  // Don't include all extensions.
+    if (visible)
+      AddExtension(extension);
+    else
+      RemoveExtension(extension);
+  }
 }
 
 void ExtensionToolbarModel::OnReady() {
   ExtensionRegistry* registry = ExtensionRegistry::Get(profile_);
   InitializeExtensionList(registry->enabled_extensions());
   // Wait until the extension system is ready before observing any further
   // changes so that the toolbar buttons can be shown in their stable ordering
   // taken from prefs.
   extension_registry_observer_.Add(registry);
   registrar_.Add(
@@ skipping 63 matching lines @@
       toolbar_items_.push_back(make_scoped_refptr(extension));
     }
   } else {
     // This is a never before seen extension, that was added to the end. Make
     // sure to reflect that. (|new_index| was set above.)
     toolbar_items_.push_back(make_scoped_refptr(extension));
     last_known_positions_.push_back(extension->id());
     UpdatePrefs();
   }
 
+  MaybeUpdateVisibilityPref(extension, new_index);
+
   // If we're currently highlighting, then even though we add a browser action
   // to the full list (|toolbar_items_|, there won't be another *visible*
   // browser action, which was what the observers care about.
   if (!is_highlighting_) {
     FOR_EACH_OBSERVER(
         Observer, observers_, ToolbarExtensionAdded(extension, new_index));
   }
 }
 
 void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
@@ skipping 31 matching lines @@
 // 1. Create a vector of extensions sorted by their pref values. This vector may
 // have holes.
 // 2. Create a vector of extensions that did not have a pref value.
 // 3. Remove holes from the sorted vector and append the unsorted vector.
 void ExtensionToolbarModel::InitializeExtensionList(
     const ExtensionSet& extensions) {
   last_known_positions_ = extension_prefs_->GetToolbarOrder();
   Populate(last_known_positions_, extensions);
 
   extensions_initialized_ = true;
+  MaybeUpdateVisibilityPrefs();
   FOR_EACH_OBSERVER(Observer, observers_, ToolbarVisibleCountChanged());
 }
 
 void ExtensionToolbarModel::Populate(const ExtensionIdList& positions,
                                      const ExtensionSet& extensions) {
   // Items that have explicit positions.
   ExtensionList sorted;
   sorted.resize(positions.size(), NULL);
   // The items that don't have explicit positions.
   ExtensionList unsorted;
@@ skipping 80 matching lines @@
 void ExtensionToolbarModel::UpdatePrefs() {
   if (!extension_prefs_)
     return;
 
   // Don't observe change caused by self.
   pref_change_registrar_.Remove(pref_names::kToolbar);
   extension_prefs_->SetToolbarOrder(last_known_positions_);
   pref_change_registrar_.Add(pref_names::kToolbar, pref_change_callback_);
 }
 
+void ExtensionToolbarModel::MaybeUpdateVisibilityPref(
+    const Extension* extension, int index) {
+  // We only update the visibility pref for hidden/not hidden based on the
+  // overflow menu with the new toolbar design.
+  if (include_all_extensions_) {
+    bool visible = index < visible_icon_count_ || visible_icon_count_ == -1;
+    if (visible != ExtensionActionAPI::GetBrowserActionVisibility(
+                       extension_prefs_, extension->id())) {
+      // Don't observe changes caused by ourselves.
+      bool registered = !registrar_.IsEmpty();
+      registrar_.RemoveAll();  // Cheat since we only have one notification.

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

Seems like you aren't saving much with this. Why not explicitly remove it and
avoid potential bugs in the future?

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

sigh... probably for the best.  But typing
NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY three times in ten lines just
hurts my fingers. :P

Just more incentive to make it an observer method instead... but that's a CL for
another time.

Done.

[Finnur, 2014/08/20 12:06:16]

That's why they invented the clipboard! ;)

[Devlin, 2014/08/20 15:31:22]

:P

+      ExtensionActionAPI::SetBrowserActionVisibility(
+          extension_prefs_, extension->id(), visible);
+      if (registered) {
+        registrar_.Add(this,
+                       NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
+                       content::Source<ExtensionPrefs>(extension_prefs_));
+      }
+    }
+  }
+}
+
+void ExtensionToolbarModel::MaybeUpdateVisibilityPrefs() {
+  for (size_t i = 0u; i < toolbar_items_.size(); ++i)
+    MaybeUpdateVisibilityPref(toolbar_items_[i], i);
+}
+
 int ExtensionToolbarModel::IncognitoIndexToOriginal(int incognito_index) {
   int original_index = 0, i = 0;
   for (ExtensionList::iterator iter = toolbar_items_.begin();
        iter != toolbar_items_.end();
        ++iter, ++original_index) {
     if (util::IsIncognitoEnabled((*iter)->id(), profile_)) {
       if (incognito_index == i)
         break;
       ++i;
     }
@@ skipping 130 matching lines @@
     is_highlighting_ = false;
     if (old_visible_icon_count_ != visible_icon_count_) {
       SetVisibleIconCount(old_visible_icon_count_);
       FOR_EACH_OBSERVER(Observer, observers_, ToolbarVisibleCountChanged());
     }
     FOR_EACH_OBSERVER(Observer, observers_, ToolbarHighlightModeChanged(false));
   }
 }
 
 }  // namespace extensions