mac: Create a static mapping of accelerators in the main menu.

chrome/browser/ui/cocoa/accelerators_cocoa.mm

Index: chrome/browser/ui/cocoa/accelerators_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/accelerators_cocoa.mm b/chrome/browser/ui/cocoa/accelerators_cocoa.mm
index 154be6f91bda7a0fc604d4a68d84a687ebd5fdfc..3f73f8770a299c4d6418812149ab36527cb5c98c 100644
--- a/chrome/browser/ui/cocoa/accelerators_cocoa.mm
+++ b/chrome/browser/ui/cocoa/accelerators_cocoa.mm
@@ -9,50 +9,151 @@
 #include "base/memory/singleton.h"
 #include "chrome/app/chrome_command_ids.h"
 #import "ui/base/accelerators/platform_accelerator_cocoa.h"
+#import "ui/events/keycodes/keyboard_code_conversion_mac.h"
 
 namespace {
 
+// These accelerators are not associated with a command_id.
+const struct AcceleratorListing {
+  NSUInteger modifiers;  // The Cocoa modifiers.
+  ui::KeyboardCode keyXP;  // The key used for cross-platform compatibility.
+} kAcceleratorList [] = {
+  {NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_H},
+  {NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_W},
+  {NSCommandKeyMask | NSAlternateKeyMask | NSShiftKeyMask, ui::VKEY_V},
+  {NSCommandKeyMask, ui::VKEY_E},
+  {NSCommandKeyMask, ui::VKEY_J},
+  {NSCommandKeyMask, ui::VKEY_OEM_1},
+  {NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_OEM_1},
+  {NSCommandKeyMask, ui::VKEY_OEM_COMMA},
+  {NSCommandKeyMask | NSControlKeyMask, ui::VKEY_SPACE},
+};
+
 const struct AcceleratorMapping {
   int command_id;
-  NSString* key;
-  NSUInteger modifiers;
+  NSUInteger modifiers;  // The Cocoa modifiers.
+  ui::KeyboardCode keyXP;  // The key used for cross-platform compatibility.

[Robert Sesek, 2014/02/11 16:19:48]

"XP" is not a common term in Chromium, except if you're talking about a version
of Windows. I'd stick with the terms "key_code" for the cross-platform version
and "key_equivalent" for the Cocoa version.

[erikchen, 2014/02/11 19:42:27]

Done.

 } kAcceleratorMap[] = {
-  { IDC_CLEAR_BROWSING_DATA, @"\x8", NSCommandKeyMask | NSShiftKeyMask },
-  { IDC_COPY, @"c", NSCommandKeyMask },
-  { IDC_CUT, @"x", NSCommandKeyMask },
-  { IDC_DEV_TOOLS, @"i", NSCommandKeyMask | NSAlternateKeyMask },
-  { IDC_DEV_TOOLS_CONSOLE, @"j", NSCommandKeyMask | NSAlternateKeyMask },
-  { IDC_FIND, @"f", NSCommandKeyMask },
-  { IDC_FULLSCREEN, @"f", NSCommandKeyMask | NSShiftKeyMask },
-  { IDC_NEW_INCOGNITO_WINDOW, @"n", NSCommandKeyMask | NSShiftKeyMask },
-  { IDC_NEW_TAB, @"t", NSCommandKeyMask },
-  { IDC_NEW_WINDOW, @"n", NSCommandKeyMask },
-  { IDC_OPTIONS, @",", NSCommandKeyMask },
-  { IDC_PASTE, @"v", NSCommandKeyMask },
-  { IDC_PRINT, @"p", NSCommandKeyMask },
-  { IDC_RESTORE_TAB, @"t", NSCommandKeyMask | NSShiftKeyMask },
-  { IDC_SAVE_PAGE, @"s", NSCommandKeyMask },
-  { IDC_SHOW_BOOKMARK_BAR, @"b", NSCommandKeyMask | NSShiftKeyMask },
-  { IDC_SHOW_BOOKMARK_MANAGER, @"b", NSCommandKeyMask | NSAlternateKeyMask },
-  { IDC_BOOKMARK_PAGE, @"d", NSCommandKeyMask },
-  { IDC_SHOW_DOWNLOADS, @"j", NSCommandKeyMask | NSShiftKeyMask },
-  { IDC_SHOW_HISTORY, @"y", NSCommandKeyMask },
-  { IDC_VIEW_SOURCE, @"u", NSCommandKeyMask | NSAlternateKeyMask },
-  { IDC_ZOOM_MINUS, @"-", NSCommandKeyMask },
-  { IDC_ZOOM_PLUS, @"+", NSCommandKeyMask }
-};
+      // Accelerators used in the toolbar menu.

[Robert Sesek, 2014/02/11 16:19:48]

nit: the indent on this was correct before (only two spaces)

[erikchen, 2014/02/11 19:42:27]

This was the output of clang-format. I'm not sure why there are 6 spaces.
Reverted to 2.
On 2014/02/11 16:19:48, rsesek wrote:

+      {IDC_CLEAR_BROWSING_DATA, NSCommandKeyMask | NSShiftKeyMask,
+       ui::VKEY_BACK},
+      {IDC_COPY, NSCommandKeyMask, ui::VKEY_C},
+      {IDC_CUT, NSCommandKeyMask, ui::VKEY_X},
+      {IDC_DEV_TOOLS, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_I},
+      {IDC_DEV_TOOLS_CONSOLE, NSCommandKeyMask | NSAlternateKeyMask,
+       ui::VKEY_J},
+      {IDC_FIND, NSCommandKeyMask, ui::VKEY_F},
+      {IDC_FULLSCREEN, NSCommandKeyMask | NSControlKeyMask, ui::VKEY_F},
+      {IDC_NEW_INCOGNITO_WINDOW, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_N},
+      {IDC_NEW_TAB, NSCommandKeyMask, ui::VKEY_T},
+      {IDC_NEW_WINDOW, NSCommandKeyMask, ui::VKEY_N},
+      {IDC_PASTE, NSCommandKeyMask, ui::VKEY_V},
+      {IDC_PRINT, NSCommandKeyMask, ui::VKEY_P},
+      {IDC_RESTORE_TAB, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_T},
+      {IDC_SAVE_PAGE, NSCommandKeyMask, ui::VKEY_S},
+      {IDC_SHOW_BOOKMARK_BAR, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_B},
+      {IDC_SHOW_BOOKMARK_MANAGER, NSCommandKeyMask | NSAlternateKeyMask,
+       ui::VKEY_B},
+      {IDC_BOOKMARK_PAGE, NSCommandKeyMask, ui::VKEY_D},
+      {IDC_SHOW_DOWNLOADS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_J},
+      {IDC_SHOW_HISTORY, NSCommandKeyMask, ui::VKEY_Y},
+      {IDC_VIEW_SOURCE, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_U},
+      {IDC_ZOOM_MINUS, NSCommandKeyMask, ui::VKEY_OEM_MINUS},
+      {IDC_ZOOM_PLUS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_OEM_PLUS},
+
+      // Accelerators used in MainMenu.xib, but not the toolbar menu.
+      {IDC_HIDE_APP, NSCommandKeyMask, ui::VKEY_H},
+      {IDC_EXIT, NSCommandKeyMask, ui::VKEY_Q},
+      {IDC_OPEN_FILE, NSCommandKeyMask, ui::VKEY_O},
+      {IDC_FOCUS_LOCATION, NSCommandKeyMask, ui::VKEY_L},
+      {IDC_CLOSE_WINDOW, NSCommandKeyMask, ui::VKEY_W},
+      {IDC_EMAIL_PAGE_LOCATION, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_I},
+      {IDC_ADVANCED_PRINT, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_P},
+      {IDC_CONTENT_CONTEXT_UNDO, NSCommandKeyMask, ui::VKEY_Z},
+      {IDC_CONTENT_CONTEXT_REDO, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_Z},
+      {IDC_CONTENT_CONTEXT_CUT, NSCommandKeyMask, ui::VKEY_X},
+      {IDC_CONTENT_CONTEXT_COPY, NSCommandKeyMask, ui::VKEY_C},
+      {IDC_CONTENT_CONTEXT_PASTE, NSCommandKeyMask, ui::VKEY_V},
+      {IDC_CONTENT_CONTEXT_PASTE_AND_MATCH_STYLE,
+       NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_V},
+      {IDC_CONTENT_CONTEXT_SELECTALL, NSCommandKeyMask, ui::VKEY_A},
+      {IDC_FOCUS_SEARCH, NSCommandKeyMask | NSAlternateKeyMask, ui::VKEY_F},
+      {IDC_FIND_NEXT, NSCommandKeyMask, ui::VKEY_G},
+      {IDC_FIND_PREVIOUS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_G},
+      {IDC_ZOOM_PLUS, NSCommandKeyMask, ui::VKEY_OEM_PLUS},
+      {IDC_ZOOM_MINUS, NSCommandKeyMask, ui::VKEY_OEM_MINUS},
+      {IDC_STOP, NSCommandKeyMask, ui::VKEY_OEM_PERIOD},
+      {IDC_RELOAD, NSCommandKeyMask, ui::VKEY_R},
+      {IDC_RELOAD_IGNORING_CACHE, NSCommandKeyMask | NSShiftKeyMask,
+       ui::VKEY_R},
+      {IDC_PRESENTATION_MODE, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_F},
+      {IDC_ZOOM_NORMAL, NSCommandKeyMask, ui::VKEY_0},
+      {IDC_HOME, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_H},
+      {IDC_BACK, NSCommandKeyMask, ui::VKEY_OEM_4},
+      {IDC_FORWARD, NSCommandKeyMask, ui::VKEY_OEM_6},
+      {IDC_BOOKMARK_ALL_TABS, NSCommandKeyMask | NSShiftKeyMask, ui::VKEY_D},
+      {IDC_MINIMIZE_WINDOW, NSCommandKeyMask, ui::VKEY_M},
+      {IDC_SELECT_NEXT_TAB, NSCommandKeyMask | NSAlternateKeyMask,
+       ui::VKEY_RIGHT},
+      {IDC_SELECT_PREVIOUS_TAB, NSCommandKeyMask | NSAlternateKeyMask,
+       ui::VKEY_LEFT},
+      {IDC_TABPOSE, NSCommandKeyMask | NSControlKeyMask, ui::VKEY_T},
+      {IDC_HELP_PAGE_VIA_MENU, NSCommandKeyMask | NSShiftKeyMask,
+       ui::VKEY_OEM_2}, };
+
+}  // Anonymous namespace.

