Commit 0d326b48 authored by Xida Chen's avatar Xida Chen Committed by Commit Bot

Reset touch action at TouchActionFilter's constructor

When we start processing a gesture event, the
OnHasTouchEventHandler(false) IPC message may not yet
be received which means the |scrolling_touch_action_|
has no value and that results a crash.

This CL fixes the problem by call ResetTouchAction()
in TouchActionFilter's constructor. At that moment, the
|has_touch_event_handler_| is default to false which
will set both |allowed_touch_action_| and
|scrolling_touch_action_| to Auto.

Bug: 850238, 851644
Change-Id: I36f3edf2b9292347f184458aa059904af1a2f597
Reviewed-on: https://chromium-review.googlesource.com/1180592
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Reviewed-by: default avatarDave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585234}
parent b5e045e5
......@@ -40,7 +40,9 @@ TouchActionFilter::TouchActionFilter()
: suppress_manipulation_events_(false),
drop_current_tap_ending_event_(false),
allow_current_double_tap_event_(true),
force_enable_zoom_(false) {}
force_enable_zoom_(false) {
ResetTouchAction();
}
TouchActionFilter::~TouchActionFilter() {}
......
......@@ -101,8 +101,9 @@ class CONTENT_EXPORT TouchActionFilter {
bool force_enable_zoom_;
// Indicates whether this page has touch event handler or not. Set by
// InputRouterImpl::OnHasTouchEventHandler.
// TODO(https://crbug.com/850238): default to true or make it Optional.
// InputRouterImpl::OnHasTouchEventHandlers. Default to false because one
// could not scroll anyways when there is no content, and this is consistent
// with the default state committed after DocumentLoader::DidCommitNavigation.
bool has_touch_event_handler_ = false;
// True if an active gesture sequence is in progress. i.e. after GTD and
......
......@@ -22,7 +22,7 @@ const blink::WebGestureDevice kSourceDevice =
class TouchActionFilterTest : public testing::Test {
public:
TouchActionFilterTest(){};
TouchActionFilterTest() { filter_.OnHasTouchEventHandlers(true); }
~TouchActionFilterTest() override {}
protected:
......@@ -461,6 +461,7 @@ TEST_F(TouchActionFilterTest, MultiTouch) {
class TouchActionFilterPinchTest : public testing::Test {
public:
void RunTest(bool force_enable_zoom) {
filter_.OnHasTouchEventHandlers(true);
filter_.SetForceEnableZoom(force_enable_zoom);
WebGestureEvent scroll_begin =
......@@ -791,6 +792,7 @@ TEST_F(TouchActionFilterTest, TouchActionResetsOnResetTouchAction) {
}
TEST_F(TouchActionFilterTest, TouchActionResetMidSequence) {
filter_.OnHasTouchEventHandlers(true);
WebGestureEvent scroll_begin =
SyntheticWebGestureEventBuilder::BuildScrollBegin(2, 3, kSourceDevice);
WebGestureEvent pinch_begin = SyntheticWebGestureEventBuilder::Build(
......@@ -915,7 +917,7 @@ TEST_F(TouchActionFilterTest, OnHasTouchEventHandlersReceivedDuringScroll) {
filter_.ResetTouchAction();
EXPECT_FALSE(filter_.allowed_touch_action().has_value());
filter_.OnHasTouchEventHandlers(false);
filter_.OnHasTouchEventHandlers(true);
// Simulate a double tap gesture: GTD-->GT-->GTD-->GDT.
filter_.OnSetTouchAction(cc::kTouchActionPan);
EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
......@@ -923,8 +925,7 @@ TEST_F(TouchActionFilterTest, OnHasTouchEventHandlersReceivedDuringScroll) {
EXPECT_EQ(ScrollingTouchAction().value(), cc::kTouchActionPan);
EXPECT_EQ(filter_.FilterGestureEvent(&tap),
FilterGestureEventResult::kFilterGestureEventAllowed);
filter_.OnHasTouchEventHandlers(true);
EXPECT_FALSE(ScrollingTouchAction().has_value());
EXPECT_TRUE(ScrollingTouchAction().has_value());
filter_.OnSetTouchAction(cc::kTouchActionPan);
EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
FilterGestureEventResult::kFilterGestureEventAllowed);
......@@ -939,6 +940,25 @@ TEST_F(TouchActionFilterTest, OnHasTouchEventHandlersReceivedDuringScroll) {
FilterGestureEventResult::kFilterGestureEventAllowed);
}
// If the renderer is busy, the gesture event might have come before the
// OnHasTouchEventHanlders IPC is received. In this case, we should allow all
// the gestures.
TEST_F(TouchActionFilterTest, GestureArrivesBeforeHasHandlerSet) {
WebGestureEvent tap_down = SyntheticWebGestureEventBuilder::Build(
WebInputEvent::kGestureTapDown, kSourceDevice);
EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
FilterGestureEventResult::kFilterGestureEventAllowed);
}
TEST_F(TouchActionFilterTest, ResetBeforeHasHandlerSet) {
// This should not crash, and should set touch action to auto.
filter_.ResetTouchAction();
WebGestureEvent tap_down = SyntheticWebGestureEventBuilder::Build(
WebInputEvent::kGestureTapDown, kSourceDevice);
EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
FilterGestureEventResult::kFilterGestureEventAllowed);
}
TEST_F(TouchActionFilterTest, TouchpadScroll) {
WebGestureEvent scroll_begin =
SyntheticWebGestureEventBuilder::BuildScrollBegin(
......
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