Suppress LongPress/Tap, and TwoFingerTap when TapDown cancels a fling.

MUST_SUPPRESS_TAP_ENDS state is added to TapSuppressionController. When
a tap end event causes to drop the stashed tap down, the controller goes
to the new state. This state shows that all following tap ends should
get suppressed till the end of the current touch sequence.

BUG=656434
TEST=TapSuppressionControllerTest.*SufficientlyLateTapUp

Review-Url: https://codereview.chromium.org/2542453003
Cr-Commit-Position: refs/heads/master@{#437923}

content/browser/renderer_host/input/gesture_event_queue.cc

@@ -163,6 +163,9 @@ bool GestureEventQueue::ShouldForwardForTapSuppression(
     case WebInputEvent::GestureTapCancel:
     case WebInputEvent::GestureTap:
     case WebInputEvent::GestureDoubleTap:
+    case WebInputEvent::GestureLongPress:
+    case WebInputEvent::GestureLongTap:
+    case WebInputEvent::GestureTwoFingerTap:
       if (gesture_event.event.sourceDevice ==
           blink::WebGestureDeviceTouchscreen) {
         return !touchscreen_tap_suppression_controller_.FilterTapEvent(

content/browser/renderer_host/input/input_router_config_helper.cc

@@ -49,9 +49,16 @@ GestureEventQueue::Config GetGestureEventQueueConfig() {
   config.touchscreen_tap_suppression_config.max_cancel_to_down_time =
       base::TimeDelta::FromMilliseconds(
           gesture_config->fling_max_cancel_to_down_time_in_ms());
+
+  // Tap suppression controller forwards the stashed tapDown and drops the rest
+  // of the stashed events when the tapDownTimer expires. If a fling cancel ack
+  // with |processed = false| arrives before the timer expiration, all stashed
+  // events will be forwarded. The timer is used to avoid waiting for an
+  // arbitrarily late fling cancel ack. Its delay should be large enough for
+  // a long press to get stashed and forwarded if needed.
   config.touchscreen_tap_suppression_config.max_tap_gap_time =
       base::TimeDelta::FromMilliseconds(
-          gesture_config->long_press_time_in_ms());
+          gesture_config->long_press_time_in_ms() + 50);
 
   config.touchpad_tap_suppression_config.enabled =
       gesture_config->fling_touchpad_tap_suppression_enabled();

content/browser/renderer_host/input/tap_suppression_controller.cc

@@ -7,13 +7,27 @@
 #include "base/logging.h"
 #include "base/trace_event/trace_event.h"
 #include "content/browser/renderer_host/input/tap_suppression_controller_client.h"
+#include "ui/events/gesture_detection/gesture_configuration.h"
 
 namespace content {
 
+// The tapDownTimer is used to avoid waiting for an arbitrarily late fling
+// cancel ack. While the timer is running, if a fling cancel ack with
+// |Processed = false| arrives, all stashed gesture events get forwarded. If
+// the timer expires, the controller forwards stashed GestureTapDown only, and
+// drops the rest of the stashed events. The timer delay should be large enough
+// for a GestureLongPress to get stashed and forwarded if needed. It's still
+// possible for a GestureLongPress to arrive after the timer expiration. In
+// this case, it will be suppressed if the controller is in SUPPRESSING_TAPS
+// state.
+
 TapSuppressionController::Config::Config()
     : enabled(false),
-      max_cancel_to_down_time(base::TimeDelta::FromMilliseconds(180)),
-      max_tap_gap_time(base::TimeDelta::FromMilliseconds(500)) {
+      max_cancel_to_down_time(base::TimeDelta::FromMilliseconds(180)) {
+  ui::GestureConfiguration* gesture_config =
+      ui::GestureConfiguration::GetInstance();
+  max_tap_gap_time = base::TimeDelta::FromMilliseconds(
+      gesture_config->long_press_time_in_ms() + 50);
 }
 
 TapSuppressionController::TapSuppressionController(
@@ -34,6 +48,7 @@ void TapSuppressionController::GestureFlingCancel() {
     case NOTHING:
     case GFC_IN_PROGRESS:
     case LAST_CANCEL_STOPPED_FLING:
+    case SUPPRESSING_TAPS:
       state_ = GFC_IN_PROGRESS;
       break;
     case TAP_DOWN_STASHED:
@@ -46,6 +61,7 @@ void TapSuppressionController::GestureFlingCancelAck(bool processed) {
   switch (state_) {
     case DISABLED:
     case NOTHING:
+    case SUPPRESSING_TAPS:
       break;
     case GFC_IN_PROGRESS:
       if (processed)
@@ -57,7 +73,9 @@ void TapSuppressionController::GestureFlingCancelAck(bool processed) {
         TRACE_EVENT0("browser",
                      "TapSuppressionController::GestureFlingCancelAck");
         StopTapDownTimer();
-        client_->ForwardStashedTapDown();
+        // If the fling cancel is not processed, forward all stashed
+        // gesture events.
+        client_->ForwardStashedGestureEvents();
         state_ = NOTHING;
       }  // Else waiting for the timer to release the stashed tap down.
       break;
@@ -89,6 +107,10 @@ bool TapSuppressionController::ShouldDeferTapDown() {
         state_ = NOTHING;
         return false;
       }
+    // Stop suppressing tap end events.
+    case SUPPRESSING_TAPS:
+      state_ = NOTHING;
+      return false;
   }
   NOTREACHED() << "Invalid state";
   return false;
@@ -101,12 +123,17 @@ bool TapSuppressionController::ShouldSuppressTapEnd() {
     case GFC_IN_PROGRESS:
       return false;
     case TAP_DOWN_STASHED:
-      state_ = NOTHING;
+      // A tap cancel happens before long tap and two finger tap events. To
+      // drop the latter events as well as the tap cancel, change the state
+      // to "SUPPRESSING_TAPS" when the stashed tap down is dropped.
+      state_ = SUPPRESSING_TAPS;
       StopTapDownTimer();
       client_->DropStashedTapDown();
       return true;
     case LAST_CANCEL_STOPPED_FLING:
       NOTREACHED() << "Invalid tap end on LAST_CANCEL_STOPPED_FLING state";
+    case SUPPRESSING_TAPS:
+      return true;
   }
   return false;
 }
@@ -128,6 +155,7 @@ void TapSuppressionController::TapDownTimerExpired() {
   switch (state_) {
     case DISABLED:
     case NOTHING:
+    case SUPPRESSING_TAPS:
       NOTREACHED() << "Timer fired on invalid state.";
       break;
     case GFC_IN_PROGRESS:
@@ -138,8 +166,10 @@ void TapSuppressionController::TapDownTimerExpired() {
     case TAP_DOWN_STASHED:
       TRACE_EVENT0("browser",
                    "TapSuppressionController::TapDownTimerExpired");
+      // When the timer expires, only forward the stashed tap down event, and
+      // drop other stashed gesture events (show press or long press).
       client_->ForwardStashedTapDown();
-      state_ = NOTHING;
+      state_ = SUPPRESSING_TAPS;
       break;
   }
 }

content/browser/renderer_host/input/tap_suppression_controller.h

@@ -70,6 +70,12 @@ class CONTENT_EXPORT TapSuppressionController {
     GFC_IN_PROGRESS,
     TAP_DOWN_STASHED,
     LAST_CANCEL_STOPPED_FLING,
+    // When the stashed TapDown event is dropped or forwarded due to tap down
+    // timer expiration, the controller enters the SUPPRESSING_TAPS state.
+    // This state shows that the controller will suppress LongTap,
+    // TwoFingerTap, and TapCancel gesture events until the next tapDown event
+    // arrives.
+    SUPPRESSING_TAPS,
   };
 
   TapSuppressionControllerClient* client_;

content/browser/renderer_host/input/tap_suppression_controller_client.h

@@ -18,9 +18,14 @@ class TapSuppressionControllerClient {
   // Called whenever the deferred tap down (if saved) should be dropped totally.
   virtual void DropStashedTapDown() = 0;
 
-  // Called whenever the deferred tap down (if saved) should be forwarded to the
-  // renderer. The tap down should go back to normal path it was
-  // on before being deferred.
+  // Called whenever the deferred tap down and other gesture events (if saved)
+  // should be forwarded to the renderer. The tap down (and possibly other
+  // gesture events) should go back to normal path they were on before being
+  // deferred.
+  virtual void ForwardStashedGestureEvents() = 0;
+
+  // Called whenever only the deferred tap down (if saved) should be forwarded
+  // to the renderer. Other saved gesture events will be dropped.
   virtual void ForwardStashedTapDown() = 0;
 
  protected:

content/browser/renderer_host/input/tap_suppression_controller_unittest.cc

@@ -22,6 +22,7 @@ class MockTapSuppressionController : public TapSuppressionController,
   using TapSuppressionController::GFC_IN_PROGRESS;
   using TapSuppressionController::TAP_DOWN_STASHED;
   using TapSuppressionController::LAST_CANCEL_STOPPED_FLING;
+  using TapSuppressionController::SUPPRESSING_TAPS;
 
   enum Action {
     NONE                                 = 0,
@@ -104,6 +105,10 @@ class MockTapSuppressionController : public TapSuppressionController,
   // TapSuppressionControllerClient implementation
   void DropStashedTapDown() override { last_actions_ |= TAP_DOWN_DROPPED; }
 
+  void ForwardStashedGestureEvents() override {
+    last_actions_ |= STASHED_TAP_DOWN_FORWARDED;
+  }
+
   void ForwardStashedTapDown() override {
     last_actions_ |= STASHED_TAP_DOWN_FORWARDED;
   }
@@ -178,7 +183,7 @@ TEST_F(TapSuppressionControllerTest, GFCAckBeforeTapFast) {
   EXPECT_EQ(MockTapSuppressionController::TAP_UP_SUPPRESSED |
             MockTapSuppressionController::TAP_DOWN_DROPPED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 }
 
@@ -219,7 +224,7 @@ TEST_F(TapSuppressionControllerTest, GFCAckBeforeTapInsufficientlyLateTapUp) {
   EXPECT_EQ(MockTapSuppressionController::TAP_UP_SUPPRESSED |
             MockTapSuppressionController::TAP_DOWN_DROPPED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 }
 
@@ -252,14 +257,16 @@ TEST_F(TapSuppressionControllerTest, GFCAckBeforeTapSufficientlyLateTapUp) {
   tap_suppression_controller_->AdvanceTime(TimeDelta::FromMilliseconds(13));
   EXPECT_EQ(MockTapSuppressionController::STASHED_TAP_DOWN_FORWARDED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 
-  // Send TapUp. This TapUp should not be suppressed.
+  // Send TapUp. This TapUp should be still suppressed.
+  // LongTap should be suppressed when the previously suppressed TapDown is
+  // forwarded because of the timer expiration.
   tap_suppression_controller_->SendTapUp();
-  EXPECT_EQ(MockTapSuppressionController::TAP_UP_FORWARDED,
+  EXPECT_EQ(MockTapSuppressionController::TAP_UP_SUPPRESSED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 }
 
@@ -300,7 +307,7 @@ TEST_F(TapSuppressionControllerTest, GFCAckBeforeTapInsufficientlyLateTapDown) {
   EXPECT_EQ(MockTapSuppressionController::TAP_UP_SUPPRESSED |
             MockTapSuppressionController::TAP_DOWN_DROPPED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 }
 
@@ -406,7 +413,7 @@ TEST_F(TapSuppressionControllerTest, GFCAckProcessedAfterTapFast) {
   EXPECT_EQ(MockTapSuppressionController::TAP_UP_SUPPRESSED |
             MockTapSuppressionController::TAP_DOWN_DROPPED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 }
 
@@ -447,7 +454,7 @@ TEST_F(TapSuppressionControllerTest, GFCAckAfterTapInsufficientlyLateTapUp) {
   EXPECT_EQ(MockTapSuppressionController::TAP_UP_SUPPRESSED |
             MockTapSuppressionController::TAP_DOWN_DROPPED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 }
 
@@ -480,14 +487,16 @@ TEST_F(TapSuppressionControllerTest, GFCAckAfterTapSufficientlyLateTapUp) {
   tap_suppression_controller_->AdvanceTime(TimeDelta::FromMilliseconds(13));
   EXPECT_EQ(MockTapSuppressionController::STASHED_TAP_DOWN_FORWARDED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 
-  // Send TapUp. This TapUp should not be suppressed.
+  // Send TapUp. This TapUp should be still suppressed.
+  // LongTap should be suppressed when the previously suppressed TapDown is
+  // forwarded because of timer expiration.
   tap_suppression_controller_->SendTapUp();
-  EXPECT_EQ(MockTapSuppressionController::TAP_UP_FORWARDED,
+  EXPECT_EQ(MockTapSuppressionController::TAP_UP_SUPPRESSED,
             tap_suppression_controller_->last_actions());
-  EXPECT_EQ(MockTapSuppressionController::NOTHING,
+  EXPECT_EQ(MockTapSuppressionController::SUPPRESSING_TAPS,
             tap_suppression_controller_->state());
 }
 

content/browser/renderer_host/input/touchpad_tap_suppression_controller.cc

@@ -37,6 +37,12 @@ bool TouchpadTapSuppressionController::ShouldSuppressMouseUp() {
 void TouchpadTapSuppressionController::DropStashedTapDown() {
 }
 
+void TouchpadTapSuppressionController::ForwardStashedGestureEvents() {
+  // Mouse downs are not handled by gesture event filter; so, they are
+  // immediately forwarded to the renderer.
+  client_->SendMouseEventImmediately(stashed_mouse_down_);
+}
+
 void TouchpadTapSuppressionController::ForwardStashedTapDown() {
   // Mouse downs are not handled by gesture event filter; so, they are
   // immediately forwarded to the renderer.

content/browser/renderer_host/input/touchpad_tap_suppression_controller.h

@@ -54,6 +54,7 @@ class TouchpadTapSuppressionController : public TapSuppressionControllerClient {
 
   // TapSuppressionControllerClient implementation.
   void DropStashedTapDown() override;
+  void ForwardStashedGestureEvents() override;
   void ForwardStashedTapDown() override;
 
   TouchpadTapSuppressionControllerClient* client_;

content/browser/renderer_host/input/touchscreen_tap_suppression_controller.cc

@@ -44,12 +44,24 @@ bool TouchscreenTapSuppressionController::FilterTapEvent(
       stashed_show_press_.reset(new GestureEventWithLatencyInfo(event));
       return true;
 
+    case WebInputEvent::GestureLongPress:
+      // It is possible that a GestureLongPress arrives after tapDownTimer
+      // expiration, in this case it should still get filtered if the
+      // controller suppresses the tap end events.
+      if (!stashed_tap_down_)
+        return controller_.ShouldSuppressTapEnd();
+
+      stashed_long_press_.reset(new GestureEventWithLatencyInfo(event));
+      return true;
+
     case WebInputEvent::GestureTapUnconfirmed:
       return !!stashed_tap_down_;
 
     case WebInputEvent::GestureTapCancel:
     case WebInputEvent::GestureTap:
     case WebInputEvent::GestureDoubleTap:
+    case WebInputEvent::GestureLongTap:
+    case WebInputEvent::GestureTwoFingerTap:
       return controller_.ShouldSuppressTapEnd();
 
     default:
@@ -61,15 +73,27 @@ bool TouchscreenTapSuppressionController::FilterTapEvent(
 void TouchscreenTapSuppressionController::DropStashedTapDown() {
   stashed_tap_down_.reset();
   stashed_show_press_.reset();
+  stashed_long_press_.reset();
 }
 
-void TouchscreenTapSuppressionController::ForwardStashedTapDown() {
+void TouchscreenTapSuppressionController::ForwardStashedGestureEvents() {
   DCHECK(stashed_tap_down_);
   ScopedGestureEvent tap_down = std::move(stashed_tap_down_);
   ScopedGestureEvent show_press = std::move(stashed_show_press_);
+  ScopedGestureEvent long_press = std::move(stashed_long_press_);
   gesture_event_queue_->ForwardGestureEvent(*tap_down);
   if (show_press)
     gesture_event_queue_->ForwardGestureEvent(*show_press);
+  if (long_press)
+    gesture_event_queue_->ForwardGestureEvent(*long_press);
+}
+
+void TouchscreenTapSuppressionController::ForwardStashedTapDown() {
+  DCHECK(stashed_tap_down_);
+  ScopedGestureEvent tap_down = std::move(stashed_tap_down_);
+  gesture_event_queue_->ForwardGestureEvent(*tap_down);
+  stashed_show_press_.reset();
+  stashed_long_press_.reset();
 }
 
 }  // namespace content

content/browser/renderer_host/input/touchscreen_tap_suppression_controller.h

@@ -40,6 +40,7 @@ class TouchscreenTapSuppressionController
  private:
   // TapSuppressionControllerClient implementation.
   void DropStashedTapDown() override;
+  void ForwardStashedGestureEvents() override;
   void ForwardStashedTapDown() override;
 
   GestureEventQueue* gesture_event_queue_;
@@ -47,6 +48,7 @@ class TouchscreenTapSuppressionController
   typedef std::unique_ptr<GestureEventWithLatencyInfo> ScopedGestureEvent;
   ScopedGestureEvent stashed_tap_down_;
   ScopedGestureEvent stashed_show_press_;
+  ScopedGestureEvent stashed_long_press_;
 
   // The core controller of tap suppression.
   TapSuppressionController controller_;