views: close menus when accelerators are pressed on Mac

This change:

1) Has MenuController consider command to be an accelerator key, along with
   control and alt, for the purpose of not searching for mnemonics;
2) Adds a Mac override of ViewsDelegate::ProcessAcceleratorWhileMenuShowing
   which tells the menu to close for an accelerator

Bug: 885138
Change-Id: Iee9d9abb3b1bb3c2d346d7aaf7e532866b4ad26a
Reviewed-on: https://chromium-review.googlesource.com/c/1334147Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607699}

chrome/browser/ui/views/chrome_views_delegate.h

@@ -35,9 +35,11 @@ class ChromeViewsDelegate : public views::ViewsDelegate {
                                ui::WindowShowState* show_state) const override;
   void NotifyAccessibilityEvent(views::View* view,
                                 ax::mojom::Event event_type) override;
-#if defined(OS_CHROMEOS)
+#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
   ProcessMenuAcceleratorResult ProcessAcceleratorWhileMenuShowing(
       const ui::Accelerator& accelerator) override;
+#endif
+#if defined(OS_CHROMEOS)
   views::NonClientFrameView* CreateDefaultNonClientFrameView(
       views::Widget* widget) override;
 #endif

chrome/browser/ui/views/chrome_views_delegate_mac.cc

@@ -17,3 +17,19 @@ views::NativeWidget* ChromeViewsDelegate::CreateNativeWidget(
 bool ChromeViewsDelegate::ShouldMirrorArrowsInRTL() const {
   return base::FeatureList::IsEnabled(features::kMacRTL);
 }
+
+views::ViewsDelegate::ProcessMenuAcceleratorResult
+ChromeViewsDelegate::ProcessAcceleratorWhileMenuShowing(
+    const ui::Accelerator& accelerator) {
+  using Result = views::ViewsDelegate::ProcessMenuAcceleratorResult;
+  bool is_modified = accelerator.IsCtrlDown() || accelerator.IsAltDown() ||
+                     accelerator.IsCmdDown();
+
+  // Using an accelerator on Mac closes any open menu. Note that Mac behavior is
+  // different between context menus (which block use of accelerators) and other
+  // types of menus, which close when an accelerator is sent and do repost the
+  // accelerator. In MacViews, this happens naturally because context menus are
+  // (modal) Cocoa menus and other menus are Views menus, which will go through
+  // this code path.
+  return is_modified ? Result::CLOSE_MENU : Result::LEAVE_MENU_OPEN;
+}

ui/views/controls/menu/menu_controller.cc

@@ -1123,9 +1123,10 @@ ui::PostDispatchAction MenuController::OnWillDispatchKeyEvent(
     if (!this_ref)
       return ui::POST_DISPATCH_NONE;
 
-    // Do not check mnemonics if the Alt or Ctrl modifiers are pressed. For
-    // example Ctrl+<T> is an accelerator, but <T> only is a mnemonic.
-    const int kKeyFlagsMask = ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN;
+    // Do not check mnemonics if the Alt, Ctrl, or Cmd modifiers are pressed.
+    // For example Ctrl+<T> is an accelerator, but <T> only is a mnemonic.
+    const int kKeyFlagsMask =
+        ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN | ui::EF_COMMAND_DOWN;
     const int flags = event->flags();
     if (exit_type() == EXIT_NONE && (flags & kKeyFlagsMask) == 0) {
       base::char16 c = event->GetCharacter();