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

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

Index: chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
diff --git a/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc b/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
index 94adeb1244953afc78f2fb0e638595674f5bb20c..5686558fb8ed4de41cfbaf57fa4c401ae95f0f05 100644
--- a/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
+++ b/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
@@ -9,13 +9,16 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/ui/views/toolbar/wrench_menu.h"
 #include "ui/views/controls/menu/menu_item_view.h"
+#include "ui/views/controls/menu/submenu_view.h"
 
 ExtensionToolbarMenuView::ExtensionToolbarMenuView(Browser* browser,
                                                    WrenchMenu* wrench_menu)
     : browser_(browser),
       wrench_menu_(wrench_menu),
       container_(NULL),
-      browser_actions_container_observer_(this) {
+      browser_actions_container_observer_(this),
+      start_padding_(0),
+      end_padding_(0) {
   BrowserActionsContainer* main =
       BrowserView::GetBrowserViewForBrowser(browser_)
           ->toolbar()->browser_actions();
@@ -25,7 +28,6 @@ ExtensionToolbarMenuView::ExtensionToolbarMenuView(Browser* browser,
       main);
   container_->Init();
   AddChildView(container_);
-
   // If we were opened for a drop command, we have to wait for the drop to
   // finish so we can close the wrench menu.
   if (wrench_menu_->for_drop()) {
@@ -37,15 +39,47 @@ ExtensionToolbarMenuView::ExtensionToolbarMenuView(Browser* browser,
 ExtensionToolbarMenuView::~ExtensionToolbarMenuView() {
 }
 
+void ExtensionToolbarMenuView::Init(views::MenuItemView* menu_root) {

[sky, 2014/09/11 19:42:27]

Can't you do this in Layout()? At Layout() time you'll know the size. Maybe you
need to know the width to calculate the preferred height? If that's the case I
would rather you change SubmenuView to make a pass using GetHeightForWidth to
calculate the necessary height.

[Devlin, 2014/09/11 20:32:13]

The problem is that we need to not just know the width of the menu, but
potentially _alter_ the width of the menu.  The behavior should be that we fill
a row with icons and, if an even number doesn't fit, we extend the menu to make
them fit perfectly (with a constant padding).  So we need to calculate and set
our width prior to the menu setting its own bounds.  Unfortunately,
MenuController::CalculateMenuBounds() is called before Layout(), so we need to
do this work sooner.

+  // We should pad on the left enough so that the first icon starts at the same
+  // point as the labels, and enough on the right so that the final icon ends
+  // at the same point as the arrows in the menu. However, there is outer
+  // padding on the container (so we can see the drop indicator), so we need to
+  // compensate.
+  // We subtract one from label_start because we want the pixel *before* the
+  // label.
+  start_padding_ = views::MenuItemView::label_start() - 1 -
+                   BrowserActionsContainer::kItemSpacing;
+  end_padding_ = menu_root->GetMenuConfig().arrow_to_edge_padding -
+                 BrowserActionsContainer::kItemSpacing;
+
+  // Find the width we have for possible content.
+  int content_width = menu_root->GetSubmenu()->GetPreferredSize().width() -
+                      start_padding_ - end_padding_;
+  // We subtract an extra padding from the content width to make division easy.
+  content_width -= BrowserActionsContainer::kItemSpacing;
+
+  int icons_per_row = content_width / BrowserActionsContainer::IconWidth(true);
+  // If it's not a perfect fit (which is very likely), we add another icon and
+  // increase the menu size.
+  if (content_width % BrowserActionsContainer::IconWidth(true))
+    ++icons_per_row;
+
+  // We should definitely have more than 0 icons per row.
+  DCHECK_NE(0, icons_per_row);
+  BrowserActionsContainer::icons_per_overflow_menu_row = icons_per_row;
+  PreferredSizeChanged();
+}
+
 gfx::Size ExtensionToolbarMenuView::GetPreferredSize() const {
-  return container_->GetPreferredSize();
+  gfx::Size size = container_->GetPreferredSize();
+  size.set_width(size.width() + start_padding_ + end_padding_);
+  return size;
 }
 
 void ExtensionToolbarMenuView::Layout() {
-  // All buttons are given the same width.
   gfx::Size sz = GetPreferredSize();
-  SetBounds(views::MenuItemView::label_start(), 0, sz.width(), sz.height());
-  container_->SetBounds(0, 0, sz.width(), sz.height());
+  container_->SetBounds(
+      start_padding_ + 1, 0, sz.width() - end_padding_, sz.height());
 }
 
 void ExtensionToolbarMenuView::OnBrowserActionDragDone() {