MacViews: Views accelerators table should match the Cocoa one.

chrome/browser/ui/views/accelerator_table.cc

Index: chrome/browser/ui/views/accelerator_table.cc
diff --git a/chrome/browser/ui/views/accelerator_table.cc b/chrome/browser/ui/views/accelerator_table.cc
index 0aded273fff70dd16988ca808ea67a2c62d73e35..7fccc9fdb7607cd80401e9533fbe96134ba203b7 100644
--- a/chrome/browser/ui/views/accelerator_table.cc
+++ b/chrome/browser/ui/views/accelerator_table.cc
@@ -6,6 +6,10 @@
 
 #include <stddef.h>
 
+#include <algorithm>
+#include <initializer_list>
+#include <tuple>
+
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
@@ -170,7 +174,7 @@ 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  // !OS_MACOSX
+#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 },
@@ -196,7 +200,7 @@ const AcceleratorMapping kAcceleratorMap[] = {
   { 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 && !OS_MACOSX
+#endif  // !OS_CHROMEOS
 
 #if defined(OS_MACOSX)

[tapted, 2016/06/28 05:19:55]

Something weird has happened with the diff - I think it's been based on a local
branch now

   // VKEY_OEM_4 is Left Brace '[{' key.
@@ -275,11 +279,14 @@ std::vector<AcceleratorMapping> GetAcceleratorList() {
     accelerators.insert(accelerators.begin(), std::begin(kAcceleratorMap),
                         std::end(kAcceleratorMap));
 #if defined(OS_MACOSX)
+#if 1
     // Alt, Control, Shift without Command and accelerators without modifiers
     // are not very typical for native Mac apps. Simplify the kAcceleratorMap
     // table by filtering them out.
-    auto it = std::remove_if(accelerators.begin(), accelerators.end(),
-                             [](const AcceleratorMapping& m) {
+    std::vector<AcceleratorMapping> accelerators_mblsha = accelerators;
+    auto remove_start_mblsha = std::remove_if(accelerators_mblsha.begin(),
+                                              accelerators_mblsha.end(),
+                                              [](const AcceleratorMapping& m) {
       if (m.modifiers & ui::EF_COMMAND_DOWN) {
         // Command is Mac-specific, so this accelerator should be enabled.
         return false;
@@ -290,7 +297,53 @@ std::vector<AcceleratorMapping> GetAcceleratorList() {
              m.modifiers & ui::EF_ALT_DOWN ||
              m.modifiers & ui::EF_CONTROL_DOWN;
     });
-    accelerators.erase(it, accelerators.end());
+    accelerators_mblsha.erase(remove_start_mblsha, accelerators_mblsha.end());
+#endif
+
+    // 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.
+    const int kMask =
+        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 & kMask) == ui::EF_ALT_DOWN;
+    });
+
+#if DCHECK_IS_ON()
+    // For everything being removed, ensure there's another mapping.
+    for (auto it = remove_start; it != accelerators.end(); ++it) {
+      auto found = std::find_if(accelerators.begin(), remove_start,
+                                [&it](const AcceleratorMapping& m) {
+        return m.command_id == it->command_id;
+      });
+      DCHECK(found != accelerators.end());
+    }
+#endif
+    accelerators.erase(remove_start, accelerators.end());
+
+#if 1
+    VLOG(1) << "accelerators";
+    for (auto d : accelerators) {
+      VLOG(1) << d.command_id << " modifiers:"
+              << (d.modifiers & ui::EF_COMMAND_DOWN ? "(Cmd)" : "")
+              << (d.modifiers & ui::EF_CONTROL_DOWN ? "(Ctrl)" : "")
+              << (d.modifiers & ui::EF_ALT_DOWN ? "(Alt)" : "")
+              << (d.modifiers & ui::EF_SHIFT_DOWN ? "(Shift)" : "")
+              << "; keycode:" << d.keycode;
+    }
+
+    VLOG(1) << "\n\n\naccelerators_mblsha";
+    for (auto d : accelerators_mblsha) {
+      VLOG(1) << d.command_id << " modifiers:"
+              << (d.modifiers & ui::EF_COMMAND_DOWN ? "(Cmd)" : "")
+              << (d.modifiers & ui::EF_CONTROL_DOWN ? "(Ctrl)" : "")
+              << (d.modifiers & ui::EF_ALT_DOWN ? "(Alt)" : "")
+              << (d.modifiers & ui::EF_SHIFT_DOWN ? "(Shift)" : "")
+              << "; keycode:" << d.keycode;
+    }
+#endif
 #endif  // OS_MACOSX
   }
   return accelerators;