Makes it so child views of menuitemview can be traversed with the

keyboard. Because I don't want the menu to actually get focus I'm
using SetHotTracked when traversing and AcceleratorPressed from
enter/return.

BUG=45734
TEST=none

Review URL: http://codereview.chromium.org/2741004

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

15 files changed, 250 insertions, 56 deletions

diff --git a/views/controls/button/custom_button.cc b/views/controls/button/custom_button.cc
index 534c8a4..8bf68b2 100644
--- a/views/controls/button/custom_button.cc
+++ b/views/controls/button/custom_button.cc
@@ -74,7 +74,8 @@ CustomButton::CustomButton(ButtonListener* listener)
     : Button(listener),
       state_(BS_NORMAL),
       animate_on_state_change_(true),
-      triggerable_event_flags_(MouseEvent::EF_LEFT_BUTTON_DOWN) {
+      triggerable_event_flags_(MouseEvent::EF_LEFT_BUTTON_DOWN),
+      request_focus_on_press_(true) {
   hover_animation_.reset(new ThrobAnimation(this));
   hover_animation_->SetSlideDuration(kHoverFadeDurationMs);
 }
@@ -101,7 +102,8 @@ bool CustomButton::OnMousePressed(const MouseEvent& e) {
   if (state_ != BS_DISABLED) {
     if (ShouldEnterPushedState(e) && HitTest(e.location()))
       SetState(BS_PUSHED);
-    RequestFocus();
+    if (request_focus_on_press_)
+      RequestFocus();
   }
   return true;
 }
diff --git a/views/controls/button/custom_button.h b/views/controls/button/custom_button.h
index 9f2cb9f..5801401 100644
--- a/views/controls/button/custom_button.h
+++ b/views/controls/button/custom_button.h
@@ -54,6 +54,13 @@ class CustomButton : public Button,
     return triggerable_event_flags_;
   }
 
+  // Sets whether |RequestFocus| should be invoked on a mouse press. The default
+  // is true.
+  void set_request_focus_on_press(bool value) {
+    request_focus_on_press_ = value;
+  }
+  bool request_focus_on_press() const { return request_focus_on_press_; }
+
  protected:
   // Construct the Button with a Listener. See comment for Button's ctor.
   explicit CustomButton(ButtonListener* listener);
@@ -108,6 +115,9 @@ class CustomButton : public Button,
   // Mouse event flags which can trigger button actions.
   int triggerable_event_flags_;
 
+  // See description above setter.
+  bool request_focus_on_press_;
+
   DISALLOW_COPY_AND_ASSIGN(CustomButton);
 };
 
diff --git a/views/controls/button/text_button.cc b/views/controls/button/text_button.cc
index 40d4d68..9fcf477 100644
--- a/views/controls/button/text_button.cc
+++ b/views/controls/button/text_button.cc
@@ -397,7 +397,8 @@ void TextButton::SetEnabled(bool enabled) {
 }
 
 bool TextButton::OnMousePressed(const MouseEvent& e) {
-  RequestFocus();
+  if (request_focus_on_press())
+    RequestFocus();
   return true;
 }
 