[Robert Sesek, 2014/02/11 16:19:48]

Revert this to what it was. "// namespace" is the correct terminating comment
for an anonymous namespace.

[erikchen, 2014/02/11 19:42:27]

Done.

 
-}  // namespace
+// Compute the cross-platform modifier from the cocoa modifier.
+int xpModifiersFromCocoaModifiers(NSUInteger modifiers) {

[Robert Sesek, 2014/02/11 16:19:48]

This should go in the namespae above, should be named with a capital letter, and
shouldn't use "xp" in the name.

[erikchen, 2014/02/11 19:42:27]

Done.

+  int modifiersXP = 0;
+  if ((modifiers & NSCommandKeyMask) != 0)
+    modifiersXP |= ui::EF_COMMAND_DOWN;
+  if ((modifiers & NSAlternateKeyMask) != 0)
+    modifiersXP |= ui::EF_ALT_DOWN;
+  if ((modifiers & NSShiftKeyMask) != 0)
+    modifiersXP |= ui::EF_SHIFT_DOWN;
+  if ((modifiers & NSControlKeyMask) != 0)
+    modifiersXP |= ui::EF_CONTROL_DOWN;
+  return modifiersXP;
+}
 
 AcceleratorsCocoa::AcceleratorsCocoa() {
   for (size_t i = 0; i < arraysize(kAcceleratorMap); ++i) {
     const AcceleratorMapping& entry = kAcceleratorMap[i];
-    ui::Accelerator accelerator(ui::VKEY_UNKNOWN, 0);
+    int xpModifiers = xpModifiersFromCocoaModifiers(entry.modifiers);

[Robert Sesek, 2014/02/11 16:19:48]

naming: here and throughout xp -> something else

[Robert Sesek, 2014/02/11 16:19:48]

I'm surprised we don't already have this function written somewhere.

[erikchen, 2014/02/11 19:42:27]

I've change xp -> x, as a shorthand for cross-platform. Let me know if you
prefer another naming.

On 2014/02/11 16:19:48, rsesek wrote:

[erikchen, 2014/02/11 19:42:27]

We have similar functions, but they all require NSEvents, and add additional
flags based on the NSEvent.

On 2014/02/11 16:19:48, rsesek wrote:

[Robert Sesek, 2014/02/11 22:18:16]

OK. It may be handy to refactor out the flag-conversion logic into a function
that can be used here along with the other places (if possible, I haven't
checked).

[Robert Sesek, 2014/02/11 22:18:16]

What about ui_modifiers vs cocoa_modifiers? Those are the names of the two
libraries between which you are converting.

[erikchen, 2014/02/11 23:16:28]

refactored completed.
On 2014/02/11 22:18:16, rsesek wrote:

[erikchen, 2014/02/11 23:16:28]

I found ui_modifiers to be confusing as well. I've decided to go with the full
name "cross_platform_modifiers"

On 2014/02/11 22:18:16, rsesek wrote:

+    ui::Accelerator accelerator(entry.keyXP, xpModifiers);
+
+    unichar character;
+    unichar characterIgnoringModifiers;

[Robert Sesek, 2014/02/11 16:19:48]

naming: here and throughout, in C++ or C functions and classes, the rule is to
use under_scores for variable names. Only in ObjC classes do you use camelCase.

[erikchen, 2014/02/11 19:42:27]

ah. got it.

On 2014/02/11 16:19:48, rsesek wrote:

+    int result = ui::MacKeyCodeForWindowsKeyCode(
+        entry.keyXP, entry.modifiers, &character, &characterIgnoringModifiers);
+    DCHECK(result != -1);
+    NSString* cocoaKeyEquivalent = [NSString stringWithFormat:@"%C", character];
+
     scoped_ptr<ui::PlatformAccelerator> platform_accelerator(
-        new ui::PlatformAcceleratorCocoa(entry.key, entry.modifiers));
+        new ui::PlatformAcceleratorCocoa(cocoaKeyEquivalent, entry.modifiers));
+
+
     accelerator.set_platform_accelerator(platform_accelerator.Pass());
     accelerators_.insert(std::make_pair(entry.command_id, accelerator));
   }
