Commit c9f8dbb5 authored by tdanderson's avatar tdanderson Committed by Commit bot

Move check for gesture end events out of RootViewTargeter

Instead of returning a NULL target for a ET_GESTURE_END
in the case where no default gesture handler has been
established, prevent processing in
RootView::OnEventProcessingStarted() by marking the
event as handled.

BUG=417771
TEST=ViewTargeterTest.ViewTargeterForGestureEvents changed,
     WidgetTest.GestureEndEvents changed,
     WidgetTest.GestureEventsNotProcessed changed,
     WidgetTest.DisabledGestureEventTarget changed

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

Cr-Commit-Position: refs/heads/master@{#297897}
parent 71653568
...@@ -343,8 +343,7 @@ TEST_F(ViewTargeterTest, ViewTargeterForGestureEvents) { ...@@ -343,8 +343,7 @@ TEST_F(ViewTargeterTest, ViewTargeterForGestureEvents) {
EXPECT_EQ(grandchild, targeter->FindTargetForEvent(root_view, &scroll_begin)); EXPECT_EQ(grandchild, targeter->FindTargetForEvent(root_view, &scroll_begin));
// If no default gesture handler is currently set, GESTURE_END events // If no default gesture handler is currently set, GESTURE_END events
// should never be targeted or re-targeted to any View. // should never be re-targeted to any View.
EXPECT_EQ(NULL, targeter->FindTargetForEvent(root_view, &end));
EXPECT_EQ(NULL, targeter->FindNextBestTarget(NULL, &end)); EXPECT_EQ(NULL, targeter->FindNextBestTarget(NULL, &end));
EXPECT_EQ(NULL, targeter->FindNextBestTarget(child, &end)); EXPECT_EQ(NULL, targeter->FindNextBestTarget(child, &end));
} }
......
...@@ -261,10 +261,12 @@ void RootView::OnEventProcessingStarted(ui::Event* event) { ...@@ -261,10 +261,12 @@ void RootView::OnEventProcessingStarted(ui::Event* event) {
return; return;
} }
// Do not process ui::ET_GESTURE_END events which do not correspond to the // Do not process ui::ET_GESTURE_END events if they do not correspond to the
// removal of the final touch point. // removal of the final touch point or if no gesture handler has already
// been set.
if (gesture_event->type() == ui::ET_GESTURE_END && if (gesture_event->type() == ui::ET_GESTURE_END &&
gesture_event->details().touch_points() > 1) { (gesture_event->details().touch_points() > 1 ||
!gesture_handler_)) {
event->SetHandled(); event->SetHandled();
return; return;
} }
......
...@@ -31,13 +31,6 @@ View* RootViewTargeter::FindTargetForGestureEvent( ...@@ -31,13 +31,6 @@ View* RootViewTargeter::FindTargetForGestureEvent(
return root_view_->gesture_handler_; return root_view_->gesture_handler_;
} }
// If no default gesture handler has already been set, do not perform any
// targeting for a ET_GESTURE_END event.
// TODO(tdanderson): This check belongs in
// RootView::OnEventProcessingStarted() instead of here.
if (gesture.type() == ui::ET_GESTURE_END)
return NULL;
// If rect-based targeting is enabled, use the gesture's bounding box to // If rect-based targeting is enabled, use the gesture's bounding box to
// determine the target. Otherwise use the center point of the gesture's // determine the target. Otherwise use the center point of the gesture's
// bounding box to determine the target. // bounding box to determine the target.
......
...@@ -1992,12 +1992,10 @@ TEST_F(WidgetTest, GestureEndEvents) { ...@@ -1992,12 +1992,10 @@ TEST_F(WidgetTest, GestureEndEvents) {
widget->Show(); widget->Show();
// If no gesture handler is set, a ui::ET_GESTURE_END event should not set // If no gesture handler is set, a ui::ET_GESTURE_END event should not set
// the gesture handler and the event should remain unhandled because the // the gesture handler.
// handle mode of |view| indicates that events should not be consumed.
EXPECT_EQ(NULL, GetGestureHandler(root_view)); EXPECT_EQ(NULL, GetGestureHandler(root_view));
GestureEventForTest end(ui::ET_GESTURE_END, 15, 15); GestureEventForTest end(ui::ET_GESTURE_END, 15, 15);
widget->OnGestureEvent(&end); widget->OnGestureEvent(&end);
EXPECT_FALSE(end.handled());
EXPECT_EQ(NULL, GetGestureHandler(root_view)); EXPECT_EQ(NULL, GetGestureHandler(root_view));
// Change the handle mode of |view| to indicate that it would like // Change the handle mode of |view| to indicate that it would like
...@@ -2091,6 +2089,22 @@ TEST_F(WidgetTest, GestureEventsNotProcessed) { ...@@ -2091,6 +2089,22 @@ TEST_F(WidgetTest, GestureEventsNotProcessed) {
v3->ResetCounts(); v3->ResetCounts();
v4->ResetCounts(); v4->ResetCounts();
// ui::ET_GESTURE_END events should not be seen by any view when there is
// no default gesture handler set, but they should be marked as handled by
// OnEventProcessingStarted().
GestureEventForTest end(ui::ET_GESTURE_END, 5, 5);
widget->OnGestureEvent(&end);
EXPECT_EQ(0, v1->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(0, v2->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(0, v3->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(0, v4->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(NULL, GetGestureHandler(root_view));
EXPECT_TRUE(end.handled());
v1->ResetCounts();
v2->ResetCounts();
v3->ResetCounts();
v4->ResetCounts();
// ui::ET_GESTURE_END events not corresponding to the release of the // ui::ET_GESTURE_END events not corresponding to the release of the
// final touch point should never be seen by any view, but they should // final touch point should never be seen by any view, but they should
// be marked as handled by OnEventProcessingStarted(). // be marked as handled by OnEventProcessingStarted().
...@@ -2548,23 +2562,6 @@ TEST_F(WidgetTest, DisabledGestureEventTarget) { ...@@ -2548,23 +2562,6 @@ TEST_F(WidgetTest, DisabledGestureEventTarget) {
v3->set_handle_mode(EventCountView::CONSUME_EVENTS); v3->set_handle_mode(EventCountView::CONSUME_EVENTS);
v3->SetEnabled(false); v3->SetEnabled(false);
// No gesture handler is set in the root view, so it should remain unset
// after a GESTURE_END. GESTURE_END events are not dispatched unless
// a gesture handler is already set in the root view, so none of the
// views should see this event and it should not be marked as handled.
GestureEventForTest end(ui::ET_GESTURE_END, 5, 5);
widget->OnGestureEvent(&end);
EXPECT_EQ(0, v1->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(0, v2->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(0, v3->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(0, v4->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(NULL, GetGestureHandler(root_view));
EXPECT_FALSE(end.handled());
v1->ResetCounts();
v2->ResetCounts();
v3->ResetCounts();
v4->ResetCounts();
// No gesture handler is set in the root view. In this case the tap event // No gesture handler is set in the root view. In this case the tap event
// should be dispatched only to |v4|, the gesture handler should be set to // should be dispatched only to |v4|, the gesture handler should be set to
// |v3|, and the event should be marked as handled. // |v3|, and the event should be marked as handled.
...@@ -2597,7 +2594,7 @@ TEST_F(WidgetTest, DisabledGestureEventTarget) { ...@@ -2597,7 +2594,7 @@ TEST_F(WidgetTest, DisabledGestureEventTarget) {
// A GESTURE_END should reset the default gesture handler to NULL. It should // A GESTURE_END should reset the default gesture handler to NULL. It should
// also not be dispatched to |v3| but still marked as handled. // also not be dispatched to |v3| but still marked as handled.
end = GestureEventForTest(ui::ET_GESTURE_END, 5, 5); GestureEventForTest end(ui::ET_GESTURE_END, 5, 5);
widget->OnGestureEvent(&end); widget->OnGestureEvent(&end);
EXPECT_EQ(0, v1->GetEventCount(ui::ET_GESTURE_END)); EXPECT_EQ(0, v1->GetEventCount(ui::ET_GESTURE_END));
EXPECT_EQ(0, v2->GetEventCount(ui::ET_GESTURE_END)); EXPECT_EQ(0, v2->GetEventCount(ui::ET_GESTURE_END));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment