Improve overscroll user experience on Windows

This patch includes 2 changes, These changes only effect Windows
touchpad because only fling events on Windows touchpad can reach
overscroll controller.

1. Prevent fling event start overscroll.
2. Only fling events in first 0.3 second will processed for
   overscroll. On Windows, we don't generate the inertial
   events (fling) but receive them from Win API. In some cases,
   we get a long tail of inertial events for a couple of seconds.
   The overscroll animation feels like stuck in these cases. So
   we only process 0.3 second inertial events then cancel the
   overscroll if it is not completed yet.

Bug: 647140
Change-Id: Iea7941bff77a84f95acf8f342c12273cb909a5d3
Reviewed-on: https://chromium-review.googlesource.com/1025391
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556890}

content/browser/renderer_host/overscroll_configuration.cc

@@ -29,6 +29,10 @@ const float kThresholdStartTouchscreen = 50.f;
 bool g_is_touchpad_overscroll_history_navigation_enabled_initialized = false;
 bool g_touchpad_overscroll_history_navigation_enabled = false;
 
+// On Windows, we only process 0.3 second inertial events then cancel the
+// overscroll if it is not completed yet.
+int g_max_inertial_events_before_overscroll_cancellation_in_ms = 300;
+
 float GetStartThresholdMultiplier() {
   base::CommandLine* cmd = base::CommandLine::ForCurrentProcess();
   if (!cmd->HasSwitch(switches::kOverscrollStartThreshold))
@@ -148,4 +152,11 @@ void OverscrollConfig::ResetTouchpadOverscrollHistoryNavigationEnabled() {
   g_is_touchpad_overscroll_history_navigation_enabled_initialized = false;
 }
 
+// static
+base::TimeDelta
+OverscrollConfig::MaxInertialEventsBeforeOverscrollCancellation() {
+  return base::TimeDelta::FromMilliseconds(
+      g_max_inertial_events_before_overscroll_cancellation_in_ms);
+}
+
 }  // namespace content

content/browser/renderer_host/overscroll_controller.cc

@@ -29,6 +29,16 @@ bool IsGestureEventFromTouchpad(const blink::WebInputEvent& event) {
   return gesture.SourceDevice() == blink::kWebGestureDeviceTouchpad;
 }
 
