[Extensions Toolbar] Make the ExtensionToolbarModel icon count more stable

chrome/browser/extensions/extension_toolbar_model.h

 // 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.
 
 #ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_
 
 #include "base/compiler_specific.h"
 #include "base/observer_list.h"
 #include "base/prefs/pref_change_registrar.h"
@@ skipping 25 matching lines @@
   ~ExtensionToolbarModel() override;
 
   // A class which is informed of changes to the model; represents the view of
   // MVC. Also used for signaling view changes such as showing extension popups.
   // TODO(devlin): Should this really be an observer? It acts more like a
   // delegate.
   class Observer {
    public:
     // TODO(devlin): Rename these methods to be OnFoo.
     // Signals that an |extension| has been added to the toolbar at |index|.
+    // This will *only* be called after the toolbar model has been initialized.
     virtual void ToolbarExtensionAdded(const Extension* extension,
                                        int index) = 0;
 
     // Signals that the given |extension| has been removed from the toolbar.
     virtual void ToolbarExtensionRemoved(const Extension* extension) = 0;
 
     // Signals that the given |extension| has been moved to |index|. |index| is
     // the desired *final* index of the extension (that is, in the adjusted
     // order, extension should be at |index|).
     virtual void ToolbarExtensionMoved(const Extension* extension,
@@ skipping 16 matching lines @@
 
     // Signals that the model has entered or exited highlighting mode, or that
     // the extensions being highlighted have (probably*) changed. Highlighting
     // mode indicates that only a subset of the extensions are actively
     // displayed, and those extensions should be highlighted for extra emphasis.
     // * probably, because if we are in highlight mode and receive a call to
     //   highlight a new set of extensions, we do not compare the current set
     //   with the new set (and just assume the new set is different).
     virtual void ToolbarHighlightModeChanged(bool is_highlighting) = 0;
 
+    // Signals that the toolbar model has been initialized, so that if any
+    // observers were postponing animation during the initialization stage, they
+    // can catch up.
+    virtual void OnToolbarModelInitialized() = 0;
+
     // Signals that the toolbar needs to be reordered for the given
     // |web_contents|. This is caused by an overflowed action wanting to run,
     // and needing to "pop itself out".
     virtual void OnToolbarReorderNecessary(
         content::WebContents* web_contents) = 0;
 
-    // Signals that the toolbar model has been initialized, so that if any
-    // observers were postponing animation during the initialization stage, they
-    // can catch up.
-    virtual void OnToolbarModelInitialized() = 0;
-
     // Returns the browser associated with the Observer.
     virtual Browser* GetBrowser() = 0;
 
    protected:
     virtual ~Observer() {}
   };
 
   // Convenience function to get the ExtensionToolbarModel for a Profile.
   static ExtensionToolbarModel* Get(Profile* profile);
 
   // Adds or removes an observer.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
   // Moves the given |extension|'s icon to the given |index|.
   void MoveExtensionIcon(const std::string& id, size_t index);
 
   // Sets the number of extension icons that should be visible.
   // If count == size(), this will set the visible icon count to -1, meaning
   // "show all actions".
   void SetVisibleIconCount(size_t count);
 
   size_t visible_icon_count() const {
+    // We have guards around this because |visible_icon_count_| can be set by
+    // prefs/sync, and we want to ensure that the icon count returned is within
+    // bounds.
     return visible_icon_count_ == -1 ?
-        toolbar_items().size() : static_cast<size_t>(visible_icon_count_);
+        toolbar_items().size() :
+        std::min(static_cast<size_t>(visible_icon_count_),
+                 toolbar_items().size());
   }
 
   bool all_icons_visible() const { return visible_icon_count_ == -1; }
 
   bool extensions_initialized() const { return extensions_initialized_; }
 
   const ExtensionList& toolbar_items() const {
     return is_highlighting_ ? highlighted_items_ : toolbar_items_;
   }
 
@@ skipping 67 matching lines @@
   // it.
   void InitializeExtensionList();
   void Populate(const ExtensionIdList& positions);
   void IncognitoPopulate();
 
   // Save the model to prefs.
   void UpdatePrefs();
 
   // Updates |extension|'s browser action visibility pref if the browser action
   // is in the overflow menu and should be considered hidden.
-  void MaybeUpdateVisibilityPref(const Extension* extension, int index);
+  void MaybeUpdateVisibilityPref(const Extension* extension, size_t index);
 
   // Calls MaybeUpdateVisibilityPref() for each extension in |toolbar_items|.
   void MaybeUpdateVisibilityPrefs();
 
   // Finds the last known visible position of the icon for an |extension|. The
   // value returned is a zero-based index into the vector of visible items.
   size_t FindNewPositionFromLastKnownGood(const Extension* extension);
 
   // Returns true if the given |extension| should be added to the toolbar.
   bool ShouldAddExtension(const Extension* extension);
@@ skipping 33 matching lines @@
   bool is_highlighting_;
 
   // The number of icons which were visible before highlighting a subset, in
   // order to restore the count when finished.
   int old_visible_icon_count_;
 
   ExtensionIdList last_known_positions_;
 
   // The number of icons visible (the rest should be hidden in the overflow
   // chevron). A value of -1 indicates that all icons should be visible.
+  // Instead of using this variable directly, use visible_icon_count() if
+  // possible.
   // TODO(devlin): Make a new variable to indicate that all icons should be
   // visible, instead of overloading this one.
   int visible_icon_count_;
 
   content::NotificationRegistrar registrar_;
 
   ScopedObserver<ExtensionActionAPI, ExtensionActionAPI::Observer>
       extension_action_observer_;
 
   // Listen to extension load, unloaded notifications.
   ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>
       extension_registry_observer_;
 
   // For observing change of toolbar order preference by external entity (sync).
   PrefChangeRegistrar pref_change_registrar_;
   base::Closure pref_change_callback_;
 
   base::WeakPtrFactory<ExtensionToolbarModel> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionToolbarModel);
 };
 
 }  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_