Squeezing the wrench menu

BUG=137345
TEST=visual


Review URL: https://chromiumcodereview.appspot.com/10919036

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@154555 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 74 insertions, 29 deletions

diff --git a/ui/views/controls/menu/menu_item_view.cc b/ui/views/controls/menu/menu_item_view.cc
index bc0c1f7..574d04c 100644
--- a/ui/views/controls/menu/menu_item_view.cc
+++ b/ui/views/controls/menu/menu_item_view.cc
@@ -542,11 +542,6 @@ void MenuItemView::Layout() {
   }
 }
 
-int MenuItemView::GetAcceleratorTextWidth() {
-  string16 text = GetAcceleratorText();
-  return text.empty() ? 0 : GetFont().GetStringWidth(text);
-}
-
 void MenuItemView::SetMargins(int top_margin, int bottom_margin) {
   top_margin_ = top_margin;
   bottom_margin_ = bottom_margin;
@@ -809,21 +804,39 @@ gfx::Size MenuItemView::GetChildPreferredSize() {
   return gfx::Size(width, height);
 }
 
-gfx::Size MenuItemView::CalculatePreferredSize() {
+MenuItemView::MenuItemDimensions MenuItemView::GetPreferredDimensions() {
   gfx::Size child_size = GetChildPreferredSize();
-  if (IsContainer()) {
-    return gfx::Size(
-        child_size.width(),
-        child_size.height() + GetBottomMargin() + GetTopMargin());
-  }
 
+  MenuItemDimensions dimensions;
+  // Get the container height.
+  dimensions.children_width = child_size.width();
+  dimensions.height = child_size.height() + GetBottomMargin() + GetTopMargin();
+
+  // In case of a container, only the container size needs to be filled.
+  if (IsContainer())
+    return dimensions;
+
+  // Determine the length of the label text.
   const gfx::Font& font = GetFont();
-  int menu_item_height = std::max(font.GetHeight(), child_size.height()) +
-                             GetBottomMargin() + GetTopMargin();
-  return gfx::Size(
-      font.GetStringWidth(title_) + label_start_ +
-          item_right_margin_ + child_size.width(),
-      std::max(menu_item_height, MenuConfig::instance().item_min_height));
+  dimensions.standard_width = font.GetStringWidth(title_) + label_start_ +
+      item_right_margin_;
+  // Determine the length of the accelerator text.
+  string16 text = GetAcceleratorText();
+  dimensions.accelerator_width =
+      text.empty() ? 0 : GetFont().GetStringWidth(text);
+
+  // Determine the height to use.
+  dimensions.height = std::max(dimensions.height,
+      font.GetHeight() + GetBottomMargin() + GetTopMargin());
+  dimensions.height = std::max(dimensions.height,
+      MenuConfig::instance().item_min_height);
+  return dimensions;
+}
+
+gfx::Size MenuItemView::CalculatePreferredSize() {
+  MenuItemView::MenuItemDimensions dimensions = GetPreferredDimensions();
+  return gfx::Size(dimensions.standard_width + dimensions.children_width,
+                   dimensions.height);
 }
 
 string16 MenuItemView::GetAcceleratorText() {
diff --git a/ui/views/controls/menu/menu_item_view.h b/ui/views/controls/menu/menu_item_view.h
index bbe258c..9b31e1b 100644
--- a/ui/views/controls/menu/menu_item_view.h
+++ b/ui/views/controls/menu/menu_item_view.h
@@ -104,6 +104,24 @@ class VIEWS_EXPORT MenuItemView : public View {
     POSITION_BELOW_BOUNDS
   };
 
+  // The data structure which is used for the menu size
+  struct MenuItemDimensions {
+    // Width of everything except the accelerator and children views.
+    int standard_width;
+    // The width of all contained views of the item.
+    int children_width;
+    // The amount of space needed to accommodate the accelerator.
+    int accelerator_width;
+    // The height of the menu item.
+    int height;
+
+    MenuItemDimensions()
+        : standard_width(0),
+          children_width(0),
+          accelerator_width(0),
+          height(0) {}
+  };
+
   // Constructor for use with the top level menu item. This menu is never
   // shown to the user, rather its use as the parent for all menu items.
   explicit MenuItemView(MenuDelegate* delegate);
@@ -252,6 +270,9 @@ class VIEWS_EXPORT MenuItemView : public View {
   // Returns the preferred size of this item.
   virtual gfx::Size GetPreferredSize() OVERRIDE;
 
+  // Return the preferred dimensions of the item in pixel.
+  MenuItemDimensions GetPreferredDimensions();
+
   // Returns the object responsible for controlling showing the menu.
   MenuController* GetMenuController();
   const MenuController* GetMenuController() const;
@@ -285,10 +306,6 @@ class VIEWS_EXPORT MenuItemView : public View {
   // Sizes any child views.
   virtual void Layout() OVERRIDE;
 
-  // Returns the amount of space needed to accommodate the accelerator. The
-  // space needed for the accelerator is NOT included in the preferred width.
-  int GetAcceleratorTextWidth();
-
   // Returns true if the menu has mnemonics. This only useful on the root menu
   // item.
   bool has_mnemonics() const { return has_mnemonics_; }
diff --git a/ui/views/controls/menu/submenu_view.cc b/ui/views/controls/menu/submenu_view.cc
index 7c923cd..a44fc7b0 100644
--- a/ui/views/controls/menu/submenu_view.cc
+++ b/ui/views/controls/menu/submenu_view.cc
@@ -123,18 +123,31 @@ gfx::Size SubmenuView::GetPreferredSize() {
     return gfx::Size();
 
   max_accelerator_width_ = 0;
-  int max_width = 0;
+  // The maximum width of items which contain maybe a label and multiple views.
+  int max_complex_width = 0;
+  // The max. width of items which contain a label and maybe an accelerator.
+  int max_simple_width = 0;
   int height = 0;
   for (int i = 0; i < child_count(); ++i) {
     View* child = child_at(i);
-    gfx::Size child_pref_size = child->visible() ? child->GetPreferredSize()
-                                                 : gfx::Size();
-    max_width = std::max(max_width, child_pref_size.width());
-    height += child_pref_size.height();
+    if (!child->visible())
+      continue;
     if (child->id() == MenuItemView::kMenuItemViewID) {
       MenuItemView* menu = static_cast<MenuItemView*>(child);
+      MenuItemView::MenuItemDimensions dimensions =
+          menu->GetPreferredDimensions();
+      max_simple_width = std::max(
+          max_simple_width, dimensions.standard_width);
       max_accelerator_width_ =
-          std::max(max_accelerator_width_, menu->GetAcceleratorTextWidth());
+          std::max(max_accelerator_width_, dimensions.accelerator_width);
+      max_complex_width = std::max(max_complex_width,
+          dimensions.standard_width + dimensions.children_width);
+      height += dimensions.height;
+    } else {
+      gfx::Size child_pref_size =
+          child->visible() ? child->GetPreferredSize() : gfx::Size();
+      max_complex_width = std::max(max_complex_width, child_pref_size.width());
+      height += child_pref_size.height();
     }
   }
   if (max_accelerator_width_ > 0) {
@@ -143,8 +156,10 @@ gfx::Size SubmenuView::GetPreferredSize() {
   }
   gfx::Insets insets = GetInsets();
   return gfx::Size(
-      std::max(max_width + max_accelerator_width_ + insets.width(),
-               minimum_preferred_width_ - 2 * insets.width()),
+      std::max(max_complex_width,
+               std::max(max_simple_width + max_accelerator_width_ +
+                        insets.width(),
+               minimum_preferred_width_ - 2 * insets.width())),
       height + insets.height());
 }