Merge 95244 - Make NetworkMenuModel a ui::MenuModel again.

This is a necessary refactoring to a single menu model to define both views menu and WebUI menu.

BUG=chromium-os:17892
TEST=none


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

TBR=rhashimoto@chromium.org
Review URL: http://codereview.chromium.org/7550060

git-svn-id: svn://svn.chromium.org/chrome/branches/835/src@95763 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 499 insertions, 157 deletions

diff --git a/chrome/browser/chromeos/status/network_menu.cc b/chrome/browser/chromeos/status/network_menu.cc
index 2795911..4ea8c83 100644
--- a/chrome/browser/chromeos/status/network_menu.cc
+++ b/chrome/browser/chromeos/status/network_menu.cc
@@ -33,6 +33,7 @@
 #include "ui/gfx/canvas_skia.h"
 #include "ui/gfx/skbitmap_operations.h"
 #include "views/controls/menu/menu_item_view.h"
+#include "views/controls/menu/menu_model_adapter.h"
 #include "views/controls/menu/submenu_view.h"
 #include "views/widget/widget.h"
 
@@ -40,7 +41,6 @@ namespace {
 
 // Offsets for views menu ids (main menu and submenu ids use the same
 // namespace).
-const int kItemIndexMask = 0x0fff;
 const int kMainIndexMask = 0x1000;
 const int kVPNIndexMask  = 0x2000;
 const int kMoreIndexMask = 0x4000;
@@ -64,11 +64,28 @@ std::string EscapeAmpersands(const std::string& input) {
   return str;
 }
 
+// Set vertical menu margins for entire menu hierarchy.
+void SetMenuMargins(views::MenuItemView* menu_item_view, int top, int bottom) {
+  menu_item_view->SetMargins(top, bottom);
+  if (menu_item_view->HasSubmenu()) {
+    views::SubmenuView* submenu = menu_item_view->GetSubmenu();
+    for (int i = 0; i < submenu->child_count(); ++i) {
+      // Must skip separators.
+      views::View* item = submenu->child_at(i);
+      if (item->id() == views::MenuItemView::kMenuItemViewID) {
+        views::MenuItemView* menu_item =
+            static_cast<views::MenuItemView*>(item);
+        SetMenuMargins(menu_item, top, bottom);
+      }
+    }
+  }
+}
+
 }  // namespace
 
 namespace chromeos {
 
-class NetworkMenuModel : public views::MenuDelegate {
+class NetworkMenuModel : public ui::MenuModel {
  public:
   struct NetworkInfo {
     NetworkInfo() : need_passphrase(false),
@@ -142,26 +159,30 @@ class NetworkMenuModel : public views::MenuDelegate {
   virtual void InitMenuItems(bool is_browser_mode,
                              bool should_open_button_options) = 0;
 
-  // PopulateMenu() clears and reinstalls the menu items defined in this
-  // instance by calling PopulateMenuItem() on each one.  Subclasses override
-  // PopulateMenuItem(), transform command_id into the correct range for
-  // the menu, and call the base class PopulateMenuItem().
-  virtual void PopulateMenu(views::MenuItemView* menu);
-  virtual void PopulateMenuItem(views::MenuItemView* menu,
-                                int index,
-                                int command_id);
-
   // Menu item field accessors.
   const MenuItemVector& menu_items() const { return menu_items_; }
-  int GetItemCount() const;
-  ui::MenuModel::ItemType GetTypeAt(int index) const;
-  string16 GetLabelAt(int index) const;
-  const gfx::Font* GetLabelFontAt(int index) const;
-  bool IsItemCheckedAt(int index) const;
-  bool GetIconAt(int index, SkBitmap* icon);
-  bool IsEnabledAt(int index) const;
-  NetworkMenuModel* GetSubmenuModelAt(int index) const;
-  void ActivatedAt(int index);
+
+  // ui::MenuModel implementation
+  // GetCommandIdAt() must be implemented by subclasses.
+  virtual bool HasIcons() const OVERRIDE;
+  virtual int GetItemCount() const OVERRIDE;
+  virtual ui::MenuModel::ItemType GetTypeAt(int index) const OVERRIDE;
+  virtual string16 GetLabelAt(int index) const OVERRIDE;
+  virtual bool IsItemDynamicAt(int index) const OVERRIDE;
+  virtual const gfx::Font* GetLabelFontAt(int index) const OVERRIDE;
+  virtual bool GetAcceleratorAt(int index,
+                                ui::Accelerator* accelerator) const OVERRIDE;
+  virtual bool IsItemCheckedAt(int index) const OVERRIDE;
+  virtual int GetGroupIdAt(int index) const OVERRIDE;
+  virtual bool GetIconAt(int index, SkBitmap* icon) OVERRIDE;
+  virtual ui::ButtonMenuItemModel* GetButtonMenuItemAt(
+      int index) const OVERRIDE;
+  virtual bool IsEnabledAt(int index) const OVERRIDE;
+  virtual bool IsVisibleAt(int index) const OVERRIDE;
+  virtual ui::MenuModel* GetSubmenuModelAt(int index) const OVERRIDE;
+  virtual void HighlightChangedTo(int index) OVERRIDE;
+  virtual void ActivatedAt(int index) OVERRIDE;
+  virtual void SetMenuModelDelegate(ui::MenuModelDelegate* delegate) OVERRIDE;
 
  protected:
   enum MenuItemFlags {
@@ -217,9 +238,9 @@ class MoreMenuModel : public NetworkMenuModel {
   // NetworkMenuModel implementation.
   virtual void InitMenuItems(bool is_browser_mode,
                              bool should_open_button_options) OVERRIDE;
-  virtual void PopulateMenuItem(views::MenuItemView* menu,
-                                int index,
-                                int command_id) OVERRIDE;
+
+  // ui::MenuModel implementation
+  virtual int GetCommandIdAt(int index) const OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MoreMenuModel);
@@ -233,9 +254,9 @@ class VPNMenuModel : public NetworkMenuModel {
   // NetworkMenuModel implementation.
   virtual void InitMenuItems(bool is_browser_mode,
                              bool should_open_button_options) OVERRIDE;
-  virtual void PopulateMenuItem(views::MenuItemView* menu,
-                                int index,
-                                int command_id) OVERRIDE;
+
+  // ui::MenuModel implementation
+  virtual int GetCommandIdAt(int index) const OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(VPNMenuModel);
@@ -253,19 +274,11 @@ class MainMenuModel : public NetworkMenuModel {
   // NetworkMenuModel implementation.
   virtual void InitMenuItems(bool is_browser_mode,
                              bool should_open_button_options) OVERRIDE;
-  virtual void PopulateMenuItem(views::MenuItemView* menu,
-                                int index,
-                                int command_id) OVERRIDE;
 
-  // views::MenuDelegate implementation.
-  virtual const gfx::Font& GetLabelFont(int id) const OVERRIDE;
-  virtual bool IsItemChecked(int id) const OVERRIDE;
-  virtual bool IsCommandEnabled(int id) const OVERRIDE;
-  virtual void ExecuteCommand(int id) OVERRIDE;
+  // ui::MenuModel implementation
+  virtual int GetCommandIdAt(int index) const OVERRIDE;
 
  private:
-  const NetworkMenuModel* GetMenuItemModel(int id) const;
-
   scoped_ptr<NetworkMenuModel> vpn_menu_model_;
   scoped_ptr<MoreMenuModel> more_menu_model_;
 
@@ -275,59 +288,6 @@ class MainMenuModel : public NetworkMenuModel {
 ////////////////////////////////////////////////////////////////////////////////
 // NetworkMenuModel, public methods:
 
-void NetworkMenuModel::PopulateMenu(views::MenuItemView* menu) {
-  if (menu->HasSubmenu()) {
-    const int old_count = menu->GetSubmenu()->child_count();
-    for (int i = 0; i < old_count; ++i)
-      menu->RemoveMenuItemAt(0);
-  }
-
-  const int menu_items_count = GetItemCount();
-  for (int i = 0; i < menu_items_count; ++i)
-    PopulateMenuItem(menu, i, i);
-}
-
-void NetworkMenuModel::PopulateMenuItem(
-    views::MenuItemView* menu, int index, int command_id) {
-  DCHECK_GT(GetItemCount(), index);
-  switch (GetTypeAt(index)) {
-    case ui::MenuModel::TYPE_SEPARATOR:
-      menu->AppendSeparator();
-      break;
-    case ui::MenuModel::TYPE_COMMAND: {
-      views::MenuItemView* item = NULL;
-      SkBitmap icon;
-      if (GetIconAt(index, &icon)) {
-        item = menu->AppendMenuItemWithIcon(command_id,
-                                            UTF16ToWide(GetLabelAt(index)),
-                                            icon);
-      } else {
-        item = menu->AppendMenuItemWithLabel(command_id,
-                                             UTF16ToWide(GetLabelAt(index)));
-      }
-      item->set_margins(kTopMargin, kBottomMargin);
-      break;
-    }
-    case ui::MenuModel::TYPE_SUBMENU: {
-      views::MenuItemView* submenu = NULL;
-      SkBitmap icon;
-      if (GetIconAt(index, &icon)) {
-        submenu = menu->AppendSubMenuWithIcon(command_id,
-                                              UTF16ToWide(GetLabelAt(index)),
-                                              icon);
-      } else {
-        submenu = menu->AppendSubMenu(command_id,
-                                      UTF16ToWide(GetLabelAt(index)));
-      }
-      submenu->set_margins(kTopMargin, kBottomMargin);
-      GetSubmenuModelAt(index)->PopulateMenu(submenu);
-      break;
-    }
-    default:
-      NOTREACHED();
-  }
-}
-
 void NetworkMenuModel::ConnectToNetworkAt(int index,
                                           const std::string& passphrase,
                                           const std::string& ssid,
@@ -416,6 +376,10 @@ void NetworkMenuModel::ConnectToNetworkAt(int index,
 ////////////////////////////////////////////////////////////////////////////////
 // NetworkMenuModel, ui::MenuModel implementation:
 
+bool NetworkMenuModel::HasIcons() const {
+  return true;
+}
+
 int NetworkMenuModel::GetItemCount() const {
   return static_cast<int>(menu_items_.size());
 }
@@ -428,6 +392,10 @@ string16 NetworkMenuModel::GetLabelAt(int index) const {
   return menu_items_[index].label;
 }
 
+bool NetworkMenuModel::IsItemDynamicAt(int index) const {
+  return false;
+}
+
 const gfx::Font* NetworkMenuModel::GetLabelFontAt(int index) const {
   const gfx::Font* font = NULL;
   if (menu_items_[index].flags & FLAG_ASSOCIATED) {
@@ -439,11 +407,20 @@ const gfx::Font* NetworkMenuModel::GetLabelFontAt(int index) const {
   return font;
 }
 
+bool NetworkMenuModel::GetAcceleratorAt(int index,
+                                        ui::Accelerator* accelerator) const {
+  return false;
+}
+
 bool NetworkMenuModel::IsItemCheckedAt(int index) const {
   // All ui::MenuModel::TYPE_CHECK menu items are checked.
   return true;
 }
 
+int NetworkMenuModel::GetGroupIdAt(int index) const {
+  return 0;
+}
+
 bool NetworkMenuModel::GetIconAt(int index, SkBitmap* icon) {
   if (!menu_items_[index].icon.empty()) {
     *icon = menu_items_[index].icon;
@@ -452,14 +429,26 @@ bool NetworkMenuModel::GetIconAt(int index, SkBitmap* icon) {
   return false;
 }
 
+ui::ButtonMenuItemModel* NetworkMenuModel::GetButtonMenuItemAt(
+    int index) const {
+  return NULL;
+}
+
 bool NetworkMenuModel::IsEnabledAt(int index) const {
   return !(menu_items_[index].flags & FLAG_DISABLED);
 }
 
-NetworkMenuModel* NetworkMenuModel::GetSubmenuModelAt(int index) const {
+bool NetworkMenuModel::IsVisibleAt(int index) const {
+  return true;
+}
+
+ui::MenuModel* NetworkMenuModel::GetSubmenuModelAt(int index) const {
   return menu_items_[index].sub_menu_model;
 }
 
+void NetworkMenuModel::HighlightChangedTo(int index) {
+}
+
 void NetworkMenuModel::ActivatedAt(int index) {
   // When we are refreshing the menu, ignore menu item activation.
   if (owner_->refreshing_menu_)
@@ -506,6 +495,9 @@ void NetworkMenuModel::ActivatedAt(int index) {
   }
 }
 
+void NetworkMenuModel::SetMenuModelDelegate(ui::MenuModelDelegate* delegate) {
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NetworkMenuModel, private methods:
 
@@ -536,16 +528,6 @@ void NetworkMenuModel::ShowOther(ConnectionType type) const {
 ////////////////////////////////////////////////////////////////////////////////
 // MainMenuModel
 
-const NetworkMenuModel* MainMenuModel::GetMenuItemModel(int id) const {
-  if (id & kMoreIndexMask)
-    return more_menu_model_.get();
-  else if (id & kVPNIndexMask)
-    return vpn_menu_model_.get();
-  else if (id & kMainIndexMask)
-    return this;
-  return NULL;
-}
-
 void MainMenuModel::InitMenuItems(bool is_browser_mode,
                                   bool should_open_button_options) {
   // This gets called on initialization, so any changes should be reflected
@@ -864,45 +846,8 @@ void MainMenuModel::InitMenuItems(bool is_browser_mode,
   }
 }
 
-void MainMenuModel::PopulateMenuItem(
-    views::MenuItemView* menu, int index, int command_id) {
-  int main_command_id = command_id | kMainIndexMask;
-  NetworkMenuModel::PopulateMenuItem(menu, index, main_command_id);
-}
-
-// views::MenuDelegate implementation.
-
-const gfx::Font& MainMenuModel::GetLabelFont(int id) const {
-  const NetworkMenuModel* model = GetMenuItemModel(id);
-  const gfx::Font* font = NULL;
-  if (model)
-    font = model->GetLabelFontAt(id & kItemIndexMask);
-  return font ? *font : views::MenuDelegate::GetLabelFont(id);
-}
-
-bool MainMenuModel::IsItemChecked(int id) const {
-  const NetworkMenuModel* model = GetMenuItemModel(id);
-  if (model)
-    return model->IsItemCheckedAt(id & kItemIndexMask);
-  return views::MenuDelegate::IsItemChecked(id);
-}
-
-bool MainMenuModel::IsCommandEnabled(int id) const {
-  const NetworkMenuModel* model = GetMenuItemModel(id);
-  if (model)
-    return model->IsEnabledAt(id & kItemIndexMask);
-  return views::MenuDelegate::IsCommandEnabled(id);
-}
-
-// Not const, so can not use GetMenuItemModel().
-void MainMenuModel::ExecuteCommand(int id) {
-  int index = id & kItemIndexMask;
-  if (id & kMoreIndexMask)
-    return more_menu_model_->ActivatedAt(index);
-  else if (id & kVPNIndexMask)
-    return vpn_menu_model_->ActivatedAt(index);
-  else if (id & kMainIndexMask)
-    return ActivatedAt(index);
+int MainMenuModel::GetCommandIdAt(int index) const {
+  return index + kMainIndexMask;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -974,10 +919,8 @@ void VPNMenuModel::InitMenuItems(bool is_browser_mode,
   }
 }
 
-void VPNMenuModel::PopulateMenuItem(
-      views::MenuItemView* menu, int index, int command_id) {
-  int vpn_command_id = command_id | kVPNIndexMask;
-  NetworkMenuModel::PopulateMenuItem(menu, index, vpn_command_id);
+int VPNMenuModel::GetCommandIdAt(int index) const {
+  return index + kVPNIndexMask;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1059,10 +1002,8 @@ void MoreMenuModel::InitMenuItems(
       address_items.begin(), address_items.end());
 }
 
-void MoreMenuModel::PopulateMenuItem(
-      views::MenuItemView* menu, int index, int command_id) {
-  int more_command_id = command_id | kMoreIndexMask;
-  NetworkMenuModel::PopulateMenuItem(menu, index, more_command_id);
+int MoreMenuModel::GetCommandIdAt(int index) const {
+  return index + kMoreIndexMask;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1074,7 +1015,9 @@ NetworkMenu::NetworkMenu(Delegate* delegate, bool is_browser_mode)
       refreshing_menu_(false),
       min_width_(kDefaultMinimumWidth) {
   main_menu_model_.reset(new MainMenuModel(this));
-  menu_item_view_.reset(new views::MenuItemView(main_menu_model_.get()));
+  menu_model_adapter_.reset(
+      new views::MenuModelAdapter(main_menu_model_.get()));
+  menu_item_view_.reset(new views::MenuItemView(menu_model_adapter_.get()));
   menu_item_view_->set_has_icons(true);
   menu_item_view_->set_menu_position(
       views::MenuItemView::POSITION_BELOW_BOUNDS);
@@ -1083,6 +1026,10 @@ NetworkMenu::NetworkMenu(Delegate* delegate, bool is_browser_mode)
 NetworkMenu::~NetworkMenu() {
 }
 
+ui::MenuModel* NetworkMenu::GetMenuModel() {
+  return main_menu_model_.get();
+}
+
 void NetworkMenu::CancelMenu() {
   menu_item_view_->Cancel();
 }
@@ -1093,8 +1040,11 @@ void NetworkMenu::UpdateMenu() {
   refreshing_menu_ = true;
   main_menu_model_->InitMenuItems(
       is_browser_mode(), delegate_->ShouldOpenButtonOptions());
-  main_menu_model_->PopulateMenu(menu_item_view_.get());
+
+  menu_model_adapter_->BuildMenu(menu_item_view_.get());
+  SetMenuMargins(menu_item_view_.get(), kTopMargin, kBottomMargin);
   menu_item_view_->ChildrenChanged();
+
   refreshing_menu_ = false;
 }
 
diff --git a/chrome/browser/chromeos/status/network_menu.h b/chrome/browser/chromeos/status/network_menu.h
index 030d79a..7f011d9 100644
--- a/chrome/browser/chromeos/status/network_menu.h
+++ b/chrome/browser/chromeos/status/network_menu.h
@@ -28,9 +28,14 @@ namespace gfx {
 class Canvas;
 }
 
+namespace ui {
+class MenuModel;
+}
+
 namespace views {
 class MenuItemView;
 class MenuButton;
+class MenuModelAdapter;
 }
 
 namespace chromeos {
@@ -81,6 +86,9 @@ class NetworkMenu {
   NetworkMenu(Delegate* delegate, bool is_browser_mode);
   virtual ~NetworkMenu();
 
+  // Access to menu definition.
+  ui::MenuModel* GetMenuModel();
+
   // Cancels the active menu.
   void CancelMenu();
 
@@ -113,8 +121,9 @@ class NetworkMenu {
   bool refreshing_menu_;
 
   // The network menu.
-  scoped_ptr<views::MenuItemView> menu_item_view_;
   scoped_ptr<NetworkMenuModel> main_menu_model_;
+  scoped_ptr<views::MenuModelAdapter> menu_model_adapter_;
+  scoped_ptr<views::MenuItemView> menu_item_view_;
 
   // Holds minimum width of the menu.
   int min_width_;
diff --git a/chrome/browser/ui/views/menu_model_adapter_test.cc b/chrome/browser/ui/views/menu_model_adapter_test.cc
new file mode 100644
index 0000000..ff7c0a4
--- /dev/null
+++ b/chrome/browser/ui/views/menu_model_adapter_test.cc
@@ -0,0 +1,372 @@
+// Copyright (c) 2011 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/automation/ui_controls.h"
+#include "chrome/test/interactive_ui/view_event_test_base.h"
+#include "ui/base/models/menu_model.h"
+#include "views/controls/button/menu_button.h"
+#include "views/controls/menu/menu_controller.h"
+#include "views/controls/menu/menu_item_view.h"
+#include "views/controls/menu/menu_model_adapter.h"
+#include "views/controls/menu/submenu_view.h"
+#include "views/controls/menu/view_menu_delegate.h"
+#include "views/test/test_views_delegate.h"
+#include "views/views_delegate.h"
+#include "views/widget/root_view.h"
+#include "views/widget/widget.h"
+
+namespace {
+
+const int kTopMenuBaseId = 100;
+const int kSubMenuBaseId = 200;
+
+// ViewsDelegate::GetDispositionForEvent() is used by views::MenuModelAdapter.
+class TestViewsDelegate : public views::ViewsDelegate {
+ public:
+  TestViewsDelegate() {
+  }
+
+  ~TestViewsDelegate() {
+  }
+
+  // views::ViewsDelegate implementation
+  virtual ui::Clipboard* GetClipboard() const OVERRIDE {
+    return NULL;
+  }
+
+  virtual views::View* GetDefaultParentView() OVERRIDE
+  {
+    return NULL;
+  }
+
+  virtual void SaveWindowPlacement(const views::Widget* widget,
+                                   const std::wstring& window_name,
+                                   const gfx::Rect& bounds,
+                                   bool maximized) OVERRIDE {
+  }
+
+  virtual bool GetSavedWindowBounds(const std::wstring& window_name,
+                                    gfx::Rect* bounds) const OVERRIDE {
+    return false;
+  }
+
+  virtual bool GetSavedMaximizedState(const std::wstring& window_name,
+                                      bool* maximized) const OVERRIDE {
+    return false;
+  }
+
+  virtual void NotifyAccessibilityEvent(
+      views::View* view, ui::AccessibilityTypes::Event event_type) OVERRIDE {
+  }
+
+  virtual void NotifyMenuItemFocused(
+      const std::wstring& menu_name,
+      const std::wstring& menu_item_name,
+      int item_index,
+      int item_count,
+      bool has_submenu) OVERRIDE {
+  }
+
+#if defined(OS_WIN)
+  virtual HICON GetDefaultWindowIcon() const OVERRIDE
+  {
+    return NULL;
+  }
+#endif
+
+  virtual void AddRef() OVERRIDE {
+  }
+
+  virtual void ReleaseRef() OVERRIDE {
+  }
+
+  // Converts views::Event::flags to a WindowOpenDisposition.
+  virtual int GetDispositionForEvent(int event_flags) OVERRIDE {
+    return 0;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestViewsDelegate);
+};
+
+// Implement most of the ui::MenuModel pure virtuals for subclasses
+//
+// Exceptions:
+//  virtual int GetItemCount() const = 0;
+//  virtual ItemType GetTypeAt(int index) const = 0;
+//  virtual int GetCommandIdAt(int index) const = 0;
+//  virtual string16 GetLabelAt(int index) const = 0;
+class CommonMenuModel : public ui::MenuModel {
+ public:
+  CommonMenuModel() {
+  }
+
+  virtual ~CommonMenuModel() {
+  }
+
+ protected:
+  // ui::MenuModel implementation.
+  virtual bool HasIcons() const OVERRIDE {
+    return false;
+  }
+
+  virtual bool IsItemDynamicAt(int index) const OVERRIDE {
+    return false;
+  }
+
+  virtual bool GetAcceleratorAt(int index,
+                                ui::Accelerator* accelerator) const OVERRIDE {
+    return false;
+  }
+
+  virtual bool IsItemCheckedAt(int index) const OVERRIDE {
+    return false;
+  }
+
+  virtual int GetGroupIdAt(int index) const OVERRIDE {
+    return 0;
+  }
+
+  virtual bool GetIconAt(int index, SkBitmap* icon) OVERRIDE {
+    return false;
+  }
+
+  virtual ui::ButtonMenuItemModel* GetButtonMenuItemAt(
+      int index) const OVERRIDE {
+    return NULL;
+  }
+
+  virtual bool IsEnabledAt(int index) const OVERRIDE {
+    return true;
+  }
+
+  virtual ui::MenuModel* GetSubmenuModelAt(int index) const OVERRIDE {
+    return NULL;
+  }
+
+  virtual void HighlightChangedTo(int index) OVERRIDE {
+  }
+
+  virtual void ActivatedAt(int index) OVERRIDE {
+  }
+
+  virtual void SetMenuModelDelegate(ui::MenuModelDelegate* delegate) OVERRIDE {
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CommonMenuModel);
+};
+
+class SubMenuModel : public CommonMenuModel {
+ public:
+  SubMenuModel()
+      : showing_(false) {
+  }
+
+  ~SubMenuModel() {
+  }
+
+  bool showing() const {
+    return showing_;
+  }
+
+ private:
+  // ui::MenuModel implementation.
+  virtual int GetItemCount() const OVERRIDE {
+    return 1;
+  }
+
+  virtual ItemType GetTypeAt(int index) const OVERRIDE {
+    return TYPE_COMMAND;
+  }
+
+  virtual int GetCommandIdAt(int index) const OVERRIDE {
+    return index + kSubMenuBaseId;
+  }
+
+  virtual string16 GetLabelAt(int index) const OVERRIDE {
+    return ASCIIToUTF16("Item");
+  }
+
+  virtual void MenuWillShow() {
+    showing_ = true;
+  }
+
+  // Called when the menu has been closed.
+  virtual void MenuClosed() {
+    showing_ = false;
+  }
+
+  bool showing_;
+
+  DISALLOW_COPY_AND_ASSIGN(SubMenuModel);
+};
+
+class TopMenuModel : public CommonMenuModel {
+ public:
+  TopMenuModel() {
+  }
+
+  ~TopMenuModel() {
+  }
+
+  bool IsSubmenuShowing() {
+    return sub_menu_model_.showing();
+  }
+
+ private:
+  // ui::MenuModel implementation.
+  virtual int GetItemCount() const OVERRIDE {
+    return 1;
+  }
+
+  virtual ItemType GetTypeAt(int index) const OVERRIDE {
+    return TYPE_SUBMENU;
+  }
+
+  virtual int GetCommandIdAt(int index) const OVERRIDE {
+    return index + kTopMenuBaseId;
+  }
+
+  virtual string16 GetLabelAt(int index) const OVERRIDE {
+    return ASCIIToUTF16("submenu");
+  }
+
+  virtual MenuModel* GetSubmenuModelAt(int index) const OVERRIDE {
+    return &sub_menu_model_;
+  }
+
+  mutable SubMenuModel sub_menu_model_;
+
+  DISALLOW_COPY_AND_ASSIGN(TopMenuModel);
+};
+
+}  // namespace
+
+class MenuModelAdapterTest : public ViewEventTestBase,
+                             public views::ViewMenuDelegate {
+ public:
+  MenuModelAdapterTest() :
+      ViewEventTestBase(),
+      button_(NULL),
+      menu_model_adapter_(&top_menu_model_) {
+    old_views_delegate_ = views::ViewsDelegate::views_delegate;
+    views::ViewsDelegate::views_delegate = &views_delegate_;
+  }
+
+  virtual ~MenuModelAdapterTest() {
+    views::ViewsDelegate::views_delegate = old_views_delegate_;
+  }
+
+  // ViewEventTestBase implementation.
+
+  virtual void SetUp() OVERRIDE {
+    button_ = new views::MenuButton(NULL, L"Menu Adapter Test", this, true);
+
+    menu_.reset(new views::MenuItemView(&menu_model_adapter_));
+    menu_model_adapter_.BuildMenu(menu_.get());
+
+    ViewEventTestBase::SetUp();
+  }
+
+  virtual void TearDown() OVERRIDE {
+    menu_.reset(NULL);
+    ViewEventTestBase::TearDown();
+  }
+
+  virtual views::View* CreateContentsView() OVERRIDE {
+    return button_;
+  }
+
+  virtual gfx::Size GetPreferredSize() OVERRIDE {
+    return button_->GetPreferredSize();
+  }
+
+  // views::ViewMenuDelegate implementation.
+  virtual void RunMenu(views::View* source, const gfx::Point& pt) OVERRIDE {
+    gfx::Point screen_location;
+    views::View::ConvertPointToScreen(source, &screen_location);
+    gfx::Rect bounds(screen_location, source->size());
+    menu_->RunMenuAt(
+        source->GetWidget(),
+        button_,
+        bounds,
+        views::MenuItemView::TOPLEFT,
+        true);
+  }
+
+  // ViewEventTestBase implementation
+  virtual void DoTestOnMessageLoop() OVERRIDE {
+    Click(button_, CreateEventTask(this, &MenuModelAdapterTest::Step1));
+  }
+
+  // Open the submenu.
+  void Step1() {
+    views::SubmenuView* topmenu = menu_->GetSubmenu();
+    ASSERT_TRUE(topmenu);
+    ASSERT_TRUE(topmenu->IsShowing());
+    ASSERT_FALSE(top_menu_model_.IsSubmenuShowing());
+
+    // Click the first item to open the submenu.
+    views::MenuItemView* item = topmenu->GetMenuItemAt(0);
+    ASSERT_TRUE(item);
+    Click(item, CreateEventTask(this, &MenuModelAdapterTest::Step2));
+  }
+
+  // Rebuild the menu which should close the submenu.
+  void Step2() {
+    views::SubmenuView* topmenu = menu_->GetSubmenu();
+    ASSERT_TRUE(topmenu);
+    ASSERT_TRUE(topmenu->IsShowing());
+    ASSERT_TRUE(top_menu_model_.IsSubmenuShowing());
+
+    menu_model_adapter_.BuildMenu(menu_.get());
+
+    MessageLoopForUI::current()->PostTask(
+        FROM_HERE,
+        CreateEventTask(this, &MenuModelAdapterTest::Step3));
+  }
+
+  // Verify that the submenu MenuModel received the close callback
+  // and close the menu.
+  void Step3() {
+    views::SubmenuView* topmenu = menu_->GetSubmenu();
+    ASSERT_TRUE(topmenu);
+    ASSERT_TRUE(topmenu->IsShowing());
+    ASSERT_FALSE(top_menu_model_.IsSubmenuShowing());
+
+    // Click the button to exit the menu.
+    Click(button_, CreateEventTask(this, &MenuModelAdapterTest::Step4));
+  }
+
+  // All done.
+  void Step4() {
+    views::SubmenuView* topmenu = menu_->GetSubmenu();
+    ASSERT_TRUE(topmenu);
+    ASSERT_FALSE(topmenu->IsShowing());
+    ASSERT_FALSE(top_menu_model_.IsSubmenuShowing());
+
+    Done();
+  }
+
+ private:
+  // Generate a mouse click on the specified view and post a new task.
+  virtual void Click(views::View* view, Task* next) {
+    ui_controls::MoveMouseToCenterAndPress(
+        view,
+        ui_controls::LEFT,
+        ui_controls::DOWN | ui_controls::UP,
+        next);
+  }
+
+  views::ViewsDelegate* old_views_delegate_;
+  TestViewsDelegate views_delegate_;
+
+  views::MenuButton* button_;
+  TopMenuModel top_menu_model_;
+  views::MenuModelAdapter menu_model_adapter_;
+  scoped_ptr<views::MenuItemView> menu_;
+};
+
+VIEW_TEST(MenuModelAdapterTest, RebuildMenu)
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index dac82c3..7a85b0c 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -503,6 +503,7 @@
         'browser/ui/views/button_dropdown_test.cc',
         'browser/ui/views/find_bar_host_interactive_uitest.cc',
         'browser/ui/views/menu_item_view_test.cc',
+        'browser/ui/views/menu_model_adapter_test.cc',
         'browser/ui/views/ssl_client_certificate_selector_browsertest.cc',
         'browser/ui/views/tabs/tab_dragging_test.cc',
         'browser/ui/webui/workers_ui_browsertest.cc',
@@ -531,6 +532,7 @@
             'browser/ui/views/button_dropdown_test.cc',
             'browser/ui/views/find_bar_host_interactive_uitest.cc',
             'browser/ui/views/menu_item_view_test.cc',
+            'browser/ui/views/menu_model_adapter_test.cc',
             'browser/ui/views/tabs/tab_dragging_test.cc',
             'browser/ui/views/tabs/tab_strip_interactive_uitest.cc',
             'test/interactive_ui/npapi_interactive_test.cc',
@@ -558,6 +560,7 @@
             'browser/ui/views/button_dropdown_test.cc',
             'browser/ui/views/find_bar_host_interactive_uitest.cc',
             'browser/ui/views/menu_item_view_test.cc',
+            'browser/ui/views/menu_model_adapter_test.cc',
             'browser/ui/views/tabs/tab_dragging_test.cc',
             'browser/ui/views/tabs/tab_strip_interactive_uitest.cc',
             'test/interactive_ui/npapi_interactive_test.cc',
diff --git a/views/controls/menu/menu_item_view.cc b/views/controls/menu/menu_item_view.cc
index e8f0026..0878dd8 100644
--- a/views/controls/menu/menu_item_view.cc
+++ b/views/controls/menu/menu_item_view.cc
@@ -508,6 +508,14 @@ int MenuItemView::GetAcceleratorTextWidth() {
   return text.empty() ? 0 : GetFont().GetStringWidth(text);
 }
 
+void MenuItemView::SetMargins(int top_margin, int bottom_margin) {
+  top_margin_ = top_margin;
+  bottom_margin_ = bottom_margin;
+
+  // invalidate GetPreferredSize() cache
+  pref_size_.SetSize(0,0);
+}
+
 MenuItemView::MenuItemView(MenuItemView* parent,
                            int command,
                            MenuItemView::Type type)
diff --git a/views/controls/menu/menu_item_view.h b/views/controls/menu/menu_item_view.h
index ba2a365..a38d87c 100644
--- a/views/controls/menu/menu_item_view.h
+++ b/views/controls/menu/menu_item_view.h
@@ -322,10 +322,7 @@ class MenuItemView : public View {
 
   // Set top and bottom margins in pixels.  If no margin is set or a
   // negative margin is specified then MenuConfig values are used.
-  void set_margins(int top_margin, int bottom_margin) {
-    top_margin_ = top_margin;
-    bottom_margin_ = bottom_margin;
-  }
+  void SetMargins(int top_margin, int bottom_margin);
 
   // Set the position of the menu with respect to the bounds (top
   // level only).
diff --git a/views/controls/menu/menu_model_adapter.cc b/views/controls/menu/menu_model_adapter.cc
index 90a99db..e2c895d 100644
--- a/views/controls/menu/menu_model_adapter.cc
+++ b/views/controls/menu/menu_model_adapter.cc
@@ -32,7 +32,11 @@ void MenuModelAdapter::BuildMenu(MenuItemView* menu) {
       menu->RemoveMenuItemAt(0);
   }
 
-  menu_map_.clear();
+  // Leave entries in the map if the menu is being shown.  This
+  // allows the map to find the menu model of submenus being closed
+  // so ui::MenuModel::MenuClosed() can be called.
+  if (!menu->GetMenuController())
+    menu_map_.clear();
   menu_map_[menu] = menu_model_;
 
   // Repopulate the menu.
diff --git a/views/controls/menu/menu_model_adapter.h b/views/controls/menu/menu_model_adapter.h
index 2fbbf4b..12b487f 100644
--- a/views/controls/menu/menu_model_adapter.h
+++ b/views/controls/menu/menu_model_adapter.h
@@ -22,8 +22,7 @@ class MenuItemView;
 class MenuModelAdapter : public MenuDelegate {
  public:
   // The caller retains ownership of the ui::MenuModel instance and
-  // must ensure it exists for the lifetime of the adapter.  The
-  // base_id argument is the command id for the first menu item.
+  // must ensure it exists for the lifetime of the adapter.
   explicit MenuModelAdapter(ui::MenuModel* menu_model);
   virtual ~MenuModelAdapter();