Fix infinite animation on fling from diagonal overscroll.

This CL fixes an issue where in if a user flings the scroller while
diagonally overscrolled, the scroller never comes out of the overscroll
and instead, plays a forward animation that takes the scroller to the
end. This happens because ElasticOverscrollController::Animate keeps
supplying zero time deltas to StretchAmountForTimeDelta and that leads
to the animation never ending. The time deltas are zero due to
momentum_animation_reset_at_next_frame_ wrongly getting reset every
frame while diagonally overscrolled.

The comment for momentum_animation_reset_at_next_frame_ claims that it
was put in place to handle programmatic scrolls while overscrolled.
However, after deleting this code and doing some tests, I don't see
any difference in how programmatic scrolls are handled while
overscrolled. This is code was carried over as a part of the overscroll
refactor (crrev.com/c/2231254). The code was originally at least 5
years old and seems to be no longer needed. Getting rid of the code
fixes the bug too.

Bug: 1122482
Change-Id: Idb78a735d023e40151fba4a69e2191d1117cf95a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377235
Commit-Queue: Rahul Arakeri <arakeri@microsoft.com>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812975}

third_party/blink/renderer/platform/widget/input/elastic_overscroll_controller.cc

@@ -249,7 +249,6 @@ void ElasticOverscrollController::EnterStateMomentumAnimated(
   momentum_animation_start_time_ = triggering_event_timestamp;
   momentum_animation_initial_stretch_ = helper_->StretchAmount();
   momentum_animation_initial_velocity_ = scroll_velocity_;
-  momentum_animation_reset_at_next_frame_ = false;
 
   // Similarly to the logic in Overscroll, prefer vertical scrolling to
   // horizontal scrolling.
@@ -276,13 +275,6 @@ void ElasticOverscrollController::Animate(base::TimeTicks time) {
   if (state_ != kStateMomentumAnimated)
     return;
 
-  if (momentum_animation_reset_at_next_frame_) {
-    momentum_animation_start_time_ = time;
-    momentum_animation_initial_stretch_ = helper_->StretchAmount();
-    momentum_animation_initial_velocity_ = gfx::Vector2dF();
-    momentum_animation_reset_at_next_frame_ = false;
-  }
-
   // If the new stretch amount is near zero, set it directly to zero and enter
   // the inactive state.
   const gfx::Vector2dF new_stretch_amount = StretchAmountForTimeDelta(
@@ -366,21 +358,10 @@ void ElasticOverscrollController::ReconcileStretchAndScroll() {
   helper_->ScrollBy(-stretch_adjustment);
   helper_->SetStretchAmount(new_stretch_amount);
 
-  // Update the internal state for the active scroll or animation to avoid
-  // discontinuities.
-  switch (state_) {
-    case kStateActiveScroll:
+  // Update the internal state for the active scroll to avoid discontinuities.
+  if (state_ == kStateActiveScroll) {
     stretch_scroll_force_ =
         AccumulatedOverscrollForStretchAmount(new_stretch_amount);
-      break;
-    case kStateMomentumAnimated:
-      momentum_animation_reset_at_next_frame_ = true;
-      break;
-    default:
-      // These cases should not be hit because the stretch must be zero in the
-      // Inactive and MomentumScroll states.
-      NOTREACHED();
-      break;
   }
 }
 

third_party/blink/renderer/platform/widget/input/elastic_overscroll_controller.h

@@ -112,6 +112,8 @@ class PLATFORM_EXPORT ElasticOverscrollController {
                            VerifyForwardAnimationTick);
   FRIEND_TEST_ALL_PREFIXES(ElasticOverscrollControllerBezierTest,
                            VerifyForwardAnimationIsNotPlayed);
+  FRIEND_TEST_ALL_PREFIXES(ElasticOverscrollControllerBezierTest,
+                           VerifyInitialStretchDelta);
 
   enum State {
     // The initial state, during which the overscroll amount is zero and
@@ -155,11 +157,6 @@ class PLATFORM_EXPORT ElasticOverscrollController {
   bool CanScrollHorizontally() const;
   bool CanScrollVertically() const;
 
-  // This is set in response to a scroll (most likely programmatic) occuring
-  // while animating the momentum phase. In this case, re-set the initial
-  // velocity, stretch, and start time at the next frame (this is the same
-  // behavior as would happen if the scroll were caused by an active scroll).
-  bool momentum_animation_reset_at_next_frame_;
   base::TimeTicks momentum_animation_start_time_;
   cc::ScrollElasticityHelper* helper_;
   State state_;

third_party/blink/renderer/platform/widget/input/elastic_overscroll_controller_bezier.h

@@ -45,6 +45,8 @@ class PLATFORM_EXPORT ElasticOverscrollControllerBezier
       const double bounce_forwards_distance) const;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(ElasticOverscrollControllerBezierTest,
+                           VerifyInitialStretchDelta);
   FRIEND_TEST_ALL_PREFIXES(ElasticOverscrollControllerBezierTest,
                            VerifyForwardAnimationIsNotPlayed);
 

third_party/blink/renderer/platform/widget/input/elastic_overscroll_controller_bezier_unittest.cc

@@ -143,6 +143,53 @@ TEST_F(ElasticOverscrollControllerBezierTest, ReconcileStretchAndScroll) {
   EXPECT_EQ(Vector2dF(-18, 0), helper_.StretchAmount());
 }
 
+// Tests that momentum_animation_start_time_ doesn't get reset when the
+// overscroll animation is ticking and the scroller is diagonally overscrolled.
+TEST_F(ElasticOverscrollControllerBezierTest, VerifyInitialStretchDelta) {
+  // Set up the state to be in kStateMomentumAnimated with some amount of
+  // diagonal stretch.
+  controller_.state_ =
+      ElasticOverscrollController::State::kStateMomentumAnimated;
+  helper_.SetStretchAmount(Vector2dF(5, 10));
+  helper_.SetScrollOffsetAndMaxScrollOffset(gfx::ScrollOffset(0, 20),
+                                            gfx::ScrollOffset(100, 100));
+  controller_.ReconcileStretchAndScroll();
+  controller_.bounce_forwards_duration_x_ =
+      base::TimeDelta::FromMilliseconds(1000);
+  controller_.bounce_forwards_duration_y_ =
+      base::TimeDelta::FromMilliseconds(1000);
+  controller_.momentum_animation_initial_stretch_ = gfx::Vector2dF(10.f, 10.f);
+
+  // Verify that the momentum_animation_start_time_ doesn't get reset when the
+  // animation ticks.
+  const base::TimeTicks animation_start_time =
+      base::TimeTicks() + base::TimeDelta::FromMilliseconds(32);
+
+  // After 2 frames.
+  controller_.Animate(animation_start_time);
+  helper_.ScrollBy(Vector2dF(0, 2));
+  EXPECT_NE(controller_.momentum_animation_start_time_, animation_start_time);
+  EXPECT_EQ(controller_.state_,
+            ElasticOverscrollController::State::kStateMomentumAnimated);
+
+  // After 8 frames.
+  controller_.Animate(animation_start_time +
+                      base::TimeDelta::FromMilliseconds(128));
+  helper_.ScrollBy(Vector2dF(0, 8));
+  EXPECT_NE(controller_.momentum_animation_start_time_, animation_start_time);
+  EXPECT_EQ(controller_.state_,
+            ElasticOverscrollController::State::kStateMomentumAnimated);
+
+  // After 64 frames the forward animation should no longer be active.
+  controller_.Animate(animation_start_time +
+                      base::TimeDelta::FromMilliseconds(1024));
+  helper_.ScrollBy(Vector2dF(0, 64));
+  EXPECT_NE(controller_.momentum_animation_start_time_, animation_start_time);
+  EXPECT_EQ(controller_.state_,
+            ElasticOverscrollController::State::kStateInactive);
+  EXPECT_EQ(Vector2dF(), helper_.StretchAmount());
+}
+
 // Tests if the overscrolled delta maps correctly to the actual amount that the
 // scroller gets stretched.
 TEST_F(ElasticOverscrollControllerBezierTest, VerifyOverscrollBounceDistance) {