Mark gesture starts on GestureTapDown

Right now in TouchActionFilter, the member gesture_sequence_in_progress_
turns to true at GestureScrollBegin and false at GestureScrollEnd.
When this is true, we do not reset the |scrolling_touch_action_| because
being true means there are more incoming gesture events.

However, when the renderer is busy, it could happen that we have
GestureTapDown, and then we receive the OnHasTouchEventHandlers(true)
message and reset the |scrolling_touch_action_|. In this case, processing
the incoming gestures such as GSB can cause a crash because the
|scrolling_touch_action_| has no value.

Bug: 851644
Change-Id: Idd9de11fc59ef87d7fa8f2d9e3d951fb4db5c2c5
Reviewed-on: https://chromium-review.googlesource.com/1180560Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585072}

content/browser/renderer_host/input/touch_action_filter.cc

@@ -63,8 +63,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
   switch (gesture_event->GetType()) {
     case WebInputEvent::kGestureScrollBegin: {
       DCHECK(!suppress_manipulation_events_);
-      DCHECK(!touchscreen_scroll_in_progress_);
-      touchscreen_scroll_in_progress_ = true;
+      gesture_sequence_in_progress_ = true;
       gesture_sequence_.append("B");
       // TODO(https://crbug.com/851644): Make sure the value is properly set.
       if (!scrolling_touch_action_.has_value()) {
@@ -115,8 +114,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
 
     case WebInputEvent::kGestureScrollEnd:
       gesture_sequence_.clear();
-      DCHECK(touchscreen_scroll_in_progress_);
-      touchscreen_scroll_in_progress_ = false;
+      gesture_sequence_in_progress_ = false;
       ReportGestureEventFiltered(suppress_manipulation_events_);
       return FilterManipulationEventAndResetState()
                  ? FilterGestureEventResult::kFilterGestureEventFiltered
@@ -134,6 +132,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
     // The double tap gesture is a tap ending event. If a double tap gesture is
     // filtered out, replace it with a tap event.
     case WebInputEvent::kGestureDoubleTap:
+      gesture_sequence_in_progress_ = false;
       gesture_sequence_.append("D");
       DCHECK_EQ(1, gesture_event->data.tap.tap_count);
       if (!allow_current_double_tap_event_)
@@ -167,6 +166,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
 
     case WebInputEvent::kGestureTap:
     case WebInputEvent::kGestureTapCancel:
+      gesture_sequence_in_progress_ = false;
       gesture_sequence_.append("A");
       if (drop_current_tap_ending_event_) {
         drop_current_tap_ending_event_ = false;
@@ -176,6 +176,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
 
     case WebInputEvent::kGestureTapDown:
       gesture_sequence_.append("O");
+      gesture_sequence_in_progress_ = true;
       // If the gesture is hitting a region that has a non-blocking (such as a
       // passive) event listener.
       if (gesture_event->is_source_touch_event_set_non_blocking)
@@ -362,8 +363,9 @@ void TouchActionFilter::OnHasTouchEventHandlers(bool has_handlers) {
   // If a page has a touch event handler, this function can be called twice with
   // has_handlers = false first and then true later. When it is true, we need to
   // reset the |scrolling_touch_action_|. However, we do not want to reset it if
-  // there is an active scroll in progress.
-  if (has_touch_event_handler_ && !touchscreen_scroll_in_progress_)
+  // there is an active gesture sequence in progress. For example, the
+  // OnHasTouchEventHandlers(true) can be received after a GestureTapDown.
+  if (has_touch_event_handler_ && !gesture_sequence_in_progress_)
     scrolling_touch_action_.reset();
 }
 

content/browser/renderer_host/input/touch_action_filter.h

@@ -105,9 +105,9 @@ class CONTENT_EXPORT TouchActionFilter {
   // TODO(https://crbug.com/850238): default to true or make it Optional.
   bool has_touch_event_handler_ = false;
 
-  // True if an active touch scroll gesture is in progress. i.e. after GSB and
+  // True if an active gesture sequence is in progress. i.e. after GTD and
   // before GSE.
-  bool touchscreen_scroll_in_progress_ = false;
+  bool gesture_sequence_in_progress_ = false;
 
   // What touch actions are currently permitted.
   base::Optional<cc::TouchAction> allowed_touch_action_;

content/browser/renderer_host/input/touch_action_filter_unittest.cc

@@ -892,6 +892,53 @@ TEST_F(TouchActionFilterTest, TouchActionNotResetWithinGestureSequence) {
   EXPECT_TRUE(ScrollingTouchAction().has_value());
 }
 
+// This test ensures that when the IPC message OnHasTouchEventHandlers is
+// received in the middle of a gesture sequence, the touch action is not reset.
+TEST_F(TouchActionFilterTest, OnHasTouchEventHandlersReceivedDuringScroll) {
+  filter_.OnHasTouchEventHandlers(false);
+
+  WebGestureEvent tap_down = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::kGestureTapDown, kSourceDevice);
+  EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  filter_.OnHasTouchEventHandlers(true);
+  EXPECT_TRUE(ScrollingTouchAction().has_value());
+
+  filter_.OnSetTouchAction(cc::kTouchActionPan);
+  // Simulate a simple tap gesture.
+  WebGestureEvent tap = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::kGestureTap, kSourceDevice);
+  EXPECT_EQ(filter_.FilterGestureEvent(&tap),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  // Gesture tap indicates that there is no scroll in progress, so this should
+  // reset the |allowed_touch_action_|.
+  filter_.ResetTouchAction();
+  EXPECT_FALSE(filter_.allowed_touch_action().has_value());
+
+  filter_.OnHasTouchEventHandlers(false);
+  // Simulate a double tap gesture: GTD-->GT-->GTD-->GDT.
+  filter_.OnSetTouchAction(cc::kTouchActionPan);
+  EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  EXPECT_EQ(ScrollingTouchAction().value(), cc::kTouchActionPan);
+  EXPECT_EQ(filter_.FilterGestureEvent(&tap),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  filter_.OnHasTouchEventHandlers(true);
+  EXPECT_FALSE(ScrollingTouchAction().has_value());
+  filter_.OnSetTouchAction(cc::kTouchActionPan);
+  EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  EXPECT_EQ(ScrollingTouchAction().value(), cc::kTouchActionPan);
+  WebGestureEvent double_tap = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::kGestureDoubleTap, kSourceDevice);
+  EXPECT_EQ(filter_.FilterGestureEvent(&tap_down),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  EXPECT_EQ(filter_.FilterGestureEvent(&tap),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+  EXPECT_EQ(filter_.FilterGestureEvent(&double_tap),
+            FilterGestureEventResult::kFilterGestureEventAllowed);
+}
+
 TEST_F(TouchActionFilterTest, TouchpadScroll) {
   WebGestureEvent scroll_begin =
       SyntheticWebGestureEventBuilder::BuildScrollBegin(