Clean up the WrenchMenuModel so that it uses SimpleMenu::Delegate.

chrome/browser/wrench_menu_model.cc

Index: chrome/browser/wrench_menu_model.cc
diff --git a/chrome/browser/wrench_menu_model.cc b/chrome/browser/wrench_menu_model.cc
index 8a52c9ddb17059b36b6376524b04bb3f0838a878..ad380b13e13d0e483ab0b6b2786169e7c47fcab3 100644
--- a/chrome/browser/wrench_menu_model.cc
+++ b/chrome/browser/wrench_menu_model.cc
@@ -20,6 +20,7 @@
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/encoding_menu_controller.h"
 #include "chrome/browser/host_zoom_map.h"
+#include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/sync_ui_util.h"
@@ -29,6 +30,7 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
+#include "chrome/common/pref_names.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
@@ -37,6 +39,10 @@
 #include <gtk/gtk.h>
 #endif
 
+#if defined(OS_MACOSX)
+#include "chrome/browser/browser_window.h"
+#endif
+
 ////////////////////////////////////////////////////////////////////////////////
 // EncodingMenuModel
 
@@ -161,10 +167,10 @@ void ToolsMenuModel::Build(Browser* browser) {
 ////////////////////////////////////////////////////////////////////////////////
 // WrenchMenuModel
 
-WrenchMenuModel::WrenchMenuModel(menus::SimpleMenuModel::Delegate* delegate,
+WrenchMenuModel::WrenchMenuModel(menus::AcceleratorProvider* provider,
                                  Browser* browser)
-    : menus::SimpleMenuModel(delegate),
-      delegate_(delegate),
+    : ALLOW_THIS_IN_INITIALIZER_LIST(model_(this)),
+      provider_(provider),
       browser_(browser),
       tabstrip_model_(browser_->tabstrip_model()) {
   Build();
@@ -178,55 +184,73 @@ WrenchMenuModel::WrenchMenuModel(menus::SimpleMenuModel::Delegate* delegate,
                  NotificationService::AllSources());
 }
 
+// For testing.
+WrenchMenuModel::WrenchMenuModel() : model_(NULL) {

[sky, 2010/08/23 21:55:59]

Position should match that of header.

+}
+
 WrenchMenuModel::~WrenchMenuModel() {
   if (tabstrip_model_)
     tabstrip_model_->RemoveObserver(this);
 }
 
-bool WrenchMenuModel::IsLabelDynamicAt(int index) const {
-  return IsDynamicItem(index) || SimpleMenuModel::IsLabelDynamicAt(index);
+bool WrenchMenuModel::IsLabelForCommandIdDynamic(int command_id) const {
+  return command_id == IDC_ZOOM_PERCENT_DISPLAY ||
+         command_id == IDC_SYNC_BOOKMARKS ||
+#if defined(OS_MACOSX)
+         command_id == IDC_FULLSCREEN ||
+#endif
+         command_id == IDC_ABOUT;
 }
 
-string16 WrenchMenuModel::GetLabelAt(int index) const {
-  if (!IsDynamicItem(index))
-    return SimpleMenuModel::GetLabelAt(index);
-
-  int command_id = GetCommandIdAt(index);
-
+string16 WrenchMenuModel::GetLabelForCommandId(int command_id) const {
   switch (command_id) {
     case IDC_ABOUT:
       return GetAboutEntryMenuLabel();
     case IDC_SYNC_BOOKMARKS:
       return GetSyncMenuLabel();
+    case IDC_ZOOM_PERCENT_DISPLAY:
+      return zoom_label_;
+#if defined(OS_MACOSX)
+    case IDC_FULLSCREEN: {
+      int string_id = IDS_ENTER_FULLSCREEN_MAC;  // Default to Enter.
+      // Note: On startup, |window()| may be NULL.
+      if (browser_->window() && browser_->window()->IsFullscreen())
+        string_id = IDS_EXIT_FULLSCREEN_MAC;
+      return l10n_util::GetStringUTF16(string_id);
+    }
+#endif
     default:
       NOTREACHED();
       return string16();
   }
 }
 
-bool WrenchMenuModel::GetIconAt(int index, SkBitmap* icon) const {
-  if (GetCommandIdAt(index) == IDC_ABOUT &&
-      Singleton<UpgradeDetector>::get()->notify_upgrade()) {
-    // Show the exclamation point next to the menu item.
-    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
-    *icon = *rb.GetBitmapNamed(IDR_UPDATE_AVAILABLE);
-    return true;
-  }
-  return false;
+void WrenchMenuModel::ExecuteCommand(int command_id) {
+  browser_->ExecuteCommand(command_id);
 }
 
-bool WrenchMenuModel::IsLabelForCommandIdDynamic(int command_id) const {
-  return command_id == IDC_ZOOM_PERCENT_DISPLAY;
+bool WrenchMenuModel::IsCommandIdChecked(int command_id) const {
+#if defined(OS_CHROMEOS)
+  if (command_id == IDC_TOGGLE_VERTICAL_TABS) {
+    return browser_->UseVerticalTabs();
+  }
+#endif
+
+  if (command_id == IDC_SHOW_BOOKMARK_BAR) {
+    return browser_->profile()->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
+  }
+
+  return false;
 }
 
-string16 WrenchMenuModel::GetLabelForCommandId(int command_id) const {
-  DCHECK_EQ(IDC_ZOOM_PERCENT_DISPLAY, command_id);
-  return zoom_label_;
+bool WrenchMenuModel::IsCommandIdEnabled(int command_id) const {
+  return browser_->command_updater()->IsCommandEnabled(command_id);
 }
 
-void WrenchMenuModel::ExecuteCommand(int command_id) {
-  if (delegate_)
-    delegate_->ExecuteCommand(command_id);
+bool WrenchMenuModel::GetAcceleratorForCommandId(
+      int command_id,
+      menus::Accelerator* accelerator) {
+  return provider_->GetAcceleratorForCommandId(command_id, accelerator);
 }
 
 void WrenchMenuModel::TabSelectedAt(TabContents* old_contents,
@@ -257,11 +281,12 @@ void WrenchMenuModel::Observe(NotificationType type,
 }
 
 void WrenchMenuModel::Build() {
-  AddItemWithStringId(IDC_NEW_TAB, IDS_NEW_TAB);
-  AddItemWithStringId(IDC_NEW_WINDOW, IDS_NEW_WINDOW);
-  AddItemWithStringId(IDC_NEW_INCOGNITO_WINDOW, IDS_NEW_INCOGNITO_WINDOW);
+  model_.AddItemWithStringId(IDC_NEW_TAB, IDS_NEW_TAB);
+  model_.AddItemWithStringId(IDC_NEW_WINDOW, IDS_NEW_WINDOW);
+  model_.AddItemWithStringId(IDC_NEW_INCOGNITO_WINDOW,
+                             IDS_NEW_INCOGNITO_WINDOW);
 
-  AddSeparator();
+  model_.AddSeparator();
 #if defined(OS_MACOSX) || (defined(OS_LINUX) && !defined(TOOLKIT_VIEWS))
   // WARNING: Mac does not use the ButtonMenuItemModel, but instead defines the
   // layout for this menu item in Toolbar.xib. It does, however, use the
@@ -270,13 +295,13 @@ void WrenchMenuModel::Build() {
   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());
+  model_.AddButtonItem(IDC_EDIT_MENU, edit_menu_item_model_.get());
 #else
   // TODO(port): Move to the above.
   CreateCutCopyPaste();
 #endif
 
-  AddSeparator();
+  model_.AddSeparator();
 #if defined(OS_MACOSX) || (defined(OS_LINUX) && !defined(TOOLKIT_VIEWS))
   // WARNING: See above comment.
   zoom_menu_item_model_.reset(
@@ -290,68 +315,79 @@ void WrenchMenuModel::Build() {
   zoom_menu_item_model_->AddSpace();
   zoom_menu_item_model_->AddItemWithImage(
       IDC_FULLSCREEN, IDR_FULLSCREEN_MENU_BUTTON);
-  AddButtonItem(IDC_ZOOM_MENU, zoom_menu_item_model_.get());
+  model_.AddButtonItem(IDC_ZOOM_MENU, zoom_menu_item_model_.get());
 #else
   // TODO(port): Move to the above.
   CreateZoomFullscreen();
 #endif
 
-  AddSeparator();
-  AddItemWithStringId(IDC_SAVE_PAGE, IDS_SAVE_PAGE);
-  AddItemWithStringId(IDC_FIND, IDS_FIND);
-  AddItemWithStringId(IDC_PRINT, IDS_PRINT);
+  model_.AddSeparator();
+  model_.AddItemWithStringId(IDC_SAVE_PAGE, IDS_SAVE_PAGE);
+  model_.AddItemWithStringId(IDC_FIND, IDS_FIND);
+  model_.AddItemWithStringId(IDC_PRINT, IDS_PRINT);
 
-  tools_menu_model_.reset(new ToolsMenuModel(delegate(), browser_));
-  AddSubMenuWithStringId(IDC_ZOOM_MENU, IDS_TOOLS_MENU,
+  tools_menu_model_.reset(new ToolsMenuModel(this, browser_));
+  model_.AddSubMenuWithStringId(IDC_ZOOM_MENU, IDS_TOOLS_MENU,
                          tools_menu_model_.get());
 
-  AddSeparator();
+  model_.AddSeparator();
 #if defined(ENABLE_REMOTING)
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableRemoting)) {
-    AddItem(IDC_REMOTING_SETUP,
+    model_.AddItem(IDC_REMOTING_SETUP,
             l10n_util::GetStringUTF16(IDS_REMOTING_SETUP_LABEL));
   }
 #endif
-  AddItemWithStringId(IDC_SHOW_BOOKMARK_MANAGER, IDS_BOOKMARK_MANAGER);
-  AddItemWithStringId(IDC_SHOW_HISTORY, IDS_SHOW_HISTORY);
-  AddItemWithStringId(IDC_SHOW_DOWNLOADS, IDS_SHOW_DOWNLOADS);
-  AddSeparator();
+  model_.AddItemWithStringId(IDC_SHOW_BOOKMARK_MANAGER, IDS_BOOKMARK_MANAGER);
+  model_.AddItemWithStringId(IDC_SHOW_HISTORY, IDS_SHOW_HISTORY);
+  model_.AddItemWithStringId(IDC_SHOW_DOWNLOADS, IDS_SHOW_DOWNLOADS);
+  model_.AddSeparator();
 
 #if defined(OS_MACOSX)
-  AddItemWithStringId(IDC_OPTIONS, IDS_PREFERENCES_MAC);
+  model_.AddItemWithStringId(IDC_OPTIONS, IDS_PREFERENCES_MAC);
 #elif defined(OS_LINUX)
   GtkStockItem stock_item;
   if (gtk_stock_lookup(GTK_STOCK_PREFERENCES, &stock_item)) {
     const char16 kUnderscore[] = { '_', 0 };
     string16 preferences;
     RemoveChars(UTF8ToUTF16(stock_item.label), kUnderscore, &preferences);
-    AddItem(IDC_OPTIONS, preferences);
+    model_.AddItem(IDC_OPTIONS, preferences);
+  } else {
+    model_.AddItemWithStringId(IDC_OPTIONS, IDS_OPTIONS);
   }
 #else
-  AddItemWithStringId(IDC_OPTIONS, IDS_OPTIONS);
+  model_.AddItemWithStringId(IDC_OPTIONS, IDS_OPTIONS);
 #endif
 
 #if defined(OS_CHROMEOS)
-  AddCheckItemWithStringId(IDC_TOGGLE_VERTICAL_TABS,
+  model_.AddCheckItemWithStringId(IDC_TOGGLE_VERTICAL_TABS,
                            IDS_TAB_CXMENU_USE_VERTICAL_TABS);
 #endif
 
+  // TODO(erg): This entire section needs to be reworked and is out of scope of
+  // the first cleanup patch I'm doing. Part 1 (crbug.com/47320) is moving most
+  // of the logic from each platform specific UI code into the model here. Part
+  // 2 (crbug.com/46221) is normalizing the about menu item/hidden update menu
+  // item behaviour across the three platforms.
+
   // On Mac, there is no About item unless it is replaced with the update
   // available notification.
   if (browser_defaults::kShowAboutMenuItem ||
       Singleton<UpgradeDetector>::get()->notify_upgrade()) {
-    AddItem(IDC_ABOUT,
+    model_.AddItem(IDC_ABOUT,
             l10n_util::GetStringFUTF16(
                 IDS_ABOUT,
                 l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)));
+    // ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+    // model_.SetIcon(model_.GetIndexOfCommandId(IDC_ABOUT),
+    //                *rb.GetBitmapNamed(IDR_UPDATE_AVAILABLE));
   }
-  AddItemWithStringId(IDC_HELP_PAGE, IDS_HELP_PAGE);
+  model_.AddItemWithStringId(IDC_HELP_PAGE, IDS_HELP_PAGE);
   if (browser_defaults::kShowExitMenuItem) {
-    AddSeparator();
+    model_.AddSeparator();
 #if defined(OS_CHROMEOS)
-    AddItemWithStringId(IDC_EXIT, IDS_SIGN_OUT);
+    model_.AddItemWithStringId(IDC_EXIT, IDS_SIGN_OUT);
 #else
-    AddItemWithStringId(IDC_EXIT, IDS_EXIT);
+    model_.AddItemWithStringId(IDC_EXIT, IDS_EXIT);
 #endif
   }
 }
@@ -359,17 +395,17 @@ void WrenchMenuModel::Build() {
 void WrenchMenuModel::CreateCutCopyPaste() {
   // 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);
+  model_.AddItemWithStringId(IDC_CUT, IDS_CUT);
+  model_.AddItemWithStringId(IDC_COPY, IDS_COPY);
+  model_.AddItemWithStringId(IDC_PASTE, IDS_PASTE);
 }
 
 void WrenchMenuModel::CreateZoomFullscreen() {
   // 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);
+  model_.AddItemWithStringId(IDC_ZOOM_MINUS, IDS_ZOOM_MINUS);
+  model_.AddItemWithStringId(IDC_ZOOM_PLUS, IDS_ZOOM_PLUS);
+  model_.AddItemWithStringId(IDC_FULLSCREEN, IDS_FULLSCREEN);
 }
 
 void WrenchMenuModel::UpdateZoomControls() {
@@ -413,15 +449,3 @@ string16 WrenchMenuModel::GetAboutEntryMenuLabel() const {
   return l10n_util::GetStringFUTF16(
       IDS_ABOUT, l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
 }
-
-bool WrenchMenuModel::IsDynamicItem(int index) const {
-  int command_id = GetCommandIdAt(index);
-  return command_id == IDC_SYNC_BOOKMARKS ||
-         command_id == IDC_ABOUT;
-}
-
-bool WrenchMenuModel::IsCommandIdEnabled(int command_id) const {
-  if (delegate_)
-    return delegate_->IsCommandIdEnabled(command_id);
-  return true;
-}