Percent based scrolling: Fix mousewheel for large scrollers

With percent based scrolling turned on, small mousewheel scrolls do not work for scrollers with very large offsets. Small percentage values like 0.1 (10%) could not be resolved against very large scrollers (>4M) with offsets > 0.5x that of the scroller on both the main thread and compositor, as they are clamped to 0 and no scroll delta is computed. This change fixes the issue by converting the percentage to a scroll delta in LayerTreeHostImpl::CanConsumeDelta for the compositor, and ScrollManager::CanScroll in the main thread. In addition, scroll speed is now ignored by gpu benchmarking when a test uses percent based scrolling. Percentage based scrolls do not require a velocity and are delivered as a single event. This prevents DCHECK_GT(total_duration_in_us, 0) from being hit in SyntheticSmoothMoveGesture::ComputeNextMoveSegment. Bug: 1057834 Change-Id: Ifec649729a1bb8ef7a5d09311ba93f90b24873ee Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145706 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Reviewed-by: Daniel Libby <dlibby@microsoft.com> Reviewed-by: Rahul Arakeri <arakeri@microsoft.com> Commit-Queue: Sahir Vellani <sahir.vellani@microsoft.com> Cr-Commit-Position: refs/heads/master@{#762989}
Denger-Network · Apr 27, 2020 · e0d798d · e0d798d
1 parent fbb4a6c
commit e0d798d
Show file tree

Hide file tree

Showing 8 changed files with 112 additions and 32 deletions.
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
@@ -4575,6 +4575,10 @@ bool LayerTreeHostImpl::CanConsumeDelta(const ScrollState& scroll_state,
       return false;
     }
     delta_to_scroll = local_scroll_delta;
+  } else if (scroll_state.delta_granularity() ==
+             ui::ScrollGranularity::kScrollByPercentage) {
+    delta_to_scroll =
+        ResolveScrollPercentageToPixels(scroll_node, delta_to_scroll);
   }
 
   if (ComputeScrollDelta(scroll_node, delta_to_scroll) != gfx::Vector2dF())

