[Extensions Toolbar] Protect against crazy bounds

chrome/browser/ui/toolbar/toolbar_actions_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_UI_TOOLBAR_TOOLBAR_ACTIONS_MODEL_H_
 #define CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_MODEL_H_
 
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_vector.h"
 #include "base/observer_list.h"
@@ skipping 10 matching lines @@
 class PrefService;
 class Profile;
 class ToolbarActionsBar;
 class ToolbarActionViewController;
 
 namespace extensions {
 class ExtensionRegistry;
 class ExtensionSet;
 }
 
-// Model for the browser actions toolbar.
+// Model for the browser actions toolbar. This is a per-profile instance, and
+// manages the user's global preferences. These will typically be reflected in
+// each window, but can be overridden in special circumstances by the window's
+// ToolbarActionsBar (see also that class for details).

[Peter Kasting, 2015/09/17 19:59:53]

Nit: Maybe instead of the sentence about "special circumstances" you could more
specifically say "when the window is narrow" or the like?  Something like:

Each browser window will attempt to show browser actions as specified by this
model, but if the window is too narrow, actions may end up pushed into the
overflow menu on a per-window basis.  Callers interested in the arrangement of
actions in a particular window should check that window's instance of
ToolbarActionsBar, which is responsible for this per-window overflow.

[Devlin, 2015/09/17 21:38:38]

Done.

 class ToolbarActionsModel : public extensions::ExtensionActionAPI::Observer,
                             public extensions::ExtensionRegistryObserver,
                             public KeyedService {
  public:
   // The different options for highlighting.
   enum HighlightType {
     HIGHLIGHT_NONE,
     HIGHLIGHT_INFO,
     HIGHLIGHT_WARNING,
   };
@@ skipping 84 matching lines @@
   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()
                : std::min(static_cast<size_t>(visible_icon_count_),
                           toolbar_items().size());
   }
 
   bool all_icons_visible() const {

[Peter Kasting, 2015/09/17 19:59:53]

Nit: Do you think you should put a warning on here about this being about the
global default, and being inappropriate if you want to know what icons are
actually visible to the user in any particular window?

[Devlin, 2015/09/17 21:38:38]

Done.

     return visible_icon_count() == toolbar_items().size();
   }
 
   bool actions_initialized() const { return actions_initialized_; }
 
   ScopedVector<ToolbarActionViewController> CreateActions(
       Browser* browser,
       ToolbarActionsBar* bar);
 
   const std::vector<ToolbarItem>& toolbar_items() const {
@@ skipping 132 matching lines @@
   // For observing change of toolbar order preference by external entity (sync).
   PrefChangeRegistrar pref_change_registrar_;
   base::Closure pref_change_callback_;
 
   base::WeakPtrFactory<ToolbarActionsModel> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ToolbarActionsModel);
 };
 
 #endif  // CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_MODEL_H_