+bool IsGestureScrollUpdateInertialEvent(const blink::WebInputEvent& event) {
+  if (event.GetType() != blink::WebInputEvent::kGestureScrollUpdate)
+    return false;
+
+  const blink::WebGestureEvent& gesture =
+      static_cast<const blink::WebGestureEvent&>(event);
+  return gesture.data.scroll_update.inertial_phase ==
+         blink::WebGestureEvent::kMomentumPhase;
+}
+
 float ClampAbsoluteValue(float value, float max_abs) {
   DCHECK_LT(0.f, max_abs);
   return std::max(-max_abs, std::min(value, max_abs));
@@ -86,17 +96,10 @@ bool OverscrollController::ShouldProcessEvent(
 
 bool OverscrollController::ShouldIgnoreInertialEvent(
     const blink::WebInputEvent& event) const {
-  if (!ignore_following_inertial_events_ ||
-      event.GetType() != blink::WebInputEvent::kGestureScrollUpdate) {
-    return false;
+  return ignore_following_inertial_events_ &&
+         IsGestureScrollUpdateInertialEvent(event);
   }
 
-  const blink::WebGestureEvent& gesture =
-      static_cast<const blink::WebGestureEvent&>(event);
-  return gesture.data.scroll_update.inertial_phase ==
-         blink::WebGestureEvent::kMomentumPhase;
-}
-
 bool OverscrollController::WillHandleEvent(const blink::WebInputEvent& event) {
   if (!ShouldProcessEvent(event))
     return false;
@@ -108,6 +111,7 @@ bool OverscrollController::WillHandleEvent(const blink::WebInputEvent& event) {
 
   if (event.GetType() == blink::WebInputEvent::kGestureScrollBegin) {
     ignore_following_inertial_events_ = false;
+    first_inertial_event_time_.reset();
     time_since_last_ignored_scroll_ =
         event.TimeStamp() - last_ignored_scroll_time_;
     // Will handle events when processing ACKs to ensure the correct order.
@@ -379,10 +383,29 @@ bool OverscrollController::ProcessEventForOverscroll(
     case blink::WebInputEvent::kGestureScrollUpdate: {
       const blink::WebGestureEvent& gesture =
           static_cast<const blink::WebGestureEvent&>(event);
+      bool is_gesture_scroll_update_inertial_event =
+          IsGestureScrollUpdateInertialEvent(event);
       event_processed = ProcessOverscroll(
           gesture.data.scroll_update.delta_x,
           gesture.data.scroll_update.delta_y,
-          gesture.SourceDevice() == blink::kWebGestureDeviceTouchpad);
+          gesture.SourceDevice() == blink::kWebGestureDeviceTouchpad,
+          is_gesture_scroll_update_inertial_event);
+      if (is_gesture_scroll_update_inertial_event) {
+        // Record the timestamp of first inertial event.
+        if (!first_inertial_event_time_) {
+          first_inertial_event_time_ = event.TimeStamp();
+          break;
+        }
+        base::TimeDelta inertial_event_interval =
+            event.TimeStamp() - first_inertial_event_time_.value();
+        if (inertial_event_interval >=
+            OverscrollConfig::MaxInertialEventsBeforeOverscrollCancellation()) {
+          ignore_following_inertial_events_ = true;
+          // Reset overscroll state if fling didn't complete the overscroll
+          // gesture within the first 20 inertial events.
+          Cancel();
+        }
+      }
       break;
     }
     case blink::WebInputEvent::kGestureFlingStart: {
@@ -422,10 +445,15 @@ bool OverscrollController::ProcessEventForOverscroll(
 
 bool OverscrollController::ProcessOverscroll(float delta_x,
                                              float delta_y,
-                                             bool is_touchpad) {
+                                             bool is_touchpad,
+                                             bool is_inertial) {
   if (scroll_state_ == ScrollState::CONTENT_CONSUMING)
     return false;
 
+  // Do not start overscroll for inertial events.
+  if (overscroll_mode_ == OVERSCROLL_NONE && is_inertial)
+    return false;
+
   overscroll_delta_x_ += delta_x;
   overscroll_delta_y_ += delta_y;
 

content/browser/renderer_host/overscroll_controller.h

@@ -7,6 +7,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "cc/input/overscroll_behavior.h"
 #include "content/common/content_export.h"
@@ -112,7 +113,10 @@ class CONTENT_EXPORT OverscrollController {
   // and the over scroll amount (i.e. |overscroll_mode_|, |overscroll_delta_x_|
   // and |overscroll_delta_y_|). Returns true if overscroll was handled by the
   // delegate.
-  bool ProcessOverscroll(float delta_x, float delta_y, bool is_touchpad);
+  bool ProcessOverscroll(float delta_x,
+                         float delta_y,
+                         bool is_touchpad,
+                         bool is_inertial);
 
   // Completes the desired action from the current gesture.
   void CompleteAction();
@@ -162,8 +166,9 @@ class CONTENT_EXPORT OverscrollController {
   bool wheel_scroll_latching_enabled_;
 
   // A inertial scroll (fling) event may complete an overscroll gesture and
-  // navigate to a new page, but the inertial scroll can continue to generate
-  // scroll-update events. These events need to be ignored.
+  // navigate to a new page or cancel the overscroll animation. In both cases
+  // inertial scroll can continue to generate scroll-update events. These events
+  // need to be ignored.
   bool ignore_following_inertial_events_ = false;
 
   // Specifies whether last overscroll was ignored, either due to a command line
@@ -177,6 +182,14 @@ class CONTENT_EXPORT OverscrollController {
   // of the current one.
   base::TimeDelta time_since_last_ignored_scroll_;
 
+  // On Windows, we don't generate the inertial events (fling) but receive them
+  // from Win API. In some cases, we get a long tail of inertial events for a
+  // couple of seconds. The overscroll animation feels like stuck in these
+  // cases. So we only process 0.3 second inertial events then cancel the
+  // overscroll if it is not completed yet.
+  // Timestamp for the first inertial event (fling) in current stream.
+  base::Optional<base::TimeTicks> first_inertial_event_time_;
+
   DISALLOW_COPY_AND_ASSIGN(OverscrollController);
 };
 

content/browser/renderer_host/overscroll_controller_unittest.cc

@@ -201,6 +201,75 @@ TEST_F(OverscrollControllerTest,
       100, 0, blink::kWebGestureDeviceTouchpad, timestamp, true));
 }
 
+// Ensure inertial gesture scroll update can not start overscroll.
+TEST_F(OverscrollControllerTest, InertialGSUsDoNotStartOverscroll) {
+  base::TimeTicks timestamp =
+      blink::WebInputEvent::GetStaticTimeStampForTests();
+  // Inertial update event complete the overscroll action.
+  EXPECT_FALSE(SimulateGestureScrollUpdate(
+      100, 0, blink::kWebGestureDeviceTouchpad, timestamp, true));
+  SimulateAck(false);
+  EXPECT_EQ(OVERSCROLL_NONE, controller_mode());
+  EXPECT_EQ(OverscrollSource::NONE, controller_source());
+  EXPECT_EQ(OVERSCROLL_NONE, delegate()->current_mode());
+  EXPECT_EQ(OVERSCROLL_NONE, delegate()->completed_mode());
+}
+
+// After 300ms inertial gesture scroll updates, overscroll must get cancelled
+// if not completed.
+TEST_F(OverscrollControllerTest, OnlyProcessLimitedInertialGSUEvents) {
+  base::TimeTicks timestamp =
+      blink::WebInputEvent::GetStaticTimeStampForTests();
+
+  EXPECT_FALSE(SimulateGestureEvent(blink::WebInputEvent::kGestureScrollBegin,
+                                    blink::kWebGestureDeviceTouchpad,
+                                    timestamp));
+  SimulateAck(false);
+
+  EXPECT_FALSE(SimulateGestureScrollUpdate(
+      61, 0, blink::kWebGestureDeviceTouchpad, timestamp, false));
+  SimulateAck(false);
+  EXPECT_EQ(OVERSCROLL_EAST, controller_mode());
+  EXPECT_EQ(OverscrollSource::TOUCHPAD, controller_source());
+  EXPECT_EQ(OVERSCROLL_EAST, delegate()->current_mode());
+  EXPECT_EQ(OVERSCROLL_NONE, delegate()->completed_mode());
+
+  // First inertial.
+  timestamp += base::TimeDelta::FromSeconds(1);
+  EXPECT_TRUE(SimulateGestureScrollUpdate(
+      1, 0, blink::kWebGestureDeviceTouchpad, timestamp, true));
+  SimulateAck(true);
+  EXPECT_EQ(OVERSCROLL_EAST, controller_mode());
+  EXPECT_EQ(OverscrollSource::TOUCHPAD, controller_source());
+  EXPECT_EQ(OVERSCROLL_EAST, delegate()->current_mode());
+  EXPECT_EQ(OVERSCROLL_NONE, delegate()->completed_mode());
+
+  // Not cancel in 10ms.
+  timestamp += base::TimeDelta::FromMilliseconds(10);
+  EXPECT_TRUE(SimulateGestureScrollUpdate(
+      1, 0, blink::kWebGestureDeviceTouchpad, timestamp, true));
+  SimulateAck(true);
+  EXPECT_EQ(OVERSCROLL_EAST, controller_mode());
+  EXPECT_EQ(OverscrollSource::TOUCHPAD, controller_source());
+  EXPECT_EQ(OVERSCROLL_EAST, delegate()->current_mode());
+  EXPECT_EQ(OVERSCROLL_NONE, delegate()->completed_mode());
+
+  // Cancel after 300ms.
+  timestamp += base::TimeDelta::FromMilliseconds(291);
+  EXPECT_TRUE(SimulateGestureScrollUpdate(
+      1, 0, blink::kWebGestureDeviceTouchpad, timestamp, true));
+  SimulateAck(true);
+  EXPECT_EQ(OVERSCROLL_NONE, controller_mode());
+  EXPECT_EQ(OverscrollSource::NONE, controller_source());
+  EXPECT_EQ(OVERSCROLL_NONE, delegate()->current_mode());
+  EXPECT_EQ(OVERSCROLL_NONE, delegate()->completed_mode());
+
+  // Next event should be ignored.
+  timestamp += base::TimeDelta::FromMilliseconds(100);
+  EXPECT_TRUE(SimulateGestureScrollUpdate(
+      1, 0, blink::kWebGestureDeviceTouchpad, timestamp, true));
+}
+
 // Verifies that when pull-to-refresh is disabled, it is not triggered for
 // neither touchpad nor touchscreen.
 TEST_F(OverscrollControllerTest, PullToRefreshDisabled) {

content/public/browser/overscroll_configuration.h

@@ -6,6 +6,7 @@
 #define CONTENT_PUBLIC_BROWSER_OVERSCROLL_CONFIGURATION_H_
 
 #include "base/macros.h"
+#include "base/time/time.h"
 #include "content/common/content_export.h"
 
 namespace content {
@@ -62,6 +63,8 @@ class CONTENT_EXPORT OverscrollConfig {
 
   static bool TouchpadOverscrollHistoryNavigationEnabled();
 
+  static base::TimeDelta MaxInertialEventsBeforeOverscrollCancellation();
+
  private:
   friend class ScopedHistoryNavigationMode;
   friend class ScopedPullToRefreshMode;