Remove TouchActionFilter::num_of_active_touches_

This variable was added to test whether the touch start / end can be
acked and released out of order or not. From the reading of the code,
I don't think is true.

This CL removes this variable, we will monitor crash reports to ensure
this doesn't cause regression.

To ensure the same behavior, we need a new variable, which is named
|touch_sequence_in_progress_| in this CL. This is specifically for this
scenario: we have received touch event ack for touch start, and then
JS removes / adds touch event listener, which triggers a
OnHasTouchEventHandlers call. In this case, we should not reset
touch action if the |touch_sequence_in_pgoress_| is true. We have the
unit test OnHasTouchEventHandlersReceivedAfterTouchStart to ensure
this behavior.

Bug: 900195
Change-Id: I53c3141905b4e2a49ab6e62ea43335c4cee17c15
Reviewed-on: https://chromium-review.googlesource.com/c/1307250
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604260}

content/browser/renderer_host/input/input_router_impl.cc

@@ -331,7 +331,7 @@ void InputRouterImpl::OnTouchEventAck(const TouchEventWithLatencyInfo& event,
         base::NumberToString(ack_result).c_str());
     touch_action_filter_.AppendToGestureSequenceForDebugging(
         base::NumberToString(event.event.unique_touch_event_id).c_str());
-    touch_action_filter_.IncreaseActiveTouches();
+    touch_action_filter_.SetTouchSequenceInProgress(true);
     // There are some cases the touch action may not have value when receiving
     // the ACK for the touch start, such as input ack state is
     // NO_CONSUMER_EXISTS, or the renderer has swapped out. In these cases, set
@@ -347,7 +347,7 @@ void InputRouterImpl::OnTouchEventAck(const TouchEventWithLatencyInfo& event,
     touch_action_filter_.AppendToGestureSequenceForDebugging("E");
     touch_action_filter_.AppendToGestureSequenceForDebugging(
         base::NumberToString(event.event.unique_touch_event_id).c_str());
-    touch_action_filter_.DecreaseActiveTouches();
+    touch_action_filter_.SetTouchSequenceInProgress(false);
     touch_action_filter_.ReportAndResetTouchAction();
     UpdateTouchAckTimeoutEnabled();
   }

content/browser/renderer_host/input/input_router_impl_unittest.cc

@@ -417,7 +417,7 @@ class InputRouterImplTest : public testing::Test {
     disposition_handler_->GetAndResetAckCount();
   }
 