+  for (size_t i = 0; i < arraysize(kAcceleratorList); ++i) {
+    const AcceleratorListing& entry = kAcceleratorList[i];
+    int xpModifiers = xpModifiersFromCocoaModifiers(entry.modifiers);
+    ui::Accelerator accelerator(entry.keyXP, xpModifiers);
+
+    unichar character;
+    unichar characterIgnoringModifiers;
+    int result = ui::MacKeyCodeForWindowsKeyCode(
+        entry.keyXP, entry.modifiers, &character, &characterIgnoringModifiers);
+    DCHECK(result != -1);
+    NSString* string = [NSString stringWithFormat:@"%C", character];
+
+    scoped_ptr<ui::PlatformAccelerator> platform_accelerator(
+        new ui::PlatformAcceleratorCocoa(string, entry.modifiers));
+    accelerator.set_platform_accelerator(platform_accelerator.Pass());
+    acceleratorVector_.push_back(accelerator);
+  }
 }
 
 AcceleratorsCocoa::~AcceleratorsCocoa() {}
@@ -69,3 +170,34 @@ const ui::Accelerator* AcceleratorsCocoa::GetAcceleratorForCommand(
     return NULL;
   return &it->second;
 }
+
+const ui::Accelerator* AcceleratorsCocoa::GetAcceleratorForHotKey(
+    NSString* keyEquivalent, NSUInteger modifiers) {
+  for (AcceleratorVector::iterator it = acceleratorVector_.begin();

[Robert Sesek, 2014/02/11 16:19:48]

const_iterator?

[erikchen, 2014/02/11 19:42:27]

Done.

+      it != acceleratorVector_.end();

[Robert Sesek, 2014/02/11 16:19:48]

nit: align with 'A'

[erikchen, 2014/02/11 19:42:27]

Done.

+      ++it) {
+    const ui::Accelerator& accelerator = *it;
+    const ui::PlatformAcceleratorCocoa* platform_accelerator =
+      static_cast<const ui::PlatformAcceleratorCocoa*>(

[Robert Sesek, 2014/02/11 16:19:48]

nit: continuations are indented 4 spaces

[erikchen, 2014/02/11 19:42:27]

Done.

+          accelerator.platform_accelerator());
+    unichar character;
+    unichar characterIgnoringModifiers;
+    int result =
+        ui::MacKeyCodeForWindowsKeyCode(accelerator.key_code(),
+                                        platform_accelerator->modifier_mask(),
+                                        &character,
+                                        &characterIgnoringModifiers);
+    if (result == -1)
+      return NULL;
+
+    // Check for a match in the modifiers and keyEquivalent.
+    NSUInteger mask = platform_accelerator->modifier_mask();
+    BOOL maskEqual =
+        (mask == modifiers) || ((mask & (~NSShiftKeyMask)) == modifiers);
+    NSString* string = [NSString stringWithFormat:@"%C", character];
+    if ([string isEqual:keyEquivalent] && maskEqual)
+      return &*it;
+  }
+
+  return NULL;
+}