diff --git a/views/controls/menu/menu_config.h b/views/controls/menu/menu_config.h
index eafbed1..adcf851 100644
--- a/views/controls/menu/menu_config.h
+++ b/views/controls/menu/menu_config.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -42,6 +42,9 @@ struct MenuConfig {
   // Font used by menus.
   gfx::Font font;
 
+  // Font used when the menu has children.
+  gfx::Font font_with_controls;
+
   // Margins between the top of the item and the label.
   int item_top_margin;
 
diff --git a/views/controls/menu/menu_config_gtk.cc b/views/controls/menu/menu_config_gtk.cc
index 84c9aa6..30b59a1 100644
--- a/views/controls/menu/menu_config_gtk.cc
+++ b/views/controls/menu/menu_config_gtk.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -15,6 +15,7 @@ MenuConfig* MenuConfig::Create() {
   MenuConfig* config = new MenuConfig();
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   config->font = rb.GetFont(ResourceBundle::BaseFont);
+  config->font_with_controls = config->font.DeriveFont(0, gfx::Font::BOLD);
   config->arrow_width = rb.GetBitmapNamed(IDR_MENU_ARROW)->width();
   // Add 4 to force some padding between check and label.
   config->check_width = rb.GetBitmapNamed(IDR_MENU_CHECK)->width() + 4;
diff --git a/views/controls/menu/menu_config_win.cc b/views/controls/menu/menu_config_win.cc
index 155f86a..bff25fd 100644
--- a/views/controls/menu/menu_config_win.cc
+++ b/views/controls/menu/menu_config_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -26,6 +26,7 @@ MenuConfig* MenuConfig::Create() {
   HFONT font = CreateFontIndirect(&metrics.lfMenuFont);
   DLOG_ASSERT(font);
   config->font = gfx::Font::CreateFont(font);
+  config->font_with_controls = config->font.DeriveFont(0, gfx::Font::BOLD);
 
   HDC dc = GetDC(NULL);
   RECT bounds = { 0, 0, 200, 200 };
diff --git a/views/controls/menu/menu_controller.cc b/views/controls/menu/menu_controller.cc
index 9005fd6..723f8bb 100644
--- a/views/controls/menu/menu_controller.cc
+++ b/views/controls/menu/menu_controller.cc
@@ -51,6 +51,73 @@ static void ScrollToVisible(View* view) {
   view->ScrollRectToVisible(gfx::Rect(gfx::Point(), view->size()));
 }
 
+// Returns the first descendant of |view| that is hot tracked.
+static View* GetFirstHotTrackedView(View* view) {
+  if (!view)
+    return NULL;
+
+  if (view->IsHotTracked())
+    return view;
+
+  for (int i = 0; i < view->GetChildViewCount(); ++i) {
+    View* hot_view = GetFirstHotTrackedView(view->GetChildViewAt(i));
+    if (hot_view)
+      return hot_view;
+  }
+  return NULL;
+}
+
+// Recurses through the child views of |view| returning the first view starting
+// at |start| that is focusable. A value of -1 for |start| indicates to start at
+// the first view (if |forward| is false, iterating starts at the last view). If
+// |forward| is true the children are considered first to last, otherwise last
+// to first.
+static View* GetFirstFocusableView(View* view, int start, bool forward) {
+  if (forward) {
+    for (int i = start == -1 ? 0 : start; i < view->GetChildViewCount(); ++i) {
+      View* deepest = GetFirstFocusableView(view->GetChildViewAt(i), -1,
+                                            forward);
+      if (deepest)
+        return deepest;
+    }
+  } else {
+    for (int i = start == -1 ? view->GetChildViewCount() - 1 : start;
+         i >= 0; --i) {
+      View* deepest = GetFirstFocusableView(view->GetChildViewAt(i), -1,
+                                            forward);
+      if (deepest)
+        return deepest;
+    }
+  }
+  return view->IsFocusable() ? view : NULL;
+}
+
+// Returns the first child of |start| that is focusable.
+static View* GetInitialFocusableView(View* start, bool forward) {
+  return GetFirstFocusableView(start, -1, forward);
+}
+
+// Returns the next view after |start_at| that is focusable. Returns NULL if
+// there are no focusable children of |ancestor| after |start_at|.
+static View* GetNextFocusableView(View* ancestor,
+                                  View* start_at,
+                                  bool forward) {
+  DCHECK(ancestor->IsParentOf(start_at));
+  View* parent = start_at;
+  do {
+    View* new_parent = parent->GetParent();
+    int index = new_parent->GetChildIndex(parent);
+    index += forward ? 1 : -1;
+    if (forward || index != -1) {
+      View* next = GetFirstFocusableView(new_parent, index, forward);
+      if (next)
+        return next;
+    }
+    parent = new_parent;
+  } while (parent != ancestor);
+  return NULL;
+}
+
 // MenuScrollTask --------------------------------------------------------------
 
 // MenuScrollTask is used when the SubmenuView does not all fit on screen and
@@ -275,6 +342,12 @@ void MenuController::SetSelection(MenuItemView* menu_item,
   size_t current_size = current_path.size();
   size_t new_size = new_path.size();
 
+  if (pending_state_.item != menu_item && pending_state_.item) {
+    View* current_hot_view = GetFirstHotTrackedView(pending_state_.item);
+    if (current_hot_view)
+      current_hot_view->SetHotTracked(false);
+  }
+
   // Notify the old path it isn't selected.
   for (size_t i = paths_differ_at; i < current_size; ++i)
     current_path[i]->SetSelected(false);
@@ -765,11 +838,16 @@ bool MenuController::OnKeyDown(int key_code
         CloseSubmenu();
       break;
 
+    case base::VKEY_SPACE:
+      SendAcceleratorToHotTrackedView();
+      break;
+
     case base::VKEY_RETURN:
       if (pending_state_.item) {
         if (pending_state_.item->HasSubmenu()) {
           OpenSubmenuChangeSelectionIfCan();
-        } else if (pending_state_.item->IsEnabled()) {
+        } else if (!SendAcceleratorToHotTrackedView() &&
+                   pending_state_.item->IsEnabled()) {
           Accept(pending_state_.item, 0);
           return false;
         }
@@ -833,6 +911,17 @@ MenuController::~MenuController() {
 #endif
 }
 
+bool MenuController::SendAcceleratorToHotTrackedView() {
+  View* hot_view = GetFirstHotTrackedView(pending_state_.item);
+  if (!hot_view)
+    return false;
+
+  Accelerator accelerator(base::VKEY_RETURN, false, false, false);
+  hot_view->AcceleratorPressed(accelerator);
+  hot_view->SetHotTracked(true);
+  return true;
+}
+
 void MenuController::UpdateInitialLocation(
     const gfx::Rect& bounds,
     MenuItemView::AnchorPosition position) {
@@ -935,7 +1024,12 @@ void MenuController::CloseAllNestedMenus() {
 }
 
 MenuItemView* MenuController::GetMenuItemAt(View* source, int x, int y) {
+  // Walk the view hierarchy until we find a menu item (or the root).
   View* child_under_mouse = source->GetViewForPoint(gfx::Point(x, y));
+  while (child_under_mouse &&
+         child_under_mouse->GetID() != MenuItemView::kMenuItemViewID) {
+    child_under_mouse = child_under_mouse->GetParent();
+  }
   if (child_under_mouse && child_under_mouse->IsEnabled() &&
       child_under_mouse->GetID() == MenuItemView::kMenuItemViewID) {
     return static_cast<MenuItemView*>(child_under_mouse);
@@ -1324,9 +1418,28 @@ void MenuController::IncrementSelection(int delta) {
     if (item->GetSubmenu()->GetMenuItemCount()) {
       SetSelection(item->GetSubmenu()->GetMenuItemAt(0), false, false);
       ScrollToVisible(item->GetSubmenu()->GetMenuItemAt(0));
-      return;  // return so else case can fall through.
+      return;
     }
   }
+
+  if (item->GetChildViewCount()) {
+    View* hot_view = GetFirstHotTrackedView(item);
+    if (hot_view) {
+      hot_view->SetHotTracked(false);
+      View* to_make_hot = GetNextFocusableView(item, hot_view, delta == 1);
+      if (to_make_hot) {
+        to_make_hot->SetHotTracked(true);
+        return;
+      }
+    } else {
+      View* to_make_hot = GetInitialFocusableView(item, delta == 1);
+      if (to_make_hot) {
+        to_make_hot->SetHotTracked(true);
+        return;
+      }
+    }
+  }
+
   if (item->GetParentMenuItem()) {
     MenuItemView* parent = item->GetParentMenuItem();
     int parent_count = parent->GetSubmenu()->GetMenuItemCount();
@@ -1335,8 +1448,12 @@ void MenuController::IncrementSelection(int delta) {
         if (parent->GetSubmenu()->GetMenuItemAt(i) == item) {
           int next_index = (i + delta + parent_count) % parent_count;
           ScrollToVisible(parent->GetSubmenu()->GetMenuItemAt(next_index));
-          SetSelection(parent->GetSubmenu()->GetMenuItemAt(next_index), false,
-                       false);
+          MenuItemView* to_select =
+              parent->GetSubmenu()->GetMenuItemAt(next_index);
+          SetSelection(to_select, false, false);
+          View* to_make_hot = GetInitialFocusableView(to_select, delta == 1);
+          if (to_make_hot)
+            to_make_hot->SetHotTracked(true);
           break;
         }
       }
diff --git a/views/controls/menu/menu_controller.h b/views/controls/menu/menu_controller.h
index 734e445..b57139f 100644
--- a/views/controls/menu/menu_controller.h
+++ b/views/controls/menu/menu_controller.h
@@ -203,6 +203,10 @@ class MenuController : public MessageLoopForUI::Dispatcher {
 
   ~MenuController();
 
+  // If there is a hot tracked view AcceleratorPressed is invoked on it and
+  // true is returned.
+  bool SendAcceleratorToHotTrackedView();
+
   void UpdateInitialLocation(const gfx::Rect& bounds,
                              MenuItemView::AnchorPosition position);
 
diff --git a/views/controls/menu/menu_item_view.cc b/views/controls/menu/menu_item_view.cc
index 29864b2..f2e269d 100644
--- a/views/controls/menu/menu_item_view.cc
+++ b/views/controls/menu/menu_item_view.cc
@@ -38,9 +38,12 @@ class EmptyMenuMenuItem : public MenuItemView {
 
 }  // namespace
 
+// Padding between child views.
+static const int kChildXPadding = 8;
+
 // MenuItemView ---------------------------------------------------------------
 
-//  static
+// static
 const int MenuItemView::kMenuItemViewID = 1001;
 
 // static
@@ -164,6 +167,28 @@ void MenuItemView::Cancel() {
   }
 }
 
+MenuItemView* MenuItemView::AppendMenuItemImpl(int item_id,
+                                               const std::wstring& label,
+                                               const SkBitmap& icon,
+                                               Type type) {
+  if (!submenu_)
+    CreateSubmenu();
+  if (type == SEPARATOR) {
+    submenu_->AddChildView(new MenuSeparator());
+    return NULL;
+  }
+  MenuItemView* item = new MenuItemView(this, item_id, type);
+  if (label.empty() && GetDelegate())
+    item->SetTitle(GetDelegate()->GetLabel(item_id));
+  else
+    item->SetTitle(label);
+  item->SetIcon(icon);
+  if (type == SUBMENU)
+    item->CreateSubmenu();
+  submenu_->AddChildView(item);
+  return item;
+}
+
 SubmenuView* MenuItemView::CreateSubmenu() {
   if (!submenu_)
     submenu_ = new SubmenuView(this);
@@ -272,6 +297,21 @@ void MenuItemView::ChildrenChanged() {
   }
 }
 
+void MenuItemView::Layout() {
+  int child_count = GetChildViewCount();
+  if (child_count == 0)
+    return;
+
+  // Child views are layed out right aligned and given the full height. To right
+  // align start with the last view and progress to the first.
+  for (int i = child_count - 1, x = width() - item_right_margin_; i >= 0; --i) {
+    View* child = GetChildViewAt(i);
+    int width = child->GetPreferredSize().width();
+    child->SetBounds(x - width, 0, width, height());
+    x -= width - kChildXPadding;
+  }
+}
+
 MenuItemView::MenuItemView(MenuItemView* parent,
                            int command,
                            MenuItemView::Type type) {
@@ -325,28 +365,6 @@ void MenuItemView::Init(MenuItemView* parent,
     SetEnabled(root_delegate->IsCommandEnabled(command));
 }
 
-MenuItemView* MenuItemView::AppendMenuItemInternal(int item_id,
-                                                   const std::wstring& label,
-                                                   const SkBitmap& icon,
-                                                   Type type) {
-  if (!submenu_)
-    CreateSubmenu();
-  if (type == SEPARATOR) {
-    submenu_->AddChildView(new MenuSeparator());
-    return NULL;
-  }
-  MenuItemView* item = new MenuItemView(this, item_id, type);
-  if (label.empty() && GetDelegate())
-    item->SetTitle(GetDelegate()->GetLabel(item_id));
-  else
-    item->SetTitle(label);
-  item->SetIcon(icon);
-  if (type == SUBMENU)
-    item->CreateSubmenu();
-  submenu_->AddChildView(item);
-  return item;
-}
-
 void MenuItemView::DropMenuClosed(bool notify_delegate) {
   DCHECK(controller_);
   DCHECK(!controller_->IsBlockingRun());
@@ -434,7 +452,6 @@ void MenuItemView::AdjustBoundsForRTLUI(gfx::Rect* rect) const {
   rect->set_x(MirroredLeftPointForRect(*rect));
 }
 
-
 void MenuItemView::DestroyAllMenuHosts() {
   if (!HasSubmenu())
     return;
@@ -460,4 +477,18 @@ int MenuItemView::GetBottomMargin() {
         MenuConfig::instance().item_no_icon_bottom_margin;
 }
 
+int MenuItemView::GetChildPreferredWidth() {
+  int child_count = GetChildViewCount();
+  if (child_count == 0)
+    return 0;
+
+  int width = 0;
+  for (int i = 0; i < child_count; ++i) {
+    if (i)
+      width += kChildXPadding;
+    width += GetChildViewAt(i)->GetPreferredSize().width();
+  }
+  return width;
+}
+
 }  // namespace views
diff --git a/views/controls/menu/menu_item_view.h b/views/controls/menu/menu_item_view.h
index db06bf1..c04c341 100644
--- a/views/controls/menu/menu_item_view.h
+++ b/views/controls/menu/menu_item_view.h
@@ -26,8 +26,13 @@ class SubmenuView;
 // of the various AddXXX methods.
 //
 // MenuItemView is itself a View, which means you can add Views to each
-// MenuItemView. This normally NOT want you want, rather add other child Views
-// to the submenu of the MenuItemView.
+// MenuItemView. This is normally NOT want you want, rather add other child
+// Views to the submenu of the MenuItemView. Any child views of the MenuItemView
+// that are focusable can be navigated to by way of the up/down arrow and can be
+// activated by way of space/return keys. Activating a focusable child results
+// in |AcceleratorPressed| being invoked. Note, that as menus try not to steal
+// focus from the hosting window child views do not actually get focus. Instead
+// |SetHotTracked| is used as the user navigates around.
 //
 // There are two ways to show a MenuItemView:
 // 1. Use RunMenuAt. This blocks the caller, executing the selected command
@@ -109,14 +114,14 @@ class MenuItemView : public View {
   void AppendMenuItem(int item_id,
                       const std::wstring& label,
                       Type type) {
-    AppendMenuItemInternal(item_id, label, SkBitmap(), type);
+    AppendMenuItemImpl(item_id, label, SkBitmap(), type);
   }
 
   // Append a submenu to this menu.
   // The returned pointer is owned by this menu.
   MenuItemView* AppendSubMenu(int item_id,
                               const std::wstring& label) {
-    return AppendMenuItemInternal(item_id, label, SkBitmap(), SUBMENU);
+    return AppendMenuItemImpl(item_id, label, SkBitmap(), SUBMENU);
   }
 
   // Append a submenu with an icon to this menu.
@@ -124,7 +129,7 @@ class MenuItemView : public View {
   MenuItemView* AppendSubMenuWithIcon(int item_id,
                                       const std::wstring& label,
                                       const SkBitmap& icon) {
-    return AppendMenuItemInternal(item_id, label, icon, SUBMENU);
+    return AppendMenuItemImpl(item_id, label, icon, SUBMENU);
   }
 
   // This is a convenience for standard text label menu items where the label
@@ -142,7 +147,7 @@ class MenuItemView : public View {
 
   // Adds a separator to this menu
   void AppendSeparator() {
-    AppendMenuItemInternal(0, std::wstring(), SkBitmap(), SEPARATOR);
+    AppendMenuItemImpl(0, std::wstring(), SkBitmap(), SEPARATOR);
   }
 
   // Appends a menu item with an icon. This is for the menu item which
@@ -151,9 +156,15 @@ class MenuItemView : public View {
   void AppendMenuItemWithIcon(int item_id,
                               const std::wstring& label,
                               const SkBitmap& icon) {
-    AppendMenuItemInternal(item_id, label, icon, NORMAL);
+    AppendMenuItemImpl(item_id, label, icon, NORMAL);
   }
 
+  // All the AppendXXX methods funnel into this.
+  MenuItemView* AppendMenuItemImpl(int item_id,
+                                   const std::wstring& label,
+                                   const SkBitmap& icon,
+                                   Type type);
+
   // Returns the view that contains child menu items. If the submenu has
   // not been creates, this creates it.
   virtual SubmenuView* CreateSubmenu();
@@ -232,6 +243,9 @@ class MenuItemView : public View {
   // recalculates the bounds.
   void ChildrenChanged();
 
+  // Sizes any child views.
+  virtual void Layout();
+
  protected:
   // Creates a MenuItemView. This is used by the various AddXXX methods.
   MenuItemView(MenuItemView* parent, int command, Type type);
@@ -248,12 +262,6 @@ class MenuItemView : public View {
             MenuItemView::Type type,
             MenuDelegate* delegate);
 
-  // All the AddXXX methods funnel into this.
-  MenuItemView* AppendMenuItemInternal(int item_id,
-                                       const std::wstring& label,
-                                       const SkBitmap& icon,
-                                       Type type);
-
   // Invoked by the MenuController when the menu closes as the result of
   // drag and drop run.
   void DropMenuClosed(bool notify_delegate);
@@ -289,6 +297,9 @@ class MenuItemView : public View {
   int GetTopMargin();
   int GetBottomMargin();
 
+  // Returns the preferred width (and padding) of any children.
+  int GetChildPreferredWidth();
+
   // 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/views/controls/menu/menu_item_view_gtk.cc b/views/controls/menu/menu_item_view_gtk.cc
index a4c7216..3f0a830 100644
--- a/views/controls/menu/menu_item_view_gtk.cc
+++ b/views/controls/menu/menu_item_view_gtk.cc
@@ -28,7 +28,8 @@ gfx::Size MenuItemView::GetPreferredSize() {
   // kFavIconSize if we're showing icons.
   int content_height = std::max(kFavIconSize, font.height());
   return gfx::Size(
-      font.GetStringWidth(title_) + label_start_ + item_right_margin_,
+      font.GetStringWidth(title_) + label_start_ + item_right_margin_ +
+          GetChildPreferredWidth(),
       content_height + GetBottomMargin() + GetTopMargin());
 }
 
@@ -36,7 +37,8 @@ void MenuItemView::Paint(gfx::Canvas* canvas, bool for_drag) {
   const MenuConfig& config = MenuConfig::instance();
   bool render_selection =
       (!for_drag && IsSelected() &&
-       parent_menu_item_->GetSubmenu()->GetShowSelection(this));
+       parent_menu_item_->GetSubmenu()->GetShowSelection(this) &&
+       GetChildViewCount() == 0);
 
   int icon_x = config.item_left_margin;
   int top_margin = GetTopMargin();
@@ -71,7 +73,8 @@ void MenuItemView::Paint(gfx::Canvas* canvas, bool for_drag) {
       IsEnabled() ? TextButton::kEnabledColor : TextButton::kDisabledColor;
 #endif
   int width = this->width() - item_right_margin_ - label_start_;
-  const gfx::Font& font = MenuConfig::instance().font;
+  const gfx::Font& font = GetChildViewCount() > 0 ?
+      MenuConfig::instance().font_with_controls : MenuConfig::instance().font;
   gfx::Rect text_bounds(label_start_, top_margin +
                         (available_height - font.height()) / 2, width,
                         font.height());
diff --git a/views/controls/menu/menu_item_view_win.cc b/views/controls/menu/menu_item_view_win.cc
index cb956cb..4a90c28 100644
--- a/views/controls/menu/menu_item_view_win.cc
+++ b/views/controls/menu/menu_item_view_win.cc
@@ -22,7 +22,8 @@ namespace views {
 gfx::Size MenuItemView::GetPreferredSize() {
   const gfx::Font& font = MenuConfig::instance().font;
   return gfx::Size(
-      font.GetStringWidth(title_) + label_start_ + item_right_margin_,
+      font.GetStringWidth(title_) + label_start_ + item_right_margin_ +
+          GetChildPreferredWidth(),
       font.height() + GetBottomMargin() + GetTopMargin());
 }
 
@@ -30,7 +31,8 @@ void MenuItemView::Paint(gfx::Canvas* canvas, bool for_drag) {
   const MenuConfig& config = MenuConfig::instance();
   bool render_selection =
       (!for_drag && IsSelected() &&
-       parent_menu_item_->GetSubmenu()->GetShowSelection(this));
+       parent_menu_item_->GetSubmenu()->GetShowSelection(this) &&
+       GetChildViewCount() == 0);
   int state = render_selection ? MPI_HOT :
                                  (IsEnabled() ? MPI_NORMAL : MPI_DISABLED);
   HDC dc = canvas->beginPlatformPaint();
@@ -94,7 +96,8 @@ void MenuItemView::Paint(gfx::Canvas* canvas, bool for_drag) {
       NativeTheme::MENU, MENU_POPUPITEM, state, TMT_TEXTCOLOR,
       default_sys_color);
   int width = this->width() - item_right_margin_ - label_start_;
-  const gfx::Font& font = MenuConfig::instance().font;
+  const gfx::Font& font = GetChildViewCount() > 0 ?
+      MenuConfig::instance().font_with_controls : MenuConfig::instance().font;
   gfx::Rect text_bounds(label_start_, top_margin, width, font.height());
   text_bounds.set_x(MirroredLeftPointForRect(text_bounds));
   if (for_drag) {
diff --git a/views/controls/menu/menu_scroll_view_container.cc b/views/controls/menu/menu_scroll_view_container.cc
index ef7a8ab..b538f5b 100644
--- a/views/controls/menu/menu_scroll_view_container.cc
+++ b/views/controls/menu/menu_scroll_view_container.cc
@@ -178,7 +178,12 @@ MenuScrollViewContainer::MenuScrollViewContainer(SubmenuView* content_view) {
                  SubmenuView::kSubmenuBorderSize));
 }
 
-void MenuScrollViewContainer::Paint(gfx::Canvas* canvas) {
+void MenuScrollViewContainer::PaintBackground(gfx::Canvas* canvas) {
+  if (background()) {
+    View::PaintBackground(canvas);
+    return;
+  }
+
 #if defined(OS_WIN)
   HDC dc = canvas->beginPlatformPaint();
   RECT bounds = {0, 0, width(), height()};
diff --git a/views/controls/menu/menu_scroll_view_container.h b/views/controls/menu/menu_scroll_view_container.h
index 41c7540..b95bfb0 100644
--- a/views/controls/menu/menu_scroll_view_container.h
+++ b/views/controls/menu/menu_scroll_view_container.h
@@ -23,7 +23,7 @@ class MenuScrollViewContainer : public View {
   View* scroll_up_button() const { return scroll_up_button_; }
 
   // View overrides.
-  virtual void Paint(gfx::Canvas* canvas);
+  virtual void PaintBackground(gfx::Canvas* canvas);
   virtual void Layout();
   virtual void DidChangeBounds(const gfx::Rect& previous,
                                const gfx::Rect& current);
diff --git a/views/controls/menu/submenu_view.h b/views/controls/menu/submenu_view.h
index fd2bd02..afef314 100644
--- a/views/controls/menu/submenu_view.h
+++ b/views/controls/menu/submenu_view.h
@@ -74,7 +74,9 @@ class SubmenuView : public View {
 
   // Shows the menu at the specified location. Coordinates are in screen
   // coordinates. max_width gives the max width the view should be.
-  void ShowAt(gfx::NativeWindow parent, const gfx::Rect& bounds, bool do_capture);
+  void ShowAt(gfx::NativeWindow parent,
+              const gfx::Rect& bounds,
+              bool do_capture);
 
   // Resets the bounds of the submenu to |bounds|.
   void Reposition(const gfx::Rect& bounds);