accelerator_table.cc.diff

diff --git a/chrome/browser/ui/views/accelerator_table.cc b/chrome/browser/ui/views/accelerator_table.cc
index e8770d2..b82b9d4 100644
--- a/chrome/browser/ui/views/accelerator_table.cc
+++ b/chrome/browser/ui/views/accelerator_table.cc
@@ -6,11 +6,6 @@
 
 #include <stddef.h>
 
-#include <algorithm>
-#include <initializer_list>
-#include <set>
-#include <tuple>
-
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
@@ -33,12 +28,12 @@ const ui::EventFlags kPlatformModifier = ui::EF_COMMAND_DOWN;
 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.
+// NOTE: Between each ifdef block, keep the 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, kPlatformModifier, IDC_BOOKMARK_PAGE },
   { ui::VKEY_D, ui::EF_SHIFT_DOWN | kPlatformModifier,
@@ -48,30 +43,14 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { 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, 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, 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, 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, kPlatformModifier, IDC_SAVE_PAGE },
   { ui::VKEY_9, kPlatformModifier, IDC_SELECT_LAST_TAB },
   { ui::VKEY_NUMPAD9, kPlatformModifier, IDC_SELECT_LAST_TAB },
@@ -82,6 +61,8 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { ui::VKEY_PRIOR, ui::EF_CONTROL_DOWN | ui::EF_SHIFT_DOWN,
     IDC_MOVE_TAB_PREVIOUS },
 #endif
+  // Control modifier is rarely used on Mac, so we allow it only in several
+  // specific cases.
   { ui::VKEY_TAB, ui::EF_CONTROL_DOWN, IDC_SELECT_NEXT_TAB },
   { ui::VKEY_NEXT, ui::EF_CONTROL_DOWN, IDC_SELECT_NEXT_TAB },
   { ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
@@ -121,17 +102,10 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { 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
+#endif  // OS_LINUX && !OS_CHROMEOS
   { 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, kPlatformModifier, IDC_ZOOM_MINUS },
   { ui::VKEY_SUBTRACT, kPlatformModifier, IDC_ZOOM_MINUS },
   { ui::VKEY_0, kPlatformModifier, IDC_ZOOM_NORMAL },
@@ -175,10 +149,8 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { 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
+#else  // !OS_CHROMEOS
   { 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 },
@@ -186,16 +158,14 @@ const AcceleratorMapping kAcceleratorMap[] = {
   // 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 },
-  // 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)
+  // For each entry until the end of the !OS_CHROMEOS block, and an entry into
+  // kChromeCmdId2AshActionId below if Ash has a corresponding accelerator.
+#if defined(GOOGLE_CHROME_BUILD) && !defined(OS_MACOSX)
   { ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, IDC_FEEDBACK },
-#endif
+#endif  // GOOGLE_CHROME_BUILD && !OS_MACOSX
+#if !defined(OS_MACOSX)
   { ui::VKEY_Q, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_EXIT },
+#endif  // !OS_MACOSX
   { ui::VKEY_N, ui::EF_SHIFT_DOWN | kPlatformModifier,
     IDC_NEW_INCOGNITO_WINDOW },
   { ui::VKEY_T, kPlatformModifier, IDC_NEW_TAB },
@@ -246,6 +216,46 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { 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 },
+#else  // !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. Mapping with just Alt appear here, and should have an
+  // alternative mapping in the block above.
+  { ui::VKEY_LEFT, ui::EF_ALT_DOWN, IDC_BACK },
+#if defined(ENABLE_BASIC_PRINTING)
+  { ui::VKEY_P, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_BASIC_PRINT},
+#endif  // ENABLE_BASIC_PRINTING
+#if !defined(OS_CHROMEOS)
+  { ui::VKEY_DELETE, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+    IDC_CLEAR_BROWSING_DATA },
+#endif  // !OS_CHROMEOS
+  { ui::VKEY_I, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN, IDC_DEV_TOOLS },
+  { 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_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_D, ui::EF_ALT_DOWN, IDC_FOCUS_LOCATION },
+  { ui::VKEY_E, ui::EF_CONTROL_DOWN, IDC_FOCUS_SEARCH },
+  { ui::VKEY_K, ui::EF_CONTROL_DOWN, IDC_FOCUS_SEARCH },
+  { ui::VKEY_T, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN, IDC_FOCUS_TOOLBAR },
+  { ui::VKEY_RIGHT, ui::EF_ALT_DOWN, IDC_FORWARD },
+  { ui::VKEY_HOME, ui::EF_ALT_DOWN, IDC_HOME },
+  { ui::VKEY_E, ui::EF_ALT_DOWN, IDC_SHOW_APP_MENU},
+  { ui::VKEY_F, ui::EF_ALT_DOWN, IDC_SHOW_APP_MENU},
+  { 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_U, ui::EF_CONTROL_DOWN, IDC_VIEW_SOURCE },
+#if !defined(OS_CHROMEOS)
+  // On Chrome OS, these keys are assigned to change UI scale.
+  { ui::VKEY_OEM_MINUS, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+    IDC_ZOOM_MINUS },
+  { ui::VKEY_OEM_PLUS, ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN,
+    IDC_ZOOM_PLUS },
+#endif  // !OS_CHROMEOS
 #endif  // OS_MACOSX
 };
 
@@ -287,53 +297,12 @@ const size_t kChromeCmdId2AshActionIdLength =
     arraysize(kChromeCmdId2AshActionId);
 #endif // defined(USE_ASH)
 
-std::vector<AcceleratorMapping> GetAllAcceleratorMappings() {
-  return std::vector<AcceleratorMapping>(std::begin(kAcceleratorMap),
-                                         std::end(kAcceleratorMap));
-}
-
 } // namespace
 
-std::vector<AcceleratorMapping> GetUnfilteredAcceleratorListForTesting() {
-  return GetAllAcceleratorMappings();
-}
-
 std::vector<AcceleratorMapping> GetAcceleratorList() {
-  static bool is_accelerator_list_initialized = false;
-  CR_DEFINE_STATIC_LOCAL(std::vector<AcceleratorMapping>, accelerators,
-                         (GetAllAcceleratorMappings()));
-  if (is_accelerator_list_initialized)
-    return accelerators;
-
-  is_accelerator_list_initialized = true;
-#if defined(OS_MACOSX)
-  // Control modifier is rarely used on Mac, so we allow it only in several
-  // whitelisted cases.
-  const std::set<int> control_whitelist = {
-      IDC_SELECT_NEXT_TAB, IDC_SELECT_PREVIOUS_TAB, IDC_FULLSCREEN,
-  };
-
-  auto remove_accelerators = [&control_whitelist](const AcceleratorMapping& m) {
-    // 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.
-    constexpr int kNonShiftMask =
-        ui::EF_COMMAND_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN;
-    if ((m.modifiers & kNonShiftMask) == ui::EF_ALT_DOWN)
-      return true;
-
-    // If command uses Control modifier it has to be whitelisted.
-    if ((m.modifiers & ui::EF_CONTROL_DOWN) != 0 &&
-        control_whitelist.count(m.command_id) == 0)
-      return true;
-
-    return false;
-  };
-  accelerators.erase(std::remove_if(accelerators.begin(), accelerators.end(),
-                                    remove_accelerators),
-                     accelerators.end());
-#endif  // OS_MACOSX
+  CR_DEFINE_STATIC_LOCAL(
+      std::vector<AcceleratorMapping>, accelerators,
+      (std::begin(kAcceleratorMap), std::end(kAcceleratorMap)));
   return accelerators;
 }
 
diff --git a/chrome/browser/ui/views/accelerator_table.h b/chrome/browser/ui/views/accelerator_table.h
index fa9255c..a3727c3 100644
--- a/chrome/browser/ui/views/accelerator_table.h
+++ b/chrome/browser/ui/views/accelerator_table.h
@@ -27,11 +27,6 @@ struct AcceleratorMapping {
 // handled by Chrome but excluding accelerators handled by Ash.
 CHROME_VIEWS_EXPORT std::vector<AcceleratorMapping> GetAcceleratorList();
 
-// Returns a full list of accelerator mapping without platform-specific
-// post-filtering.
-CHROME_VIEWS_EXPORT std::vector<AcceleratorMapping>
-GetUnfilteredAcceleratorListForTesting();
-
 // Returns true on Ash and if the command id has an associated accelerator which
 // is handled by Ash. If the return is true the accelerator is returned via the
 // second argument.
diff --git a/chrome/browser/ui/views/accelerator_table_unittest_mac.mm b/chrome/browser/ui/views/accelerator_table_unittest_mac.mm
index 9bf94ee..265b7d2 100644
--- a/chrome/browser/ui/views/accelerator_table_unittest_mac.mm
+++ b/chrome/browser/ui/views/accelerator_table_unittest_mac.mm
@@ -104,68 +104,6 @@ TEST(AcceleratorTableTest, CheckMacOSAltAccelerators) {
   }
 }
 
-// Vefifies that accelerator filtering works correctly and we have an
-// alternative shortcut for each filtered-out accelerator.
-TEST(AcceleratorTableTest, CheckMacOSFilteredOutAcceleratorsHaveAlternatives) {
-  // It's okay not to have an alternative accelerator for these commands as
-  // there's no alternative for them in the Cocoa browser.
-  const std::set<int> whitelisted_unique_shortcuts = {
-      IDC_FOCUS_TOOLBAR,
-      IDC_FOCUS_BOOKMARKS,
-      IDC_FOCUS_INFOBARS,
-      IDC_SHOW_APP_MENU,
-  };
-
-  // Convert the accelerator lists to sets for easy searching/filtering.
-  auto ToSet = [](const std::vector<AcceleratorMapping>& v) {
-    return std::set<AcceleratorMapping, bool (*)(const AcceleratorMapping&,
-                                                 const AcceleratorMapping&)>(
-        v.begin(), v.end(),
-        [](const AcceleratorMapping& lhs, const AcceleratorMapping& rhs) {
-          return std::tie(lhs.command_id, lhs.keycode, lhs.modifiers) <
-                 std::tie(rhs.command_id, rhs.keycode, rhs.modifiers);
-        });
-  };
-
-  const auto accelerators = ToSet(GetAcceleratorList());
-  const auto full_accelerators =
-      ToSet(GetUnfilteredAcceleratorListForTesting());
-
-  // Ensure that we still have alternative mappings for all filtered-out
-  // accelerators.
-  auto filtered_accelerators(full_accelerators);
-  // Remove all accelerators that are present in |accelerators|.
-  for (const auto& m : accelerators)
-    filtered_accelerators.erase(m);
-  EXPECT_GT(filtered_accelerators.size(), 1u);
-
-  for (const auto& removed_entry : filtered_accelerators) {
-    if (base::ContainsKey(whitelisted_unique_shortcuts,
-                          removed_entry.command_id))
-      continue;
-
-    const auto entry =
-        std::find_if(accelerators.begin(), accelerators.end(),
-                     [&removed_entry](const AcceleratorMapping& m) {
-          return removed_entry.command_id == m.command_id;
-        });
-    EXPECT_NE(entry, accelerators.end())
-        << "Filtered command doesn't have an alternative mapping: "
-        << removed_entry.command_id;
-  }
-
-  for (const auto& whitelist_entry : whitelisted_unique_shortcuts) {
-    const auto entry =
-        std::find_if(filtered_accelerators.begin(), filtered_accelerators.end(),
-                     [&whitelist_entry](const AcceleratorMapping& m) {
-          return whitelist_entry == m.command_id;
-        });
-    EXPECT_NE(entry, filtered_accelerators.end())
-        << "Whitelisted accelerator not found in the filtered-out list: "
-        << whitelist_entry;
-  }
-}
-
 // Verifies that we're not processing any duplicate accelerators in
 // global_keyboard_shortcuts_mac.mm functions.
 TEST(AcceleratorTableTest, CheckNoDuplicatesGlobalKeyboardShortcutsMac) {