Dynamically calculate the number of extension icons to show per row in overflow

chrome/browser/ui/views/toolbar/browser_actions_container.cc

 // Copyright 2013 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/ui/views/toolbar/browser_actions_container.h"
 
 #include "base/compiler_specific.h"
 #include "base/stl_util.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_util.h"
@@ skipping 31 matching lines @@
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/resize_area.h"
 #include "ui/views/metrics.h"
 #include "ui/views/painter.h"
 #include "ui/views/widget/widget.h"
 
 using extensions::Extension;
 
 namespace {
 
-// Horizontal spacing between most items in the container, as well as after the
-// last item or chevron (if visible).
-const int kItemSpacing = ToolbarView::kStandardSpacing;
-
 // Horizontal spacing before the chevron (if visible).
-const int kChevronSpacing = kItemSpacing - 2;
-
-// The maximum number of icons to show per row when in overflow mode (showing
-// icons in the application menu).
-// TODO(devlin): Compute the right number of icons to show, depending on the
-//               menu width.
-#if defined(OS_LINUX)
-const int kIconsPerMenuRow = 8;  // The menu on Linux is wider.
-#else
-const int kIconsPerMenuRow = 7;
-#endif
+const int kChevronSpacing = ToolbarView::kStandardSpacing - 2;
 
 // A version of MenuButton with almost empty insets to fit properly on the
 // toolbar.
 class ChevronMenuButton : public views::MenuButton {
  public:
   ChevronMenuButton(views::ButtonListener* listener,
                     const base::string16& text,
                     views::MenuButtonListener* menu_button_listener,
                     bool show_menu_marker)
       : views::MenuButton(listener,
@@ skipping 34 matching lines @@
 
 BrowserActionsContainer::DropPosition::DropPosition(
     size_t row, size_t icon_in_row)
     : row(row), icon_in_row(icon_in_row) {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserActionsContainer
 
 // static
+int BrowserActionsContainer::icons_per_menu_row = 1;
+
+// static
+const int BrowserActionsContainer::kItemSpacing = ToolbarView::kStandardSpacing;
+
+// static
 bool BrowserActionsContainer::disable_animations_during_testing_ = false;
 
 BrowserActionsContainer::BrowserActionsContainer(
     Browser* browser,
     View* owner_view,
     BrowserActionsContainer* main_container)
     : profile_(browser->profile()),
       browser_(browser),
       owner_view_(owner_view),
       main_container_(main_container),
@@ skipping 186 matching lines @@
 
   // If there are no actions to show, or we are in overflow mode and the main
   // container is already showing them all, then no further work is required.
   if (icon_count == 0)
     return gfx::Size();
 
   if (in_overflow_mode()) {
     // When in overflow, y is multiline, so the pixel count is IconHeight()
     // times the number of rows needed.
     return gfx::Size(
-        IconCountToWidth(kIconsPerMenuRow, false),
-        (((icon_count - 1) / kIconsPerMenuRow) + 1) * IconHeight());
+        IconCountToWidth(icons_per_menu_row, false),
+        (((icon_count - 1) / icons_per_menu_row) + 1) * IconHeight());
   }
 
   // We calculate the size of the view by taking the current width and
   // subtracting resize_amount_ (the latter represents how far the user is
   // resizing the view or, if animating the snapping, how far to animate it).
   // But we also clamp it to a minimum size and the maximum size, so that the
   // container can never shrink too far or take up more space than it needs.
   // In other words: MinimumNonemptyWidth() < width() - resize < ClampTo(MAX).
   int preferred_width = std::min(
       std::max(MinimumNonemptyWidth(), container_width_ - resize_amount_),
@@ skipping 39 matching lines @@
          i < main_container_->VisibleBrowserActionsAfterAnimation(); ++i) {
       // Ensure that any browser actions shown in the main view are hidden in
       // the overflow view.
       browser_action_views_[i]->SetVisible(false);
     }
 
     for (size_t i = main_container_->VisibleBrowserActionsAfterAnimation();
          i < browser_action_views_.size(); ++i) {
       BrowserActionView* view = browser_action_views_[i];
       size_t index = i - main_container_->VisibleBrowserActionsAfterAnimation();
-      int row_index = static_cast<int>(index) / kIconsPerMenuRow;
+      int row_index = static_cast<int>(index) / icons_per_menu_row;
       int x = kItemSpacing + (index * IconWidth(true)) -
-          (row_index * IconWidth(true) * kIconsPerMenuRow);
+          (row_index * IconWidth(true) * icons_per_menu_row);
       gfx::Rect rect_bounds(
           x, IconHeight() * row_index, icon_width, IconHeight());
       view->SetBoundsRect(rect_bounds);
       view->SetVisible(true);
     }
   } else {
     for (BrowserActionViews::const_iterator it = browser_action_views_.begin();
          it < browser_action_views_.end(); ++it) {
       BrowserActionView* view = *it;
       int x = ToolbarView::kStandardSpacing +
@@ skipping 74 matching lines @@
       kItemSpacing) / IconWidth(true);
 
   // We need to figure out how many icons are visible on the relevant row.
   // In the main container, this will just be the visible actions.
   int visible_icons_on_row = VisibleBrowserActionsAfterAnimation();
   if (in_overflow_mode()) {
     // If this is the final row of the overflow, then this is the remainder of
     // visible icons. Otherwise, it's a full row (kIconsPerRow).
     visible_icons_on_row =
         row_index ==
-            static_cast<size_t>(visible_icons_on_row / kIconsPerMenuRow) ?
-                visible_icons_on_row % kIconsPerMenuRow :
-                kIconsPerMenuRow;
+            static_cast<size_t>(visible_icons_on_row / icons_per_menu_row) ?
+                visible_icons_on_row % icons_per_menu_row :
+                icons_per_menu_row;
   }
 
   // Because the user can drag outside the container bounds, we need to clamp to
   // the valid range.  Note that the maximum allowable value is (num icons), not
   // (num icons - 1), because we represent the indicator being past the last
   // icon as being "before the (last + 1) icon".
   size_t before_icon_in_row =
       std::min(std::max(before_icon_unclamped, 0), visible_icons_on_row);
 
   if (!drop_position_.get() ||
@@ skipping 17 matching lines @@
   BrowserActionDragData data;
   if (!data.Read(event.data()))
     return ui::DragDropTypes::DRAG_NONE;
 
   // Make sure we have the same view as we started with.
   DCHECK_EQ(browser_action_views_[data.index()]->extension()->id(),
             data.id());
   DCHECK(model_);
 
   size_t i =
-      drop_position_->row * kIconsPerMenuRow + drop_position_->icon_in_row;
+      drop_position_->row * icons_per_menu_row + drop_position_->icon_in_row;

[Finnur, 2014/09/10 10:48:27]

Danger! Danger! Drop can be broken if the wrench menu is opened, triggering a
change to this static value and then someone tries to drag to the main
container. Would have been nasty to track down. :)

[Devlin, 2014/09/10 16:14:26]

Can it, though?

In the main container, there are no "rows", and, as such, drop_position_->row is
always 0, so the value of icons_per_menu_row doesn't matter at all.  The only
time it matters is for the overflow container (in which case it's correct by
definition, since the overflow container uses this to determine how many icons
it has).  I think this is actually safe.  (I've verified by hand that dragging
still works, just to be sure.)

[Finnur, 2014/09/11 09:12:43]

Good.

   if (in_overflow_mode())
     i += GetFirstVisibleIconIndex();
   // |i| now points to the item to the right of the drop indicator*, which is
   // correct when dragging an icon to the left. When dragging to the right,
   // however, we want the icon being dragged to get the index of the item to
   // the left of the drop indicator, so we subtract one.
   // * Well, it can also point to the end, but not when dragging to the left. :)
   if (i > data.index())
     --i;
 
@@ skipping 464 matching lines @@
 int BrowserActionsContainer::IconCountToWidth(int icons,
                                               bool display_chevron) const {
   if (icons < 0)
     icons = browser_action_views_.size();
   if ((icons == 0) && !display_chevron)
     return ToolbarView::kStandardSpacing;
   int icons_size =
       (icons == 0) ? 0 : ((icons * IconWidth(true)) - kItemSpacing);
   int chevron_size = chevron_ && display_chevron ?
       (kChevronSpacing + chevron_->GetPreferredSize().width()) : 0;
-  return ToolbarView::kStandardSpacing + icons_size + chevron_size +
-      ToolbarView::kStandardSpacing;
+  // In overflow mode, our padding is to use item spacing on either end (just so
+  // we can see the drop indicator). Otherwise we use the standard toolbar
+  // spacing.
+  // Note: These are actually the same thing, but, on the offchance one
+  // changes, let's get it right.
+  int padding =
+      2 * (in_overflow_mode() ? kItemSpacing : ToolbarView::kStandardSpacing);
+  return icons_size + chevron_size + padding;
 }
 
 size_t BrowserActionsContainer::WidthToIconCount(int pixels) const {
   // Check for widths large enough to show the entire icon set.
   if (pixels >= IconCountToWidth(-1, false))
     return browser_action_views_.size();
 
   // We need to reserve space for the resize area, chevron, and the spacing on
   // either side of the chevron.
   int available_space = pixels - ToolbarView::kStandardSpacing -
@@ skipping 51 matching lines @@
   size_t absolute_model_size =
       model_size == -1 ? extensions.size() : model_size;
 
   // The main container will try to show |model_size| icons, but reduce if there
   // aren't enough displayable icons to do so.
   size_t main_displayed = std::min(displayable_icon_count, absolute_model_size);
   // The overflow will display the extras, if any.
   return in_overflow_mode() ?
       displayable_icon_count - main_displayed : main_displayed;
 }