Changed MenuItemView to check if any menu item has a checkbox or radio button and then adjust it's layout accordingly.

When MenuModelAdapter::BuildMenuImpl() was going recursively through all submenus it used to set has_icon to each submenu instead of top menu item. Changed it to set it to the top menu item.

BUG=172990

TEST=When testing please make sure other menus with checkboxes/radio buttons have proper padding.


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

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

4 files changed, 29 insertions, 10 deletions

diff --git a/ui/views/controls/menu/menu_item_view.cc b/ui/views/controls/menu/menu_item_view.cc
index 0c66f17..eb19fbb 100644
--- a/ui/views/controls/menu/menu_item_view.cc
+++ b/ui/views/controls/menu/menu_item_view.cc
@@ -622,14 +622,14 @@ void MenuItemView::UpdateMenuPartSizes() {
 
   label_start_ = config.item_left_margin + icon_area_width_;
   int padding = 0;
-  if (config.always_use_icon_to_label_padding)
+  if (config.always_use_icon_to_label_padding) {
     padding = config.icon_to_label_padding;
-  else if (config.render_gutter)
+  } else if (config.render_gutter) {
     padding = config.item_left_margin;
-  else
-    // If there are no icons don't pad by the icon to label padding. This
-    // makes us look close to system menus.
-    padding = has_icons_ ? config.icon_to_label_padding : 0;
+  } else {
+    padding = (has_icons_ || HasChecksOrRadioButtons()) ?
+        config.icon_to_label_padding : 0;
+  }
   label_start_ += padding;
 
   if (config.render_gutter)
@@ -1020,4 +1020,19 @@ int MenuItemView::GetMaxIconViewWidth() const {
   return width;
 }
 
+bool MenuItemView::HasChecksOrRadioButtons() const {
+  for (int i = 0; i < submenu_->GetMenuItemCount(); ++i) {
+    MenuItemView* menu_item = submenu_->GetMenuItemAt(i);
+    if (menu_item->HasSubmenu()) {
+      if (menu_item->HasChecksOrRadioButtons())
+        return true;
+    } else {
+      const Type& type = menu_item->GetType();
+      if (type == CHECKBOX || type == RADIO)
+        return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace views
diff --git a/ui/views/controls/menu/menu_item_view.h b/ui/views/controls/menu/menu_item_view.h
index 0afcdf5..3c87ff5 100644
--- a/ui/views/controls/menu/menu_item_view.h
+++ b/ui/views/controls/menu/menu_item_view.h
@@ -306,6 +306,7 @@ class VIEWS_EXPORT MenuItemView : public View {
   void set_has_icons(bool has_icons) {
     has_icons_ = has_icons;
   }
+  bool has_icons() const { return has_icons_; }
 
   // Returns the descendant with the specified command.
   MenuItemView* GetMenuItemByID(int id);
@@ -444,6 +445,9 @@ class VIEWS_EXPORT MenuItemView : public View {
   // Returns the max icon width; recurses over submenus.
   int GetMaxIconViewWidth() const;
 
+  // Returns true if the menu has items with a checkbox or a radio button.
+  bool HasChecksOrRadioButtons() const;
+
   // The delegate. This is only valid for the root menu item. You shouldn't
   // use this directly, instead use GetDelegate() which walks the tree as
   // as necessary.
diff --git a/ui/views/controls/menu/menu_model_adapter.cc b/ui/views/controls/menu/menu_model_adapter.cc
index 1d0bae89..1b1c45a 100644
--- a/ui/views/controls/menu/menu_model_adapter.cc
+++ b/ui/views/controls/menu/menu_model_adapter.cc
@@ -180,6 +180,7 @@ void MenuModelAdapter::WillHideMenu(MenuItemView* menu) {
 void MenuModelAdapter::BuildMenuImpl(MenuItemView* menu, ui::MenuModel* model) {
   DCHECK(menu);
   DCHECK(model);
+  bool has_icons = model->HasIcons();
   const int item_count = model->GetItemCount();
   for (int i = 0; i < item_count; ++i) {
     MenuItemView* item = menu->AppendMenuItemFromModel(
@@ -194,12 +195,13 @@ void MenuModelAdapter::BuildMenuImpl(MenuItemView* menu, ui::MenuModel* model) {
       ui::MenuModel* submodel = model->GetSubmenuModelAt(i);
       DCHECK(submodel);
       BuildMenuImpl(item, submodel);
+      has_icons = has_icons || item->has_icons();
 
       menu_map_[item] = submodel;
     }
   }
 
-  menu->set_has_icons(model->HasIcons());
+  menu->set_has_icons(has_icons);
 }
 
 }  // namespace views
diff --git a/ui/views/controls/menu/menu_model_adapter.h b/ui/views/controls/menu/menu_model_adapter.h
index 9a15006..9c3e84e 100644
--- a/ui/views/controls/menu/menu_model_adapter.h
+++ b/ui/views/controls/menu/menu_model_adapter.h
@@ -55,9 +55,7 @@ class VIEWS_EXPORT MenuModelAdapter : public MenuDelegate {
   virtual void WillHideMenu(MenuItemView* menu) OVERRIDE;
 
  private:
-  // Implementation of BuildMenu().  index_offset is both input and output;
-  // on input it contains the offset from index to command id for the model,
-  // and on output it contains the offset for the next model.
+  // Implementation of BuildMenu().
   void BuildMenuImpl(MenuItemView* menu, ui::MenuModel* model);
 
   // Container of ui::MenuModel pointers as encountered by preorder