MacViews: Views accelerators table should match the Cocoa one.

chrome/browser/ui/views/frame/browser_view.cc

Index: chrome/browser/ui/views/frame/browser_view.cc
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index dbaf157c6e078beaa61153346d4d98c12344512c..71fc86a771230f17139da5028a1093067da76ce2 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -72,6 +72,7 @@
 #include "chrome/browser/ui/views/find_bar_host.h"
 #include "chrome/browser/ui/views/frame/browser_view_layout.h"
 #include "chrome/browser/ui/views/frame/browser_view_layout_delegate.h"
+#include "chrome/browser/ui/views/frame/browser_view_platform.h"
 #include "chrome/browser/ui/views/frame/contents_layout_manager.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
 #include "chrome/browser/ui/views/frame/top_container_view.h"
@@ -1417,6 +1418,9 @@ bool BrowserView::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event,
   }
 #endif  // defined(OS_CHROMEOS)
 
+  if (BrowserViewPlatform::PreHandleKeyboardEvent(event, browser()))

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

I'm not an owner here, but based on the surrounding code, it might be OK to put
this in #ifdef. That would allow an in-place comment like

#if defined(OS_MACOSX)
  // On Mac, users can configure their own shortcuts for commands in the
  // mainMenu via System Preferences (even while Chrome is running). So
  // first scan through system menus and possibly handle the event natively
  // so the menu bar flashes.
  ...
#endif

But! There's actually already a mechanism for adding platform-specific codepaths
using the |native_browser_frame_| member.

So I think PreHandleKeyboardEvent probably belongs in native_browser_frame.h
(and probably get a native_browser_frame.cc so that we can return false
everywhere but mac). But we'd need to defer to whatever an OWNER wants --
pkasting or sky

edit: based on later code - maybe NativeBrowserFrame would allow a lot more code
reuse here

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

Thanks! Moved the code there.

+    return true;
+
   chrome::BrowserCommandController* controller = browser_->command_controller();
 
   // Here we need to retrieve the command id (if any) associated to the
@@ -1454,6 +1458,9 @@ bool BrowserView::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event,
 }
 
 void BrowserView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {
+  if (BrowserViewPlatform::HandleKeyboardEvent(event, browser()))
+    return;
+
   unhandled_keyboard_event_handler_.HandleKeyboardEvent(event,

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

There is already a mac-specific UnhandledKeyboardEventHandler -- can we reuse
that here? Or comment why we need the separate HandleKeyboardEvent for Mac

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

Tried to do that:
https://gist.github.com/mblsha/d3ddab85dfff543efe8d4b808ae203aa

But got unresolved external linker error due to the use of BrowserView, and
decided to leave this as is.

                                                         GetFocusManager());
 }