Use a pre-target handler in RootView to open keyboard-generated conte…

…xt menus Move the code which checks whether a particular key event should trigger a context menu from RootView::DispatchKeyEvent() into a pre-target handler on RootView. BUG=357286 TEST=RootViewTest.ContextMenuFromKeyEvent Review URL: https://codereview.chromium.org/217073005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260617 0039d316-1c4b-4281-b951-d872f2087c98
hylschwy · Mar 31, 2014 · e6bdf2a · e6bdf2a
1 parent 1fdd4f4
commit e6bdf2a
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 16 deletions.
diff --git a/ui/views/view.h b/ui/views/view.h
@@ -70,6 +70,7 @@ class ScrollView;
 class Widget;
 
 namespace internal {
+class PreEventDispatchHandler;
 class PostEventDispatchHandler;
 class RootView;
 }
@@ -1210,6 +1211,7 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
 #endif
 
  private:
+  friend class internal::PreEventDispatchHandler;
   friend class internal::PostEventDispatchHandler;
   friend class internal::RootView;
   friend class FocusManager;

diff --git a/ui/views/widget/root_view.cc b/ui/views/widget/root_view.cc
@@ -49,6 +49,48 @@ class MouseEnterExitEvent : public ui::MouseEvent {
 
 }  // namespace
 
+// This event handler receives events in the pre-target phase and takes care of
+// the following:
+//   - Shows keyboard-triggered context menus.
+class PreEventDispatchHandler : public ui::EventHandler {
+ public:
+  explicit PreEventDispatchHandler(View* owner)
+      : owner_(owner) {
+  }
+  virtual ~PreEventDispatchHandler() {}
+
+ private:
+  // ui::EventHandler:
+  virtual void OnKeyEvent(ui::KeyEvent* event) OVERRIDE {
+    CHECK_EQ(ui::EP_PRETARGET, event->phase());
+    if (event->handled())
+      return;
+
+    View* v = NULL;
+    if (owner_->GetFocusManager())  // Can be NULL in unittests.
+      v = owner_->GetFocusManager()->GetFocusedView();
+
+    // Special case to handle keyboard-triggered context menus.
+    if (v && v->enabled() && ((event->key_code() == ui::VKEY_APPS) ||
+       (event->key_code() == ui::VKEY_F10 && event->IsShiftDown()))) {
+      // Clamp the menu location within the visible bounds of each ancestor view
+      // to avoid showing the menu over a completely different view or window.
+      gfx::Point location = v->GetKeyboardContextMenuLocation();
+      for (View* parent = v->parent(); parent; parent = parent->parent()) {
+        const gfx::Rect& parent_bounds = parent->GetBoundsInScreen();
+        location.SetToMax(parent_bounds.origin());
+        location.SetToMin(parent_bounds.bottom_right());
+      }
+      v->ShowContextMenu(location, ui::MENU_SOURCE_KEYBOARD);
+      event->StopPropagation();
+    }
+  }
+
+  View* owner_;
+
+  DISALLOW_COPY_AND_ASSIGN(PreEventDispatchHandler);
+};
+
 // static
 const char RootView::kViewClassName[] = "RootView";
 
@@ -69,11 +111,13 @@ RootView::RootView(Widget* widget)
       touch_pressed_handler_(NULL),
       gesture_handler_(NULL),
       scroll_gesture_handler_(NULL),
