[Extensions Toolbar] Make the ExtensionToolbarModel icon count more stable

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 104 matching lines @@
     last_known_positions_.push_back(id);
   }
 
   FOR_EACH_OBSERVER(
       Observer, observers_, ToolbarExtensionMoved(extension.get(), index));
   MaybeUpdateVisibilityPref(extension.get(), index);
   UpdatePrefs();
 }
 
 void ExtensionToolbarModel::SetVisibleIconCount(size_t count) {
-  visible_icon_count_ = (count == toolbar_items_.size()) ? -1 : 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
   //  not persist across browser restarts (though it may be re-entered), and we
   // don't store anything in incognito.
   if (!is_highlighting_ && !profile_->IsOffTheRecord()) {
     // 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.
@@ skipping 84 matching lines @@
     // It's possible that we haven't added this extension yet, if its
     // visibility was adjusted in the course of its initialization.
     if (std::find(toolbar_items_.begin(), toolbar_items_.end(), extension) ==
             toolbar_items_.end())
       return;
 
     int new_size = 0;
     int new_index = 0;
     if (visible) {
       // If this action used to be hidden, we can't possibly be showing all.
-      DCHECK_NE(-1, visible_icon_count_);
+      DCHECK_LT(visible_icon_count(), toolbar_items_.size());
       // Grow the bar by one and move the extension to the end of the visibles.
-      new_size = visible_icon_count_ + 1;
+      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);
+      DCHECK_GE(visible_icon_count(), 0u);
       // 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_size = visible_icon_count() - 1;
       new_index = new_size;
     }
     SetVisibleIconCount(new_size);
     MoveExtensionIcon(extension->id(), new_index);
   } else {  // Don't include all extensions.
     if (visible)
       AddExtension(extension);
     else
       RemoveExtension(extension);
   }
@@ skipping 53 matching lines @@
     // the overflow menu by default.
     return action_manager->GetExtensionAction(*extension) != NULL;
   }
 
   return action_manager->GetBrowserAction(*extension) &&
          ExtensionActionAPI::GetBrowserActionVisibility(
              extension_prefs_, extension->id());
 }
 
 void ExtensionToolbarModel::AddExtension(const Extension* extension) {
+  // We only use AddExtension() once the system is initialized.
+  DCHECK(extensions_initialized_);
   if (!ShouldAddExtension(extension))
     return;
 
   // See if we have a last known good position for this extension.
   bool is_new_extension =
       std::find(last_known_positions_.begin(),
                 last_known_positions_.end(),
                 extension->id()) == last_known_positions_.end();
   size_t new_index = is_new_extension ? toolbar_items_.size() :
       FindNewPositionFromLastKnownGood(extension);
@@ skipping 14 matching lines @@
         Observer, observers_, ToolbarExtensionAdded(extension, new_index));
 
     // If this is an incognito profile, we also have to check to make sure the
     // overflow matches the main bar's status.
     if (profile_->IsOffTheRecord()) {
       ExtensionToolbarModel* main_model =
           ExtensionToolbarModel::Get(profile_->GetOriginalProfile());
       // Find what the index will be in the main bar. Because Observer calls are
       // nondeterministic, we can't just assume the main bar will have the
       // extension and look it up.
-      int main_index = is_new_extension ?
+      size_t main_index = is_new_extension ?
           main_model->toolbar_items_.size() :
           main_model->FindNewPositionFromLastKnownGood(extension);
-      bool visible = main_index < main_model->visible_icon_count_ ||
-                     main_model->visible_icon_count_ == -1;
+      bool visible = main_index < main_model->visible_icon_count();
       // We may need to adjust the visible count if the incognito bar isn't
       // showing all icons and this one is visible, or if it is showing all
       // icons and this is hidden.
-      if (visible && visible_icon_count_ != -1)
-        SetVisibleIconCount(visible_icon_count_ + 1);
-      else if (!visible && visible_icon_count_ == -1)
-        SetVisibleIconCount(toolbar_items_.size() - 1);
+      if (visible && !all_icons_visible())
+        SetVisibleIconCount(visible_icon_count() + 1);
+      else if (!visible && all_icons_visible())
+        SetVisibleIconCount(visible_icon_count() - 1);
     }
   }
 }
 
 void ExtensionToolbarModel::RemoveExtension(const Extension* extension) {
   ExtensionList::iterator pos =
       std::find(toolbar_items_.begin(), toolbar_items_.end(), extension);
   if (pos == toolbar_items_.end())
     return;
 
@@ skipping 35 matching lines @@
 }
 
 // Combine the currently enabled extensions that have browser actions (which
 // we get from the ExtensionRegistry) with the ordering we get from the
 // pref service. For robustness we use a somewhat inefficient process:
 // 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() {
