[Reland] Fix for thumb drag and pointer offset.

The original CL was reverted because one of the content_browsertests
(ScrollLatencyCompositedScrollbarBrowserTest.ScrollbarThumbDragLatency)
got flaky. This failure was not caught because all TryJobs passed the
dry run on retries (crbug.com/988443)

Reason for the flakiness:
This CL exposed a pre existing race condition. The issue only now came
to light because this CL changes how thumb drag takes place.
ScrollbarController::HandleMouseMove now calls ComputeScrollDelta
for calculating the clamped delta. ComputeScrollDelta is where execution
aborts due to the scroll_node being null. The reason the scroll_node is
sometimes null here is because the mousemove reaches ComputeScrollDelta
before GSB has had a chance to set up the scroll_node (due to queueing).
See crbug.com/988308 for more details.

Reverted CL is crrev.com/c/1684532
Reverted CL is in PS2.
New CL is in PS6.

[Description from the original CL]:
Fixes an issue with thumb drags for compositor threaded scrollbar
scrolling. When you click and drag the thumb past the track and then
reverse your drag direction, the pointer no longer sticks to the thumb
and instead, maintains a constant offset with the thumb as it moves.
The reason this happens is because the scroll offset calculation in the
ScrollbarController simply uses 2 factors to calculate the delta. One
of them is the previous_pointer_position_ and the other is the current
pointer location (i.e position_in_widget). There is no logic to check
if the difference between these 2 factors should indeed generate a GSU
or not. The fix is to make the thumb drag delta calculation relative to
the layer and then use LayerTreeHostImpl::CanConsumeDelta to determine
if a scroll should be initiated.

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

cc/input/scrollbar_controller.cc

@@ -25,7 +25,6 @@ ScrollbarController::ScrollbarController(
     LayerTreeHostImpl* layer_tree_host_impl)
     : layer_tree_host_impl_(layer_tree_host_impl),
       scrollbar_scroll_is_active_(false),
-      thumb_drag_in_progress_(false),
       autoscroll_state_(AutoScrollState::NO_AUTOSCROLL),
       currently_captured_scrollbar_(nullptr),
       previous_pointer_position_(gfx::PointF(0, 0)),
@@ -41,6 +40,21 @@ void ScrollbarController::WillBeginImplFrame() {
     layer_tree_host_impl_->mutator_host()->ScrollAnimationAbort();
 }
 
