[Extensions Toolbar] Protect against crazy bounds

chrome/browser/ui/toolbar/toolbar_actions_bar.h

 // Copyright 2014 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_BAR_H_
 #define CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_BAR_H_
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/scoped_vector.h"
@@ skipping 83 matching lines @@
   // Returns the [minimum|maximum] possible width for the toolbar.
   int GetMinimumWidth() const;
   int GetMaximumWidth() const;
 
   // Returns the width for the given number of icons.
   int IconCountToWidth(int icons) const;
 
   // Returns the number of icons that can fit within the given width.
   size_t WidthToIconCount(int width) const;
 
-  // Returns the number of icons that should be displayed.
+  // Returns the number of icons that should be displayed if space allows.
   size_t GetIconCount() const;
 
+  // Returns the starting index (inclusive) for displayable icons.
+  size_t GetStartIndexInBounds() const;
+
+  // Returns the ending index (exclusive) for displayable icons.
+  size_t GetEndIndexInBounds() const;
+
+  // Returns true if an overflow container is necessary to display any other
+  // icons. This is different than ToolbarActionsModel::all_icons_visible()
+  // because the ToolbarActionsBar (which represents the UI for a specific
+  // window) might be unable to display all the icons that the model (which
+  // represents the underlying model of *all* windows) is set to, such as in

[Peter Kasting, 2015/09/17 00:18:04]

So does this mean that if all windows are equally narrow, the two will be in
agreement, i.e. the model describes the visibility state of the widest window? 
It's still a bit unclear to me; see next comment also.

[Devlin, 2015/09/17 17:00:27]

I've cleared up this comment, and added some better class comments both here and
in the ToolbarActionsModel.  Lemme know if it's a bit better. :)

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

Yeah, these help.

+  // the case of a very narrow window.

[Peter Kasting, 2015/09/17 00:18:04]

This comment helps, but it might be necessary to instead/additionally use
class-scope comments or more function-scope comments not only here but in
ToolbarActionsModel as well to make clear the different concepts of visibility. 
Bonus points if you invent terminology to distinguish these so the classes
aren't both talking about "visible" but meaning different things by it.

[Devlin, 2015/09/17 17:00:27]

See above.

+  bool NeedsOverflow() const;
+
   // Returns the frame (bounds) that the specified index should have, taking
   // into account if this is the main or overflow bar. If this is the overflow
   // bar and the index should not be displayed (i.e., it is shown on the main
   // bar), returns an empty rect.
   gfx::Rect GetFrameForIndex(size_t index) const;
 
   // Returns the actions in the proper order; this may differ from the
   // underlying order in the case of actions being popped out to show a popup.
   std::vector<ToolbarActionViewController*> GetActions() const;
 
@@ skipping 170 matching lines @@
   // The controller of the bubble to show once animation finishes, if any.
   scoped_ptr<extensions::ExtensionMessageBubbleController>
       pending_extension_bubble_controller_;
 
   base::WeakPtrFactory<ToolbarActionsBar> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ToolbarActionsBar);
 };
 
 #endif  // CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_BAR_H_