Properly resume scrolling if a fling ends during a suppressed scroll

As a boosted fling may suppress a new scroll sequence in anticipation of a
follow-up fling event, it's important that scrolling properly resume if the
boosted fling terminates during said suppressed scroll. Normally, a synthetic
GestureScrollBegin is used to kickstart the scroll in such cases. However, if
the fling naturally terminates by either overscrolling or ticking beyond its
lifetime, we failed to resume touch scrolling. This led to unexpected cases
where a GestureScrollUpdate would be processed without an antecedent
GestureScrollBegin. 

Address this by always inserting the appropriate synthetic GestureScrollBegin
if the fling terminates for any reason.

BUG=402077

Review URL: https://codereview.chromium.org/473053002

Cr-Commit-Position: refs/heads/master@{#290027}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290027 0039d316-1c4b-4281-b951-d872f2087c98

content/renderer/input/input_handler_proxy.cc

@@ -336,7 +336,7 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleInputEvent(
         *static_cast<const WebGestureEvent*>(&event);
     return HandleGestureFling(gesture_event);
   } else if (event.type == WebInputEvent::GestureFlingCancel) {
-    if (CancelCurrentFling(true))
+    if (CancelCurrentFling())
       return DID_HANDLE;
     else if (!fling_may_be_active_on_main_thread_)
       return DROP_EVENT;
@@ -357,7 +357,7 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleInputEvent(
     // Only call |CancelCurrentFling()| if a fling was active, as it will
     // otherwise disrupt an in-progress touch scroll.
     if (fling_curve_)
-      CancelCurrentFling(true);
+      CancelCurrentFling();
   } else if (event.type == WebInputEvent::MouseMove) {
     const WebMouseEvent& mouse_event =
         *static_cast<const WebMouseEvent*>(&event);
@@ -485,7 +485,7 @@ bool InputHandlerProxy::FilterInputEventForFlingBoosting(
 
   // Gestures from a different source should immediately interrupt the fling.
   if (gesture_event.sourceDevice != fling_parameters_.sourceDevice) {
-    FlingBoostCancelAndResumeScrollingIfNecessary();
+    CancelCurrentFling();
     return false;
   }
 
@@ -500,13 +500,13 @@ bool InputHandlerProxy::FilterInputEventForFlingBoosting(
               fling_parameters_.sourceDevice == blink::WebGestureDeviceTouchpad
                   ? cc::InputHandler::NonBubblingGesture
                   : cc::InputHandler::Gesture)) {
-        CancelCurrentFling(true);
+        CancelCurrentFling();
         return false;
       }
 
       // TODO(jdduke): Use |gesture_event.data.scrollBegin.delta{X,Y}Hint| to
       // determine if the ScrollBegin should immediately cancel the fling.
-      FlingBoostExtend(gesture_event);
+      ExtendBoostedFlingTimeout(gesture_event);
       return true;
 
     case WebInputEvent::GestureScrollUpdate: {
@@ -515,16 +515,19 @@ bool InputHandlerProxy::FilterInputEventForFlingBoosting(
       if (ShouldSuppressScrollForFlingBoosting(current_fling_velocity_,
                                                gesture_event,
                                                time_since_last_boost_event)) {
-        FlingBoostExtend(gesture_event);
+        ExtendBoostedFlingTimeout(gesture_event);
         return true;
       }
 
-      FlingBoostCancelAndResumeScrollingIfNecessary();
+      CancelCurrentFling();
       return false;
     }
 
     case WebInputEvent::GestureScrollEnd:
-      CancelCurrentFling(true);
+      // Clear the last fling boost event *prior* to fling cancellation,
+      // preventing insertion of a synthetic GestureScrollBegin.
+      last_fling_boost_event_ = WebGestureEvent();
+      CancelCurrentFling();
       return true;
 
     case WebInputEvent::GestureFlingStart: {
@@ -576,36 +579,21 @@ bool InputHandlerProxy::FilterInputEventForFlingBoosting(
     default:
       // All other types of gestures (taps, presses, etc...) will complete the
       // deferred fling cancellation.
-      FlingBoostCancelAndResumeScrollingIfNecessary();
+      CancelCurrentFling();
       return false;
   }
 }
 
-void InputHandlerProxy::FlingBoostExtend(const blink::WebGestureEvent& event) {
-  TRACE_EVENT_INSTANT0(
-      "input", "InputHandlerProxy::FlingBoostExtend", TRACE_EVENT_SCOPE_THREAD);
+void InputHandlerProxy::ExtendBoostedFlingTimeout(
+    const blink::WebGestureEvent& event) {
+  TRACE_EVENT_INSTANT0("input",
+                       "InputHandlerProxy::ExtendBoostedFlingTimeout",
+                       TRACE_EVENT_SCOPE_THREAD);
   deferred_fling_cancel_time_seconds_ =
       event.timeStampSeconds + kFlingBoostTimeoutDelaySeconds;
   last_fling_boost_event_ = event;
 }
 
-void InputHandlerProxy::FlingBoostCancelAndResumeScrollingIfNecessary() {
-  TRACE_EVENT_INSTANT0(
-      "input", "InputHandlerProxy::FlingBoostCancel", TRACE_EVENT_SCOPE_THREAD);
-  DCHECK(deferred_fling_cancel_time_seconds_);
-
-  // Note: |last_fling_boost_event_| is cleared by |CancelCurrentFling()|.
-  WebGestureEvent last_fling_boost_event = last_fling_boost_event_;
-
-  CancelCurrentFling(true);
-
-  if (last_fling_boost_event.type == WebInputEvent::GestureScrollBegin ||
-      last_fling_boost_event.type == WebInputEvent::GestureScrollUpdate) {
-    // Synthesize a GestureScrollBegin, as the original was suppressed.
-    HandleInputEvent(ObtainGestureScrollBegin(last_fling_boost_event));
-  }
-}
-
 void InputHandlerProxy::Animate(base::TimeTicks time) {
   if (!fling_curve_)
     return;
@@ -614,7 +602,7 @@ void InputHandlerProxy::Animate(base::TimeTicks time) {
 
   if (deferred_fling_cancel_time_seconds_ &&
       monotonic_time_sec > deferred_fling_cancel_time_seconds_) {
-    FlingBoostCancelAndResumeScrollingIfNecessary();
+    CancelCurrentFling();
     return;
   }
 
@@ -645,7 +633,7 @@ void InputHandlerProxy::Animate(base::TimeTicks time) {
     TRACE_EVENT_INSTANT0("input",
                          "InputHandlerProxy::animate::flingOver",
                          TRACE_EVENT_SCOPE_THREAD);
-    CancelCurrentFling(true);
+    CancelCurrentFling();
   }
 }
 
@@ -685,8 +673,15 @@ void InputHandlerProxy::DidOverscroll(
   client_->DidOverscroll(params);
 }
 
-bool InputHandlerProxy::CancelCurrentFling(
-    bool send_fling_stopped_notification) {
+bool InputHandlerProxy::CancelCurrentFling() {
+  if (CancelCurrentFlingWithoutNotifyingClient()) {
+    client_->DidStopFlinging();
+    return true;
+  }
+  return false;
+}
+
+bool InputHandlerProxy::CancelCurrentFlingWithoutNotifyingClient() {
   bool had_fling_animation = fling_curve_;
   if (had_fling_animation &&
       fling_parameters_.sourceDevice == blink::WebGestureDeviceTouchscreen) {
@@ -707,10 +702,19 @@ bool InputHandlerProxy::CancelCurrentFling(
   gesture_scroll_on_impl_thread_ = false;
   current_fling_velocity_ = gfx::Vector2dF();
   fling_parameters_ = blink::WebActiveWheelFlingParameters();
-  deferred_fling_cancel_time_seconds_ = 0;
-  last_fling_boost_event_ = WebGestureEvent();
-  if (send_fling_stopped_notification && had_fling_animation)
-    client_->DidStopFlinging();
+
+  if (deferred_fling_cancel_time_seconds_) {
+    deferred_fling_cancel_time_seconds_ = 0;
+
+    WebGestureEvent last_fling_boost_event = last_fling_boost_event_;
+    last_fling_boost_event_ = WebGestureEvent();
+    if (last_fling_boost_event.type == WebInputEvent::GestureScrollBegin ||
+        last_fling_boost_event.type == WebInputEvent::GestureScrollUpdate) {
+      // Synthesize a GestureScrollBegin, as the original was suppressed.
+      HandleInputEvent(ObtainGestureScrollBegin(last_fling_boost_event));
+    }
+  }
+
   return had_fling_animation;
 }
 
@@ -746,7 +750,7 @@ bool InputHandlerProxy::TouchpadFlingScroll(
       // the subarea but then is flung "under" the pointer.
       client_->TransferActiveWheelFlingAnimation(fling_parameters_);
       fling_may_be_active_on_main_thread_ = true;
-      CancelCurrentFling(false);
+      CancelCurrentFlingWithoutNotifyingClient();
       break;
   }
 

content/renderer/input/input_handler_proxy.h

@@ -66,16 +66,19 @@ class CONTENT_EXPORT InputHandlerProxy
 
   // Schedule a time in the future after which a boost-enabled fling will
   // terminate without further momentum from the user (see |Animate()|).
-  void FlingBoostExtend(const blink::WebGestureEvent& event);
-
-  // Cancel the current fling and insert a GestureScrollBegin if necessary.
-  void FlingBoostCancelAndResumeScrollingIfNecessary();
+  void ExtendBoostedFlingTimeout(const blink::WebGestureEvent& event);
 
   // Returns true if we scrolled by the increment.
   bool TouchpadFlingScroll(const blink::WebFloatSize& increment);
 
+  // Returns true if we actually had an active fling to cancel, also notifying
+  // the client that the fling has ended. Note that if a boosted fling is active
+  // and suppressing an active scroll sequence, a synthetic GestureScrollBegin
+  // will be injected to resume scrolling.
+  bool CancelCurrentFling();
+
   // Returns true if we actually had an active fling to cancel.
-  bool CancelCurrentFling(bool send_fling_stopped_notification);
+  bool CancelCurrentFlingWithoutNotifyingClient();
 
   scoped_ptr<blink::WebGestureCurve> fling_curve_;
   // Parameters for the active fling animation, stored in case we need to
@@ -91,7 +94,7 @@ class CONTENT_EXPORT InputHandlerProxy
 
   // The last event that extended the lifetime of the boosted fling. If the
   // event was a scroll gesture, a GestureScrollBegin will be inserted if the
-  // fling terminates (via |FlingBoostCancelAndResumeScrollingIfNecessary()|).
+  // fling terminates (via |CancelCurrentFling()|).
   blink::WebGestureEvent last_fling_boost_event_;
 
 #ifndef NDEBUG

content/renderer/input/input_handler_proxy_unittest.cc

@@ -1856,5 +1856,75 @@ TEST_F(InputHandlerProxyTest, NoFlingBoostIfFlingTooSlow) {
   VERIFY_AND_RESET_MOCKS();
 }
 
+TEST_F(InputHandlerProxyTest, FlingBoostTerminatedDuringScrollSequence) {
+  base::TimeDelta dt = base::TimeDelta::FromMilliseconds(10);
+  base::TimeTicks time = base::TimeTicks() + dt;
+  base::TimeTicks last_animate_time = time;
+  WebFloatPoint fling_delta = WebFloatPoint(1000, 0);
+  WebPoint fling_point = WebPoint(7, 13);
+  StartFling(
+      time, blink::WebGestureDeviceTouchscreen, fling_delta, fling_point);
+
+  // Now cancel the fling.  The fling cancellation should be deferred to allow
+  // fling boosting events to arrive.
+  time += dt;
+  CancelFling(time);
+
+  // The GestureScrollBegin should be swallowed by the fling.
+  time += dt;
+  gesture_.timeStampSeconds = InSecondsF(time);
+  gesture_.type = WebInputEvent::GestureScrollBegin;
+  EXPECT_CALL(mock_input_handler_,
+              IsCurrentlyScrollingLayerAt(testing::_, testing::_))
+      .WillOnce(testing::Return(true));
+  EXPECT_EQ(expected_disposition_, input_handler_->HandleInputEvent(gesture_));
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // Now animate the fling to completion (in this case, the fling should
+  // terminate because the input handler reports a failed scroll). As the fling
+  // was cancelled during an active scroll sequence, a synthetic
+  // GestureScrollBegin should be processed, resuming the scroll.
+  time += dt;
+  float expected_delta =
+      (time - last_animate_time).InSecondsF() * -fling_delta.x;
+  EXPECT_CALL(mock_input_handler_,
+              ScrollBy(testing::_,
+                       testing::Property(&gfx::Vector2dF::x,
+                                         testing::Eq(expected_delta))))
+      .WillOnce(testing::Return(false));
+  EXPECT_CALL(mock_input_handler_, ScrollEnd());
+  EXPECT_CALL(mock_input_handler_, ScrollBegin(testing::_, testing::_))
+      .WillOnce(testing::Return(cc::InputHandler::ScrollStarted));
+  input_handler_->Animate(time);
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // Subsequent GestureScrollUpdates after the cancelled, boosted fling should
+  // cause scrolling as usual.
+  time += dt;
+  expected_delta = 7.3f;
+  gesture_.timeStampSeconds = InSecondsF(time);
+  gesture_.type = WebInputEvent::GestureScrollUpdate;
+  gesture_.data.scrollUpdate.deltaX = -expected_delta;
+  EXPECT_CALL(mock_input_handler_,
+              ScrollBy(testing::_,
+                       testing::Property(&gfx::Vector2dF::x,
+                                         testing::Eq(expected_delta))))
+      .WillOnce(testing::Return(true));
+  EXPECT_EQ(expected_disposition_, input_handler_->HandleInputEvent(gesture_));
+
+  VERIFY_AND_RESET_MOCKS();
+
+  // GestureScrollEnd should terminate the resumed scroll properly.
+  time += dt;
+  gesture_.timeStampSeconds = InSecondsF(time);
+  gesture_.type = WebInputEvent::GestureScrollEnd;
+  EXPECT_CALL(mock_input_handler_, ScrollEnd());
+  EXPECT_EQ(expected_disposition_, input_handler_->HandleInputEvent(gesture_));
+
+  VERIFY_AND_RESET_MOCKS();
+}
+
 } // namespace
 } // namespace content