[Compositor threaded scrollbar scrolling] DSF fix.

This fixes an issue when scroll deltas for arrow clicks, shift+click
and autoscrolling was improperly scaled. The fix here is to use the
painted_device_scale_factor (when use-zoom-for-dsf is true). As a
part of this change, 2 VirtualTestSuites have also been introduced.
One of them runs the new tests with DSF=2 on the main thread and the
other one on the cc thread. New test had to be added because the scroll
offsets when DSF=2 (and use-zoom-for-dsf=true) are different compared to
offsets when DSF=1 (and use-zoom-for-dsf=true) both in case of main and
cc scrollbars in some cases (eg: shift+click).

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

cc/input/scrollbar_controller.cc

@@ -354,7 +354,6 @@ float ScrollbarController::GetScrollerToScrollbarRatio(
   // - scrollbar_thumb_length
   // - scroll_layer_length
   // - viewport_length
-  // - device_scale_factor
   // - position_in_widget
   //
   // When a thumb drag is in progress, for every pixel that the pointer moves,
@@ -364,6 +363,8 @@ float ScrollbarController::GetScrollerToScrollbarRatio(
   //  (scroll_layer_length - viewport_length) /
   //   (scrollbar_track_length - scrollbar_thumb_length)
   //
+  // PS: Note that since this is a "ratio", it need not be scaled by the DSF.
+  //
   // |<--------------------- scroll_layer_length -------------------------->|
   //
   // +------------------------------------------------+......................
@@ -403,17 +404,8 @@ float ScrollbarController::GetScrollerToScrollbarRatio(
           ? owner_scroll_layer->scroll_container_bounds().height()
           : (owner_scroll_layer->scroll_container_bounds().width());
 
-  // For platforms which have use_zoom_for_dsf set to false (like Mac), the
-  // device_scale_factor should not be used while determining the
-  // scaled_scroller_to_scrollbar_ratio as thumb drag would appear jittery due
-  // to constant over and under corrections.
-  // (See ScrollbarController::ScreenSpaceScaleFactor()).
-  float scaled_scroller_to_scrollbar_ratio =
-      ((scroll_layer_length - viewport_length) /
-       (scrollbar_track_length - scrollbar_thumb_length)) *
-      ScreenSpaceScaleFactor();
-
-  return scaled_scroller_to_scrollbar_ratio;
+  return ((scroll_layer_length - viewport_length) /
+          (scrollbar_track_length - scrollbar_thumb_length));
 }
 
 void ScrollbarController::ResetState() {
@@ -608,7 +600,7 @@ int ScrollbarController::GetScrollDeltaForScrollbarPart(
   switch (scrollbar_part) {
     case ScrollbarPart::BACK_BUTTON:
     case ScrollbarPart::FORWARD_BUTTON:
-      scroll_delta = kPixelsPerLineStep;
+      scroll_delta = kPixelsPerLineStep * ScreenSpaceScaleFactor();
       break;
     case ScrollbarPart::BACK_TRACK:
     case ScrollbarPart::FORWARD_TRACK:
@@ -629,7 +621,7 @@ int ScrollbarController::GetScrollDeltaForScrollbarPart(
       scroll_delta = 0;
   }
 
-  return scroll_delta * ScreenSpaceScaleFactor();
+  return scroll_delta;
 }
 
 float ScrollbarController::ScreenSpaceScaleFactor() const {
@@ -640,7 +632,8 @@ float ScrollbarController::ScreenSpaceScaleFactor() const {
   // on arrows would be incorrectly calculated as 80px instead of 40px. This is
   // also necessary to ensure that hit testing works as intended.
   return layer_tree_host_impl_->settings().use_zoom_for_dsf
-             ? layer_tree_host_impl_->active_tree()->device_scale_factor()
+             ? layer_tree_host_impl_->active_tree()
+                   ->painted_device_scale_factor()
              : 1.f;
 }
 

third_party/blink/web_tests/TestExpectations

@@ -2573,6 +2573,11 @@ crbug.com/953847 [ Mac ] virtual/compositor_threaded_scrollbar_scrolling/fast/sc
 crbug.com/1009892 [ Win ] virtual/compositor_threaded_scrollbar_scrolling_hidpi_nozoomfordsf/fast/scrolling/scrollbars/mouse-scrolling-on-div-scrollbar.html [ Failure ]
 crbug.com/1009892 [ Win ] virtual/compositor_threaded_scrollbar_scrolling_hidpi_nozoomfordsf/fast/scrolling/scrollbars/scrollbar-thumb-snapping.html [ Failure ]
 
+# TODO(arakeri): Mac is the only platform that runs with use_zoom_for_dsf=false. Get rid of this suite when crbug.com/716231 is fixed.
+# When DSF = 2 and use_zoom_for_dsf=false, the scroll offsets are different when compared to DSF = 2 and use_zoom_for_dsf=true.
+crbug.com/1016186 [ Win ] virtual/compositor_threaded_scrollbar_scrolling_hidpi_nozoomfordsf/fast/scrolling/scrollbars/dsf-ready/mouse-interactions-dsf-2.html [ Failure Timeout ]
+crbug.com/1016186 [ Linux ] virtual/compositor_threaded_scrollbar_scrolling_hidpi_nozoomfordsf/fast/scrolling/scrollbars/dsf-ready/mouse-interactions-dsf-2.html [ Failure Timeout ]
+
 # Some control characters still not visible
 crbug.com/893490 [ Mac ] external/wpt/css/css-text/white-space/control-chars-001.html [ Failure ]
 crbug.com/893490 [ Mac ] external/wpt/css/css-text/white-space/control-chars-002.html [ Failure ]

third_party/blink/web_tests/VirtualTestSuites

@@ -524,6 +524,21 @@
              "--enable-prefer-compositing-to-lcd-text",
              "--disable-smooth-scrolling"]
   },
+  {
+    "prefix": "hidpi",
+    "base": "fast/scrolling/scrollbars/dsf-ready",
+    "args": ["--disable-smooth-scrolling",
+             "--force-device-scale-factor=2"]
+  },
+  {
+    "prefix": "compositor_threaded_scrollbar_scrolling_hidpi",
+    "base": "fast/scrolling/scrollbars/dsf-ready",
+    "args": ["--enable-features=CompositorThreadedScrollbarScrolling",
+             "--enable-threaded-compositing",
+             "--enable-prefer-compositing-to-lcd-text",
+             "--disable-smooth-scrolling",
+             "--force-device-scale-factor=2"]
+  },
   {
     "prefix": "compositor_threaded_scrollbar_scrolling_hidpi_nozoomfordsf",
     "base": "fast/scrolling/scrollbars",

third_party/blink/web_tests/fast/scrolling/scrollbars/dsf-ready/mouse-interactions-dsf-2.html

+<!DOCTYPE html>
+<title>Tests mouse interactions when DSF=2 and --enable-use-zoom-for-dsf=true.</title>
+<script src="../../../../resources/testharness.js"></script>
+<script src="../../../../resources/testharnessreport.js"></script>
+<script src="../../../../resources/gesture-util.js"></script>
+<script src="../../../../resources/scrollbar-util.js"></script>
+<style>
+.appearance {
+  width: 100px;
+  height: 100px;
+  overflow: scroll;
+  border: 1px solid black;
+}
+.standardLocation {
+  position: absolute;
+  top: 100px;
+  left: 100px;
+}
+.customLocation {
+  position: absolute;
+  top: 250px;
+  left: 100px;
+}
+
+.space {
+  height: 1000px;
+  width: 1000px;
+}
+</style>
+
+<!-- Composited non-custom fast scroller -->
+<div id="standard" class="appearance standardLocation">
+  <div class="space"></div>
+</div>
+
+<script>
+if (window.internals)
+    internals.settings.setScrollAnimatorEnabled(false);
+
+window.onload = () => {
+  // http://crrev.com/c/1856359 disables mock theme and makes tests use the
+  // native theme. Hence, values will differ between platforms for both, main
+  // and compositor thread scrolling.
+  const onLinuxPlatform = navigator.userAgent.search(/\bLinux\b/) != -1;
+  const onMacPlatform =  navigator.userAgent.search(/\bMac OS X\b/) != -1;
+
+  const standardDivFast = document.getElementById("standard");
+  const standardRectFast = standardDivFast.getBoundingClientRect();
+
+  const TRACK_WIDTH = calculateScrollbarThickness();
+  const BUTTON_WIDTH = TRACK_WIDTH;
+  const SCROLL_CORNER = TRACK_WIDTH;
+  const SCROLLBAR_BUTTON_FWD = {
+    x: standardRectFast.right - BUTTON_WIDTH / 2,
+    y: standardRectFast.bottom - SCROLL_CORNER - BUTTON_WIDTH / 2
+  }
+  const SCROLLBAR_THUMB = {
+    x: standardRectFast.right - TRACK_WIDTH / 2,
+    y: standardRectFast.top + BUTTON_WIDTH + 5
+  }
+
+  promise_test (async () => {
+    resetScrollOffset(standardDivFast);
+
+    // Testing the vertical scrollbar thumb.
+    await mouseDragAndDrop(SCROLLBAR_THUMB.x, SCROLLBAR_THUMB.y, SCROLLBAR_THUMB.x, SCROLLBAR_THUMB.y + 20);
+    let expected_offset = 0;
+
+    // TODO(arakeri): crbug.com/1016188 has been filed to investigate the differences
+    // in scroll offsets between platforms.
+    if (onLinuxPlatform)
+      expected_offset = window.devicePixelRatio == 1 ? 732 : 457.5;
+    else if (onMacPlatform)
+      expected_offset = 281;
+    else
+      expected_offset = window.devicePixelRatio == 1 ? 481 : 362;
+    assert_equals(standardDivFast.scrollTop, expected_offset, "Vertical thumb drag downwards did not scroll as expected.");
+  }, "Test mouse drags on non-custom composited div scrollbar thumb.");
+
+  promise_test (async (test) => {
+    // Since scrollbars on Mac don't have buttons, this test is irrelevant.
+    if(onMacPlatform)
+      test.done();
+
+    resetScrollOffset(standardDivFast);
+
+    // Click on the Down arrow for standardRectFast.
+    await mouseClickOn(SCROLLBAR_BUTTON_FWD.x, SCROLLBAR_BUTTON_FWD.y);
+    assert_equals(standardDivFast.scrollTop, 40, "Pressing the down arrow didn't scroll.");
+  }, "Test mouse click on non-custom composited div scrollbar arrows.");
+
+  promise_test (async (test) => {
+    // TODO (arakeri): Track clicks on Mac don't work yet (crbug.com/1016452).
+    if(onMacPlatform)
+      test.done();
+
+    resetScrollOffset(standardDivFast);
+
+    // Click on the track part just above the down arrow.
+    await mouseClickOn(SCROLLBAR_BUTTON_FWD.x, SCROLLBAR_BUTTON_FWD.y - 10);
+    assert_equals(standardDivFast.scrollTop, 74, "Pressing the down trackpart didn't scroll.");
+  }, "Test mouse click on non-custom composited div scrollbar empty trackparts.");
+
+  promise_test (async (test) => {
+    // TODO (arakeri): Track clicks on Mac don't work yet (crbug.com/1016452).
+    if(onMacPlatform)
+      test.done();
+
+    resetScrollOffset(standardDivFast);
+
+    // Testing forward scroll on vertical scrollbar.
+    await mouseClickOn(SCROLLBAR_BUTTON_FWD.x, standardRectFast.top + 50, /*left_click*/0, /*modifier*/ "Shift");
+    let expected_offset = 0;
+    if (onLinuxPlatform)
+      expected_offset = window.devicePixelRatio == 1 ? 695 : 606;
+    else
+      expected_offset = window.devicePixelRatio == 1 ? 626 : 579.5;
+    assert_equals(standardDivFast.scrollTop, expected_offset, "Shift + click forward didn't scroll.");
+  }, "Test shift + click on non-custom composited scrollbars.");
+}
+</script>

third_party/blink/web_tests/virtual/compositor_threaded_scrollbar_scrolling_hidpi/README.md

+This directory is dedicated for testing scrollbar interactions on the
+compositor thread with device_scale_factor set to 2.

third_party/blink/web_tests/virtual/hidpi/README.md

+This directory is dedicated for testing scrollbar interactions on the
+main thread with device_scale_factor set to 2.