views: fix Mac menu key handling

This change:

1) Adds logic to EventMonitorMac so that if an event is marked handled by the
   client of EventMonitorMac, the event is not handled further - i.e.,
   EventMonitorMac can now eat events;
2) Renames the existing MenuPreTargetHandler to MenuPreTargetHandlerAura;
3) Adds MenuPreTargetHandlerMac which uses EventMonitorMac;
4) Adds MenuPreTargetHandler::Create to manufacture the platform-appropriate
   MenuPreTargetHandler instance.

Bug: 867127
Change-Id: I33f7c750598b0496fa291434098f8019e4b99c47
Reviewed-on: https://chromium-review.googlesource.com/1174568
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583254}

ui/views/BUILD.gn

@@ -455,6 +455,7 @@ jumbo_component("views") {
     "cocoa/widget_owner_nswindow_adapter.mm",
     "controls/button/label_button_label.cc",
     "controls/button/label_button_label.h",
+    "controls/menu/menu_pre_target_handler.h",
   ]
 
   sources += get_target_outputs(":views_vector_icons")
@@ -588,7 +589,6 @@ jumbo_component("views") {
       "accessibility/ax_widget_obj_wrapper.h",
       "accessibility/ax_window_obj_wrapper.h",
       "bubble/tray_bubble_view.h",
-      "controls/menu/menu_pre_target_handler.h",
       "controls/native/native_view_host_aura.h",
       "corewm/cursor_height_provider_win.h",
       "corewm/tooltip.h",
@@ -622,7 +622,8 @@ jumbo_component("views") {
       "accessibility/ax_window_obj_wrapper.cc",
       "bubble/tray_bubble_view.cc",
       "controls/menu/display_change_listener_aura.cc",
-      "controls/menu/menu_pre_target_handler.cc",
+      "controls/menu/menu_pre_target_handler_aura.cc",
+      "controls/menu/menu_pre_target_handler_aura.h",
       "controls/native/native_view_host_aura.cc",
       "corewm/cursor_height_provider_win.cc",
       "corewm/tooltip_aura.cc",
@@ -722,6 +723,10 @@ jumbo_component("views") {
 
   if (is_mac) {
     sources -= [ "controls/views_text_services_context_menu.cc" ]
+    sources += [
+      "controls/menu/menu_pre_target_handler_mac.h",
+      "controls/menu/menu_pre_target_handler_mac.mm",
+    ]
     deps += [
       "//components/crash/core/common",
       "//ui/accelerated_widget_mac",

ui/views/controls/menu/menu_controller.cc

@@ -31,6 +31,7 @@
 #include "ui/views/controls/menu/menu_controller_delegate.h"
 #include "ui/views/controls/menu/menu_host_root_view.h"
 #include "ui/views/controls/menu/menu_item_view.h"
+#include "ui/views/controls/menu/menu_pre_target_handler.h"
 #include "ui/views/controls/menu/menu_scroll_view_container.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/drag_utils.h"
@@ -55,7 +56,6 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tree_host.h"
-#include "ui/views/controls/menu/menu_pre_target_handler.h"
 #endif
 
 using base::TimeDelta;
@@ -461,12 +461,9 @@ void MenuController::Run(Widget* parent,
     if (owner_)
       owner_->AddObserver(this);
 
-#if defined(USE_AURA)
     // Only create a MenuPreTargetHandler for non-nested menus. Nested menus
     // will use the existing one.
-    menu_pre_target_handler_ =
-        std::make_unique<MenuPreTargetHandler>(this, owner_);
-#endif
+    menu_pre_target_handler_ = MenuPreTargetHandler::Create(this, owner_);
   }
 
 #if defined(OS_MACOSX)

ui/views/controls/menu/menu_controller.h

@@ -39,15 +39,12 @@ namespace views {
 class MenuButton;
 class MenuHostRootView;
 class MenuItemView;
+class MenuPreTargetHandler;
 class MouseEvent;
 class SubmenuView;
 class View;
 class ViewTracker;
 
-#if defined(USE_AURA)
-class MenuPreTargetHandler;
-#endif
-
 namespace internal {
 class MenuControllerDelegate;
 class MenuRunnerImpl;
@@ -731,9 +728,7 @@ class VIEWS_EXPORT MenuController
   std::unique_ptr<MenuCocoaWatcherMac> menu_cocoa_watcher_;
 #endif
 
-#if defined(USE_AURA)
   std::unique_ptr<MenuPreTargetHandler> menu_pre_target_handler_;
-#endif
 
   DISALLOW_COPY_AND_ASSIGN(MenuController);
 };

ui/views/controls/menu/menu_pre_target_handler.h

-// Copyright 2016 The Chromium Authors. All rights reserved.
+// Copyright 2018 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.
 
-#ifndef UI_VIEWS_CONTROLS_MENU_PRE_TARGET_HANDLER_H_
-#define UI_VIEWS_CONTROLS_MENU_PRE_TARGET_HANDLER_H_
+#ifndef UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_H_
+#define UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_H_
 
-#include "ui/aura/window_observer.h"
-#include "ui/events/event_handler.h"
-#include "ui/views/views_export.h"
-#include "ui/wm/public/activation_change_observer.h"
+#include <memory>
 
-namespace aura {
-class Window;
-}  // namespace aura
+#include "base/macros.h"
 
 namespace views {
 
 class MenuController;
 class Widget;
 
-// MenuPreTargetHandler is used to observe activation changes, cancel events,
-// and root window destruction, to shutdown the menu.
-//
-// Additionally handles key events to provide accelerator support to menus.
-class VIEWS_EXPORT MenuPreTargetHandler : public wm::ActivationChangeObserver,
-                                          public aura::WindowObserver,
-                                          public ui::EventHandler {
+// A MenuPreTargetHandler is responsible for intercepting events destined for
+// another widget (the menu's owning widget) and letting the menu's controller
+// try dispatching them first.
+class MenuPreTargetHandler {
  public:
-  MenuPreTargetHandler(MenuController* controller, Widget* owner);
-  ~MenuPreTargetHandler() override;
+  virtual ~MenuPreTargetHandler() = default;
 
-  // aura::client:ActivationChangeObserver:
-  void OnWindowActivated(wm::ActivationChangeObserver::ActivationReason reason,
-                         aura::Window* gained_active,
-                         aura::Window* lost_active) override;
+  // There are separate implementations of this method for Aura platforms and
+  // for Mac.
+  static std::unique_ptr<MenuPreTargetHandler> Create(
+      MenuController* controller,
+      Widget* owner);
 
-  // aura::WindowObserver:
-  void OnWindowDestroying(aura::Window* window) override;
-
-  // ui::EventHandler:
-  void OnCancelMode(ui::CancelModeEvent* event) override;
-  void OnKeyEvent(ui::KeyEvent* event) override;
+ protected:
+  MenuPreTargetHandler() = default;
 
  private:
-  void Cleanup();
-
-  MenuController* controller_;
-  aura::Window* root_;
-
   DISALLOW_COPY_AND_ASSIGN(MenuPreTargetHandler);
 };
 
 }  // namespace views
 
-#endif  // UI_VIEWS_CONTROLS_MENU_PRE_TARGET_HANDLER_H_
+#endif  //  UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_H_

ui/views/controls/menu/menu_pre_target_handler.cc → ui/views/controls/menu/menu_pre_target_handler_aura.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ui/views/controls/menu/menu_pre_target_handler.h"
+#include "ui/views/controls/menu/menu_pre_target_handler_aura.h"
 
 #include "ui/aura/env.h"
 #include "ui/aura/window.h"
@@ -20,7 +20,7 @@ aura::Window* GetOwnerRootWindow(views::Widget* owner) {
 
 }  // namespace
 
-MenuPreTargetHandler::MenuPreTargetHandler(MenuController* controller,
+MenuPreTargetHandlerAura::MenuPreTargetHandlerAura(MenuController* controller,
                                                    Widget* owner)
     : controller_(controller), root_(GetOwnerRootWindow(owner)) {
   aura::Env::GetInstance()->AddPreTargetHandler(
@@ -31,12 +31,12 @@ MenuPreTargetHandler::MenuPreTargetHandler(MenuController* controller,
   }
 }
 
-MenuPreTargetHandler::~MenuPreTargetHandler() {
+MenuPreTargetHandlerAura::~MenuPreTargetHandlerAura() {
   aura::Env::GetInstance()->RemovePreTargetHandler(this);
   Cleanup();
 }
 
-void MenuPreTargetHandler::OnWindowActivated(
+void MenuPreTargetHandlerAura::OnWindowActivated(
     wm::ActivationChangeObserver::ActivationReason reason,
     aura::Window* gained_active,
     aura::Window* lost_active) {
@@ -44,19 +44,19 @@ void MenuPreTargetHandler::OnWindowActivated(
     controller_->CancelAll();
 }
 
-void MenuPreTargetHandler::OnWindowDestroying(aura::Window* window) {
+void MenuPreTargetHandlerAura::OnWindowDestroying(aura::Window* window) {
   Cleanup();
 }
 
-void MenuPreTargetHandler::OnCancelMode(ui::CancelModeEvent* event) {
+void MenuPreTargetHandlerAura::OnCancelMode(ui::CancelModeEvent* event) {
   controller_->CancelAll();
 }
 
-void MenuPreTargetHandler::OnKeyEvent(ui::KeyEvent* event) {
+void MenuPreTargetHandlerAura::OnKeyEvent(ui::KeyEvent* event) {
   controller_->OnWillDispatchKeyEvent(event);
 }
 
-void MenuPreTargetHandler::Cleanup() {
+void MenuPreTargetHandlerAura::Cleanup() {
   if (!root_)
     return;
   // The ActivationClient may have been destroyed by the time we get here.
@@ -67,4 +67,11 @@ void MenuPreTargetHandler::Cleanup() {
   root_ = nullptr;
 }
 
+// static
+std::unique_ptr<MenuPreTargetHandler> MenuPreTargetHandler::Create(
+    MenuController* controller,
+    Widget* owner) {
+  return std::make_unique<MenuPreTargetHandlerAura>(controller, owner);
+}
+
 }  // namespace views

ui/views/controls/menu/menu_pre_target_handler_aura.h

+// 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.
+
+#ifndef UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_AURA_H_
+#define UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_AURA_H_
+
+#include "ui/aura/window_observer.h"
+#include "ui/events/event_handler.h"
+#include "ui/views/controls/menu/menu_pre_target_handler.h"
+#include "ui/views/views_export.h"
+#include "ui/wm/public/activation_change_observer.h"
+
+namespace aura {
+class Window;
+}  // namespace aura
+
+namespace views {
+
+class MenuController;
+class Widget;
+
+// MenuPreTargetHandlerAura is used to observe activation changes, cancel
+// events, and root window destruction, to shutdown the menu.
+//
+// Additionally handles key events to provide accelerator support to menus.
+class VIEWS_EXPORT MenuPreTargetHandlerAura
+    : public wm::ActivationChangeObserver,
+      public aura::WindowObserver,
+      public ui::EventHandler,
+      public MenuPreTargetHandler {
+ public:
+  MenuPreTargetHandlerAura(MenuController* controller, Widget* owner);
+  ~MenuPreTargetHandlerAura() override;
+
+  // aura::client:ActivationChangeObserver:
+  void OnWindowActivated(wm::ActivationChangeObserver::ActivationReason reason,
+                         aura::Window* gained_active,
+                         aura::Window* lost_active) override;
+
+  // aura::WindowObserver:
+  void OnWindowDestroying(aura::Window* window) override;
+
+  // ui::EventHandler:
+  void OnCancelMode(ui::CancelModeEvent* event) override;
+  void OnKeyEvent(ui::KeyEvent* event) override;
+
+ private:
+  void Cleanup();
+
+  MenuController* controller_;
+  aura::Window* root_;
+
+  DISALLOW_COPY_AND_ASSIGN(MenuPreTargetHandlerAura);
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_AURA_H_

ui/views/controls/menu/menu_pre_target_handler_mac.h

+// Copyright 2018 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.
+
+#ifndef UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_MAC_H_
+#define UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_MAC_H_
+
+#include "ui/events/event_handler.h"
+#include "ui/views/controls/menu/menu_pre_target_handler.h"
+#include "ui/views/event_monitor_mac.h"
+
+namespace views {
+
+class MenuPreTargetHandlerMac : public MenuPreTargetHandler,
+                                public ui::EventHandler {
+ public:
+  MenuPreTargetHandlerMac(MenuController* controller, Widget* widget);
+  ~MenuPreTargetHandlerMac() override;
+
+  // ui::EventHandler:
+  void OnKeyEvent(ui::KeyEvent* event) override;
+
+ private:
+  MenuController* controller_;  // Weak. Owns |this|.
+  std::unique_ptr<EventMonitorMac> event_monitor_;
+
+  DISALLOW_COPY_AND_ASSIGN(MenuPreTargetHandlerMac);
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_CONTROLS_MENU_MENU_PRE_TARGET_HANDLER_MAC_H_

ui/views/controls/menu/menu_pre_target_handler_mac.mm

+// Copyright 2018 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.
+
+#include "ui/views/controls/menu/menu_pre_target_handler_mac.h"
+
+#include "ui/views/controls/menu/menu_controller.h"
+#include "ui/views/widget/widget.h"
+
+namespace views {
+
+MenuPreTargetHandlerMac::MenuPreTargetHandlerMac(MenuController* controller,
+                                                 Widget* widget)
+    : controller_(controller),
+      event_monitor_(
+          std::make_unique<EventMonitorMac>(this, widget->GetNativeWindow())) {}
+
+MenuPreTargetHandlerMac::~MenuPreTargetHandlerMac() {}
+
+void MenuPreTargetHandlerMac::OnKeyEvent(ui::KeyEvent* event) {
+  if (controller_->OnWillDispatchKeyEvent(event) !=
+      ui::POST_DISPATCH_PERFORM_DEFAULT) {
+    event->SetHandled();
+  }
+}
+
+// static
+std::unique_ptr<MenuPreTargetHandler> MenuPreTargetHandler::Create(
+    MenuController* controller,
+    Widget* widget) {
+  return std::make_unique<MenuPreTargetHandlerMac>(controller, widget);
+}
+
+}  // namespace views

ui/views/event_monitor_mac.mm

@@ -48,8 +48,13 @@ EventMonitorMac::EventMonitorMac(ui::EventHandler* event_handler,
 
     if (!target_window || [event window] == target_window) {
       std::unique_ptr<ui::Event> ui_event = ui::EventFromNative(event);
-      if (ui_event)
+      if (ui_event) {
         event_handler->OnEvent(ui_event.get());
+        // If an event is handled, swallow it by returning nil so the event
+        // never proceeds to the normal event handling machinery.
+        if (ui_event->handled())
+          return nil;
+      }
     }
     return event;
   };