+      pre_dispatch_handler_(new internal::PreEventDispatchHandler(this)),
       focus_search_(this, false, false),
       focus_traversable_parent_(NULL),
       focus_traversable_parent_view_(NULL),
       event_dispatch_target_(NULL),
       old_dispatch_target_(NULL) {
+  AddPreTargetHandler(pre_dispatch_handler_.get());
 }
 
 RootView::~RootView() {
@@ -660,23 +704,8 @@ View::DragInfo* RootView::GetDragInfo() {
 
 void RootView::DispatchKeyEvent(ui::KeyEvent* event) {
   View* v = NULL;
-  if (GetFocusManager())  // NULL in unittests.
+  if (GetFocusManager())  // Can be NULL in unittests.
     v = GetFocusManager()->GetFocusedView();
-  // Special case to handle keyboard-triggered context menus.
-  if (v && v->enabled() && ((event->key_code() == ui::VKEY_APPS) ||
-     (event->key_code() == ui::VKEY_F10 && event->IsShiftDown()))) {
-    // Clamp the menu location within the visible bounds of each ancestor view
-    // to avoid showing the menu over a completely different view or window.
-    gfx::Point location = v->GetKeyboardContextMenuLocation();
-    for (View* parent = v->parent(); parent; parent = parent->parent()) {
-      const gfx::Rect& parent_bounds = parent->GetBoundsInScreen();
-      location.SetToMax(parent_bounds.origin());
-      location.SetToMin(parent_bounds.bottom_right());
-    }
-    v->ShowContextMenu(location, ui::MENU_SOURCE_KEYBOARD);
-    event->StopPropagation();
-    return;
-  }
 
   DispatchKeyEventStartAt(v, event);
 }

diff --git a/ui/views/widget/root_view.h b/ui/views/widget/root_view.h
@@ -25,6 +25,7 @@ class Widget;
 // This is a views-internal API and should not be used externally.
 // Widget exposes this object as a View*.
 namespace internal {
+class PreEventDispatchHandler;
 
 ////////////////////////////////////////////////////////////////////////////////
 // RootView class
@@ -210,6 +211,8 @@ class VIEWS_EXPORT RootView : public View,
   // The view currently handling scroll gesture events.
   View* scroll_gesture_handler_;
 
+  scoped_ptr<internal::PreEventDispatchHandler> pre_dispatch_handler_;
+
   // Focus ---------------------------------------------------------------------
 
   // The focus search algorithm.

diff --git a/ui/views/widget/root_view_unittest.cc b/ui/views/widget/root_view_unittest.cc
@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ui/views/widget/root_view.h"
+
+#include "ui/views/context_menu_controller.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/root_view_test_helper.h"
 
@@ -51,5 +54,95 @@ TEST_F(RootViewTest, DeleteViewDuringKeyEventDispatch) {
   EXPECT_TRUE(got_key_event);
 }
 
+// Used to determine whether or not a context menu is shown as a result of
+// a keypress.
+class TestContextMenuController : public ContextMenuController {
+ public:
+  TestContextMenuController()
+      : show_context_menu_calls_(0),
+        menu_source_view_(NULL),
+        menu_source_type_(ui::MENU_SOURCE_NONE) {
+  }
+  virtual ~TestContextMenuController() {}
+
+  int show_context_menu_calls() const { return show_context_menu_calls_; }
+  View* menu_source_view() const { return menu_source_view_; }
+  ui::MenuSourceType menu_source_type() const { return menu_source_type_; }
+
+  void Reset() {
+    show_context_menu_calls_ = 0;
+    menu_source_view_ = NULL;
+    menu_source_type_ = ui::MENU_SOURCE_NONE;
+  }
+
+  // ContextMenuController:
+  virtual void ShowContextMenuForView(
+      View* source,
+      const gfx::Point& point,
+      ui::MenuSourceType source_type) OVERRIDE {
+    show_context_menu_calls_++;
+    menu_source_view_ = source;
+    menu_source_type_ = source_type;
+  }
+
+ private:
+  int show_context_menu_calls_;
+  View* menu_source_view_;
+  ui::MenuSourceType menu_source_type_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestContextMenuController);
+};
+
+// Tests that context menus are shown for certain key events (Shift+F10
+// and VKEY_APPS) by the pre-target handler installed on RootView.
+TEST_F(RootViewTest, ContextMenuFromKeyEvent) {
+  Widget widget;
+  Widget::InitParams init_params =
+      CreateParams(Widget::InitParams::TYPE_POPUP);
+  init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  widget.Init(init_params);
+  internal::RootView* root_view =
+      static_cast<internal::RootView*>(widget.GetRootView());
+
+  TestContextMenuController controller;
+  View* focused_view = new View;
+  focused_view->set_context_menu_controller(&controller);
+  widget.SetContentsView(focused_view);
+  focused_view->SetFocusable(true);
+  focused_view->RequestFocus();
+
+  // No context menu should be shown for a keypress of 'A'.
+  ui::KeyEvent nomenu_key_event(ui::ET_KEY_PRESSED, ui::VKEY_A, 0, true);
+  ui::EventDispatchDetails details =
+      root_view->OnEventFromSource(&nomenu_key_event);
+  EXPECT_FALSE(details.target_destroyed);
+  EXPECT_FALSE(details.dispatcher_destroyed);
+  EXPECT_EQ(0, controller.show_context_menu_calls());
+  EXPECT_EQ(NULL, controller.menu_source_view());
+  EXPECT_EQ(ui::MENU_SOURCE_NONE, controller.menu_source_type());
+  controller.Reset();
+
+  // A context menu should be shown for a keypress of Shift+F10.
+  ui::KeyEvent menu_key_event(
+      ui::ET_KEY_PRESSED, ui::VKEY_F10, ui::EF_SHIFT_DOWN, false);
+  details = root_view->OnEventFromSource(&menu_key_event);
+  EXPECT_FALSE(details.target_destroyed);
+  EXPECT_FALSE(details.dispatcher_destroyed);
+  EXPECT_EQ(1, controller.show_context_menu_calls());
+  EXPECT_EQ(focused_view, controller.menu_source_view());
+  EXPECT_EQ(ui::MENU_SOURCE_KEYBOARD, controller.menu_source_type());
+  controller.Reset();
+
+  // A context menu should be shown for a keypress of VKEY_APPS.
+  ui::KeyEvent menu_key_event2(ui::ET_KEY_PRESSED, ui::VKEY_APPS, 0, false);
+  details = root_view->OnEventFromSource(&menu_key_event2);
+  EXPECT_FALSE(details.target_destroyed);
+  EXPECT_FALSE(details.dispatcher_destroyed);
+  EXPECT_EQ(1, controller.show_context_menu_calls());
+  EXPECT_EQ(focused_view, controller.menu_source_view());
+  EXPECT_EQ(ui::MENU_SOURCE_KEYBOARD, controller.menu_source_type());
+  controller.Reset();
+}
+
 }  // namespace test
 }  // namespace views