Snap after pressing PgUp/PgDn and Home/End

This patch implements snapping after pressing PgUp/PgDn and Home/End
keys. The logic is implemented in ScrollManager::LogicalScroll().
These keys are different from arrow keys. Pressing the arrow key is
considered as scrolling with intended direction only. Pressing the
PgUp/PgDn is considered as scrolling with intended direction and end
position. Pressing the Home/End key is considered as scrolling with
intended end position only. So we need to use different strategies to
snap for these keys.

Bug: 821928
Change-Id: If4d04ff58214a3304d09838f321e65c6e68f7d1d
Reviewed-on: https://chromium-review.googlesource.com/c/1361648Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616062}

third_party/blink/renderer/core/input/scroll_manager.cc

@@ -280,8 +280,35 @@ bool ScrollManager::LogicalScroll(ScrollDirection direction,
     ScrollOffset delta = ToScrollDelta(physical_direction, 1);
     delta.Scale(scrollable_area->ScrollStep(granularity, kHorizontalScrollbar),
                 scrollable_area->ScrollStep(granularity, kVerticalScrollbar));
-    if (snap_coordinator->SnapForDirection(*box, delta))
-      return true;
+    // Pressing the arrow key is considered as a scroll with intended direction
+    // only. Pressing the PgUp/PgDn key is considered as a scroll with intended
+    // direction and end position. Pressing the Home/End key is considered as a
+    // scroll with intended end position only.
+    switch (granularity) {
+      case kScrollByLine: {
+        if (snap_coordinator->SnapForDirection(*box, delta))
+          return true;
+        break;
+      }
+      case kScrollByPage: {
+        if (snap_coordinator->SnapForEndAndDirection(*box, delta))
+          return true;
+        break;
+      }
+      case kScrollByDocument: {
+        FloatPoint end_position = scrollable_area->ScrollPosition() + delta;
+        bool scrolled_x = physical_direction == kScrollLeft ||
+                          physical_direction == kScrollRight;
+        bool scrolled_y = physical_direction == kScrollUp ||
+                          physical_direction == kScrollDown;
+        if (snap_coordinator->SnapForEndPosition(*box, end_position, scrolled_x,
+                                                 scrolled_y))
+          return true;
+        break;
+      }
+      default:
+        NOTREACHED();
+    }
 
     ScrollResult result = scrollable_area->UserScroll(
         granularity, ToScrollDelta(physical_direction, 1));
@@ -656,9 +683,9 @@ void ScrollManager::SnapAtGestureScrollEnd() {
   if (!snap_coordinator || !layout_box)
     return;
 
-  snap_coordinator->SnapForEndPosition(*layout_box,
-                                       did_scroll_x_for_scroll_gesture_,
-                                       did_scroll_y_for_scroll_gesture_);
+  snap_coordinator->SnapAtCurrentPosition(*layout_box,
+                                          did_scroll_x_for_scroll_gesture_,
+                                          did_scroll_y_for_scroll_gesture_);
 }
 
 bool ScrollManager::GetSnapFlingInfo(

third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc

@@ -255,16 +255,24 @@ base::Optional<FloatPoint> SnapCoordinator::GetSnapPosition(
   return base::nullopt;
 }
 
-bool SnapCoordinator::SnapForEndPosition(const LayoutBox& snap_container,
-                                         bool scrolled_x,
-                                         bool scrolled_y) const {
+bool SnapCoordinator::SnapAtCurrentPosition(const LayoutBox& snap_container,
+                                            bool scrolled_x,
+                                            bool scrolled_y) const {
   ScrollableArea* scrollable_area = ScrollableAreaForSnapping(snap_container);
   if (!scrollable_area)
     return false;
   FloatPoint current_position = scrollable_area->ScrollPosition();
+  return SnapForEndPosition(snap_container, current_position, scrolled_x,
+                            scrolled_y);
+}
+
+bool SnapCoordinator::SnapForEndPosition(const LayoutBox& snap_container,
+                                         const FloatPoint& end_position,
+                                         bool scrolled_x,
+                                         bool scrolled_y) const {
   std::unique_ptr<SnapSelectionStrategy> strategy =
       SnapSelectionStrategy::CreateForEndPosition(
-          gfx::ScrollOffset(current_position), scrolled_x, scrolled_y);
+          gfx::ScrollOffset(end_position), scrolled_x, scrolled_y);
   return PerformSnapping(snap_container, *strategy);
 }
 
@@ -281,6 +289,19 @@ bool SnapCoordinator::SnapForDirection(const LayoutBox& snap_container,
   return PerformSnapping(snap_container, *strategy);
 }
 
+bool SnapCoordinator::SnapForEndAndDirection(const LayoutBox& snap_container,
+                                             const ScrollOffset& delta) const {
+  ScrollableArea* scrollable_area = ScrollableAreaForSnapping(snap_container);
+  if (!scrollable_area)
+    return false;
+  FloatPoint current_position = scrollable_area->ScrollPosition();
+  std::unique_ptr<SnapSelectionStrategy> strategy =
+      SnapSelectionStrategy::CreateForEndAndDirection(
+          gfx::ScrollOffset(current_position),
+          gfx::ScrollOffset(delta.Width(), delta.Height()));
+  return PerformSnapping(snap_container, *strategy);
+}
+
 bool SnapCoordinator::PerformSnapping(
     const LayoutBox& snap_container,
     const SnapSelectionStrategy& strategy) const {
@@ -295,7 +316,7 @@ bool SnapCoordinator::PerformSnapping(
 
   scrollable_area->CancelScrollAnimation();
   scrollable_area->CancelProgrammaticScrollAnimation();
-  if (gfx::ScrollOffset(snap_point.value()) != strategy.current_position()) {
+  if (snap_point.value() != scrollable_area->ScrollPosition()) {
     scrollable_area->SetScrollOffset(
         scrollable_area->ScrollPositionToOffset(snap_point.value()),
         kProgrammaticScroll, kScrollBehaviorSmooth);

third_party/blink/renderer/core/page/scrolling/snap_coordinator.h

@@ -53,13 +53,22 @@ class CORE_EXPORT SnapCoordinator final
   void UpdateAllSnapContainerData();
   void UpdateSnapContainerData(const LayoutBox&);
 
-  // SnapForEndPosition() and SnapForDirection() return true if snapping was
-  // performed, and false otherwise.
+  // SnapAtCurrentPosition(), SnapForEndPosition(), SnapForDirection(), and
+  // SnapForEndAndDirection() return true if snapping was performed, and false
+  // otherwise.
+  // SnapAtCurrentPosition() calls SnapForEndPosition() with the current scroll
+  // position.
+  bool SnapAtCurrentPosition(const LayoutBox& snap_container,
+                             bool scrolled_x,
+                             bool scrolled_y) const;
   bool SnapForEndPosition(const LayoutBox& snap_container,
+                          const FloatPoint& end_position,
                           bool scrolled_x,
                           bool scrolled_y) const;
   bool SnapForDirection(const LayoutBox& snap_container,
                         const ScrollOffset& delta) const;
+  bool SnapForEndAndDirection(const LayoutBox& snap_container,
+                              const ScrollOffset& delta) const;
 
   base::Optional<FloatPoint> GetSnapPosition(
       const LayoutBox& snap_container,

third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc

@@ -1626,9 +1626,9 @@ void PaintLayerScrollableArea::SnapAfterScrollbarScrolling(
   if (!snap_coordinator)
     return;
 
-  snap_coordinator->SnapForEndPosition(*GetLayoutBox(),
-                                       orientation == kHorizontalScrollbar,
-                                       orientation == kVerticalScrollbar);
+  snap_coordinator->SnapAtCurrentPosition(*GetLayoutBox(),
+                                          orientation == kHorizontalScrollbar,
+                                          orientation == kVerticalScrollbar);
 }
 
 void PaintLayerScrollableArea::PositionOverflowControls() {

third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint

@@ -346,10 +346,11 @@ Bug(none) fast/forms/suggestion-picker/month-suggestion-picker-mouse-operations.
 Bug(none) fast/forms/suggestion-picker/time-suggestion-picker-mouse-operations.html [ Failure ]
 Bug(none) fast/forms/suggestion-picker/week-suggestion-picker-mouse-operations.html [ Failure ]
 
-Bug(none) fast/scroll-snap/snaps-after-scrollbar-scrolling.html [ Failure ]
-Bug(none) fast/scroll-snap/snaps-after-keyboard-scrolling.html [ Failure ]
-Bug(none) fast/scroll-snap/snaps-after-keyboard-scrolling-rtl.html [ Failure ]
-Bug(none) fast/scroll-snap/snap-scrolls-visual-viewport.html [ Failure ]
+crbug.com/913955 fast/scroll-snap/snaps-after-scrollbar-scrolling.html [ Failure ]
+crbug.com/913955 fast/scroll-snap/snaps-after-keyboard-scrolling.html [ Failure ]
+crbug.com/913955 fast/scroll-snap/snaps-after-keyboard-scrolling-rtl.html [ Failure ]
+crbug.com/913955 fast/scroll-snap/snaps-for-different-key-granularity.html [ Failure ]
+crbug.com/913955 fast/scroll-snap/snap-scrolls-visual-viewport.html [ Failure ]
 Bug(none) fast/events/remove-child-onscroll.html [ Timeout ]
 
 # Text failures due to layerization differences

third_party/blink/web_tests/fast/scroll-snap/snaps-for-different-key-granularity.html

+<!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>
+:root {
+  --page-size: 600px;
+  --line-size: 200px;
+}
+
+div {
+  position: absolute;
+}
+
+#scroller {
+  width: 600px;
+  height: var(--page-size);
+  overflow: scroll;
+  scroll-snap-type: both mandatory;
+}
+#space {
+  width: 2000px;
+  height: 2000px;
+}
+.snap {
+  width: 100px;
+  height: 100px;
+  background-color: blue;
+  scroll-snap-align: start;
+}
+</style>
+
+<div id="scroller">
+  <div id="space"></div>
+  <div class="snap" id="page1" style="left: 0px; top: 0px;"></div>
+  <div class="snap" id="page1-line1" style="left: 0px; top: var(--line-size);"></div>
+  <div class="snap" id="page2" style="left: 0px; top: var(--page-size);"></div>
+  <div class="snap" id="page2-line1" style="left: 0px; top: calc(var(--page-size) + var(--line-size));"></div>
+  <div class="snap" id="page2-line2" style="left: 0px; top: calc(2 * var(--page-size) - var(--line-size));"></div>
+  <div class="snap" id="page3" style="left: 0px; top: calc(2 * var(--page-size));"></div>
+</div>
+
+<script>
+var scroller = document.getElementById("scroller");
+
+function scrollLeft() {
+  return scroller.scrollLeft;
+}
+
+function scrollTop() {
+  return scroller.scrollTop;
+}
+
+function keyPress(key) {
+  return new Promise((resolve, reject) => {
+    if (window.eventSender) {
+      eventSender.keyDown(key);
+      resolve();
+    }
+    else {
+      reject('This test requires window.eventSender');
+    }
+  })
+}
+
+promise_test (async () => {
+  await mouseClickOn(10, 10);
+  scroller.scrollTo(0, 0);
+  await keyPress("ArrowDown");
+  await waitForAnimationEnd(scrollTop, 500, 15);
+  assert_equals(scroller.scrollTop, 200);
+}, "Snaps to page1-line1 after pressing ArrowDown at page1.");
+
+promise_test (async () => {
+  await mouseClickOn(10, 10);
+  scroller.scrollTo(0, 1200);
+  await keyPress("ArrowUp");
+  await waitForAnimationEnd(scrollTop, 500, 15);
+  assert_equals(scroller.scrollTop, 1000);
+}, "Snaps to page2-line2 after pressing ArrowUp at page3.");
+
+promise_test (async () => {
+  await mouseClickOn(10, 10);
+  scroller.scrollTo(0, 0);
+  await keyPress("PageDown");
+  await waitForAnimationEnd(scrollTop, 500, 15);
+  assert_equals(scroller.scrollTop, 600);
+}, "Snaps to page2 after pressing PageDown at page1.");
+
+promise_test (async () => {
+  await mouseClickOn(10, 10);
+  scroller.scrollTo(0, 1200);
+  await keyPress("PageUp");
+  await waitForAnimationEnd(scrollTop, 500, 15);
+  assert_equals(scroller.scrollTop, 600);
+}, "Snaps to page2 after pressing PageUp at page3.");
+
+promise_test (async () => {
+  await mouseClickOn(10, 10);
+  scroller.scrollTo(0, 0);
+  await keyPress("End");
+  await waitForAnimationEnd(scrollTop, 500, 15);
+  assert_equals(scroller.scrollTop, 1200);
+}, "Snaps to page3 after pressing End at page1.");
+
+promise_test (async () => {
+  await mouseClickOn(10, 10);
+  scroller.scrollTo(0, 1200);
+  await keyPress("Home");
+  await waitForAnimationEnd(scrollTop, 500, 15);
+  assert_equals(scroller.scrollTop, 0);
+}, "Snaps to page1 after pressing Home at page3.");
+</script>
\ No newline at end of file