diff --git a/content/browser/renderer_host/input/synthetic_smooth_move_gesture.cc b/content/browser/renderer_host/input/synthetic_smooth_move_gesture.cc
@@ -370,14 +370,20 @@ gfx::Vector2dF SyntheticSmoothMoveGesture::GetPositionDeltaAtTime(
 void SyntheticSmoothMoveGesture::ComputeNextMoveSegment() {
   current_move_segment_++;
   DCHECK_LT(current_move_segment_, static_cast<int>(params_.distances.size()));
-  int64_t total_duration_in_us = static_cast<int64_t>(
-      1e6 * (params_.distances[current_move_segment_].Length() /
-             params_.speed_in_pixels_s));
-  DCHECK_GT(total_duration_in_us, 0);
-  current_move_segment_start_time_ = current_move_segment_stop_time_;
-  current_move_segment_stop_time_ =
-      current_move_segment_start_time_ +
-      base::TimeDelta::FromMicroseconds(total_duration_in_us);
+  // Percentage based scrolls do not require velocity and are delivered in a
+  // single segment. No need to compute another segment
+  if (params_.granularity == ui::ScrollGranularity::kScrollByPercentage) {
+    current_move_segment_start_time_ = current_move_segment_stop_time_;
+  } else {
+    int64_t total_duration_in_us = static_cast<int64_t>(
+        1e6 * (params_.distances[current_move_segment_].Length() /
+               params_.speed_in_pixels_s));
+    DCHECK_GT(total_duration_in_us, 0);
+    current_move_segment_start_time_ = current_move_segment_stop_time_;
+    current_move_segment_stop_time_ =
+        current_move_segment_start_time_ +
+        base::TimeDelta::FromMicroseconds(total_duration_in_us);
+  }
 }
 
 base::TimeTicks SyntheticSmoothMoveGesture::ClampTimestamp(

diff --git a/content/renderer/gpu_benchmarking_extension.cc b/content/renderer/gpu_benchmarking_extension.cc
@@ -715,6 +715,8 @@ bool GpuBenchmarking::SmoothScrollBy(gin::Arguments* args) {
   // Scroll by percentage only for mouse inputs.
   DCHECK(!scroll_by_percentage ||
          gesture_source_type == SyntheticGestureParams::MOUSE_INPUT);
+  // Scroll by percentage does not require speed in pixels
+  DCHECK(!scroll_by_percentage || (speed_in_pixels_s == 800));
 
   base::Optional<gfx::Vector2dF> pixels_to_scrol_vector =
       ToVector(direction, pixels_to_scroll);

diff --git a/third_party/blink/renderer/core/input/scroll_manager.cc b/third_party/blink/renderer/core/input/scroll_manager.cc
@@ -228,6 +228,14 @@ bool ScrollManager::CanScroll(const ScrollState& scroll_state,
   if (!scrollable_area->UserInputScrollable(kVerticalScrollbar))
     delta_y = 0;
 
+  if (scroll_state.deltaGranularity() ==
+      static_cast<double>(ui::ScrollGranularity::kScrollByPercentage)) {
+    delta_x *= scrollable_area->ScrollStep(
+        ui::ScrollGranularity::kScrollByPercentage, kHorizontalScrollbar);
+    delta_y *= scrollable_area->ScrollStep(
+        ui::ScrollGranularity::kScrollByPercentage, kVerticalScrollbar);
+  }
+
   ScrollOffset current_offset = scrollable_area->GetScrollOffset();
   ScrollOffset target_offset = current_offset + ScrollOffset(delta_x, delta_y);
   ScrollOffset clamped_offset =
@@ -473,6 +481,7 @@ WebInputEventResult ScrollManager::HandleGestureScrollBegin(
   scroll_state_data->position_y = position.Y();
   scroll_state_data->delta_x_hint = -gesture_event.DeltaXInRootFrame();
   scroll_state_data->delta_y_hint = -gesture_event.DeltaYInRootFrame();
+  scroll_state_data->delta_granularity = gesture_event.DeltaUnits();
   scroll_state_data->is_beginning = true;
   scroll_state_data->from_user_input = true;
   scroll_state_data->is_direct_manipulation =

diff --git a/third_party/blink/web_tests/fast/scrolling/percentage-mousewheel-large-scroll-offset.html b/third_party/blink/web_tests/fast/scrolling/percentage-mousewheel-large-scroll-offset.html
@@ -0,0 +1,57 @@
+<!doctype html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../resources/gesture-util.js"></script>
+<style>
+#scroll-div {
+  width:400px; height:100px;
+  overflow: auto;
+  position:relative;
+}
+#range-div {
+  width:4194410px; height: 4194410px;
+  border:1px solid red;
+  position:absolute;
+  top: 10px;
+  left: 10px;
+}
+</style>
+<div id="scroll-div">
+  <div id="range-div"></div>
+</div>
+<script>
+  var scroller = document.querySelector("#scroll-div");
+  // Scroll div to some large offset that is > 0.5*4194410
+  const LARGE_SCROLL_OFFSET = 2097206;
+  scroller.scrollTop = LARGE_SCROLL_OFFSET;
+  scroller.scrollLeft = LARGE_SCROLL_OFFSET;
+  const SCROLL_PERCENTAGE = 0.1;
+  promise_test(async () => {
+    // Scroll the inner scroller by the SCROLL_PERCENTAGE.
+    var originalTopOffset = scroller.scrollTop;
+    var originalLeftOffset = scroller.scrollLeft;
+
+    // Use negative scroll percentages because we want to scroll up and left
+    await percentScroll(-SCROLL_PERCENTAGE,
+                        -SCROLL_PERCENTAGE,
+                        50,
+                        50,
+                        GestureSourceType.MOUSE_INPUT);
+
+    function isCorrectOffset() {
+      return scroller.scrollTop <=
+          Math.round(originalTopOffset - scroller.clientHeight * SCROLL_PERCENTAGE);
+    }
+    await waitFor(isCorrectOffset,
+                  "Scrolling by 10% must scroll the correct amount.");
+
+    assert_less_than(scroller.scrollTop, originalTopOffset,
+        "Must be able to scroll vertically by percentage even when at large offset");
+    assert_less_than(scroller.scrollLeft, originalLeftOffset,
+        "Must be able to scroll horizontally by percentage even when at large offset");
+
+
+  }, "Can scroll by percentage at large offset");
+
+
+</script>
diff --git a/third_party/blink/web_tests/fast/scrolling/percentage-mousewheel-scroll-on-iframe.html b/third_party/blink/web_tests/fast/scrolling/percentage-mousewheel-scroll-on-iframe.html
@@ -38,14 +38,8 @@
 
   const SCROLL_PERCENTAGE = 0.4;
   const WHEEL_DELTA_CONSTANT = 100;
-  await smoothScroll(SCROLL_PERCENTAGE, 200, 150,
-                     GestureSourceType.MOUSE_INPUT,
-                     'downright',
-                     SPEED_INSTANT /* speed_in_pixels_s */,
-                     false /* precise_scrolling_deltas */,
-                     false /* scroll_by_page */,
-                     true /* cursor_visible */,
-                     true /* scroll_by_percentage */);
+  await percentScroll(SCROLL_PERCENTAGE, SCROLL_PERCENTAGE, 200, 150,
+                     GestureSourceType.MOUSE_INPUT);
 
   function isCorrectXOffset() {
     return isNear(iframeElement.contentDocument.scrollingElement.scrollLeft,
@@ -68,13 +62,12 @@
                 "Wheel event deltaY must be a constant");
 
   // Scroll back to the top and ensure we scroll back to the origin and have
-  // negated wheel deltas
-  await smoothScroll(SCROLL_PERCENTAGE, 200, 150, GestureSourceType.MOUSE_INPUT,
-               'upleft', 100 /* speed_in_pixels_s */,
-               false /* precise_scrolling_deltas */,
-               false /* scroll_by_page */,
-               true /* cursor_visible */,
-               true /* scroll_by_percentage */);
+  // negated wheel deltas. Scrolling upleft so use negative pixels
+  await percentScroll(-SCROLL_PERCENTAGE,
+                      -SCROLL_PERCENTAGE,
+                      200,
+                      150,
+                      GestureSourceType.MOUSE_INPUT);
 
   await waitFor(() =>
       iframeElement.contentDocument.scrollingElement.scrollTop === 0,

diff --git a/third_party/blink/web_tests/fast/scrolling/percentage-mousewheel-scroll.html b/third_party/blink/web_tests/fast/scrolling/percentage-mousewheel-scroll.html
@@ -14,12 +14,7 @@
 const SCROLL_PERCENTAGE = 0.6;
 
 function scrollAt(x, y) {
-  return smoothScroll(SCROLL_PERCENTAGE, x, y, GestureSourceType.MOUSE_INPUT,
-                      'down', SPEED_INSTANT /* speed_in_pixels_s */,
-                      false /* precise_scrolling_deltas */,
-                      false /* scroll_by_page */, 
-                      true /* cursor_visible */,
-                      true /* scroll_by_percentage */);
+  return percentScroll(0, SCROLL_PERCENTAGE, x, y, GestureSourceType.MOUSE_INPUT);
 }
 
 async function test_scroller(scrollElement, x, y) {
@@ -29,14 +24,14 @@
   await scrollAt(x, y);
 
   function isCorrectOffset() {
-    return Math.abs(scrollElement.scrollTop - 
+    return Math.abs(scrollElement.scrollTop -
         scrollElement.clientHeight * SCROLL_PERCENTAGE) <= 0.5;
   }
   await waitFor(isCorrectOffset,
                 "Scrolling by 10% must scroll the correct amount.");
-  await conditionHolds(isCorrectOffset, 
+  await conditionHolds(isCorrectOffset,
                 "Scrolling 10% must not scroll past the correct ammount");
-  
+
   assert_equals(deltaY, WHEEL_DELTA,
                 "Wheel scrolling must report a constant delta value.");
 }

diff --git a/third_party/blink/web_tests/resources/gesture-util.js b/third_party/blink/web_tests/resources/gesture-util.js
@@ -198,6 +198,20 @@ function smoothScroll(pixels_to_scroll, start_x, start_y, gesture_source_type,
                             cursor_visible, scroll_by_percentage);
 }
 
+// Perform a percent based scroll using smoothScrollWithXY
+function percentScroll(percent_to_scroll_x, percent_to_scroll_y, start_x, start_y, gesture_source_type) {
+  return smoothScrollWithXY(percent_to_scroll_x, percent_to_scroll_y, start_x, start_y,
+    gesture_source_type,
+    undefined /* speed_in_pixels_s - not defined for percent based scrolls */,
+    false /* precise_scrolling_deltas */,
+    false /* scroll_by_page */,
+    true /* cursor_visible */,
+    true /* scroll_by_percentage */);
+}
+
+// modifier_keys means the keys pressed while doing the mouse wheel scroll, it
+// should be one of the values in the |Modifiers| or a comma separated string
+// to specify multiple values.
 function smoothScrollWithXY(pixels_to_scroll_x, pixels_to_scroll_y, start_x,
                             start_y, gesture_source_type, speed_in_pixels_s,
                             precise_scrolling_deltas, scroll_by_page,