Skip to content

GitLab

T
tangled
Commit ce09732e authored by Xida Chen's avatar Xida Chen Committed by Commit Bot
Browse files
Options

Send GFS/GFC to FlingController before TouchActionFilter 

In our current implementation, we ask touch action filter to process
a gesture event first and then check whether the event should be queued
or not. For FlingStart and FlingCancel we never queue them even if the
touch action filter says that they are allowed. As a result, FlingStart
gesture will always sent to touch action filter and that's causing
some bugs.

This CL fixes it by re-ordering how we process a gesture event. We apply
three different filters in the following order:
1-The fling controller decides to either filter the event or forward it.
The filtering happens either when the controller processes the GSF/GFC
events or when it suppresses GSB/GSU events during fling boosting.

2- If the event is not filtered by the fling controller, the touch action
filter checks to filter the touchscreen gesture scroll events, with this
change this filter doesn't receive GFS events anymore since they are
already filtered by the fling controller.

3- If a gesture event is not filtered by any of the two filters above,
the gesture_event_queue chooses to either add the event to the event
queue to be sent to the renderer or filter the event and add it to
debouncing_deferral_queue_.

Bug: 838616
Change-Id: I0e363d2fff12fdf5f91c51d42c3752355bb7cccd
Reviewed-on: https://chromium-review.googlesource.com/1075554Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Reviewed-by: default avatarSahel Sharifymoghaddam <sahel@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562831}
parent 3234223c
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 141 additions and 225 deletions
content/browser/renderer_host/input/fling_browsertest.cc
View file @ ce09732e
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "build/build_config.h" #include "build/build_config.h"
#include "content/browser/renderer_host/input/synthetic_smooth_scroll_gesture.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test.h"
...@@ -28,6 +29,19 @@ const std::string kBrowserFlingDataURL = R"HTML( ...@@ -28,6 +29,19 @@ const std::string kBrowserFlingDataURL = R"HTML(
<script> <script>
document.title='ready'; document.title='ready';
</script>)HTML"; </script>)HTML";
const std::string kTouchActionFilterDataURL = R"HTML(
<!DOCTYPE html>
<meta name='viewport' content='width=device-width'/>
<style>
body {
height: 10000px;
touch-action: pan-y;
}
</style>
<script>
document.title='ready';
</script>)HTML";
} // namespace } // namespace
namespace content { namespace content {
...@@ -42,6 +56,11 @@ class BrowserSideFlingBrowserTest : public ContentBrowserTest { ...@@ -42,6 +56,11 @@ class BrowserSideFlingBrowserTest : public ContentBrowserTest {
"MiddleClickAutoscroll"); "MiddleClickAutoscroll");
} }
void OnSyntheticGestureCompleted(SyntheticGesture::Result result) {
EXPECT_EQ(SyntheticGesture::GESTURE_FINISHED, result);
run_loop_->Quit();
}
protected: protected:
RenderWidgetHostImpl* GetWidgetHost() { RenderWidgetHostImpl* GetWidgetHost() {
return RenderWidgetHostImpl::From( return RenderWidgetHostImpl::From(
...@@ -81,6 +100,8 @@ class BrowserSideFlingBrowserTest : public ContentBrowserTest { ...@@ -81,6 +100,8 @@ class BrowserSideFlingBrowserTest : public ContentBrowserTest {
GetWidgetHost()->ForwardMouseEvent(up_event); GetWidgetHost()->ForwardMouseEvent(up_event);
} }
std::unique_ptr<base::RunLoop> run_loop_;
private: private:
DISALLOW_COPY_AND_ASSIGN(BrowserSideFlingBrowserTest); DISALLOW_COPY_AND_ASSIGN(BrowserSideFlingBrowserTest);
}; };
...@@ -220,4 +241,63 @@ IN_PROC_BROWSER_TEST_F(BrowserSideFlingBrowserTest, ...@@ -220,4 +241,63 @@ IN_PROC_BROWSER_TEST_F(BrowserSideFlingBrowserTest,
} }
} }
// Disabled on MacOS because it doesn't support touchscreen scroll.
#if defined(OS_MACOSX)
#define MAYBE_ScrollEndGeneratedForFilteredFling \
DISABLED_ScrollEndGeneratedForFilteredFling
#else
#define MAYBE_ScrollEndGeneratedForFilteredFling \
ScrollEndGeneratedForFilteredFling
#endif
IN_PROC_BROWSER_TEST_F(BrowserSideFlingBrowserTest,
MAYBE_ScrollEndGeneratedForFilteredFling) {
LoadURL(kTouchActionFilterDataURL);
// Necessary for checking the ACK source of the sent events. The events are
// filtered when the Browser is the source.
auto scroll_begin_watcher = std::make_unique<InputMsgWatcher>(
GetWidgetHost(), blink::WebInputEvent::kGestureScrollBegin);
auto fling_start_watcher = std::make_unique<InputMsgWatcher>(
GetWidgetHost(), blink::WebInputEvent::kGestureFlingStart);
auto scroll_end_watcher = std::make_unique<InputMsgWatcher>(
GetWidgetHost(), blink::WebInputEvent::kGestureScrollEnd);
// Do a horizontal touchscreen scroll followed by a fling. The GFS must get
// filtered since the GSB is filtered.
SyntheticSmoothScrollGestureParams params;
params.gesture_source_type = SyntheticGestureParams::TOUCH_INPUT;
params.anchor = gfx::PointF(10, 10);
params.distances.push_back(gfx::Vector2d(-60, 0));
params.prevent_fling = false;
run_loop_ = std::make_unique<base::RunLoop>();
std::unique_ptr<SyntheticSmoothScrollGesture> gesture(
new SyntheticSmoothScrollGesture(params));
GetWidgetHost()->QueueSyntheticGesture(
std::move(gesture),
base::BindOnce(&BrowserSideFlingBrowserTest::OnSyntheticGestureCompleted,
base::Unretained(this)));
// Runs until we get the OnSyntheticGestureCompleted callback.
run_loop_->Run();
scroll_begin_watcher->GetAckStateWaitIfNecessary();
EXPECT_EQ(InputEventAckSource::BROWSER,
scroll_begin_watcher->last_event_ack_source());
fling_start_watcher->GetAckStateWaitIfNecessary();
EXPECT_EQ(InputEventAckSource::BROWSER,
fling_start_watcher->last_event_ack_source());
// Since the GFS is filtered. the input_router_impl will generate and forward
// a GSE to make sure that the scrolling sequence and the touch action filter
// state get reset properly. The generated GSE will also get filtered since
// its equivalent GSB is filtered. The test will timeout if the GSE is not
// generated.
scroll_end_watcher->GetAckStateWaitIfNecessary();
EXPECT_EQ(InputEventAckSource::BROWSER,
scroll_end_watcher->last_event_ack_source());
}
} // namespace content } // namespace content
content/browser/renderer_host/input/gesture_event_queue.cc
View file @ ce09732e
...@@ -47,13 +47,22 @@ GestureEventQueue::GestureEventQueue( ...@@ -47,13 +47,22 @@ GestureEventQueue::GestureEventQueue(
GestureEventQueue::~GestureEventQueue() { } GestureEventQueue::~GestureEventQueue() { }
bool GestureEventQueue::QueueEvent( bool GestureEventQueue::DebounceOrQueueEvent(
const GestureEventWithLatencyInfo& gesture_event) { const GestureEventWithLatencyInfo& gesture_event) {
TRACE_EVENT0("input", "GestureEventQueue::QueueEvent"); // GFS should have been filtered in FlingControllerFilterEvent.
if (!ShouldForwardForBounceReduction(gesture_event) || DCHECK_NE(gesture_event.event.GetType(), WebInputEvent::kGestureFlingStart);
fling_controller_.FilterGestureEvent(gesture_event)) { if (!ShouldForwardForBounceReduction(gesture_event))
return false; return false;
}
QueueAndForwardIfNecessary(gesture_event);
return true;
}
bool GestureEventQueue::FlingControllerFilterEvent(
const GestureEventWithLatencyInfo& gesture_event) {
TRACE_EVENT0("input", "GestureEventQueue::QueueEvent");
if (fling_controller_.FilterGestureEvent(gesture_event))
return true;
// fling_controller_ is in charge of handling GFS events and the events are // fling_controller_ is in charge of handling GFS events and the events are
// not sent to the renderer, the controller processes the fling and generates // not sent to the renderer, the controller processes the fling and generates
...@@ -62,7 +71,7 @@ bool GestureEventQueue::QueueEvent( ...@@ -62,7 +71,7 @@ bool GestureEventQueue::QueueEvent(
if (gesture_event.event.GetType() == WebInputEvent::kGestureFlingStart) { if (gesture_event.event.GetType() == WebInputEvent::kGestureFlingStart) {
fling_controller_.ProcessGestureFlingStart(gesture_event); fling_controller_.ProcessGestureFlingStart(gesture_event);
fling_in_progress_ = true; fling_in_progress_ = true;
return false; return true;
} }
// If the GestureFlingStart event is processed by the fling_controller_, the // If the GestureFlingStart event is processed by the fling_controller_, the
...@@ -70,11 +79,10 @@ bool GestureEventQueue::QueueEvent( ...@@ -70,11 +79,10 @@ bool GestureEventQueue::QueueEvent(
if (gesture_event.event.GetType() == WebInputEvent::kGestureFlingCancel) { if (gesture_event.event.GetType() == WebInputEvent::kGestureFlingCancel) {
fling_controller_.ProcessGestureFlingCancel(gesture_event); fling_controller_.ProcessGestureFlingCancel(gesture_event);
fling_in_progress_ = false; fling_in_progress_ = false;
return false; return true;
} }
QueueAndForwardIfNecessary(gesture_event); return false;
return true;
} }
void GestureEventQueue::StopFling() { void GestureEventQueue::StopFling() {
......
content/browser/renderer_host/input/gesture_event_queue.h
View file @ ce09732e
...@@ -77,10 +77,13 @@ class CONTENT_EXPORT GestureEventQueue { ...@@ -77,10 +77,13 @@ class CONTENT_EXPORT GestureEventQueue {
const Config& config); const Config& config);
~GestureEventQueue(); ~GestureEventQueue();
// Adds a gesture to the queue if it passes the relevant filters. If // Uses fling controller to filter the gesture event. Returns true if the
// there are no events currently queued, the event will be forwarded // event wasn't queued and was filtered.
// immediately. Returns false if the event wasn't queued and was filtered. bool FlingControllerFilterEvent(const GestureEventWithLatencyInfo&);
bool QueueEvent(const GestureEventWithLatencyInfo&);
// Check for debouncing, or add the gesture event to the queue. Returns false
// if the event wasn't queued.
bool DebounceOrQueueEvent(const GestureEventWithLatencyInfo&);
// Indicates that the caller has received an acknowledgement from the renderer // Indicates that the caller has received an acknowledgement from the renderer
// with state |ack_result| and event |type|. May send events if the queue is // with state |ack_result| and event |type|. May send events if the queue is
......
content/browser/renderer_host/input/gesture_event_queue_unittest.cc
View file @ ce09732e
...@@ -118,7 +118,10 @@ class GestureEventQueueTest : public testing::Test, ...@@ -118,7 +118,10 @@ class GestureEventQueueTest : public testing::Test,
} }
void SimulateGestureEvent(const WebGestureEvent& gesture) { void SimulateGestureEvent(const WebGestureEvent& gesture) {
queue()->QueueEvent(GestureEventWithLatencyInfo(gesture)); GestureEventWithLatencyInfo gesture_event(gesture);
if (!queue()->FlingControllerFilterEvent(gesture_event)) {
queue()->DebounceOrQueueEvent(gesture_event);
}
} }
void SimulateGestureEvent(WebInputEvent::Type type, void SimulateGestureEvent(WebInputEvent::Type type,
...@@ -1053,64 +1056,6 @@ TEST_F(GestureEventQueueTest, DebounceDefersFollowingGestureEvents) { ...@@ -1053,64 +1056,6 @@ TEST_F(GestureEventQueueTest, DebounceDefersFollowingGestureEvents) {
} }
} }
// Test that a GestureFlingStart is not filtered out by debouncing and that
// GestureFlingStart causes debouncing queue to empty into gesture queue.
TEST_F(GestureEventQueueTest, DebounceEndsWithFlingStartEvent) {
SetUpForDebounce(3);
SimulateGestureEvent(WebInputEvent::kGestureScrollUpdate,
blink::kWebGestureDeviceTouchpad);
EXPECT_EQ(1U, GetAndResetSentGestureEventCount());
EXPECT_EQ(1U, GestureEventQueueSize());
EXPECT_EQ(0U, GestureEventDebouncingQueueSize());
EXPECT_TRUE(ScrollingInProgress());
SimulateGestureEvent(WebInputEvent::kGestureScrollEnd,
blink::kWebGestureDeviceTouchpad);
EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
EXPECT_EQ(1U, GestureEventQueueSize());
EXPECT_EQ(1U, GestureEventDebouncingQueueSize());
// The deferred events are correctly queued in coalescing queue. The GFS with
// touchpad source is not queued since it is handled by fling controller.
SimulateGestureFlingStartEvent(0, 10, blink::kWebGestureDeviceTouchpad);
EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
EXPECT_EQ(2U, GestureEventQueueSize());
EXPECT_EQ(0U, GestureEventDebouncingQueueSize());
EXPECT_FALSE(ScrollingInProgress());
EXPECT_TRUE(FlingInProgress());
// While fling is in progress events don't get debounced.
SimulateGestureEvent(WebInputEvent::kGestureScrollBegin,
blink::kWebGestureDeviceTouchpad);
EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
EXPECT_EQ(3U, GestureEventQueueSize());
EXPECT_EQ(0U, GestureEventDebouncingQueueSize());
SimulateGestureEvent(WebInputEvent::kGestureScrollUpdate,
blink::kWebGestureDeviceTouchpad);
EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
EXPECT_EQ(4U, GestureEventQueueSize());
EXPECT_EQ(0U, GestureEventDebouncingQueueSize());
SimulateGestureEvent(WebInputEvent::kGestureScrollEnd,
blink::kWebGestureDeviceTouchpad);
EXPECT_EQ(0U, GetAndResetSentGestureEventCount());
EXPECT_EQ(5U, GestureEventQueueSize());
EXPECT_EQ(0U, GestureEventDebouncingQueueSize());
// Verify that the coalescing queue contains the correct events.
WebInputEvent::Type expected[] = {
WebInputEvent::kGestureScrollUpdate, WebInputEvent::kGestureScrollEnd,
WebInputEvent::kGestureScrollBegin, WebInputEvent::kGestureScrollUpdate};
for (unsigned i = 0; i < sizeof(expected) / sizeof(WebInputEvent::Type);
i++) {
WebGestureEvent merged_event = GestureEventQueueEventAt(i);
EXPECT_EQ(expected[i], merged_event.GetType());
}
}
// Test that non-scroll events are deferred while scrolling during the debounce // Test that non-scroll events are deferred while scrolling during the debounce
// interval and are discarded if a GestureScrollUpdate event arrives before the // interval and are discarded if a GestureScrollUpdate event arrives before the
// interval end. // interval end.
......
content/browser/renderer_host/input/input_router_impl.cc
View file @ ce09732e
...@@ -136,6 +136,13 @@ void InputRouterImpl::SendGestureEvent( ...@@ -136,6 +136,13 @@ void InputRouterImpl::SendGestureEvent(
GestureEventWithLatencyInfo gesture_event(original_gesture_event); GestureEventWithLatencyInfo gesture_event(original_gesture_event);
if (gesture_event_queue_.FlingControllerFilterEvent(gesture_event)) {
disposition_handler_->OnGestureEventAck(gesture_event,
InputEventAckSource::BROWSER,
INPUT_EVENT_ACK_STATE_CONSUMED);
return;
}
if (touch_action_filter_.FilterGestureEvent(&gesture_event.event) == if (touch_action_filter_.FilterGestureEvent(&gesture_event.event) ==
FilterGestureEventResult::kFilterGestureEventFiltered) { FilterGestureEventResult::kFilterGestureEventFiltered) {
disposition_handler_->OnGestureEventAck(gesture_event, disposition_handler_->OnGestureEventAck(gesture_event,
...@@ -164,7 +171,7 @@ void InputRouterImpl::SendGestureEvent( ...@@ -164,7 +171,7 @@ void InputRouterImpl::SendGestureEvent(
touch_event_queue_.OnGestureScrollEvent(gesture_event); touch_event_queue_.OnGestureScrollEvent(gesture_event);
} }
if (!gesture_event_queue_.QueueEvent(gesture_event)) { if (!gesture_event_queue_.DebounceOrQueueEvent(gesture_event)) {
disposition_handler_->OnGestureEventAck(gesture_event, disposition_handler_->OnGestureEventAck(gesture_event,
InputEventAckSource::BROWSER, InputEventAckSource::BROWSER,
INPUT_EVENT_ACK_STATE_CONSUMED); INPUT_EVENT_ACK_STATE_CONSUMED);
......
content/browser/renderer_host/input/touch_action_filter.cc
View file @ ce09732e
...@@ -51,6 +51,8 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -51,6 +51,8 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
switch (gesture_event->GetType()) { switch (gesture_event->GetType()) {
case WebInputEvent::kGestureScrollBegin: case WebInputEvent::kGestureScrollBegin:
DCHECK(!suppress_manipulation_events_); DCHECK(!suppress_manipulation_events_);
DCHECK(!touchscreen_scroll_in_progress_);
touchscreen_scroll_in_progress_ = true;
suppress_manipulation_events_ = suppress_manipulation_events_ =
ShouldSuppressManipulation(*gesture_event); ShouldSuppressManipulation(*gesture_event);
return suppress_manipulation_events_ return suppress_manipulation_events_
...@@ -79,29 +81,14 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent( ...@@ -79,29 +81,14 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
break; break;
case WebInputEvent::kGestureFlingStart: case WebInputEvent::kGestureFlingStart:
ReportGestureEventFiltered(suppress_manipulation_events_); // Fling controller processes FlingStart event, and we should never get
// Touchscreen flings should always have non-zero velocity. // it here.
DCHECK(gesture_event->data.fling_start.velocity_x || NOTREACHED();
gesture_event->data.fling_start.velocity_y); break;
if (!suppress_manipulation_events_) {
// Flings restricted to a specific axis shouldn't permit velocity
// in the perpendicular axis.
if (IsYAxisActionDisallowed(allowed_touch_action_))
gesture_event->data.fling_start.velocity_y = 0;
else if (IsXAxisActionDisallowed(allowed_touch_action_))
gesture_event->data.fling_start.velocity_x = 0;
// As the renderer expects a scroll-ending event, but does not expect a
// zero-velocity fling, convert the now zero-velocity fling accordingly.
if (!gesture_event->data.fling_start.velocity_x &&
!gesture_event->data.fling_start.velocity_y) {
gesture_event->SetType(WebInputEvent::kGestureScrollEnd);
}
}
return FilterManipulationEventAndResetState()
? FilterGestureEventResult::kFilterGestureEventFiltered
: FilterGestureEventResult::kFilterGestureEventAllowed;
case WebInputEvent::kGestureScrollEnd: case WebInputEvent::kGestureScrollEnd:
DCHECK(touchscreen_scroll_in_progress_);
touchscreen_scroll_in_progress_ = false;
ReportGestureEventFiltered(suppress_manipulation_events_); ReportGestureEventFiltered(suppress_manipulation_events_);
return FilterManipulationEventAndResetState() return FilterManipulationEventAndResetState()
? FilterGestureEventResult::kFilterGestureEventFiltered ? FilterGestureEventResult::kFilterGestureEventFiltered
......
content/browser/renderer_host/input/touch_action_filter.h
View file @ ce09732e
...@@ -83,6 +83,10 @@ class CONTENT_EXPORT TouchActionFilter { ...@@ -83,6 +83,10 @@ class CONTENT_EXPORT TouchActionFilter {
// Force enable zoom for Accessibility. // Force enable zoom for Accessibility.
bool force_enable_zoom_; bool force_enable_zoom_;
// True if an active touch scroll gesture is in progress. i.e. after GSB and
// before GSE.
bool touchscreen_scroll_in_progress_ = false;
// What touch actions are currently permitted. // What touch actions are currently permitted.
cc::TouchAction allowed_touch_action_; cc::TouchAction allowed_touch_action_;
......
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