MacViews: Views accelerators table should match the Cocoa one.

chrome/browser/ui/views/frame/browser_view_platform_mac.mm

Index: chrome/browser/ui/views/frame/browser_view_platform_mac.mm
diff --git a/chrome/browser/ui/views/frame/browser_view_platform_mac.mm b/chrome/browser/ui/views/frame/browser_view_platform_mac.mm
new file mode 100644
index 0000000000000000000000000000000000000000..bf49bcbb1858e03f6eb2356efcdd818ba30d00da
--- /dev/null
+++ b/chrome/browser/ui/views/frame/browser_view_platform_mac.mm
@@ -0,0 +1,125 @@
+// Copyright 2016 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.
+
+#import <Cocoa/Cocoa.h>
+
+#include "chrome/browser/ui/views/frame/browser_view_platform.h"
+
+#import "base/mac/foundation_util.h"
+#include "chrome/browser/global_keyboard_shortcuts_mac.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_window.h"
+
+namespace {
+
+// Type of functions listed in global_keyboard_shortcuts_mac.h.
+typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int, unichar);
+
+// If the key combination has an associated command, execute the command.
+bool HandleExtraKeyboardShortcut(

[tapted, 2016/10/17 07:02:47]

We shouldn't copy all this stuff. Is there a way to move/re-use the stuff
already in chrome_command_dispatcher_delegate.mm ?

browser_frame_mac.mm is already installing a ChromeCommandDispatcherDelegate --
can we access it via |BrowserFrame::native_browser_frame_|?

[themblsha, 2016/10/20 16:41:05]

Made this function public in chrome_command_dispatcher_delegate.mm. I've
uncommented the shortcuts in global_keyboard_shortcuts_mac.mm, and they're
mostly duplicating the ones in accelerator_table.cc. I think this duplication is
bad, and ChromeCommandDispatcherDelegate on MacViews should behave differently,
without consulting those tables (and consult the single MacViews-specific one).

+    NSEvent* event,
+    Browser* browser,
+    KeyToCommandMapper command_for_keyboard_shortcut) {
+  if (!browser)
+    return false;
+
+  // Extract info from |event|.
+  NSUInteger modifers = [event modifierFlags];
+  const bool command = modifers & NSCommandKeyMask;
+  const bool shift = modifers & NSShiftKeyMask;
+  const bool control = modifers & NSControlKeyMask;
+  const bool option = modifers & NSAlternateKeyMask;
+  const int key_code = [event keyCode];
+  const unichar key_char = KeyCharacterForEvent(event);
+
+  int cmd = command_for_keyboard_shortcut(command, shift, control, option,
+                                          key_code, key_char);
+
+  if (cmd == -1)
+    return false;
+
+  chrome::ExecuteCommand(browser, cmd);
+  return true;
+}
+
+bool HandleExtraWindowKeyboardShortcut(NSEvent* event, Browser* browser) {
+  return HandleExtraKeyboardShortcut(event, browser,
+                                     CommandForWindowKeyboardShortcut);
+}
+
+bool HandleDelayedWindowKeyboardShortcut(NSEvent* event, Browser* browser) {
+  return HandleExtraKeyboardShortcut(event, browser,
+                                     CommandForDelayedWindowKeyboardShortcut);
+}
+
+bool HandleExtraBrowserKeyboardShortcut(NSEvent* event, Browser* browser) {
+  return HandleExtraKeyboardShortcut(event, browser,
+                                     CommandForBrowserKeyboardShortcut);
+}
+
+bool ShouldHandleKeyboardEvent(const content::NativeWebKeyboardEvent& event) {
+  // Do not fire shortcuts on key up.
+  if ([event type] != NSKeyDown)
+    return false;
+
+  if (event.skip_in_browser ||
+      event.type == content::NativeWebKeyboardEvent::Char)
+    return false;
+
+  DCHECK(event.os_event != NULL);
+  return true;
+}
+
+// Copied from chrome_command_dispatcher_delegate.mm's
+// HandleExtraBrowserKeyboardShortcut.
+bool HandleExtraKeyboardShortcut(NSEvent* event, Browser* browser) {
+  // Send the event to the menu before sending it to the browser/window
+  // shortcut handling, so that if a user configures cmd-left to mean
+  // "previous tab", it takes precedence over the built-in "history back"
+  // binding. Other than that, the |-redispatchKeyEvent:| call would take care
+  // of invoking the original menu item shortcut as well.
+
+  if ([[NSApp mainMenu] performKeyEquivalent:event])
+    return true;
+
+  return HandleExtraBrowserKeyboardShortcut(event, browser) ||
+         HandleExtraWindowKeyboardShortcut(event, browser) ||
+         HandleDelayedWindowKeyboardShortcut(event, browser);
+}
+
+}  // namespace
+
+// static. Copied from BrowserWindowCocoa::PreHandleKeyboardEvent

[tapted, 2016/10/17 07:02:47]

nit: Move Copied from.. into the function body.

[themblsha, 2016/10/20 16:41:05]

Done.

+// implementation.
+bool BrowserViewPlatform::PreHandleKeyboardEvent(
+    const content::NativeWebKeyboardEvent& event,
+    Browser* browser) {
+  if (!ShouldHandleKeyboardEvent(event))
+    return false;
+
+  // CommandForKeyEvent consults the [NSApp mainMenu] and
+  // CommandForWindowKeyboardShortcut + CommandForBrowserKeyboardShortcut
+  // accelerator tables internally.
+  int command_id = CommandForKeyEvent(event.os_event);
+  if (command_id == -1)
+    return false;
+
+  if (!browser->command_controller()->IsReservedCommandOrKey(command_id, event))
+    return false;
+
+  return HandleExtraKeyboardShortcut(event.os_event, browser);
+}
+
+// static. Copied from BrowserWindowCocoa::HandleKeyboardEvent implementation.
+bool BrowserViewPlatform::HandleKeyboardEvent(
+    const content::NativeWebKeyboardEvent& event,
+    Browser* browser) {
+  if (!ShouldHandleKeyboardEvent(event))
+    return false;
+
+  return HandleExtraKeyboardShortcut(event.os_event, browser);
+}