Remove RootView::DispatchGestureEvent()

Remove RootView::DispatchGestureEvent() and instead
call up to the unified event processing logic in
EventProcessor::OnEventFromSource(). This CL
also introduces the virtual method
EventProcessor::OnEventProcessingFinished(),
which is called when the processing of an
event is finished.

BUG=404234
TEST=EventProcessorTest.OnEventProcessingFinished

Review URL: https://codereview.chromium.org/533323004

Cr-Commit-Position: refs/heads/master@{#294249}

ui/events/event_processor.cc

@@ -27,8 +27,9 @@ EventDispatchDetails EventProcessor::OnEventFromSource(Event* event) {
   }
 
   EventTarget* target = targeter->FindTargetForEvent(root, event_to_dispatch);
+  EventDispatchDetails details;
   while (target) {
-    EventDispatchDetails details = DispatchEvent(target, event_to_dispatch);
+    details = DispatchEvent(target, event_to_dispatch);
 
     if (!dispatch_original_event) {
       if (event_to_dispatch->stopped_propagation())
@@ -37,19 +38,23 @@ EventDispatchDetails EventProcessor::OnEventFromSource(Event* event) {
         event->SetHandled();
     }
 
-    if (details.dispatcher_destroyed ||
-        details.target_destroyed ||
-        event->handled()) {
+    if (details.dispatcher_destroyed)
       return details;
-    }
+
+    if (details.target_destroyed || event->handled())
+      break;
 
     target = targeter->FindNextBestTarget(target, event_to_dispatch);
   }
 
-  return EventDispatchDetails();
+  OnEventProcessingFinished(event);
+  return details;
 }
 
 void EventProcessor::PrepareEventForDispatch(Event* event) {
 }
 
+void EventProcessor::OnEventProcessingFinished(Event* event) {
+}
+
 }  // namespace ui

ui/events/event_processor.h

@@ -33,6 +33,12 @@ class EVENTS_EXPORT EventProcessor : public EventDispatcherDelegate {
   // it can be used to updated the location of the event when disptaching from
   // an EventSource in high-DPI).
   virtual void PrepareEventForDispatch(Event* event);
+
+  // Invoked when the processing of |event| has finished (i.e., when no further
+  // dispatching of |event| will be performed by this EventProcessor). Note
+  // that the last target to which |event| was dispatched may have been
+  // destroyed.
+  virtual void OnEventProcessingFinished(Event* event);
 };
 
 }  // namespace ui

ui/events/event_processor_unittest.cc