-  void ActiveTouchSequenceCountTest(
+  void TouchSequenceInProgressTest(
       const base::Optional<cc::TouchAction>& touch_action,
       InputEventAckState state) {
     PressTouchPoint(1, 1);
@@ -426,11 +426,13 @@ class InputRouterImplTest : public testing::Test {
     input_router_->TouchEventHandled(
         TouchEventWithLatencyInfo(touch_event_), InputEventAckSource::BROWSER,
         ui::LatencyInfo(), state, overscroll, touch_action);
-    EXPECT_EQ(input_router_->touch_action_filter_.num_of_active_touches_, 1);
+    EXPECT_TRUE(
+        input_router_->touch_action_filter_.touch_sequence_in_progress_);
     ReleaseTouchPoint(0);
     input_router_->OnTouchEventAck(TouchEventWithLatencyInfo(touch_event_),
                                    InputEventAckSource::BROWSER, state);
-    EXPECT_EQ(input_router_->touch_action_filter_.num_of_active_touches_, 0);
+    EXPECT_FALSE(
+        input_router_->touch_action_filter_.touch_sequence_in_progress_);
   }
 
   void OnTouchEventAckWithAckState(InputEventAckState ack_state) {
@@ -614,31 +616,31 @@ TEST_F(InputRouterImplTest, CoalescesWheelEvents) {
 
 // Test that the active touch sequence count increment when the touch start is
 // not ACKed from the main thread.
-TEST_F(InputRouterImplTest, ActiveTouchSequenceCountWithoutTouchAction) {
+TEST_F(InputRouterImplTest, TouchSequenceInProgressWithoutTouchAction) {
   base::Optional<cc::TouchAction> touch_action;
-  ActiveTouchSequenceCountTest(touch_action,
-                               INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  TouchSequenceInProgressTest(touch_action,
+                              INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
 }
 
 TEST_F(InputRouterImplTest,
-       ActiveTouchSequenceCountWithoutTouchActionNoConsumer) {
+       TouchSequenceInProgressWithoutTouchActionNoConsumer) {
   base::Optional<cc::TouchAction> touch_action;
-  ActiveTouchSequenceCountTest(touch_action,
-                               INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
+  TouchSequenceInProgressTest(touch_action,
+                              INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
 }
 
 // Test that the active touch sequence count increment when the touch start is
 // ACKed from the main thread.
-TEST_F(InputRouterImplTest, ActiveTouchSequenceCountWithTouchAction) {
+TEST_F(InputRouterImplTest, TouchSequenceInProgressWithTouchAction) {
   base::Optional<cc::TouchAction> touch_action(cc::kTouchActionPanY);
-  ActiveTouchSequenceCountTest(touch_action,
-                               INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
+  TouchSequenceInProgressTest(touch_action,
+                              INPUT_EVENT_ACK_STATE_SET_NON_BLOCKING);
 }
 
-TEST_F(InputRouterImplTest, ActiveTouchSequenceCountWithTouchActionNoConsumer) {
+TEST_F(InputRouterImplTest, TouchSequenceInProgressWithTouchActionNoConsumer) {
   base::Optional<cc::TouchAction> touch_action(cc::kTouchActionPanY);
-  ActiveTouchSequenceCountTest(touch_action,
-                               INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
+  TouchSequenceInProgressTest(touch_action,
+                              INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
 }
 
 TEST_F(InputRouterImplTest, TouchActionAutoWithAckStateConsumed) {

content/browser/renderer_host/input/touch_action_filter.cc

@@ -188,10 +188,7 @@ FilterGestureEventResult TouchActionFilter::FilterGestureEvent(
       gesture_sequence_in_progress_ = true;
       // If the gesture is hitting a region that has a non-blocking (such as a
       // passive) event listener.
-      // In theory, the num_of_active_touches_ should be > 0 at this point. But
-      // crash reports suggest otherwise.
-      if (gesture_event->is_source_touch_event_set_non_blocking ||
-          num_of_active_touches_ <= 0)
+      if (gesture_event->is_source_touch_event_set_non_blocking)
         SetTouchAction(cc::kTouchActionAuto);
       active_touch_action_ = allowed_touch_action_;
       if (active_touch_action_.has_value())
@@ -269,12 +266,9 @@ void TouchActionFilter::OnSetTouchAction(cc::TouchAction touch_action) {
   active_touch_action_ = allowed_touch_action_;
 }
 
-void TouchActionFilter::IncreaseActiveTouches() {
-  num_of_active_touches_++;
-}
-
-void TouchActionFilter::DecreaseActiveTouches() {
-  num_of_active_touches_--;
+void TouchActionFilter::SetTouchSequenceInProgress(
+    bool touch_sequence_in_progress) {
+  touch_sequence_in_progress_ = touch_sequence_in_progress;
 }
 
 void TouchActionFilter::ReportAndResetTouchAction() {
@@ -283,8 +277,8 @@ void TouchActionFilter::ReportAndResetTouchAction() {
   else
     gesture_sequence_.append("RN");
   ReportTouchAction();
-  if (num_of_active_touches_ <= 0)
-    ResetTouchAction();
+  DCHECK(!touch_sequence_in_progress_);
+  ResetTouchAction();
 }
 
 void TouchActionFilter::ReportTouchAction() {
@@ -396,7 +390,7 @@ void TouchActionFilter::OnHasTouchEventHandlers(bool has_handlers) {
   // We have set the associated touch action if the touch start already happened
   // or there is a gesture in progress. In these cases, we should not reset the
   // associated touch action.
-  if (!gesture_sequence_in_progress_ && num_of_active_touches_ <= 0) {
+  if (!gesture_sequence_in_progress_ && !touch_sequence_in_progress_) {
     ResetTouchAction();
     if (has_touch_event_handler_)
       active_touch_action_.reset();

content/browser/renderer_host/input/touch_action_filter.h

@@ -60,8 +60,7 @@ class CONTENT_EXPORT TouchActionFilter {
 
   void OnHasTouchEventHandlers(bool has_handlers);
 
-  void IncreaseActiveTouches();
-  void DecreaseActiveTouches();
+  void SetTouchSequenceInProgress(bool touch_sequence_in_progress);
 
   void ForceResetTouchActionForTest();
 
@@ -111,8 +110,8 @@ class CONTENT_EXPORT TouchActionFilter {
   // before GSE.
   bool gesture_sequence_in_progress_ = false;
 
-  // Increment at receiving ACK for touch start and decrement at touch end.
-  int num_of_active_touches_ = 0;
+  // True at touch sequence start and false at touch sequence end.
+  bool touch_sequence_in_progress_ = false;
 
   // What touch actions are currently permitted.
   base::Optional<cc::TouchAction> allowed_touch_action_;