+  DCHECK(toolbar_items_.empty());  // We shouldn't have any items yet.
+
   last_known_positions_ = extension_prefs_->GetToolbarOrder();
   if (profile_->IsOffTheRecord())
     IncognitoPopulate();
   else
     Populate(last_known_positions_);
 
   extensions_initialized_ = true;
   MaybeUpdateVisibilityPrefs();
   FOR_EACH_OBSERVER(Observer, observers_, OnToolbarModelInitialized());
 }
@@ skipping 18 matching lines @@
     }
 
     ExtensionIdList::const_iterator pos =
         std::find(positions.begin(), positions.end(), extension->id());
     if (pos != positions.end())
       sorted[pos - positions.begin()] = extension;
     else
       unsorted.push_back(extension);
   }
 
-  // Clear the current items, if any.
-  ClearItems();
-
   // Merge the lists.
-  toolbar_items_.reserve(sorted.size() + unsorted.size());
+  sorted.insert(sorted.end(), unsorted.begin(), unsorted.end());
+  toolbar_items_.reserve(sorted.size());
 
   for (const scoped_refptr<const Extension>& extension : sorted) {
     // It's possible for the extension order to contain items that aren't
     // actually loaded on this machine.  For example, when extension sync is on,
     // we sync the extension order as-is but double-check with the user before
     // syncing NPAPI-containing extensions, so if one of those is not actually
     // synced, we'll get a NULL in the list.  This sort of case can also happen
     // if some error prevents an extension from loading.
-    if (extension.get() != NULL) {
+    if (extension.get()) {
+      // We don't notify observers of the added extension yet. Rather, observers
+      // should wait for the "OnToolbarModelInitialized" notification, and then
+      // bulk-update. (This saves a lot of bouncing-back-and-forth here, and
+      // allows observers to ensure that the extension system is always
+      // initialized before using the extensions).
       toolbar_items_.push_back(extension);
-      FOR_EACH_OBSERVER(
-          Observer,
-          observers_,
-          ToolbarExtensionAdded(extension.get(), toolbar_items_.size() - 1));
-    }
-  }
-  for (const scoped_refptr<const Extension>& extension : unsorted) {
-    if (extension.get() != NULL) {
-      toolbar_items_.push_back(extension);
-      FOR_EACH_OBSERVER(
-          Observer,
-          observers_,
-          ToolbarExtensionAdded(extension.get(), toolbar_items_.size() - 1));
     }
   }
 
   UMA_HISTOGRAM_COUNTS_100(
       "ExtensionToolbarModel.BrowserActionsPermanentlyHidden", hidden);
   UMA_HISTOGRAM_COUNTS_100("ExtensionToolbarModel.BrowserActionsCount",
                            toolbar_items_.size());
 
   if (!toolbar_items_.empty()) {
     // Visible count can be -1, meaning: 'show all'. Since UMA converts negative
     // values to 0, this would be counted as 'show none' unless we convert it to
     // max.
     UMA_HISTOGRAM_COUNTS_100("ExtensionToolbarModel.BrowserActionsVisible",
                              visible_icon_count_ == -1 ?
                                  base::HistogramBase::kSampleType_MAX :
                                  visible_icon_count_);
   }
 }
 
 void ExtensionToolbarModel::IncognitoPopulate() {
   DCHECK(profile_->IsOffTheRecord());
-  // Clear the current items, if any.
-  ClearItems();
-
   const ExtensionToolbarModel* original_model =
       ExtensionToolbarModel::Get(profile_->GetOriginalProfile());
 
   // Find the absolute value of the original model's count.
   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
   // mode are still overflowed. Order is the same as in regular mode.
   visible_icon_count_ = 0;
   for (ExtensionList::const_iterator iter =
            original_model->toolbar_items_.begin();
        iter != original_model->toolbar_items_.end(); ++iter) {
     if (ShouldAddExtension(iter->get())) {
       toolbar_items_.push_back(*iter);
       if (iter - original_model->toolbar_items_.begin() < original_visible)
         ++visible_icon_count_;
-      FOR_EACH_OBSERVER(
-          Observer,
-          observers_,
-          ToolbarExtensionAdded(iter->get(), toolbar_items_.size() - 1));
     }
   }
 }
 
 void ExtensionToolbarModel::UpdatePrefs() {
   if (!extension_prefs_ || profile_->IsOffTheRecord())
     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) {
+    const Extension* extension, size_t 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_ && !profile_->IsOffTheRecord()) {
-    bool visible = index < visible_icon_count_ || visible_icon_count_ == -1;
+    bool visible = index < visible_icon_count();
     if (visible != ExtensionActionAPI::GetBrowserActionVisibility(
                        extension_prefs_, extension->id())) {
       // Don't observe changes caused by ourselves.
       bool was_registered = false;
       if (registrar_.IsRegistered(
               this,
               NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
               content::Source<ExtensionPrefs>(extension_prefs_))) {
         was_registered = true;
         registrar_.Remove(
@@ skipping 27 matching lines @@
   ExtensionIdList pref_positions = extension_prefs_->GetToolbarOrder();
   size_t pref_position_size = pref_positions.size();
   for (size_t i = 0; i < last_known_positions_.size(); ++i) {
     if (std::find(pref_positions.begin(), pref_positions.end(),
                   last_known_positions_[i]) == pref_positions.end()) {
       pref_positions.push_back(last_known_positions_[i]);
     }
   }
   last_known_positions_.swap(pref_positions);
 
+  ClearItems();
+
   // Re-populate.
   Populate(last_known_positions_);
 
   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()));
   }
 }
 
 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_;
