Fix premature termination of scroll animations while selecting text.

During text selection, AutoscrollController calls scrollIntoView continuously to make the scroll position follow the selection. This had the side effect of resetting ScrollAnimator before it could finish a main-thread smooth scroll. There are early returns for zero-delta scrolls in FrameView::setScrollPosition and RootFrameViewport::distributeScrollBetweenViewports, but scrollIntoView's conversion of fractional scroll offsets to LayoutUnits resulted in a very small (< 1px) non-zero scroll delta. We now avoid this by directly comparing the input and output of getRectToExpose and skipping the unneeded calls to setScrollPosition. BUG=591918 Review URL: https://codereview.chromium.org/1775213002 Cr-Commit-Position: refs/heads/master@{#380162}
huningxin · Mar 9, 2016 · 67a7b9d · 67a7b9d
1 parent 0a95746
commit 67a7b9d
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 8 deletions.
diff --git a/...ebKit/LayoutTests/fast/scroll-behavior/smooth-scroll/scroll-during-selection-expected.txt b/...ebKit/LayoutTests/fast/scroll-behavior/smooth-scroll/scroll-during-selection-expected.txt
@@ -0,0 +1,11 @@
+TEXT
+This test verifies that text selection does not prevent smooth scrolls running on the main thread.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scrollY became 40
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/..._party/WebKit/LayoutTests/fast/scroll-behavior/smooth-scroll/scroll-during-selection.html b/..._party/WebKit/LayoutTests/fast/scroll-behavior/smooth-scroll/scroll-during-selection.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<script src="../../../resources/js-test.js"></script>
+<style>
+body {
+    height: 1000px;
+}
+#fixed {
+    position:fixed;
+    right: 20px;
+    width: 100px;
+    height: 40px;
+    background-color: #ace;
+}
+#text {
+    font: bold 18pt monospace;
+}
+</style>
+<div id="fixed"></div>
+<div id="text">TEXT</div>
+<div id="console"></div>
+<script>
+
+var jsTestIsAsync = true;
+
+description(
+    "This test verifies that text selection does not prevent smooth " +
+    "scrolls running on the main thread.");
+
+eventSender.dragMode = false;
+
+// Start a text selection.
+eventSender.mouseMoveTo(20, 20);
+eventSender.mouseDown();
+eventSender.mouseMoveTo(40, 20);
+
+eventSender.mouseScrollBy(0, -1);
+shouldBecomeEqual("scrollY", "40", finishJSTest);
+
+</script>
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -3751,11 +3751,8 @@ LayoutRect FrameView::scrollIntoView(const LayoutRect& rectInContent, const Scro
 {
     LayoutRect viewRect(visibleContentRect());
     LayoutRect exposeRect = ScrollAlignment::getRectToExpose(viewRect, rectInContent, alignX, alignY);
-
-    double xOffset = exposeRect.x();
-    double yOffset = exposeRect.y();
-
-    setScrollPosition(DoublePoint(xOffset, yOffset), scrollType);
+    if (exposeRect != viewRect)
+        setScrollPosition(DoublePoint(exposeRect.x(), exposeRect.y()), scrollType);
 
     // Scrolling the FrameView cannot change the input rect's location relative to the document.
     return rectInContent;

diff --git a/third_party/WebKit/Source/core/frame/RootFrameViewport.cpp b/third_party/WebKit/Source/core/frame/RootFrameViewport.cpp
@@ -109,9 +109,8 @@ LayoutRect RootFrameViewport::scrollIntoView(const LayoutRect& rectInContent, co
     LayoutRect viewRectInContent = intersection(visualRectInContent, frameRectInContent);
     LayoutRect targetViewport =
         ScrollAlignment::getRectToExpose(viewRectInContent, rectInContent, alignX, alignY);
-    DoublePoint targetOffset(targetViewport.x(), targetViewport.y());
-
-    setScrollPosition(targetOffset, scrollType, ScrollBehaviorInstant);
+    if (targetViewport != viewRectInContent)
+        setScrollPosition(DoublePoint(targetViewport.x(), targetViewport.y()), scrollType);
 
     // RootFrameViewport only changes the viewport relative to the document so we can't change the input
     // rect's location relative to the document origin.