views: Show accelerators in tabstrip context menu.

As it happens, the command-id is a TabStripModel::ContextMenuCommand. So it
needs to be converted into a browser command first, before the accelerator is
looked up.

BUG=None
TEST=Right click on a tab. The context menu should have proper accelerators
displayed (e.g. Ctrl+T for the 'New Tab' option).

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

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

5 files changed, 55 insertions, 53 deletions

diff --git a/chrome/browser/gtk/tabs/tab_gtk.cc b/chrome/browser/gtk/tabs/tab_gtk.cc
index b59fa84..b61c967 100644
--- a/chrome/browser/gtk/tabs/tab_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_gtk.cc
@@ -64,27 +64,10 @@ class TabGtk::ContextMenuController : public menus::SimpleMenuModel::Delegate,
   virtual bool GetAcceleratorForCommandId(
       int command_id,
       menus::Accelerator* accelerator) {
-    int browser_command = 0;
-    switch (command_id) {
-      case TabStripModel::CommandNewTab:
-        browser_command = IDC_NEW_TAB;
-        break;
-      case TabStripModel::CommandReload:
-        browser_command = IDC_RELOAD;
-        break;
-      case TabStripModel::CommandCloseTab:
-        browser_command = IDC_CLOSE_TAB;
-        break;
-      case TabStripModel::CommandRestoreTab:
-        browser_command = IDC_RESTORE_TAB;
-        break;
-      case TabStripModel::CommandBookmarkAllTabs:
-        browser_command = IDC_BOOKMARK_ALL_TABS;
-        break;
-      default:
-        return false;
-    }
-
+    int browser_command;
+    if (!TabStripModel::ContextMenuCommandToBrowserCommand(command_id,
+                                                           &browser_command))
+      return false;
     const menus::AcceleratorGtk* accelerator_gtk =
         Singleton<AcceleratorsGtk>()->GetPrimaryAcceleratorForCommand(
             browser_command);
@@ -101,34 +84,11 @@ class TabGtk::ContextMenuController : public menus::SimpleMenuModel::Delegate,
   }
 
   GtkWidget* GetImageForCommandId(int command_id) const {
-    int browser_cmd_id = 0;
-    switch (command_id) {
-      case TabStripModel::CommandNewTab:
-        browser_cmd_id = IDC_NEW_TAB;
-        break;
-
-      case TabStripModel::CommandReload:
-        browser_cmd_id = IDC_RELOAD;
-        break;
-
-      case TabStripModel::CommandCloseTab:
-        browser_cmd_id = IDC_CLOSE_TAB;
-        break;
-
-      case TabStripModel::CommandRestoreTab:
-        browser_cmd_id = IDC_RESTORE_TAB;
-        break;
-
-      case TabStripModel::CommandDuplicate:
-      case TabStripModel::CommandCloseOtherTabs:
-      case TabStripModel::CommandCloseTabsToRight:
-      case TabStripModel::CommandTogglePinned:
-      case TabStripModel::CommandBookmarkAllTabs:
-      case TabStripModel::CommandUseVerticalTabs:
-        return NULL;
-    }
-
-    return MenuGtk::Delegate::GetDefaultImageForCommandId(browser_cmd_id);
+    int browser_cmd_id;
+    return TabStripModel::ContextMenuCommandToBrowserCommand(command_id,
+                                                             &browser_cmd_id) ?
+        MenuGtk::Delegate::GetDefaultImageForCommandId(browser_cmd_id) :
+        NULL;
   }
 
   // The context menu.
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 1341578..51582ad 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -10,6 +10,7 @@
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "build/build_config.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/defaults.h"
@@ -795,6 +796,39 @@ void TabStripModel::Observe(NotificationType type,
   }
 }
 
+// static
+bool TabStripModel::ContextMenuCommandToBrowserCommand(int cmd_id,
+    int* browser_cmd) {
+  switch (cmd_id) {
+    case CommandNewTab:
+      *browser_cmd = IDC_NEW_TAB;
+      break;
+    case CommandReload:
+      *browser_cmd = IDC_RELOAD;
+      break;
+    case CommandDuplicate:
+      *browser_cmd = IDC_DUPLICATE_TAB;
+      break;
+    case CommandCloseTab:
+      *browser_cmd = IDC_CLOSE_TAB;
+      break;
+    case CommandRestoreTab:
+      *browser_cmd = IDC_RESTORE_TAB;
+      break;
+    case CommandBookmarkAllTabs:
+      *browser_cmd = IDC_BOOKMARK_ALL_TABS;
+      break;
+    case CommandUseVerticalTabs:
+      *browser_cmd = IDC_TOGGLE_VERTICAL_TABS;
+      break;
+    default:
+      *browser_cmd = 0;
+      return false;
+  }
+
+  return true;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // TabStripModel, private:
 
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 8d385cb..2aae16e 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -411,6 +411,10 @@ class TabStripModel : public NotificationObserver {
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
+  // Convert a ContextMenuCommand into a browser command. Returns true if a
+  // corresponding browser command exists, false otherwise.
+  static bool ContextMenuCommandToBrowserCommand(int cmd_id, int* browser_cmd);
+
  private:
   // We cannot be constructed without a delegate.
   TabStripModel();
diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
index 0ecfacf..426fe58 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
@@ -75,8 +75,12 @@ class BrowserTabStripController::TabContextMenuContents
   virtual bool GetAcceleratorForCommandId(
       int command_id,
       menus::Accelerator* accelerator) {
-    return controller_->tabstrip_->GetWidget()->GetAccelerator(command_id,
-                                                               accelerator);
+    int browser_cmd;
+    return TabStripModel::ContextMenuCommandToBrowserCommand(command_id,
+                                                             &browser_cmd) ?
+        controller_->tabstrip_->GetWidget()->GetAccelerator(browser_cmd,
+                                                            accelerator) :
+        false;
   }
   virtual void CommandIdHighlighted(int command_id) {
     controller_->StopHighlightTabsForCommand(last_command_, tab_);
@@ -169,7 +173,7 @@ void BrowserTabStripController::ExecuteCommandForTab(
     model_->ExecuteContextMenuCommand(model_index, command_id);
 }
 
-bool BrowserTabStripController::IsTabPinned(BaseTab* tab) {
+bool BrowserTabStripController::IsTabPinned(BaseTab* tab) const {
   return IsTabPinned(tabstrip_->GetModelIndexOfBaseTab(tab));
 }
 
diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
index 5fd7e11..1dcd1f4 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
@@ -37,7 +37,7 @@ class BrowserTabStripController : public TabStripController,
                               BaseTab* tab) const;
   void ExecuteCommandForTab(TabStripModel::ContextMenuCommand command_id,
                             BaseTab* tab);
-  bool IsTabPinned(BaseTab* tab);
+  bool IsTabPinned(BaseTab* tab) const;
 
   // TabStripController implementation:
   virtual int GetCount() const;