Snaps after scrollbar clicking.

After user clicking on the scrollbar's track or arrows, we should snap to the closest snap position of the container. This is done by simply moving SnapAfterScrollbarDragging() from ScrollableArea::MouseReleasedScrollbar() up to Scrollbar::MouseUp(). This patch also rewrites the previous snaps-after-scrollbar-dragging.html test using the gesture-util.js, and adds the new clicking tests to it as well. Bug: 860768 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I75267c3fc0917ecf5c14673a1eb480a047d03824 Reviewed-on: https://chromium-review.googlesource.com/1127153 Commit-Queue: Sandra Sun <[email protected]> Reviewed-by: David Bokan <[email protected]> Reviewed-by: Majid Valipour <[email protected]> Cr-Commit-Position: refs/heads/master@{#575191}
feiyue1206 · Jul 16, 2018 · e98df9c · e98df9c
1 parent 73f3b43
commit e98df9c
Show file tree

Hide file tree

Showing 10 changed files with 129 additions and 121 deletions.
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 b/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
@@ -860,6 +860,7 @@ Bug(none) fast/forms/suggestion-picker/week-suggestion-picker-mouse-operations.h
 
 Bug(none) fast/scrolling/absolute-position-behind-scrollbar.html [ Failure ]
 Bug(none) fast/scrolling/fixed-position-behind-scrollbar.html [ Failure Timeout ]
+Bug(none) fast/scroll-snap/snaps-after-scrollbar-scrolling.html [ Failure ]
 Bug(none) fast/events/remove-child-onscroll.html [ Timeout ]
 Bug(none) fast/events/mouse-wheel-main-frame-scroll.html [ Timeout ]
 Bug(none) fast/scrolling/hover-during-scroll.html [ Timeout ]

diff --git a/third_party/WebKit/LayoutTests/fast/scroll-behavior/snaps-after-scrollbar-dragging.html b/third_party/WebKit/LayoutTests/fast/scroll-behavior/snaps-after-scrollbar-dragging.html
diff --git a/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-scrollbar-scrolling.html b/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-scrollbar-scrolling.html
@@ -0,0 +1,99 @@
+<!DOCTYPE html>
+<link rel="help" href="https://drafts.csswg.org/css-scroll-snap-1" />
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../resources/gesture-util.js"></script>
+<style>
+body {
+  margin: 0px;
+}
+div {
+  position: absolute;
+}
+#scroller {
+  width: 400px;
+  height: 400px;
+  overflow: scroll;
+  scroll-snap-type: both mandatory;
+  padding: 0px;
+}
+.snap {
+  width: 200px;
+  height: 200px;
+  background-color: blue;
+  scroll-snap-align: start;
+}
+#space {
+  width: 1000px;
+  height: 1000px;
+}
+#left-top {
+  left: 0px;
+  top: 0px;
+}
+#right-top {
+  left: 400px;
+  top: 0px;
+}
+#left-bottom {
+  left: 0px;
+  top: 400px;
+}
+
+</style>
+
+<div id='scroller'>
+  <div id="space"></div>
+  <div class="snap" id="left-top"></div>
+  <div class="snap" id="right-top"></div>
+  <div class="snap" id="left-bottom"></div>
+</div>
+
+<script>
+var scroller = document.getElementById("scroller");
+
+function scrollLeft() {
+  return scroller.scrollLeft;
+}
+
+function scrollTop() {
+  return scroller.scrollTop;
+}
+
+promise_test (async () => {
+  scroller.scrollTo(0, 0);
+  await mouseDragAndDrop(398, 20, 398, 120);
+  await waitForAnimationEnd(scrollTop, 500, 5);
+  await waitFor( () => {
+    return scroller.scrollTop == 400;
+  });
+}, "Snaps after dragging the vertical scrollbar.");
+
+promise_test (async () => {
+  scroller.scrollTo(0, 0);
+  await mouseDragAndDrop(20, 398, 120, 398);
+  await waitForAnimationEnd(scrollLeft, 500, 5);
+  await waitFor( () => {
+    return scroller.scrollLeft == 400;
+  });
+}, "Snaps after dragging the horizontal scrollbar.");
+
+promise_test (async () => {
+  scroller.scrollTo(0, 0);
+  await mousePressOn(398, 350, 1);
+  await waitForAnimationEnd(scrollTop, 500, 5);
+  await waitFor( () => {
+    return scroller.scrollTop == 400;
+  });
+}, "Snaps after clicking the vertical scrollbar.");
+
+promise_test (async () => {
+  scroller.scrollTo(0, 0);
+  await mousePressOn(350, 398, 1);
+  await waitForAnimationEnd(scrollLeft, 500, 5);
+  await waitFor( () => {
+    return scroller.scrollLeft == 400;
+  });
+}, "Snaps after clicking the horizontal scrollbar.");
+
+</script>
diff --git a/third_party/WebKit/LayoutTests/resources/gesture-util.js b/third_party/WebKit/LayoutTests/resources/gesture-util.js
@@ -195,6 +195,26 @@ function mouseClickOn(x, y) {
   });
 }
 