+  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 {
@@ skipping 34 matching lines @@
   // (like we do for LocationBar), but sadly, we don't.
   while ((obs = it.GetNext()) != NULL) {
     if (obs->GetBrowser() == browser)
       return obs->ShowExtensionActionPopup(extension, grant_active_tab);
   }
   return false;
 }
 
 void ExtensionToolbarModel::EnsureVisibility(
     const ExtensionIdList& extension_ids) {
-  if (visible_icon_count_ == -1)
+  if (all_icons_visible())
     return;  // Already showing all.
 
   // Otherwise, make sure we have enough room to show all the extensions
   // requested.
-  if (visible_icon_count_ < static_cast<int>(extension_ids.size()))
+  if (visible_icon_count() < extension_ids.size())
     SetVisibleIconCount(extension_ids.size());
 
-  if (visible_icon_count_ == -1)
+  if (all_icons_visible())
     return;  // May have been set to max by SetVisibleIconCount.
 
   // Guillotine's Delight: Move an orange noble to the front of the line.
   for (ExtensionIdList::const_iterator it = extension_ids.begin();
        it != extension_ids.end(); ++it) {
     for (ExtensionList::const_iterator extension = toolbar_items_.begin();
          extension != toolbar_items_.end(); ++extension) {
       if ((*extension)->id() == (*it)) {
-        if (extension - toolbar_items_.begin() >= visible_icon_count_)
+        if (extension - toolbar_items_.begin() >=
+                static_cast<int>(visible_icon_count()))
           MoveExtensionIcon((*extension)->id(), 0);
         break;
       }
     }
   }
 }
 
 bool ExtensionToolbarModel::HighlightExtensions(
     const ExtensionIdList& extension_ids) {
   highlighted_items_.clear();
 
   for (ExtensionIdList::const_iterator id = extension_ids.begin();
        id != extension_ids.end();
        ++id) {
     for (ExtensionList::const_iterator extension = toolbar_items_.begin();
          extension != toolbar_items_.end();
          ++extension) {
       if (*id == (*extension)->id())
         highlighted_items_.push_back(*extension);
     }
   }
 
   // If we have any items in |highlighted_items_|, then we entered highlighting
   // mode.
   if (highlighted_items_.size()) {
     old_visible_icon_count_ = visible_icon_count_;
     is_highlighting_ = true;
-    if (visible_icon_count_ != -1 &&
-        visible_icon_count_ < static_cast<int>(extension_ids.size())) {
+    if (visible_icon_count() < extension_ids.size())
       SetVisibleIconCount(extension_ids.size());
-    }
 
     FOR_EACH_OBSERVER(Observer, observers_, ToolbarHighlightModeChanged(true));
     return true;
   }
 
   // Otherwise, we didn't enter highlighting mode (and, in fact, exited it if
   // we were otherwise in it).
   if (is_highlighting_)
     StopHighlighting();
   return false;
 }
 
 void ExtensionToolbarModel::StopHighlighting() {
   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