Infinite scrolling thumb displacement.

Bug:
When a thumb drag is in progress, the scroller will jump every time the
scroller length changes. This happens because the pointer and the thumb
always try to stay in sync with each other. Main thread scrollbars dont
have this issue as they allow the thumb and pointer to get out of sync.

Fix:
If the scroller length increases mid thumb drag, we record the
"displacement" delta and early out. This delta is then applied to every
subsequent pointermove to ensure that that scroller retains its offset
even when the scroller length changes. The downside of this approach is
that the thumb and the pointer will get out of sync but the upside is
that the scroller won't suddenly jump.

Bug: 1067518
Change-Id: I773335db538b8595e96464f3306d2df9e82a58f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134923
Commit-Queue: Rahul Arakeri <arakeri@microsoft.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759288}

cc/input/scrollbar_controller.cc

@@ -102,6 +102,8 @@ InputHandlerPointerResult ScrollbarController::HandlePointerDown(
     // thumb back to its original position if the pointer moves too far away
     // from the track during a thumb drag.
     drag_state_->scroll_position_at_start_ = scrollbar->current_pos();
+    drag_state_->scroller_length_at_previous_move =
+        scrollbar->scroll_layer_length();
   }
 
   if (!scroll_result.scroll_offset.IsZero()) {
@@ -233,7 +235,7 @@ float ScrollbarController::GetScrollDeltaForAbsoluteJump(
   return delta * GetScrollerToScrollbarRatio(scrollbar);
 }
 
-gfx::ScrollOffset ScrollbarController::GetScrollDeltaForDragPosition(
+int ScrollbarController::GetScrollDeltaForDragPosition(
     const ScrollbarLayerImplBase* scrollbar,
     const gfx::PointF pointer_position_in_widget) {
   const float pointer_delta =
@@ -247,9 +249,7 @@ gfx::ScrollOffset ScrollbarController::GetScrollDeltaForDragPosition(
                              new_offset - scrollbar->current_pos();
 
   // Scroll delta floored to match main thread per pixel behavior
-  return scrollbar->orientation() == ScrollbarOrientation::VERTICAL
-             ? gfx::ScrollOffset(0, floorf(scroll_delta))
-             : gfx::ScrollOffset(floorf(scroll_delta), 0);
+  return floorf(scroll_delta);
 }
 
 // Performs hit test and prepares scroll deltas that will be used by GSU.
@@ -302,9 +302,27 @@ InputHandlerPointerResult ScrollbarController::HandlePointerMove(
   // valid ScrollNode.
   DCHECK(target_node);
 
+  int delta = GetScrollDeltaForDragPosition(scrollbar, position_in_widget);
+  if (drag_state_->scroller_length_at_previous_move !=
+      scrollbar->scroll_layer_length()) {
+    drag_state_->scroller_displacement = delta;
+    drag_state_->scroller_length_at_previous_move =
+        scrollbar->scroll_layer_length();
+
+    // This is done to ensure that, when the scroller length changes mid thumb
+    // drag, the scroller shouldn't jump. We early out because the delta would
+    // be zero in this case anyway (since drag_state_->scroller_displacement =
+    // delta). So that means, in the worst case you'd miss 1 GSU every time the
+    // scroller expands while a thumb drag is in progress.
+    return scroll_result;
+  }
+  delta -= drag_state_->scroller_displacement;
+
   // If scroll_offset can't be consumed, there's no point in continuing on.
-  const gfx::ScrollOffset scroll_offset(
-      GetScrollDeltaForDragPosition(scrollbar, position_in_widget));
+  const gfx::ScrollOffset scroll_offset(scrollbar->orientation() ==
+                                                ScrollbarOrientation::VERTICAL
+                                            ? gfx::ScrollOffset(0, delta)
+                                            : gfx::ScrollOffset(delta, 0));
   const gfx::Vector2dF clamped_scroll_offset(
       layer_tree_host_impl_->ComputeScrollDelta(
           *target_node, ScrollOffsetToVector2dF(scroll_offset)));

cc/input/scrollbar_controller.h

@@ -169,6 +169,13 @@ class CC_EXPORT ScrollbarController {
     // This is needed for thumb snapping when the pointer moves too far away
     // from the track while scrolling.
     float scroll_position_at_start_;
+
+    // The |scroller_displacement| indicates the scroll offset compensation that
+    // needs to be applied when the scroller's length changes dynamically mid
+    // thumb drag. This is needed done to ensure that the scroller does not jump
+    // while a thumb drag is in progress.
+    float scroller_displacement;
+    float scroller_length_at_previous_move;
   };
 
   struct CC_EXPORT CapturedScrollbarMetadata {
@@ -232,8 +239,8 @@ class CC_EXPORT ScrollbarController {
   ui::ScrollGranularity Granularity(const ScrollbarPart scrollbar_part,
                                     bool shift_modifier);
 
-  // Calculates the scroll_offset based on position_in_widget and drag_origin.
-  gfx::ScrollOffset GetScrollDeltaForDragPosition(
+  // Calculates the delta based on position_in_widget and drag_origin.
+  int GetScrollDeltaForDragPosition(
       const ScrollbarLayerImplBase* scrollbar,
       const gfx::PointF pointer_position_in_widget);
 

cc/trees/layer_tree_host_impl_unittest.cc

@@ -12710,6 +12710,109 @@ TEST_F(LayerTreeHostImplTest, SingleGSUForScrollbarThumbDragPerFrame) {
   host_impl_ = nullptr;
 }
 
+// Tests that changing the scroller length in the middle of a thumb drag doesn't
+// cause the scroller to jump.
+TEST_F(LayerTreeHostImplTest, ThumbDragScrollerLengthIncrease) {
+  LayerTreeSettings settings = DefaultSettings();
+  settings.compositor_threaded_scrollbar_scrolling = true;
+  CreateHostImpl(settings, CreateLayerTreeFrameSink());
+
+  // Setup the viewport.
+  const gfx::Size viewport_size = gfx::Size(360, 600);
+  const gfx::Size content_size = gfx::Size(345, 3800);
+  SetupViewportLayersOuterScrolls(viewport_size, content_size);
+  LayerImpl* scroll_layer = OuterViewportScrollLayer();
+
+  // Set up the scrollbar and its dimensions.
+  LayerTreeImpl* layer_tree_impl = host_impl_->active_tree();
+  auto* scrollbar = AddLayer<PaintedScrollbarLayerImpl>(
+      layer_tree_impl, VERTICAL, /*is_left_side_vertical_scrollbar*/ false,
+      /*is_overlay*/ false);
+
+  // TODO(arakeri): crbug.com/1070063 Setting the dimensions for scrollbar parts
+  // (like thumb, track etc) should be moved to SetupScrollbarLayer.
+  SetupScrollbarLayer(scroll_layer, scrollbar);
+  const gfx::Size scrollbar_size = gfx::Size(15, 600);
+  scrollbar->SetBounds(scrollbar_size);
+
+  // Set up the thumb dimensions.
+  scrollbar->SetThumbThickness(15);
+  scrollbar->SetThumbLength(50);
+  scrollbar->SetTrackRect(gfx::Rect(0, 15, 15, 575));
+
+  // Set up scrollbar arrows.
+  scrollbar->SetBackButtonRect(
+      gfx::Rect(gfx::Point(345, 0), gfx::Size(15, 15)));
+  scrollbar->SetForwardButtonRect(
+      gfx::Rect(gfx::Point(345, 570), gfx::Size(15, 15)));
+  scrollbar->SetOffsetToTransformParent(gfx::Vector2dF(345, 0));
+
+  TestInputHandlerClient input_handler_client;
+  host_impl_->BindToClient(&input_handler_client);
+
+  // ----------------------------- Start frame 0 -----------------------------
+  viz::BeginFrameArgs begin_frame_args =
+      viz::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
+  base::TimeTicks start_time =
+      base::TimeTicks() + base::TimeDelta::FromMilliseconds(200);
+  begin_frame_args.frame_time = start_time;
+  begin_frame_args.frame_id.sequence_number++;
+  host_impl_->WillBeginImplFrame(begin_frame_args);
+  InputHandlerPointerResult result =
+      host_impl_->MouseDown(gfx::PointF(350, 18), /*shift_modifier*/ false);
+  EXPECT_EQ(result.scroll_offset.y(), 0);
+  EXPECT_EQ(result.type, PointerResultType::kScrollbarScroll);
+  host_impl_->DidFinishImplFrame(begin_frame_args);
+
+  // ----------------------------- Start frame 1 -----------------------------
+  begin_frame_args.frame_time =
+      start_time + base::TimeDelta::FromMilliseconds(250);
+  begin_frame_args.frame_id.sequence_number++;
+  host_impl_->WillBeginImplFrame(begin_frame_args);
+
+  result = host_impl_->MouseMoveAt(gfx::Point(350, 20));
+  EXPECT_EQ(result.scroll_offset.y(), 12);
+
+  // This is intentional. The thumb drags that follow will test the behavior
+  // *after* the scroller length expansion.
+  scrollbar->SetScrollLayerLength(7000);
+  host_impl_->DidFinishImplFrame(begin_frame_args);
+
+  // ----------------------------- Start frame 2 -----------------------------
+  // The very first mousemove after the scroller length change will result in a
+  // zero offset. This is done to ensure that the scroller doesn't jump ahead
+  // when the length changes mid thumb drag. So, every time the scroller length
+  // changes mid thumb drag, a GSU is lost (in the worst case).
+  begin_frame_args.frame_time =
+      start_time + base::TimeDelta::FromMilliseconds(300);
+  begin_frame_args.frame_id.sequence_number++;
+  host_impl_->WillBeginImplFrame(begin_frame_args);
+  result = host_impl_->MouseMoveAt(gfx::Point(350, 22));
+  EXPECT_EQ(result.scroll_offset.y(), 0);
+  host_impl_->DidFinishImplFrame(begin_frame_args);
+
+  // ----------------------------- Start frame 3 -----------------------------
+  // All subsequent mousemoves then produce deltas which are "displaced" by a
+  // certain amount. The previous mousemove (to y = 22) would have calculated
+  // the drag_state_->scroller_displacement value as 48 (based on the pointer
+  // movement). The current pointermove (to y = 26) calculates the delta as 97.
+  // Since delta -= drag_state_->scroller_displacement, the actual delta becomes
+  // 97 - 48 which is 49. The math that calculates the deltas (i.e 97 and 48)
+  // can be found in ScrollbarController::GetScrollDeltaForDragPosition.
+  begin_frame_args.frame_time =
+      start_time + base::TimeDelta::FromMilliseconds(350);
+  begin_frame_args.frame_id.sequence_number++;
+  host_impl_->WillBeginImplFrame(begin_frame_args);
+  result = host_impl_->MouseMoveAt(gfx::Point(350, 26));
+  EXPECT_EQ(result.scroll_offset.y(), 49);
+  host_impl_->MouseUp(gfx::PointF(350, 26));
+  host_impl_->DidFinishImplFrame(begin_frame_args);
+
+  // Tear down the LayerTreeHostImpl before the InputHandlerClient.
+  host_impl_->ReleaseLayerTreeFrameSink();
+  host_impl_ = nullptr;
+}
+
 TEST_F(LayerTreeHostImplTest, MainThreadFallback) {
   LayerTreeSettings settings = DefaultSettings();
   settings.compositor_threaded_scrollbar_scrolling = true;