MacViews: Views accelerators table should match the Cocoa one.

chrome/browser/ui/views/accelerator_table.cc

 // Copyright (c) 2012 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/ui/views/accelerator_table.h"
 
 #include <stddef.h>
 
+#include <algorithm>
+#include <initializer_list>
+#include <set>
+#include <tuple>
+
+#include "base/logging.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/events/event_constants.h"
 
 #if defined(USE_ASH)
 #include "ash/common/accelerators/accelerator_table.h"  // nogncheck
 #endif
 
 namespace chrome {
 namespace {
 
 // For many commands, the Mac equivalent uses Cmd instead of Ctrl. We only need
 // to list the ones that do not have a key equivalent in the main menu, i.e.
 // only the ones in global_keyboard_shortcuts_mac.mm.
-// TODO(jackhou): If-def out the accelerators that should not be on Mac.
 #if defined(OS_MACOSX)
 const ui::EventFlags kPlatformModifier = ui::EF_COMMAND_DOWN;
 #else
 const ui::EventFlags kPlatformModifier = ui::EF_CONTROL_DOWN;
 #endif
 
 // NOTE: Keep this list in the same (mostly-alphabetical) order as
 // the Windows accelerators in ../../app/chrome_dll.rc.
 // Do not use Ctrl-Alt as a shortcut modifier, as it is used by i18n keyboards:
 // http://blogs.msdn.com/b/oldnewthing/archive/2004/03/29/101121.aspx
 const AcceleratorMapping kAcceleratorMap[] = {
   { ui::VKEY_LEFT, ui::EF_ALT_DOWN, IDC_BACK },
   { ui::VKEY_BACK, ui::EF_NONE, IDC_BACKSPACE_BACK },
-  { ui::VKEY_D, ui::EF_CONTROL_DOWN, IDC_BOOKMARK_PAGE },
-  { ui::VKEY_D, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+  { ui::VKEY_D, kPlatformModifier, IDC_BOOKMARK_PAGE },
+  { ui::VKEY_D, ui::EF_SHIFT_DOWN | kPlatformModifier,
     IDC_BOOKMARK_ALL_TABS },
-  { ui::VKEY_W, ui::EF_CONTROL_DOWN, IDC_CLOSE_TAB },
-  { ui::VKEY_W, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_CLOSE_WINDOW },
-  { ui::VKEY_F, ui::EF_CONTROL_DOWN, IDC_FIND },
-  { ui::VKEY_G, ui::EF_CONTROL_DOWN, IDC_FIND_NEXT },
-  { ui::VKEY_G, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_FIND_PREVIOUS },
+  { ui::VKEY_W, kPlatformModifier, IDC_CLOSE_TAB },
+  { ui::VKEY_W, ui::EF_SHIFT_DOWN | kPlatformModifier, IDC_CLOSE_WINDOW },
+  { ui::VKEY_F, kPlatformModifier, IDC_FIND },
+  { ui::VKEY_G, kPlatformModifier, IDC_FIND_NEXT },
+  { ui::VKEY_G, ui::EF_SHIFT_DOWN | kPlatformModifier, IDC_FIND_PREVIOUS },
   { ui::VKEY_D, ui::EF_ALT_DOWN, IDC_FOCUS_LOCATION },
-  { ui::VKEY_L, ui::EF_CONTROL_DOWN, IDC_FOCUS_LOCATION },
+  { ui::VKEY_L, kPlatformModifier, IDC_FOCUS_LOCATION },
   { ui::VKEY_K, ui::EF_CONTROL_DOWN, IDC_FOCUS_SEARCH },
   { ui::VKEY_E, ui::EF_CONTROL_DOWN, IDC_FOCUS_SEARCH },
   { ui::VKEY_T, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, IDC_FOCUS_TOOLBAR },
   { ui::VKEY_B, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, IDC_FOCUS_BOOKMARKS },
   { ui::VKEY_A, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, IDC_FOCUS_INFOBARS },
   { ui::VKEY_RIGHT, ui::EF_ALT_DOWN, IDC_FORWARD },
   { ui::VKEY_BACK, ui::EF_SHIFT_DOWN, IDC_BACKSPACE_FORWARD },
   { ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_DEV_TOOLS },
   { ui::VKEY_F12, ui::EF_NONE, IDC_DEV_TOOLS_TOGGLE },
   { ui::VKEY_J, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
     IDC_DEV_TOOLS_CONSOLE },
   { ui::VKEY_C, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
     IDC_DEV_TOOLS_INSPECT },
-  { ui::VKEY_O, ui::EF_CONTROL_DOWN, IDC_OPEN_FILE },
-  { ui::VKEY_P, ui::EF_CONTROL_DOWN, IDC_PRINT},
+  { ui::VKEY_O, kPlatformModifier, IDC_OPEN_FILE },
+  { ui::VKEY_P, kPlatformModifier, IDC_PRINT },
 #if defined(ENABLE_BASIC_PRINTING)
   { ui::VKEY_P, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_BASIC_PRINT},
 #endif  // ENABLE_BASIC_PRINTING
-  { ui::VKEY_R, ui::EF_CONTROL_DOWN, IDC_RELOAD },
-  { ui::VKEY_R, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+  { ui::VKEY_R, kPlatformModifier, IDC_RELOAD },
+  { ui::VKEY_R, ui::EF_SHIFT_DOWN | kPlatformModifier,
     IDC_RELOAD_BYPASSING_CACHE },
   { ui::VKEY_HOME, ui::EF_ALT_DOWN, IDC_HOME },
-  { ui::VKEY_S, ui::EF_CONTROL_DOWN, IDC_SAVE_PAGE },
+  { ui::VKEY_S, kPlatformModifier, IDC_SAVE_PAGE },
   { ui::VKEY_9, kPlatformModifier, IDC_SELECT_LAST_TAB },
   { ui::VKEY_NUMPAD9, kPlatformModifier, IDC_SELECT_LAST_TAB },
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   { ui::VKEY_9, ui::EF_ALT_DOWN, IDC_SELECT_LAST_TAB },
   { ui::VKEY_NUMPAD9, ui::EF_ALT_DOWN, IDC_SELECT_LAST_TAB },
   { ui::VKEY_NEXT, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN, IDC_MOVE_TAB_NEXT },
   { ui::VKEY_PRIOR, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN,
     IDC_MOVE_TAB_PREVIOUS },
 #endif
   { ui::VKEY_TAB, ui::EF_CONTROL_DOWN, IDC_SELECT_NEXT_TAB },
@@ skipping 29 matching lines @@
   { ui::VKEY_5, ui::EF_ALT_DOWN, IDC_SELECT_TAB_4 },
   { ui::VKEY_NUMPAD5, ui::EF_ALT_DOWN, IDC_SELECT_TAB_4 },
   { ui::VKEY_6, ui::EF_ALT_DOWN, IDC_SELECT_TAB_5 },
   { ui::VKEY_NUMPAD6, ui::EF_ALT_DOWN, IDC_SELECT_TAB_5 },
   { ui::VKEY_7, ui::EF_ALT_DOWN, IDC_SELECT_TAB_6 },
   { ui::VKEY_NUMPAD7, ui::EF_ALT_DOWN, IDC_SELECT_TAB_6 },
   { ui::VKEY_8, ui::EF_ALT_DOWN, IDC_SELECT_TAB_7 },
   { ui::VKEY_NUMPAD8, ui::EF_ALT_DOWN, IDC_SELECT_TAB_7 },
   { ui::VKEY_BROWSER_FAVORITES, ui::EF_NONE, IDC_SHOW_BOOKMARK_BAR },
 #endif
-  { ui::VKEY_B, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+  { ui::VKEY_B, ui::EF_SHIFT_DOWN | kPlatformModifier,
     IDC_SHOW_BOOKMARK_BAR },
   { ui::VKEY_O, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
     IDC_SHOW_BOOKMARK_MANAGER },
   { ui::VKEY_J, ui::EF_CONTROL_DOWN, IDC_SHOW_DOWNLOADS },
   { ui::VKEY_H, ui::EF_CONTROL_DOWN, IDC_SHOW_HISTORY },
   { ui::VKEY_F, ui::EF_ALT_DOWN, IDC_SHOW_APP_MENU},
   { ui::VKEY_E, ui::EF_ALT_DOWN, IDC_SHOW_APP_MENU},
   { ui::VKEY_ESCAPE, ui::EF_NONE, IDC_STOP },
   { ui::VKEY_U, ui::EF_CONTROL_DOWN, IDC_VIEW_SOURCE },
-  { ui::VKEY_OEM_MINUS, ui::EF_CONTROL_DOWN, IDC_ZOOM_MINUS },
-  { ui::VKEY_SUBTRACT, ui::EF_CONTROL_DOWN, IDC_ZOOM_MINUS },
-  { ui::VKEY_0, ui::EF_CONTROL_DOWN, IDC_ZOOM_NORMAL },
-  { ui::VKEY_NUMPAD0, ui::EF_CONTROL_DOWN, IDC_ZOOM_NORMAL },
-  { ui::VKEY_OEM_PLUS, ui::EF_CONTROL_DOWN, IDC_ZOOM_PLUS },
-  { ui::VKEY_ADD, ui::EF_CONTROL_DOWN, IDC_ZOOM_PLUS },
+  { ui::VKEY_OEM_MINUS, kPlatformModifier, IDC_ZOOM_MINUS },
+  { ui::VKEY_SUBTRACT, kPlatformModifier, IDC_ZOOM_MINUS },
+  { ui::VKEY_0, kPlatformModifier, IDC_ZOOM_NORMAL },
+  { ui::VKEY_NUMPAD0, kPlatformModifier, IDC_ZOOM_NORMAL },
+  { ui::VKEY_OEM_PLUS, kPlatformModifier, IDC_ZOOM_PLUS },
+  { ui::VKEY_ADD, kPlatformModifier, IDC_ZOOM_PLUS },
+
+#if !defined(OS_MACOSX)  // Function keys aren't mapped on Mac.
   { ui::VKEY_F1, ui::EF_NONE, IDC_HELP_PAGE_VIA_KEYBOARD },
   { ui::VKEY_F3, ui::EF_NONE, IDC_FIND_NEXT },
   { ui::VKEY_F3, ui::EF_SHIFT_DOWN, IDC_FIND_PREVIOUS },
   { ui::VKEY_F4, ui::EF_CONTROL_DOWN, IDC_CLOSE_TAB },
   { ui::VKEY_F4, ui::EF_ALT_DOWN, IDC_CLOSE_WINDOW },
   { ui::VKEY_F5, ui::EF_NONE, IDC_RELOAD },
   { ui::VKEY_F5, ui::EF_CONTROL_DOWN, IDC_RELOAD_BYPASSING_CACHE },
   { ui::VKEY_F5, ui::EF_SHIFT_DOWN, IDC_RELOAD_BYPASSING_CACHE },
   { ui::VKEY_F6, ui::EF_NONE, IDC_FOCUS_NEXT_PANE },
   { ui::VKEY_F6, ui::EF_SHIFT_DOWN, IDC_FOCUS_PREVIOUS_PANE },
   { ui::VKEY_F10, ui::EF_NONE, IDC_FOCUS_MENU_BAR },
   { ui::VKEY_F11, ui::EF_NONE, IDC_FULLSCREEN },
+#endif  // !OS_MACOSX
 
   // Platform-specific key maps.
 #if defined(OS_LINUX)
   { ui::VKEY_BROWSER_BACK, ui::EF_NONE, IDC_BACK },
   { ui::VKEY_BROWSER_FORWARD, ui::EF_NONE, IDC_FORWARD },
   { ui::VKEY_BROWSER_HOME, ui::EF_NONE, IDC_HOME },
   { ui::VKEY_BROWSER_REFRESH, ui::EF_NONE, IDC_RELOAD },
   { ui::VKEY_BROWSER_REFRESH, ui::EF_CONTROL_DOWN, IDC_RELOAD_BYPASSING_CACHE },
   { ui::VKEY_BROWSER_REFRESH, ui::EF_SHIFT_DOWN, IDC_RELOAD_BYPASSING_CACHE },
 #endif  // defined(OS_LINUX)
 
 #if defined(OS_CHROMEOS)
   // On Chrome OS, VKEY_BROWSER_SEARCH is handled in Ash.
   { ui::VKEY_BACK, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
     IDC_CLEAR_BROWSING_DATA },
   { ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN, IDC_HELP_PAGE_VIA_KEYBOARD },
   { ui::VKEY_OEM_2, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
     IDC_HELP_PAGE_VIA_KEYBOARD },
   { ui::VKEY_BROWSER_FAVORITES, ui::EF_NONE, IDC_SHOW_BOOKMARK_MANAGER },
   { ui::VKEY_BROWSER_STOP, ui::EF_NONE, IDC_STOP },
   { ui::VKEY_P, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
     IDC_TOUCH_HUD_PROJECTION_TOGGLE },
-#else  // OS_CHROMEOS
+#else
   { ui::VKEY_ESCAPE, ui::EF_SHIFT_DOWN, IDC_TASK_MANAGER },
   { ui::VKEY_DELETE, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
     IDC_CLEAR_BROWSING_DATA },
   { ui::VKEY_LMENU, ui::EF_NONE, IDC_FOCUS_MENU_BAR },
   { ui::VKEY_MENU, ui::EF_NONE, IDC_FOCUS_MENU_BAR },
   { ui::VKEY_RMENU, ui::EF_NONE, IDC_FOCUS_MENU_BAR },
   // On Windows, all VKEY_BROWSER_* keys except VKEY_BROWSER_SEARCH are handled
   // via WM_APPCOMMAND.
   { ui::VKEY_BROWSER_SEARCH, ui::EF_NONE, IDC_FOCUS_SEARCH },
-  { ui::VKEY_M, ui::EF_SHIFT_DOWN | kPlatformModifier, IDC_SHOW_AVATAR_MENU},
+  { ui::VKEY_M, ui::EF_SHIFT_DOWN | kPlatformModifier, IDC_SHOW_AVATAR_MENU },
   // On Chrome OS, these keys are assigned to change UI scale.
   { ui::VKEY_OEM_PLUS, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_ZOOM_PLUS },
   { ui::VKEY_OEM_MINUS, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
     IDC_ZOOM_MINUS },
   // For each entry here add an entry into kChromeCmdId2AshActionId below
   // if Ash has a corresponding accelerator.
 #if defined(GOOGLE_CHROME_BUILD)
   { ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, IDC_FEEDBACK },
 #endif
   { ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_EXIT },
-  { ui::VKEY_N, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+  { ui::VKEY_N, ui::EF_SHIFT_DOWN | kPlatformModifier,
     IDC_NEW_INCOGNITO_WINDOW },
-  { ui::VKEY_T, ui::EF_CONTROL_DOWN, IDC_NEW_TAB },
-  { ui::VKEY_N, ui::EF_CONTROL_DOWN, IDC_NEW_WINDOW },
-  { ui::VKEY_T, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_RESTORE_TAB },
-#endif
+  { ui::VKEY_T, kPlatformModifier, IDC_NEW_TAB },
+  { ui::VKEY_N, kPlatformModifier, IDC_NEW_WINDOW },
+  { ui::VKEY_T, ui::EF_SHIFT_DOWN | kPlatformModifier, IDC_RESTORE_TAB },
+#endif  // !OS_CHROMEOS
+
+#if defined(OS_MACOSX)
+  // VKEY_OEM_4 is Left Brace '[{' key.
+  { ui::VKEY_OEM_4, ui::EF_COMMAND_DOWN, IDC_BACK },
+  { ui::VKEY_LEFT, ui::EF_COMMAND_DOWN, IDC_BACK },
+#if defined(ENABLE_BASIC_PRINTING)
+  { ui::VKEY_P, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN, IDC_BASIC_PRINT },
+#endif  // ENABLE_BASIC_PRINTING
+  { ui::VKEY_BACK, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
+    IDC_CLEAR_BROWSING_DATA },
+  { ui::VKEY_V, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
+    IDC_CONTENT_CONTEXT_PASTE_AND_MATCH_STYLE },
+  { ui::VKEY_Z, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
+    IDC_CONTENT_CONTEXT_REDO },
+  { ui::VKEY_A, ui::EF_COMMAND_DOWN, IDC_CONTENT_CONTEXT_SELECTALL },
+  { ui::VKEY_Z, ui::EF_COMMAND_DOWN, IDC_CONTENT_CONTEXT_UNDO },
+  { ui::VKEY_I, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN, IDC_DEV_TOOLS },
+  { ui::VKEY_J, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN, IDC_DEV_TOOLS_CONSOLE },
+  { ui::VKEY_C, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN, IDC_DEV_TOOLS_INSPECT},
+  { ui::VKEY_I, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
+    IDC_EMAIL_PAGE_LOCATION },
+  { ui::VKEY_Q, ui::EF_COMMAND_DOWN, IDC_EXIT },
+  { ui::VKEY_F, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN, IDC_FOCUS_SEARCH },
+  // VKEY_OEM_6 is Right Brace ']}' key.
+  { ui::VKEY_OEM_6, ui::EF_COMMAND_DOWN, IDC_FORWARD },
+  { ui::VKEY_RIGHT, ui::EF_COMMAND_DOWN, IDC_FORWARD },
+  { ui::VKEY_F, ui::EF_COMMAND_DOWN | ui::EF_CONTROL_DOWN, IDC_FULLSCREEN },
+  // VKEY_OEM_2 is Slash '/?' key

[tapted, 2016/11/01 00:22:50]

nit: fullstop at end.

[themblsha, 2016/11/01 14:36:24]

Done.

+  { ui::VKEY_OEM_2, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN,
+    IDC_HELP_PAGE_VIA_MENU },
+  { ui::VKEY_H, ui::EF_COMMAND_DOWN, IDC_HIDE_APP },
+  { ui::VKEY_H, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN, IDC_HOME },
+  { ui::VKEY_M, ui::EF_COMMAND_DOWN, IDC_MINIMIZE_WINDOW },
+  { ui::VKEY_RIGHT, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN,
+    IDC_SELECT_NEXT_TAB },
+  { ui::VKEY_LEFT, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN,
+    IDC_SELECT_PREVIOUS_TAB },
+  { ui::VKEY_B, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN,
+    IDC_SHOW_BOOKMARK_MANAGER },
+  { ui::VKEY_J, ui::EF_COMMAND_DOWN | ui::EF_SHIFT_DOWN, IDC_SHOW_DOWNLOADS },
+  { ui::VKEY_L, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN, IDC_SHOW_DOWNLOADS },
+  { ui::VKEY_Y, ui::EF_COMMAND_DOWN, IDC_SHOW_HISTORY },
+  { ui::VKEY_OEM_PERIOD, ui::EF_COMMAND_DOWN, IDC_STOP },
+  { ui::VKEY_U, ui::EF_COMMAND_DOWN | ui::EF_ALT_DOWN, IDC_VIEW_SOURCE },
+#endif  // OS_MACOSX
 };
-const size_t kAcceleratorMapLength = arraysize(kAcceleratorMap);
 
 const int kRepeatableCommandIds[] = {
   IDC_FIND_NEXT,
   IDC_FIND_PREVIOUS,
   IDC_FOCUS_NEXT_PANE,
   IDC_FOCUS_PREVIOUS_PANE,
   IDC_MOVE_TAB_NEXT,
   IDC_MOVE_TAB_PREVIOUS,
   IDC_SELECT_NEXT_TAB,
   IDC_SELECT_PREVIOUS_TAB,
@@ skipping 21 matching lines @@
 #if defined(OS_CHROMEOS)
   { IDC_TOUCH_HUD_PROJECTION_TOGGLE, ash::TOUCH_HUD_PROJECTION_TOGGLE },
 #endif
 };
 const size_t kChromeCmdId2AshActionIdLength =
     arraysize(kChromeCmdId2AshActionId);
 #endif // defined(USE_ASH)
 
 } // namespace
 
+std::vector<AcceleratorMapping> GetUnfilteredAcceleratorListForTesting() {
+  return std::vector<AcceleratorMapping>(std::begin(kAcceleratorMap),
+                                         std::end(kAcceleratorMap));

[Peter Kasting, 2016/10/31 18:07:32]

Nit: Or just inline kAcceleratorMap here:

  return {
      { ui::VKEY_LEFT, ui::EF_ALT_DOWN, IDC_BACK },
      { ui::VKEY_BACK, ui::EF_NONE, IDC_BACKSPACE_BACK },
      ....
  };

Then use this function when building |accelerators| in GetAcceleratorList()
below.

[themblsha, 2016/11/01 14:36:24]

I tried to preserve the original formatting of the kAcceleratorMap table in
order not to break the git blame. Should I reformat the table as well?

[Peter Kasting, 2016/11/01 18:20:58]

Yes, you should reformat/move it (don't insert your global accessor in the
middle of the anonymous namespace).  Don't worry about git blame.

If you want to make sure someone can really understand what's going on, do the
change to inline this as its own CL, with all the functional changes from this
current CL separate.

[themblsha, 2016/11/02 17:31:37]

Ok, I'd better do it separately alongside the de-chrome-ing of
AcceleratorMapping. I should create a crbug for that or use the one I'm using
for this CL? Or use https://bugs.chromium.org/p/chromium/issues/detail?id=289619
?

[Peter Kasting, 2016/11/02 17:51:28]

For de-chrome-ing, use bug 289619.  For inlining kAcceleratorMap, I'd probably
do it with no bug at all, but you're welcome to do whatever you want there.

+}
+
 std::vector<AcceleratorMapping> GetAcceleratorList() {
-  return std::vector<AcceleratorMapping>(
-      kAcceleratorMap, kAcceleratorMap + kAcceleratorMapLength);
+  CR_DEFINE_STATIC_LOCAL(std::vector<AcceleratorMapping>, accelerators, ());
+  if (!accelerators.empty())
+    return accelerators;
+
+  accelerators.insert(accelerators.begin(), std::begin(kAcceleratorMap),
+                      std::end(kAcceleratorMap));
+#if defined(OS_MACOSX)
+  // Alt by itself (or with just shift) is never used on Mac since it's used
+  // to generate non-ASCII characters. Such commands are given Mac-specific
+  // bindings as well, so remove the mappings with Alt, but not those with
+  // Command or Control.

[Peter Kasting, 2016/10/31 18:07:32]

Nit: I suggest combining these two remove calls:

  auto RemoveAccelerators = [](const AcceleratorMapping& m) {
    // Alt by itself (or with just shift) is never used on Mac since it's used
    // ...
    ...

    // Control modifier is rarely used on Mac, so we allow it only in several
    // ...
    ...
  };
  accelerators.erase(std::remove_if(accelerators.begin(), accelerators.end(),
                                    RemoveAccelerators),
                     accelerators.end());

This is not only more efficient, it reduces the amount of boilerplate (yay
readability) and makes it simpler to safely add more such tweaks in the future.

[themblsha, 2016/11/01 14:36:24]

Yep, looks much better now, thanks!

+  const int kNonShiftMask =

[Peter Kasting, 2016/10/31 18:07:32]

Nit: constexpr (2 places)

[themblsha, 2016/11/01 14:36:24]

Tried to apply it to the std::set as well, but got this error:
../../chrome/browser/ui/views/accelerator_table.cc:318:29: error: constexpr
variable cannot have non-literal type 'const std::set<int>'
    constexpr std::set<int> control_whitelist = {

Did you mean some other const expression?

[Peter Kasting, 2016/11/01 18:20:58]

That means those sets can't be constexpr because their constructors are not
declared constexpr.  Don't worry about it, they can just be const.

+      ui::EF_COMMAND_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN;
+  auto remove_start = std::remove_if(accelerators.begin(), accelerators.end(),
+                                     [](const AcceleratorMapping& m) {
+    return (m.modifiers & kNonShiftMask) == ui::EF_ALT_DOWN;
+  });
+  accelerators.erase(remove_start, accelerators.end());
+
+  // Control modifier is rarely used on Mac, so we allow it only in several
+  // whiltelisted cases.

[Peter Kasting, 2016/10/31 18:07:32]

Nit: whitelisted

[themblsha, 2016/11/01 14:36:24]

Done.

+  const std::set<int> control_whitelist = {

[Peter Kasting, 2016/10/31 18:07:32]

Nit: kControlWhitelist

[themblsha, 2016/11/01 14:36:24]

Done.

+      IDC_SELECT_NEXT_TAB, IDC_SELECT_PREVIOUS_TAB, IDC_FULLSCREEN,
+  };
+  remove_start =
+      std::remove_if(accelerators.begin(), accelerators.end(),
+                     [&control_whitelist](const AcceleratorMapping& m) {
+        return (m.modifiers & ui::EF_CONTROL_DOWN) != 0 &&
+               control_whitelist.count(m.command_id) == 0;
+      });
+  accelerators.erase(remove_start, accelerators.end());
+#endif  // OS_MACOSX
+  return accelerators;
 }
 
 bool GetAshAcceleratorForCommandId(int command_id,
                                    ui::Accelerator* accelerator) {
 #if defined(USE_ASH)
   for (size_t i = 0; i < kChromeCmdId2AshActionIdLength; ++i) {
     if (command_id == kChromeCmdId2AshActionId[i].chrome_cmd_id) {
       for (size_t j = 0; j < ash::kAcceleratorDataLength; ++j) {
         if (kChromeCmdId2AshActionId[i].ash_action_id ==
             ash::kAcceleratorData[j].action) {
           *accelerator = ui::Accelerator(ash::kAcceleratorData[j].keycode,
                                          ash::kAcceleratorData[j].modifiers);
           return true;
         }
       }
     }
   }
 #endif // defined(USE_ASH)
   return false;
 }
 
 bool GetStandardAcceleratorForCommandId(int command_id,
                                         ui::Accelerator* accelerator) {
   // The standard Ctrl-X, Ctrl-V and Ctrl-C are not defined as accelerators
   // anywhere else.
   switch (command_id) {
     case IDC_CUT:
-      *accelerator = ui::Accelerator(ui::VKEY_X, ui::EF_CONTROL_DOWN);
+      *accelerator = ui::Accelerator(ui::VKEY_X, kPlatformModifier);
       return true;
     case IDC_COPY:
-      *accelerator = ui::Accelerator(ui::VKEY_C, ui::EF_CONTROL_DOWN);
+      *accelerator = ui::Accelerator(ui::VKEY_C, kPlatformModifier);
       return true;
     case IDC_PASTE:
-      *accelerator = ui::Accelerator(ui::VKEY_V, ui::EF_CONTROL_DOWN);
+      *accelerator = ui::Accelerator(ui::VKEY_V, kPlatformModifier);
       return true;
   }
   return false;
 }
 
 bool IsCommandRepeatable(int command_id) {
   for (size_t i = 0; i < kRepeatableCommandIdsLength; ++i) {
     if (kRepeatableCommandIds[i] == command_id)
       return true;
   }
   return false;
 }
 
 }  // namespace chrome