Make extensions that desire to act pop out if in overflow

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 <algorithm>
 #include <string>
 
 #include "base/message_loop/message_loop.h"
@@ skipping 134 matching lines @@
 }
 
 void ExtensionToolbarModel::OnExtensionActionUpdated(
     ExtensionAction* extension_action,
     content::WebContents* web_contents,
     content::BrowserContext* browser_context) {
   const Extension* extension =
       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()) >=
+            GetAbsoluteVisibleIconCount()) {
+      FOR_EACH_OBSERVER(
+          Observer, observers_, ToolbarReorderNecessary(web_contents));
+    }
   }
 }
 
 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++) {
@@ skipping 390 matching lines @@
   if (last_known_positions_.size() > pref_position_size) {
     // Need to update pref because we have extra icons. But can't call
     // UpdatePrefs() directly within observation closure.
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(&ExtensionToolbarModel::UpdatePrefs,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 }
 
+int ExtensionToolbarModel::GetVisibleIconCountForTab(
+    content::WebContents* web_contents) const {
+  int base_visible = visible_icon_count_;

[Peter Kasting, 2014/10/30 22:05:59]

Why do we need this temp?  Does visible_icon_count_ change during the course of
the function?  If not, just use it directly.  Or, keep it but eliminate
|overflowed_actions_wanting_to_run| below and just increment this value
directly.

[Devlin, 2014/10/31 17:44:09]

Whoops, leftover from old implementation. Fixed.

+  if (base_visible == -1)
+    return base_visible;  // Already displaying all actions.
+
+  ExtensionActionAPI* extension_action_api = ExtensionActionAPI::Get(profile_);
+  size_t overflowed_actions_wanting_to_run = 0u;
+  for (size_t i = base_visible; i < toolbar_items_.size(); ++i) {
+    if (extension_action_api->ExtensionWantsToRun(toolbar_items_[i].get(),
+                                                  web_contents))
+      ++overflowed_actions_wanting_to_run;
+  }
+  return base_visible + overflowed_actions_wanting_to_run;
+}
+
+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_);
+  int base_visible = GetAbsoluteVisibleIconCount();
+  int index = toolbar_items_.size() - 1;
+
+  // We need to pop out any extensions that are overflowed, but want to act.
+  // Iterate over the overflowed extensions in reverse, and append those that
+  // don't want to act, while storing those that do separately.
+  for (int i = toolbar_items_.size() - 1; i >= base_visible; --i) {

[Peter Kasting, 2014/10/30 22:05:59]

This is a very complicated way of doing what I think would be easier with
std::rotate().

Just rotate the extensions that wish to run up to the divider point between the
visible and overflowed extensions.  No need for a spare ExtensionList or two
loops.

[Devlin, 2014/10/31 17:44:09]

Ah, yes, that is quite a bit simpler.  Done.

+    if (extension_action_api->ExtensionWantsToRun(toolbar_items_[i].get(),
+                                                  web_contents))
+      overflowed_actions_wanting_to_run.push_back(toolbar_items_[i]);
+    else
+      result[index--] = toolbar_items_[i];
+  }
+  // Now, append any that do want to act but were otherwise overflowed.
+  for (const scoped_refptr<const Extension>& extension :
+           overflowed_actions_wanting_to_run) {
+    result[index--] = extension;
+  }
+  // We should have appended all of (and only) those items that were in
+  // overflow.
+  DCHECK_EQ(base_visible, index + 1);
+  // The rest of the vector should be fine, since we don't need to reorder any
+  // of the already-visible extensions.
+  return result;
+}
+
 bool ExtensionToolbarModel::ShowExtensionActionPopup(
     const Extension* extension,
     Browser* browser,
     bool grant_active_tab) {
   ObserverListBase<Observer>::Iterator it(observers_);
   Observer* obs = NULL;
   // Look for the Observer associated with the browser.
   // This would be cleaner if we had an abstract class for the Toolbar UI
   // (like we do for LocationBar), but sadly, we don't.
   while ((obs = it.GetNext()) != NULL) {
@@ skipping 70 matching lines @@
   if (is_highlighting_) {
     highlighted_items_.clear();
     is_highlighting_ = false;
     if (old_visible_icon_count_ != visible_icon_count_)
       SetVisibleIconCount(old_visible_icon_count_);
     FOR_EACH_OBSERVER(Observer, observers_, ToolbarHighlightModeChanged(false));
   }
 }
 
 }  // namespace extensions