+gfx::Vector2dF ScrollbarController::GetThumbRelativePoint(
+    const gfx::PointF position_in_widget) {
+  bool clipped;
+  const gfx::PointF position_in_layer =
+      GetScrollbarRelativePosition(position_in_widget, &clipped);
+
+  if (clipped)
+    return gfx::Vector2d(0, 0);
+
+  const gfx::RectF thumb_rect(
+      currently_captured_scrollbar_->ComputeThumbQuadRect());
+  DCHECK(thumb_rect.Contains(position_in_layer));
+  return position_in_layer - gfx::PointF(thumb_rect.origin());
+}
+
 // Performs hit test and prepares scroll deltas that will be used by GSB and
 // GSU.
 InputHandlerPointerResult ScrollbarController::HandleMouseDown(
@@ -56,16 +70,16 @@ InputHandlerPointerResult ScrollbarController::HandleMouseDown(
   currently_captured_scrollbar_ = layer_impl->ToScrollbarLayer();
   scroll_result.type = PointerResultType::kScrollbarScroll;
   layer_tree_host_impl_->active_tree()->UpdateScrollbarGeometries();
-  ScrollbarPart scrollbar_part =
+  const ScrollbarPart scrollbar_part =
       GetScrollbarPartFromPointerDown(position_in_widget);
   scroll_result.scroll_offset = GetScrollOffsetForScrollbarPart(
       scrollbar_part, currently_captured_scrollbar_->orientation());
   previous_pointer_position_ = position_in_widget;
   scrollbar_scroll_is_active_ = true;
   if (scrollbar_part == ScrollbarPart::THUMB) {
-    thumb_drag_in_progress_ = true;
     scroll_result.scroll_units =
         ui::input_types::ScrollGranularity::kScrollByPrecisePixel;
+    drag_anchor_relative_to_thumb_ = GetThumbRelativePoint(position_in_widget);
   } else {
     // TODO(arakeri): This needs to be updated to kLine once cc implements
     // handling it. crbug.com/959441
@@ -73,10 +87,12 @@ InputHandlerPointerResult ScrollbarController::HandleMouseDown(
         ui::input_types::ScrollGranularity::kScrollByPixel;
   }
 
-  // Thumb drag is the only scrollbar manipulation that cannot produce an
-  // autoscroll. All other interactions like clicking on arrows/trackparts have
-  // the potential of initiating an autoscroll (if held down long enough).
-  if (!scroll_result.scroll_offset.IsZero() && !thumb_drag_in_progress_) {
+  if (!scroll_result.scroll_offset.IsZero()) {
+    // Thumb drag is the only scrollbar manipulation that cannot produce an
+    // autoscroll. All other interactions like clicking on arrows/trackparts
+    // have the potential of initiating an autoscroll (if held down for long
+    // enough).
+    DCHECK(scrollbar_part != ScrollbarPart::THUMB);
     cancelable_autoscroll_task_ = std::make_unique<base::CancelableClosure>(
         base::Bind(&ScrollbarController::StartAutoScrollAnimation,
                    base::Unretained(this), scroll_result.scroll_offset,
@@ -89,29 +105,79 @@ InputHandlerPointerResult ScrollbarController::HandleMouseDown(
   return scroll_result;
 }
 
+gfx::ScrollOffset ScrollbarController::GetScrollOffsetForDragPosition(
+    const gfx::PointF pointer_position_in_widget) {
+  layer_tree_host_impl_->active_tree()->UpdateScrollbarGeometries();
+
+  const gfx::Rect thumb_rect(
+      currently_captured_scrollbar_->ComputeThumbQuadRect());
+  const gfx::PointF drag_position_relative_to_layer =
+      gfx::PointF(thumb_rect.origin()) + drag_anchor_relative_to_thumb_.value();
+
+  bool clipped = false;
+  const gfx::PointF pointer_position_in_layer =
+      GetScrollbarRelativePosition(pointer_position_in_widget, &clipped);
+
+  if (clipped)
+    return gfx::ScrollOffset(0, 0);
+
+  // Calculate the delta based on the previously known thumb drag point.
+  const gfx::Vector2dF pointer_delta =
+      pointer_position_in_layer - drag_position_relative_to_layer;
+
+  gfx::ScrollOffset scaled_thumb_drag_delta;
+  const ScrollbarOrientation orientation =
+      currently_captured_scrollbar_->orientation();
+  orientation == ScrollbarOrientation::VERTICAL
+      ? scaled_thumb_drag_delta.set_y(pointer_delta.y())
+      : scaled_thumb_drag_delta.set_x(pointer_delta.x());
+
+  float scaled_scroller_to_scrollbar_ratio = GetScrollerToScrollbarRatio();
+  scaled_thumb_drag_delta.Scale(scaled_scroller_to_scrollbar_ratio);
+  return scaled_thumb_drag_delta;
+}
+
 // Performs hit test and prepares scroll deltas that will be used by GSU.
 InputHandlerPointerResult ScrollbarController::HandleMouseMove(
     const gfx::PointF position_in_widget) {
-  const gfx::PointF previous_pointer_position = previous_pointer_position_;
   previous_pointer_position_ = position_in_widget;
   InputHandlerPointerResult scroll_result;
-  if (!thumb_drag_in_progress_)
+
+  // If a thumb drag is not in progress, there's no point in continuing on.
+  if (!drag_anchor_relative_to_thumb_.has_value())
+    return scroll_result;
+
+  const ScrollNode* currently_scrolling_node =
+      layer_tree_host_impl_->CurrentlyScrollingNode();
+
+  // Thumb drag needs a scroll_node. Clear the thumb drag state and exit if it
+  // is unset.
+  if (currently_scrolling_node == nullptr) {
+    drag_anchor_relative_to_thumb_ = base::nullopt;
     return scroll_result;
+  }
 
+  // If scroll_offset can't be consumed, there's no point in continuing on.
+  const gfx::ScrollOffset scroll_offset(
+      GetScrollOffsetForDragPosition(position_in_widget));
+  const gfx::Vector2dF clamped_scroll_offset(
+      layer_tree_host_impl_->ComputeScrollDelta(
+          *currently_scrolling_node, ScrollOffsetToVector2dF(scroll_offset)));
+
+  if (clamped_scroll_offset.IsZero())
+    return scroll_result;
+
+  // Thumb drags have more granularity and are purely dependent on the pointer
+  // movement. Hence we use kPrecisePixel when dragging the thumb.
+  scroll_result.scroll_units =
+      ui::input_types::ScrollGranularity::kScrollByPrecisePixel;
   scroll_result.type = PointerResultType::kScrollbarScroll;
-  const ScrollbarOrientation orientation =
-      currently_captured_scrollbar_->orientation();
-  if (orientation == ScrollbarOrientation::VERTICAL)
-    scroll_result.scroll_offset.set_y(position_in_widget.y() -
-                                      previous_pointer_position.y());
-  else
-    scroll_result.scroll_offset.set_x(position_in_widget.x() -
-                                      previous_pointer_position.x());
-
-  LayerImpl* owner_scroll_layer =
-      layer_tree_host_impl_->active_tree()->ScrollableLayerByElementId(
-          currently_captured_scrollbar_->scroll_element_id());
+  scroll_result.scroll_offset = gfx::ScrollOffset(clamped_scroll_offset);
+
+  return scroll_result;
+}
 
+float ScrollbarController::GetScrollerToScrollbarRatio() {
   // Calculating the delta by which the scroller layer should move when
   // dragging the thumb depends on the following factors:
   // - scrollbar_track_length
@@ -155,10 +221,15 @@ InputHandlerPointerResult ScrollbarController::HandleMouseMove(
       currently_captured_scrollbar_->scroll_layer_length();
   float scrollbar_track_length = currently_captured_scrollbar_->TrackLength();
   gfx::Rect thumb_rect(currently_captured_scrollbar_->ComputeThumbQuadRect());
+  const ScrollbarOrientation orientation =
+      currently_captured_scrollbar_->orientation();
   float scrollbar_thumb_length = orientation == ScrollbarOrientation::VERTICAL
                                      ? thumb_rect.height()
                                      : thumb_rect.width();
 
+  const LayerImpl* owner_scroll_layer =
+      layer_tree_host_impl_->active_tree()->ScrollableLayerByElementId(
+          currently_captured_scrollbar_->scroll_element_id());
   float viewport_length =
       orientation == ScrollbarOrientation::VERTICAL
           ? owner_scroll_layer->scroll_container_bounds().height()
@@ -167,13 +238,17 @@ InputHandlerPointerResult ScrollbarController::HandleMouseMove(
       ((scroll_layer_length - viewport_length) /
        (scrollbar_track_length - scrollbar_thumb_length)) *
       layer_tree_host_impl_->active_tree()->device_scale_factor();
-  scroll_result.scroll_offset.Scale(scaled_scroller_to_scrollbar_ratio);
-  // Thumb drags have more granularity and are purely dependent on the pointer
-  // movement. Hence we use kPrecisePixel when dragging the thumb.
-  scroll_result.scroll_units =
-      ui::input_types::ScrollGranularity::kScrollByPrecisePixel;
 
-  return scroll_result;
+  // Avoid precision loss later on by rounding up 3 decimal places here.
+  // TODO(arakeri): Revisit this while fixing crbug.com/986174. There is a
+  // precision loss that is happening somewhere which affects root scrollbars
+  // only. Even though it is not visible to the end user, without rounding up,
+  // the scrolling tests will fail due to this precision loss.
+  DCHECK_GT(scaled_scroller_to_scrollbar_ratio, 0);
+  scaled_scroller_to_scrollbar_ratio =
+      ceil(scaled_scroller_to_scrollbar_ratio * 1000.0) / 1000.0;
+
+  return scaled_scroller_to_scrollbar_ratio;
 }
 
 bool ScrollbarController::ShouldCancelTrackAutoscroll() {
@@ -285,7 +360,7 @@ InputHandlerPointerResult ScrollbarController::HandleMouseUp(
     cancelable_autoscroll_task_.reset();
   }
 
-  thumb_drag_in_progress_ = false;
+  drag_anchor_relative_to_thumb_ = base::nullopt;
   autoscroll_state_ = AutoScrollState::NO_AUTOSCROLL;
   return scroll_result;
 }

cc/input/scrollbar_controller.h

@@ -58,6 +58,18 @@ class CC_EXPORT ScrollbarController {
   // Decides whether a track autoscroll should be aborted.
   bool ShouldCancelTrackAutoscroll();
 
+  // Calculates the scroll_offset based on position_in_widget and
+  // drag_anchor_relative_to_thumb_.
+  gfx::ScrollOffset GetScrollOffsetForDragPosition(
+      const gfx::PointF pointer_position_in_widget);
+
+  // Returns a Vector2dF for position_in_widget relative to the scrollbar thumb.
+  gfx::Vector2dF GetThumbRelativePoint(const gfx::PointF position_in_widget);
+
+  // Returns the ratio of the scroller length to the scrollbar length. This is
+  // needed to scale the scroll delta for thumb drag.
+  float GetScrollerToScrollbarRatio();
+
   LayerTreeHostImpl* layer_tree_host_impl_;
 
   // Used to safeguard against firing GSE without firing GSB and GSU. For
@@ -65,9 +77,6 @@ class CC_EXPORT ScrollbarController {
   // moving inside the scrollbar, a GSE will get queued up without this flag.
   bool scrollbar_scroll_is_active_;
 
-  // Used to tell if the scrollbar thumb is getting dragged.
-  bool thumb_drag_in_progress_;
-
   // "Autoscroll" here means the continuous scrolling that occurs when the
   // pointer is held down on a hit-testable area of the scrollbar such as an
   // arrows of the track itself.
@@ -77,6 +86,10 @@ class CC_EXPORT ScrollbarController {
   // This is relative to the RenderWidget's origin.
   gfx::PointF previous_pointer_position_;
 
+  // This is used to track the pointer location relative to the thumb origin
+  // when a drag has started.
+  base::Optional<gfx::Vector2dF> drag_anchor_relative_to_thumb_;
+
   std::unique_ptr<base::CancelableClosure> cancelable_autoscroll_task_;
 };
 

third_party/blink/web_tests/fast/scrolling/scrollbars/mouse-scrolling-on-div-scrollbar.html

@@ -140,6 +140,12 @@ window.onload = () => {
     await mouseDownAt(x, y);
     assert_equals(standardDivFast.scrollTop, 0, "Mousedown on vertical scrollbar thumb is not expected to scroll.");
 
+    await mouseMoveTo(x, y-10);
+    assert_equals(standardDivFast.scrollTop, 0, "Vertical thumb drag beyond the track should not cause a scroll.");
+
+    await mouseMoveTo(x, y);
+    assert_equals(standardDivFast.scrollTop, 0, "Vertical thumb drag beyond the track and back should not cause a scroll.");
+
     y += SCROLL_DELTA;
     await mouseMoveTo(x, y);
     assert_equals(standardDivFast.scrollTop, 915, "Vertical thumb drag downwards did not scroll as expected.");
@@ -159,6 +165,12 @@ window.onload = () => {
     await mouseDownAt(x, y);
     assert_equals(standardDivFast.scrollLeft, 0, "Mousedown on horizontal scrollbar thumb is not expected to scroll.");
 
+    await mouseMoveTo(x-10, y);
+    assert_equals(standardDivFast.scrollLeft, 0, "Horizontal thumb drag beyond the track should not cause a scroll.");
+
+    await mouseMoveTo(x, y);
+    assert_equals(standardDivFast.scrollLeft, 0, "Horizontal thumb drag beyond the track and back should not cause a scroll.");
+
     x += SCROLL_DELTA;
     await mouseMoveTo(x, y);
     assert_equals(standardDivFast.scrollLeft, 915, "Horizontal thumb drag towards the right did not scroll as expected.");

third_party/blink/web_tests/fast/scrolling/scrollbars/mouse-scrolling-on-root-scrollbar.html

@@ -85,6 +85,12 @@ window.onload = () => {
     await mouseDownAt(x, y);
     assert_equals(window.scrollY, 0, "Mousedown on vertical scrollbar thumb is not expected to scroll.");
 
+    await mouseMoveTo(x, y-10);
+    assert_equals(window.scrollY, 0, "Vertical thumb drag beyond the track should not cause a scroll.");
+
+    await mouseMoveTo(x, y);
+    assert_equals(window.scrollY, 0, "Vertical thumb drag beyond the track and back should not cause a scroll.");
+
     y += SCROLL_DELTA;
     await mouseMoveTo(x, y);
     // TODO(arakeri): There is currently a 1 px difference when dragging the thumb on the compositor thread
@@ -107,6 +113,12 @@ window.onload = () => {
     await mouseDownAt(x, y);
     assert_equals(window.scrollX, 0, "Mousedown on horizontal scrollbar thumb is not expected to scroll.");
 
+    await mouseMoveTo(x-10, y);
+    assert_equals(window.scrollX, 0, "Horizontal thumb drag beyond the track should not cause a scroll.");
+
+    await mouseMoveTo(x, y);
+    assert_equals(window.scrollX, 0, "Horizontal thumb drag beyond the track and back should not cause a scroll.");
+
     x += SCROLL_DELTA;
     await mouseMoveTo(x, y);
     assert_approx_equals(window.scrollX, 133, 1, "Horizontal thumb drag towards the right did not scroll as expected.");