Make extensions that desire to act pop out if in overflow

chrome/browser/extensions/extension_toolbar_model.cc

Index: chrome/browser/extensions/extension_toolbar_model.cc
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index 7c64c1f11755eeeb4da6a38295c15c4e9b823458..1facabc12d66925c330b9fb2aab6695cf33fbe75 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -121,9 +121,8 @@ void ExtensionToolbarModel::MoveExtensionIcon(const std::string& id,
   UpdatePrefs();
 }
 
-void ExtensionToolbarModel::SetVisibleIconCount(int count) {
-  visible_icon_count_ =
-      count == static_cast<int>(toolbar_items_.size()) ? -1 : count;
+void ExtensionToolbarModel::SetVisibleIconCount(size_t count) {
+  visible_icon_count_ = (count == toolbar_items_.size()) ? -1 : count;
 
   // Only set the prefs if we're not in highlight mode and the profile is not
   // incognito. Highlight mode is designed to be a transitory state, and should
@@ -152,11 +151,18 @@ void ExtensionToolbarModel::OnExtensionActionUpdated(
       ExtensionRegistry::Get(profile_)->enabled_extensions().GetByID(
           extension_action->extension_id());
   // Notify observers if the extension exists and is in the model.
-  if (extension &&
-      std::find(toolbar_items_.begin(),
-                toolbar_items_.end(),
-                extension) != toolbar_items_.end()) {
-    FOR_EACH_OBSERVER(Observer, observers_, ToolbarExtensionUpdated(extension));
+  ExtensionList::const_iterator iter =
+      std::find(toolbar_items_.begin(), toolbar_items_.end(), extension);
+  if (iter != toolbar_items_.end()) {
+    FOR_EACH_OBSERVER(
+        Observer, observers_, ToolbarExtensionUpdated(extension));
+    // If the action was in the overflow menu, we have to alert observers that
+    // the toolbar needs to be reordered (to show the action).
+    if (static_cast<size_t>(iter - toolbar_items_.begin()) >=
+            visible_icon_count()) {
+      FOR_EACH_OBSERVER(
+          Observer, observers_, OnToolbarReorderNecessary(web_contents));
+    }
   }
 }
 
@@ -490,9 +496,7 @@ void ExtensionToolbarModel::IncognitoPopulate() {
       ExtensionToolbarModel::Get(profile_->GetOriginalProfile());
 
   // Find the absolute value of the original model's count.
-  int original_visible = original_model->GetVisibleIconCount();
-  if (original_visible == -1)
-    original_visible = original_model->toolbar_items_.size();
+  int original_visible = original_model->visible_icon_count();
 
   // In incognito mode, we show only those extensions that are
   // incognito-enabled. Further, any actions that were overflowed in regular
@@ -589,6 +593,51 @@ void ExtensionToolbarModel::OnExtensionToolbarPrefChange() {
   }
 }
 
+size_t ExtensionToolbarModel::GetVisibleIconCountForTab(
+    content::WebContents* web_contents) const {
+  if (all_icons_visible())
+    return visible_icon_count();  // Already displaying all actions.
+
+  ExtensionActionAPI* extension_action_api = ExtensionActionAPI::Get(profile_);
+  size_t total_icons = visible_icon_count_;
+  for (size_t i = total_icons; i < toolbar_items_.size(); ++i) {
+    if (extension_action_api->ExtensionWantsToRun(toolbar_items_[i].get(),
+                                                  web_contents))
+      ++total_icons;
+  }
+  return total_icons;
+}
+
+ExtensionList ExtensionToolbarModel::GetItemOrderForTab(
+    content::WebContents* web_contents) const {
+  // If we're highlighting, the items are always the same.
+  if (is_highlighting_)
+    return highlighted_items_;
+
+  // Start by initializing the array to be the same as toolbar items (this isn't
+  // any more expensive than initializing it to be of the same size with all
+  // nulls, and saves us time at the end).
+  ExtensionList result = toolbar_items_;
+  if (toolbar_items_.empty())
+    return result;
+
+  ExtensionList overflowed_actions_wanting_to_run;
+  ExtensionActionAPI* extension_action_api = ExtensionActionAPI::Get(profile_);
+  size_t boundary = visible_icon_count();
+  // Rotate any actions that want to run to the boundary between visible and
+  // overflowed actions.
+  for (size_t i = boundary; i < result.size(); ++i) {
+    if (extension_action_api->ExtensionWantsToRun(result[i].get(),
+                                                  web_contents)) {
+      std::rotate(result.rbegin() + (result.size() - i - 1),
+                  result.rbegin() + (result.size() - i),
+                  result.rbegin() + (result.size() - boundary));

[Peter Kasting, 2014/10/31 19:01:08]

Why use rbegin() + result.size() - x in all these?  It seems like this could be
clearer using begin() everywhere.

Furthermore, I suspect you could make your loop index an iterator instead and
get an even simpler function body.

[Devlin, 2014/10/31 20:49:29]

Excellent question. For some reason, I thought a right rotation made more sense,
but it really shouldn't matter.  Changed.
 
Thought std::rotate would invalidate iters.  Turns out not.  Changed.

+      ++boundary;
+    }
+  }
+  return result;
+}
+
 bool ExtensionToolbarModel::ShowExtensionActionPopup(
     const Extension* extension,
     Browser* browser,