MacViews: Gets wrench menu working

WrenchMenuModel was set up differently between Mac and Views at compile
time. This change unifies the model so that the wrench menu can work with
MacViews.

Use ButtonMenuItemModel as submodels for Cut/Copy/Paste and
Zoom/Fullscreen menu buttons. Update Views menu to work with this model
change.

BUG=425229

Review URL: https://codereview.chromium.org/1003853004

Cr-Commit-Position: refs/heads/master@{#321016}

7 files changed, 70 insertions, 61 deletions

diff --git a/chrome/app/chrome_command_ids.h b/chrome/app/chrome_command_ids.h
index f7b0604..cd11b91 100644
--- a/chrome/app/chrome_command_ids.h
+++ b/chrome/app/chrome_command_ids.h
@@ -214,6 +214,7 @@
 #define IDC_SHOW_SRT_BUBBLE             40246
 #define IDC_ELEVATED_RECOVERY_DIALOG    40247
 #define IDC_TAKE_SCREENSHOT             40248
+#define IDC_MORE_TOOLS_MENU             40249
 
 // Spell-check
 // Insert any additional suggestions before _LAST; these have to be consecutive.
diff --git a/chrome/browser/ui/toolbar/wrench_menu_model.cc b/chrome/browser/ui/toolbar/wrench_menu_model.cc
index ad3717f..c7a0991 100644
--- a/chrome/browser/ui/toolbar/wrench_menu_model.cc
+++ b/chrome/browser/ui/toolbar/wrench_menu_model.cc
@@ -955,7 +955,7 @@ void WrenchMenuModel::Build() {
 
   AddSeparator(ui::NORMAL_SEPARATOR);
   AddSubMenuWithStringId(
-      IDC_ZOOM_MENU, IDS_MORE_TOOLS_MENU, tools_menu_model_.get());
+      IDC_MORE_TOOLS_MENU, IDS_MORE_TOOLS_MENU, tools_menu_model_.get());
 
   bool show_exit_menu = browser_defaults::kShowExitMenuItem;
 #if defined(OS_WIN)
@@ -1027,22 +1027,14 @@ void WrenchMenuModel::CreateExtensionToolbarOverflowMenu() {
 void WrenchMenuModel::CreateCutCopyPasteMenu() {
   AddSeparator(ui::LOWER_SEPARATOR);
 
-#if defined(OS_MACOSX)
   // WARNING: Mac does not use the ButtonMenuItemModel, but instead defines the
-  // layout for this menu item in Toolbar.xib. It does, however, use the
+  // layout for this menu item in WrenchMenu.xib. It does, however, use the
   // command_id value from AddButtonItem() to identify this special item.
   edit_menu_item_model_.reset(new ui::ButtonMenuItemModel(IDS_EDIT, this));
   edit_menu_item_model_->AddGroupItemWithStringId(IDC_CUT, IDS_CUT);
   edit_menu_item_model_->AddGroupItemWithStringId(IDC_COPY, IDS_COPY);
   edit_menu_item_model_->AddGroupItemWithStringId(IDC_PASTE, IDS_PASTE);
   AddButtonItem(IDC_EDIT_MENU, edit_menu_item_model_.get());
-#else
-  // WARNING: views/wrench_menu assumes these items are added in this order. If
-  // you change the order you'll need to update wrench_menu as well.
-  AddItemWithStringId(IDC_CUT, IDS_CUT);
-  AddItemWithStringId(IDC_COPY, IDS_COPY);
-  AddItemWithStringId(IDC_PASTE, IDS_PASTE);
-#endif
 
   AddSeparator(ui::UPPER_SEPARATOR);
 }
@@ -1051,29 +1043,18 @@ void WrenchMenuModel::CreateZoomMenu() {
   // This menu needs to be enclosed by separators.
   AddSeparator(ui::LOWER_SEPARATOR);
 
-#if defined(OS_MACOSX)
   // WARNING: Mac does not use the ButtonMenuItemModel, but instead defines the
-  // layout for this menu item in Toolbar.xib. It does, however, use the
+  // layout for this menu item in WrenchMenu.xib. It does, however, use the
   // command_id value from AddButtonItem() to identify this special item.
   zoom_menu_item_model_.reset(
       new ui::ButtonMenuItemModel(IDS_ZOOM_MENU, this));
-  zoom_menu_item_model_->AddGroupItemWithStringId(
-      IDC_ZOOM_MINUS, IDS_ZOOM_MINUS2);
-  zoom_menu_item_model_->AddButtonLabel(IDC_ZOOM_PERCENT_DISPLAY,
-                                        IDS_ZOOM_PLUS2);
-  zoom_menu_item_model_->AddGroupItemWithStringId(
-      IDC_ZOOM_PLUS, IDS_ZOOM_PLUS2);
-  zoom_menu_item_model_->AddSpace();
-  zoom_menu_item_model_->AddItemWithImage(
-      IDC_FULLSCREEN, IDR_FULLSCREEN_MENU_BUTTON);
+  zoom_menu_item_model_->AddGroupItemWithStringId(IDC_ZOOM_MINUS,
+                                                  IDS_ZOOM_MINUS2);
+  zoom_menu_item_model_->AddGroupItemWithStringId(IDC_ZOOM_PLUS,
+                                                  IDS_ZOOM_PLUS2);
+  zoom_menu_item_model_->AddItemWithImage(IDC_FULLSCREEN,
+                                          IDR_FULLSCREEN_MENU_BUTTON);
   AddButtonItem(IDC_ZOOM_MENU, zoom_menu_item_model_.get());
-#else
-  // WARNING: views/wrench_menu assumes these items are added in this order. If
-  // you change the order you'll need to update wrench_menu as well.
-  AddItemWithStringId(IDC_ZOOM_MINUS, IDS_ZOOM_MINUS);
-  AddItemWithStringId(IDC_ZOOM_PLUS, IDS_ZOOM_PLUS);
-  AddItemWithStringId(IDC_FULLSCREEN, IDS_FULLSCREEN);
-#endif
 
   AddSeparator(ui::UPPER_SEPARATOR);
 }
diff --git a/chrome/browser/ui/views/toolbar/wrench_menu.cc b/chrome/browser/ui/views/toolbar/wrench_menu.cc
index 8b96fd8..b206686 100644
--- a/chrome/browser/ui/views/toolbar/wrench_menu.cc
+++ b/chrome/browser/ui/views/toolbar/wrench_menu.cc
@@ -64,6 +64,7 @@
 using base::UserMetricsAction;
 using bookmarks::BookmarkModel;
 using content::WebContents;
+using ui::ButtonMenuItemModel;
 using ui::MenuModel;
 using views::CustomButton;
 using views::ImageButton;
@@ -238,7 +239,7 @@ class InMenuButtonBackground : public views::Background {
 };
 
 base::string16 GetAccessibleNameForWrenchMenuItem(
-      MenuModel* model, int item_index, int accessible_string_id) {
+    ButtonMenuItemModel* model, int item_index, int accessible_string_id) {
   base::string16 accessible_name =
       l10n_util::GetStringUTF16(accessible_string_id);
   base::string16 accelerator_text;
@@ -314,7 +315,7 @@ class WrenchMenuView : public views::View,
                        public views::ButtonListener,
                        public WrenchMenuObserver {
  public:
-  WrenchMenuView(WrenchMenu* menu, MenuModel* menu_model)
+  WrenchMenuView(WrenchMenu* menu, ButtonMenuItemModel* menu_model)
       : menu_(menu),
         menu_model_(menu_model) {
     menu_->AddObserver(this);
@@ -373,7 +374,7 @@ class WrenchMenuView : public views::View,
 
  protected:
   WrenchMenu* menu() { return menu_; }
-  MenuModel* menu_model() { return menu_model_; }
+  ButtonMenuItemModel* menu_model() { return menu_model_; }
 
  private:
   // Hosting WrenchMenu.
@@ -382,7 +383,7 @@ class WrenchMenuView : public views::View,
 
   // The menu model containing the increment/decrement/reset items.
   // WARNING: this may be NULL during shutdown.
-  MenuModel* menu_model_;
+  ButtonMenuItemModel* menu_model_;
 
   DISALLOW_COPY_AND_ASSIGN(WrenchMenuView);
 };
@@ -430,7 +431,7 @@ class HoveredImageSource : public gfx::ImageSkiaSource {
 class WrenchMenu::CutCopyPasteView : public WrenchMenuView {
  public:
   CutCopyPasteView(WrenchMenu* menu,
-                   MenuModel* menu_model,
+                   ButtonMenuItemModel* menu_model,
                    int cut_index,
                    int copy_index,
                    int paste_index)
@@ -484,7 +485,7 @@ class WrenchMenu::CutCopyPasteView : public WrenchMenuView {
 class WrenchMenu::ZoomView : public WrenchMenuView {
  public:
   ZoomView(WrenchMenu* menu,
-           MenuModel* menu_model,
+           ButtonMenuItemModel* menu_model,
            int decrement_index,
            int increment_index,
            int fullscreen_index)
@@ -851,8 +852,9 @@ void WrenchMenu::RunMenu(views::MenuButton* host) {
     if (model)
       model->RemoveObserver(this);
   }
-  if (selected_menu_model_)
+  if (selected_menu_model_) {
     selected_menu_model_->ActivatedAt(selected_index_);
+  }
 }
 
 void WrenchMenu::CloseMenu() {
@@ -995,7 +997,8 @@ bool WrenchMenu::IsCommandEnabled(int command_id) const {
   // The items representing the cut menu (cut/copy/paste), zoom menu
   // (increment/decrement/reset) and extension toolbar view are always enabled.
   // The child views of these items enabled state updates appropriately.
-  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+  if (command_id == IDC_EDIT_MENU || command_id == IDC_ZOOM_MENU ||
+      command_id == IDC_MORE_TOOLS_MENU ||
       command_id == IDC_EXTENSIONS_OVERFLOW_MENU)
     return true;
 
@@ -1013,7 +1016,8 @@ void WrenchMenu::ExecuteCommand(int command_id, int mouse_event_flags) {
     return;
   }
 
-  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+  if (command_id == IDC_EDIT_MENU || command_id == IDC_ZOOM_MENU ||
+      command_id == IDC_MORE_TOOLS_MENU ||
       command_id == IDC_EXTENSIONS_OVERFLOW_MENU) {
     // These items are represented by child views. If ExecuteCommand is invoked
     // it means the user clicked on the area around the buttons and we should
@@ -1030,7 +1034,8 @@ bool WrenchMenu::GetAccelerator(int command_id,
   if (IsBookmarkCommand(command_id))
     return false;
 
-  if (command_id == IDC_CUT || command_id == IDC_ZOOM_MINUS ||
+  if (command_id == IDC_EDIT_MENU || command_id == IDC_ZOOM_MENU ||
+      command_id == IDC_MORE_TOOLS_MENU ||
       command_id == IDC_EXTENSIONS_OVERFLOW_MENU) {
     // These have special child views; don't show the accelerator for them.
     return false;
@@ -1101,8 +1106,9 @@ void WrenchMenu::PopulateMenu(MenuItemView* parent,
     MenuItemView* item =
         AddMenuItem(parent, menu_index, model, i, model->GetTypeAt(i));
 
-    if (model->GetCommandIdAt(i) == IDC_CUT ||
-        model->GetCommandIdAt(i) == IDC_ZOOM_MINUS) {
+    if (model->GetCommandIdAt(i) == IDC_EDIT_MENU ||
+        model->GetCommandIdAt(i) == IDC_ZOOM_MENU ||
+        model->GetCommandIdAt(i) == IDC_MORE_TOOLS_MENU) {
       const MenuConfig& config = item->GetMenuConfig();
       int top_margin = config.item_top_margin + config.separator_height / 2;
       int bottom_margin =
@@ -1131,25 +1137,25 @@ void WrenchMenu::PopulateMenu(MenuItemView* parent,
         break;
       }
 
-      case IDC_CUT:
-        DCHECK_EQ(MenuModel::TYPE_COMMAND, model->GetTypeAt(i));
-        DCHECK_LT(i + 2, max);
-        DCHECK_EQ(IDC_COPY, model->GetCommandIdAt(i + 1));
-        DCHECK_EQ(IDC_PASTE, model->GetCommandIdAt(i + 2));
+      case IDC_EDIT_MENU: {
+        ui::ButtonMenuItemModel* submodel = model->GetButtonMenuItemAt(i);
+        DCHECK_EQ(IDC_CUT, submodel->GetCommandIdAt(0));
+        DCHECK_EQ(IDC_COPY, submodel->GetCommandIdAt(1));
+        DCHECK_EQ(IDC_PASTE, submodel->GetCommandIdAt(2));
         item->SetTitle(l10n_util::GetStringUTF16(IDS_EDIT2));
-        item->AddChildView(new CutCopyPasteView(this, model,
-                                                i, i + 1, i + 2));
-        i += 2;
+        item->AddChildView(new CutCopyPasteView(this, submodel, 0, 1, 2));
         break;
+      }
 
-      case IDC_ZOOM_MINUS:
-        DCHECK_EQ(MenuModel::TYPE_COMMAND, model->GetTypeAt(i));
-        DCHECK_EQ(IDC_ZOOM_PLUS, model->GetCommandIdAt(i + 1));
-        DCHECK_EQ(IDC_FULLSCREEN, model->GetCommandIdAt(i + 2));
+      case IDC_ZOOM_MENU: {
+        ui::ButtonMenuItemModel* submodel = model->GetButtonMenuItemAt(i);
+        DCHECK_EQ(IDC_ZOOM_MINUS, submodel->GetCommandIdAt(0));
+        DCHECK_EQ(IDC_ZOOM_PLUS, submodel->GetCommandIdAt(1));
+        DCHECK_EQ(IDC_FULLSCREEN, submodel->GetCommandIdAt(2));
         item->SetTitle(l10n_util::GetStringUTF16(IDS_ZOOM_MENU2));
-        item->AddChildView(new ZoomView(this, model, i, i + 1, i + 2));
-        i += 2;
+        item->AddChildView(new ZoomView(this, submodel, 0, 1, 2));
         break;
+      }
 
       case IDC_BOOKMARKS_MENU:
         DCHECK(!bookmark_menu_);
@@ -1223,7 +1229,7 @@ MenuItemView* WrenchMenu::AddMenuItem(MenuItemView* parent,
   return menu_item;
 }
 
-void WrenchMenu::CancelAndEvaluate(MenuModel* model, int index) {
+void WrenchMenu::CancelAndEvaluate(ButtonMenuItemModel* model, int index) {
   selected_menu_model_ = model;
   selected_index_ = index;
   root_->Cancel();
diff --git a/chrome/browser/ui/views/toolbar/wrench_menu.h b/chrome/browser/ui/views/toolbar/wrench_menu.h
index 22fb897..d9f9fc6 100644
--- a/chrome/browser/ui/views/toolbar/wrench_menu.h
+++ b/chrome/browser/ui/views/toolbar/wrench_menu.h
@@ -138,7 +138,7 @@ class WrenchMenu : public views::MenuDelegate,
 
   // Invoked from the cut/copy/paste menus. Cancels the current active menu and
   // activates the menu item in |model| at |index|.
-  void CancelAndEvaluate(ui::MenuModel* model, int index);
+  void CancelAndEvaluate(ui::ButtonMenuItemModel* model, int index);
 
   // Creates the bookmark menu if necessary. Does nothing if already created or
   // the bookmark model isn't loaded.
@@ -164,7 +164,7 @@ class WrenchMenu : public views::MenuDelegate,
   // If |selected_menu_model_| is non-null after the menu completes
   // ActivatedAt is invoked. This is done so that ActivatedAt isn't invoked
   // while the message loop is nested.
-  ui::MenuModel* selected_menu_model_;
+  ui::ButtonMenuItemModel* selected_menu_model_;
   int selected_index_;
 
   // Used for managing the bookmark menu items.
diff --git a/ui/base/models/button_menu_item_model.cc b/ui/base/models/button_menu_item_model.cc
index 83bd973..dd05fe2 100644
--- a/ui/base/models/button_menu_item_model.cc
+++ b/ui/base/models/button_menu_item_model.cc
@@ -88,6 +88,15 @@ bool ButtonMenuItemModel::IsItemDynamicAt(int index) const {
   return false;
 }
 
+bool ButtonMenuItemModel::GetAcceleratorAt(int index,
+                                           ui::Accelerator* accelerator) const {
+  if (delegate_) {
+    return delegate_->GetAcceleratorForCommandId(GetCommandIdAt(index),
+                                                 accelerator);
+  }
+  return false;
+}
+
 base::string16 ButtonMenuItemModel::GetLabelAt(int index) const {
   if (IsItemDynamicAt(index))
     return delegate_->GetLabelForCommandId(GetCommandIdAt(index));
@@ -106,9 +115,9 @@ bool ButtonMenuItemModel::PartOfGroup(int index) const {
   return items_[index].part_of_group;
 }
 
-void ButtonMenuItemModel::ActivatedCommand(int command_id) {
+void ButtonMenuItemModel::ActivatedAt(int index) {
   if (delegate_)
-    delegate_->ExecuteCommand(command_id, 0);
+    delegate_->ExecuteCommand(GetCommandIdAt(index), 0);
 }
 
 bool ButtonMenuItemModel::IsEnabledAt(int index) const {
diff --git a/ui/base/models/button_menu_item_model.h b/ui/base/models/button_menu_item_model.h
index b0bc95b..01666fe 100644
--- a/ui/base/models/button_menu_item_model.h
+++ b/ui/base/models/button_menu_item_model.h
@@ -12,6 +12,8 @@
 
 namespace ui {
 
+class Accelerator;
+
 // A model representing the rows of buttons that should be inserted in a button
 // containing menu item.
 class UI_BASE_EXPORT ButtonMenuItemModel {
@@ -34,6 +36,11 @@ class UI_BASE_EXPORT ButtonMenuItemModel {
     virtual bool IsCommandIdEnabled(int command_id) const;
     virtual bool DoesCommandIdDismissMenu(int command_id) const;
 
+    // Gets the accelerator for the specified command id. Returns true if the
+    // command id has a valid accelerator, false otherwise.
+    virtual bool GetAcceleratorForCommandId(int command_id,
+                                            ui::Accelerator* accelerator) = 0;
+
    protected:
     virtual ~Delegate() {}
   };
@@ -68,6 +75,10 @@ class UI_BASE_EXPORT ButtonMenuItemModel {
   // Whether the label for item |index| changes.
   bool IsItemDynamicAt(int index) const;
 
+  // Gets the accelerator information for the specified index, returning true if
+  // there is a shortcut accelerator for the item, false otherwise.
+  bool GetAcceleratorAt(int index, ui::Accelerator* accelerator) const;
+
   // Returns the current label value for the button at |index|.
   base::string16 GetLabelAt(int index) const;
 
@@ -79,8 +90,8 @@ class UI_BASE_EXPORT ButtonMenuItemModel {
   // other items that have their PartOfGroup bit set.
   bool PartOfGroup(int index) const;
 
-  // Called from implementations.
-  void ActivatedCommand(int command_id);
+  // Called when the item at the specified index has been activated.
+  void ActivatedAt(int index);
 
   // Returns the enabled state of the button at |index|.
   bool IsEnabledAt(int index) const;
diff --git a/ui/views/controls/menu/menu_model_adapter.cc b/ui/views/controls/menu/menu_model_adapter.cc
index 195658f..b04fd0b 100644
--- a/ui/views/controls/menu/menu_model_adapter.cc
+++ b/ui/views/controls/menu/menu_model_adapter.cc
@@ -66,6 +66,7 @@ MenuItemView* MenuModelAdapter::AddMenuItemFromModelAt(ui::MenuModel* model,
 
   switch (menu_type) {
     case ui::MenuModel::TYPE_COMMAND:
+    case ui::MenuModel::TYPE_BUTTON_ITEM:
       type = MenuItemView::NORMAL;
       label = model->GetLabelAt(model_index);
       sublabel = model->GetSublabelAt(model_index);