@@ -25,6 +25,7 @@ class EventProcessorTest : public testing::Test {
   // testing::Test:
   virtual void SetUp() OVERRIDE {
     processor_.SetRoot(scoped_ptr<EventTarget>(new TestEventTarget()));
+    processor_.ResetCounts();
     root()->SetEventTargeter(make_scoped_ptr(new EventTargeter()));
   }
 
@@ -32,6 +33,10 @@ class EventProcessorTest : public testing::Test {
     return static_cast<TestEventTarget*>(processor_.GetRootTarget());
   }
 
+  TestEventProcessor* processor() {
+    return &processor_;
+  }
+
   void DispatchEvent(Event* event) {
     processor_.OnEventFromSource(event);
   }
@@ -257,6 +262,24 @@ TEST_F(EventProcessorTest, NestedEventProcessing) {
   EXPECT_TRUE(mouse2.handled());
 }
 
+// Verifies that OnEventProcessingFinished() is called when an event
+// has been handled.
+TEST_F(EventProcessorTest, OnEventProcessingFinished) {
+  scoped_ptr<TestEventTarget> child(new TestEventTarget());
+  child->set_mark_events_as_handled(true);
+  root()->AddChild(child.Pass());
+
+  // Dispatch a mouse event. We expect the event to be seen by the target,
+  // handled, and we expect OnEventProcessingFinished() to be invoked once.
+  MouseEvent mouse(ET_MOUSE_MOVED, gfx::Point(10, 10), gfx::Point(10, 10),
+                   EF_NONE, EF_NONE);
+  DispatchEvent(&mouse);
+  EXPECT_TRUE(root()->child_at(0)->DidReceiveEvent(ET_MOUSE_MOVED));
+  EXPECT_FALSE(root()->DidReceiveEvent(ET_MOUSE_MOVED));
+  EXPECT_TRUE(mouse.handled());
+  EXPECT_EQ(1, processor()->num_times_processing_finished());
+}
+
 class IgnoreEventTargeter : public EventTargeter {
  public:
   IgnoreEventTargeter() {}

ui/events/test/test_event_processor.cc

@@ -9,13 +9,17 @@
 namespace ui {
 namespace test {
 
-TestEventProcessor::TestEventProcessor() {}
+TestEventProcessor::TestEventProcessor() : num_times_processing_finished_(0) {}
 TestEventProcessor::~TestEventProcessor() {}
 
 void TestEventProcessor::SetRoot(scoped_ptr<EventTarget> root) {
   root_ = root.Pass();
 }
 
+void TestEventProcessor::ResetCounts() {
+  num_times_processing_finished_ = 0;
+}
+
 bool TestEventProcessor::CanDispatchToTarget(EventTarget* target) {
   return true;
 }
@@ -28,5 +32,9 @@ EventDispatchDetails TestEventProcessor::OnEventFromSource(Event* event) {
   return EventProcessor::OnEventFromSource(event);
 }
 
+void TestEventProcessor::OnEventProcessingFinished(Event* event) {
+  num_times_processing_finished_++;
+}
+
 }  // namespace test
 }  // namespace ui

ui/events/test/test_event_processor.h

@@ -16,16 +16,25 @@ class TestEventProcessor : public EventProcessor {
   TestEventProcessor();
   virtual ~TestEventProcessor();
 
+  int num_times_processing_finished() const {
+    return num_times_processing_finished_;
+  }
+
   void SetRoot(scoped_ptr<EventTarget> root);
+  void ResetCounts();
 
   // EventProcessor:
   virtual bool CanDispatchToTarget(EventTarget* target) OVERRIDE;
   virtual EventTarget* GetRootTarget() OVERRIDE;
   virtual EventDispatchDetails OnEventFromSource(Event* event) OVERRIDE;
+  virtual void OnEventProcessingFinished(Event* event) OVERRIDE;
 
  private:
   scoped_ptr<EventTarget> root_;
 
+  // Counts the number of times OnEventProcessingFinished() has been called.
+  int num_times_processing_finished_;
+
   DISALLOW_COPY_AND_ASSIGN(TestEventProcessor);
 };
 

ui/views/view_targeter_unittest.cc

@@ -88,9 +88,9 @@ class ViewTargeterTest : public ViewsTestBase {
     root_view->gesture_handler_ = handler;
   }
 
-  void SetAllowGestureEventRetargeting(internal::RootView* root_view,
-                                       bool allow) {
-    root_view->allow_gesture_event_retargeting_ = allow;
+  void SetGestureHandlerSetBeforeProcessing(internal::RootView* root_view,
+                                            bool set) {
+    root_view->gesture_handler_set_before_processing_ = set;
   }
 
  private:
@@ -282,7 +282,7 @@ TEST_F(ViewTargeterTest, ViewTargeterForGestureEvents) {
   // re-targeting should be prohibited for TAP but permitted for
   // GESTURE_SCROLL_BEGIN (which should be re-targeted to the parent of
   // |grandchild|).
-  SetAllowGestureEventRetargeting(root_view, false);
+  SetGestureHandlerSetBeforeProcessing(root_view, true);
   SetGestureHandler(root_view, grandchild);
   EXPECT_EQ(grandchild, targeter->FindTargetForEvent(root_view, &tap));
   EXPECT_EQ(NULL, targeter->FindNextBestTarget(grandchild, &tap));
@@ -299,7 +299,7 @@ TEST_F(ViewTargeterTest, ViewTargeterForGestureEvents) {
   // in the process of finding the View to which subsequent gestures will be
   // dispatched, so TAP and SCROLL_BEGIN events should be re-targeted up
   // the ancestor chain.
-  SetAllowGestureEventRetargeting(root_view, true);
+  SetGestureHandlerSetBeforeProcessing(root_view, false);
   EXPECT_EQ(child, targeter->FindNextBestTarget(grandchild, &tap));
   EXPECT_EQ(child, targeter->FindNextBestTarget(grandchild, &scroll_begin));
 
@@ -313,7 +313,7 @@ TEST_F(ViewTargeterTest, ViewTargeterForGestureEvents) {
   // re-targeted in this case (regardless of the view that is passed in to
   // FindNextBestTarget() as the previous target).
   SetGestureHandler(root_view, NULL);
-  SetAllowGestureEventRetargeting(root_view, false);
+  SetGestureHandlerSetBeforeProcessing(root_view, true);
   EXPECT_EQ(NULL, targeter->FindNextBestTarget(child, &tap));
   EXPECT_EQ(NULL, targeter->FindNextBestTarget(NULL, &tap));
   EXPECT_EQ(NULL, targeter->FindNextBestTarget(content, &scroll_begin));
@@ -338,7 +338,7 @@ TEST_F(ViewTargeterTest, ViewTargeterForGestureEvents) {
   // If no default gesture handler is currently set, targeting should be
   // performed using the location of the gesture event for a TAP and a
   // SCROLL_BEGIN.
-  SetAllowGestureEventRetargeting(root_view, true);
+  SetGestureHandlerSetBeforeProcessing(root_view, false);
   EXPECT_EQ(grandchild, targeter->FindTargetForEvent(root_view, &tap));
   EXPECT_EQ(grandchild, targeter->FindTargetForEvent(root_view, &scroll_begin));
 

ui/views/widget/root_view.cc

@@ -159,7 +159,7 @@ RootView::RootView(Widget* widget)
       last_mouse_event_x_(-1),
       last_mouse_event_y_(-1),
       gesture_handler_(NULL),
-      allow_gesture_event_retargeting_(true),
+      gesture_handler_set_before_processing_(false),
       pre_dispatch_handler_(new internal::PreEventDispatchHandler(this)),
       post_dispatch_handler_(new internal::PostEventDispatchHandler),
       focus_search_(this, false, false),
@@ -287,13 +287,8 @@ ui::EventDispatchDetails RootView::OnEventFromSource(ui::Event* event) {
       return DispatchDetails();
     }
 
-    // If |gesture_handler_| is non-null (as a result of dispatching a previous
-    // gesture event), then |gesture_event| should be dispatched only to
-    // |gesture_handler_|.
-    allow_gesture_event_retargeting_ = gesture_handler_ ? false : true;
-
-    DispatchGestureEvent(gesture_event);
-    return DispatchDetails();
+    gesture_handler_set_before_processing_ = !!gesture_handler_;
+    return EventProcessor::OnEventFromSource(event);
   }
 
   if (event->IsTouchEvent())
@@ -305,6 +300,17 @@ ui::EventDispatchDetails RootView::OnEventFromSource(ui::Event* event) {
   return DispatchDetails();
 }
 
+void RootView::OnEventProcessingFinished(ui::Event* event) {
+  // If |event| was not handled and |gesture_handler_| was not set by the
+  // dispatch of a previous gesture event, then no default gesture handler
+  // should be set prior to the next gesture event being received.
+  if (event->IsGestureEvent() &&
+      !event->handled() &&
+      !gesture_handler_set_before_processing_) {
+    gesture_handler_ = NULL;
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // RootView, View overrides:
 
@@ -645,44 +651,6 @@ View::DragInfo* RootView::GetDragInfo() {
 
 // Input -----------------------------------------------------------------------
 
-void RootView::DispatchGestureEvent(ui::GestureEvent* event) {
-  bool gesture_handler_set_before_dispatch = !!gesture_handler_;
-  scoped_ptr<ui::Event> event_copy = ui::Event::Clone(*event);
-  View* target = static_cast<View*>(
-      targeter()->FindTargetForEvent(this, event_copy.get()));
-  while (target && target != this) {
-    // Create and dispatch a copy of |event|.
-    ui::EventDispatchDetails dispatch_details =
-        DispatchEvent(target, event_copy.get());
-    if (dispatch_details.dispatcher_destroyed)
-      return;
-
-    if (event_copy->stopped_propagation())
-      event->StopPropagation();
-    else if (event_copy->handled())
-      event->SetHandled();
-
-    // If the event was handled by the previous dispatch or if the target
-    // was destroyed, do not allow any further processing of |event|.
-    if (event->handled() || dispatch_details.target_destroyed)
-      return;
-
-    // The event was not handled by |target|, so continue processing by
-    // re-targeting the event.
-    target = static_cast<View*>(
-        targeter()->FindNextBestTarget(target, event_copy.get()));
-  }
-
-  // |event| was not handled, so if |gesture_handler_| was not set by the
-  // dispatch of a previous gesture event, then no default gesture handler
-  // should be set prior to the next gesture event being received.
-  // TODO(tdanderson): Move this into a new virtual function
-  //                   EventProcessor::OnEventProcessingFinished(), to be called
-  //                   at the end of EventProcessor::OnEventFromSource().
-  if (!gesture_handler_set_before_dispatch)
-    gesture_handler_ = NULL;
-}
-
 void RootView::UpdateCursor(const ui::MouseEvent& event) {
   if (!(event.flags() & ui::EF_IS_NON_CLIENT)) {
     View* v = GetEventHandlerForPoint(event.location());
@@ -728,6 +696,10 @@ ui::EventDispatchDetails RootView::PreDispatchEvent(ui::EventTarget* target,
   if (event->IsGestureEvent()) {
     // Update |gesture_handler_| to indicate which View is currently handling
     // gesture events.
+    // TODO(tdanderson): Look into moving this to PostDispatchEvent() and
+    //                   using |event_dispatch_target_| instead of
+    //                   |gesture_handler_| to detect if the view has been
+    //                   removed from the tree.
     gesture_handler_ = view;
 
     // Disabled views are permitted to be targets of gesture events, but

ui/views/widget/root_view.h

@@ -94,6 +94,7 @@ class VIEWS_EXPORT RootView : public View,
   // Overridden from ui::EventProcessor:
   virtual ui::EventTarget* GetRootTarget() OVERRIDE;
   virtual ui::EventDispatchDetails OnEventFromSource(ui::Event* event) OVERRIDE;
+  virtual void OnEventProcessingFinished(ui::Event* event) OVERRIDE;
 
   // Overridden from View:
   virtual const Widget* GetWidget() const OVERRIDE;
@@ -132,11 +133,6 @@ class VIEWS_EXPORT RootView : public View,
 
   // Input ---------------------------------------------------------------------
 
-  // TODO(tdanderson): Remove RootView::DispatchGestureEvent() once
-  //                   its targeting and dispatch logic has been moved
-  //                   elsewhere. See crbug.com/348083.
-  void DispatchGestureEvent(ui::GestureEvent* event);
-
   // Update the cursor given a mouse event. This is called by non mouse_move
   // event handlers to honor the cursor desired by views located under the
   // cursor during drag operations. The location of the mouse should be in the
@@ -197,9 +193,13 @@ class VIEWS_EXPORT RootView : public View,
   // The View currently handling gesture events.
   View* gesture_handler_;
 
-  // If true, then gesture events received from Widget are permitted to be
-  // re-targeted and re-dispatched while they remain unhandled.
-  bool allow_gesture_event_retargeting_;
+  // Used to indicate if the |gesture_handler_| member was set prior to the
+  // processing of the current event (i.e., if |gesture_handler_| was set
+  // by the dispatch of a previous gesture event).
+  // TODO(tdanderson): It may be possible to eliminate the need for this
+  //                   member if |event_dispatch_target_| can be used in
+  //                   its place.
+  bool gesture_handler_set_before_processing_;
 
   scoped_ptr<internal::PreEventDispatchHandler> pre_dispatch_handler_;
   scoped_ptr<internal::PostEventDispatchHandler> post_dispatch_handler_;

ui/views/widget/root_view_targeter.cc

@@ -27,7 +27,7 @@ View* RootViewTargeter::FindTargetForGestureEvent(
 
   // Return the default gesture handler if one is already set.
   if (root_view_->gesture_handler_) {
-    CHECK(!root_view_->allow_gesture_event_retargeting_);
+    CHECK(root_view_->gesture_handler_set_before_processing_);
     return root_view_->gesture_handler_;
   }
 
@@ -60,9 +60,10 @@ ui::EventTarget* RootViewTargeter::FindNextBestTargetForGestureEvent(
   if (gesture.type() == ui::ET_GESTURE_END)
     return NULL;
 
-  // GESTURE_SCROLL_BEGIN events are always permitted to be re-targeted, even
-  // if |allow_gesture_event_retargeting_| is false.
-  if (!root_view_->allow_gesture_event_retargeting_ &&
+  // Prohibit re-targeting of gesture events (except for GESTURE_SCROLL_BEGIN
+  // events) if the default gesture handler was set by the dispatch of a
+  // previous gesture event.
+  if (root_view_->gesture_handler_set_before_processing_ &&
       gesture.type() != ui::ET_GESTURE_SCROLL_BEGIN) {
     return NULL;
   }