+// Simulate a mouse press on point for a certain time.
+function mousePressOn(x, y, t) {
+  return new Promise((resolve, reject) => {
+    if (chrome && chrome.gpuBenchmarking) {
+      let pointerActions = [{
+        source: 'mouse',
+        actions: [
+          { 'name': 'pointerMove', 'x': x, 'y': y },
+          { 'name': 'pointerDown', 'x': x, 'y': y },
+          { 'name': 'pause', duration: t},
+          { 'name': 'pointerUp' },
+        ]
+      }];
+      chrome.gpuBenchmarking.pointerActionSequence(pointerActions, resolve);
+    } else {
+      reject('This test requires chrome.gpuBenchmarking');
+    }
+  });
+}
+
 // Simulate a mouse drag and drop. mouse down at {start_x, start_y}, move to
 // {end_x, end_y} and release.
 function mouseDragAndDrop(start_x, start_y, end_x, end_y, button = 'left') {

diff --git a/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc b/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
@@ -270,6 +270,8 @@ void SnapCoordinator::PerformSnapping(const LayoutBox& snap_container,
   if (!snap_point.has_value())
     return;
 
+  scrollable_area->CancelScrollAnimation();
+  scrollable_area->CancelProgrammaticScrollAnimation();
   if (snap_point.value() != current_position) {
     scrollable_area->SetScrollOffset(
         scrollable_area->ScrollPositionToOffset(snap_point.value()),

diff --git a/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc b/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
@@ -1621,7 +1621,7 @@ int PaintLayerScrollableArea::HorizontalScrollbarHeight(
   return HorizontalScrollbar()->ScrollbarThickness();
 }
 
-void PaintLayerScrollableArea::SnapAfterScrollbarDragging(
+void PaintLayerScrollableArea::SnapAfterScrollbarScrolling(
     ScrollbarOrientation orientation) {
   SnapCoordinator* snap_coordinator =
       GetLayoutBox()->GetDocument().GetSnapCoordinator();

diff --git a/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h b/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h
@@ -583,7 +583,7 @@ class CORE_EXPORT PaintLayerScrollableArea final
   bool SetHasHorizontalScrollbar(bool has_scrollbar);
   bool SetHasVerticalScrollbar(bool has_scrollbar);
 
-  void SnapAfterScrollbarDragging(ScrollbarOrientation) override;
+  void SnapAfterScrollbarScrolling(ScrollbarOrientation) override;
 
   void UpdateScrollCornerStyle();
   LayoutSize MinimumSizeForResizing(float zoom_factor);

diff --git a/third_party/blink/renderer/platform/scroll/scrollable_area.cc b/third_party/blink/renderer/platform/scroll/scrollable_area.cc
@@ -390,11 +390,10 @@ void ScrollableArea::MouseCapturedScrollbar() {
     fade_overlay_scrollbars_timer_->Stop();
 }
 
-void ScrollableArea::MouseReleasedScrollbar(ScrollbarOrientation orientation) {
+void ScrollableArea::MouseReleasedScrollbar() {
   scrollbar_captured_ = false;
   // This will kick off the fade out timer.
   ShowOverlayScrollbars();
-  SnapAfterScrollbarDragging(orientation);
 }
 
 void ScrollableArea::ContentAreaDidShow() const {

diff --git a/third_party/blink/renderer/platform/scroll/scrollable_area.h b/third_party/blink/renderer/platform/scroll/scrollable_area.h
@@ -113,11 +113,11 @@ class PLATFORM_EXPORT ScrollableArea : public GarbageCollectedMixin {
   void MouseEnteredScrollbar(Scrollbar&);
   void MouseExitedScrollbar(Scrollbar&);
   void MouseCapturedScrollbar();
-  void MouseReleasedScrollbar(ScrollbarOrientation);
+  void MouseReleasedScrollbar();
   void ContentAreaDidShow() const;
   void ContentAreaDidHide() const;
 
-  virtual void SnapAfterScrollbarDragging(ScrollbarOrientation) {}
+  virtual void SnapAfterScrollbarScrolling(ScrollbarOrientation) {}
 
   void FinishCurrentScrollAnimations() const;
 

diff --git a/third_party/blink/renderer/platform/scroll/scrollbar.cc b/third_party/blink/renderer/platform/scroll/scrollbar.cc
@@ -484,7 +484,8 @@ void Scrollbar::MouseUp(const WebMouseEvent& mouse_event) {
 
   if (scrollable_area_) {
     if (is_captured)
-      scrollable_area_->MouseReleasedScrollbar(orientation_);
+      scrollable_area_->MouseReleasedScrollbar();
+    scrollable_area_->SnapAfterScrollbarScrolling(orientation_);
 
     ScrollbarPart part = GetTheme().HitTest(
         *this, FlooredIntPoint(mouse_event.PositionInRootFrame()));