Account for scroll origin in scroll animators.

Blink and cc have different notions of scroll offset - Blink's is relative to the scroll origin which is non-zero in RTL documents. The CompositorScrollOffsetAnimationCurve must work entirely in cc scroll offsets since it is handed over to cc which doesn't know about the scroll origin. This patch teaches ScrollAnimator and ProgrammaticScrollAnimator to convert in both directions when creating and using the curve object. BUG=581264 Review URL: https://codereview.chromium.org/1776503002 Cr-Commit-Position: refs/heads/master@{#379974}
huningxin · Mar 8, 2016 · 0bb1660 · 0bb1660
1 parent 4d90c49
commit 0bb1660
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 10 deletions.
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
@@ -890,6 +890,7 @@ crbug.com/364614 [ Mac ] virtual/threaded/fast/scroll-behavior/overflow-scroll-r
 crbug.com/364614 [ Mac ] virtual/scroll_customization/fast/scroll-behavior/overflow-scroll-root-frame-animates.html [ Skip ]
 
 crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/fixed-background-in-iframe.html [ Skip ]
+crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html [ Skip ]
 crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/main-thread-scrolling-reason-added.html [ Skip ]
 crbug.com/574283 [ Mac ] virtual/threaded/fast/scroll-behavior/smooth-scroll/main-thread-scrolling-reason-correctness.html [ Skip ]
 

diff --git a/.../threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl-expected.txt b/.../threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl-expected.txt
@@ -0,0 +1,11 @@
+This test verifies that both input-driven and programmatic smooth scrolls serviced by the compositor thread go to the correct scroll position on RTL pages with horizontal overflow.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scrollX became -80
+PASS scrollX became -40
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/.../virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html b/.../virtual/threaded/fast/scroll-behavior/smooth-scroll/horizontal-smooth-scroll-in-rtl.html
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<script src="../../../../../resources/js-test.js"></script>
+<style>
+
+html {
+    writing-mode: vertical-rl;
+    width: 1000px;
+}
+
+</style>
+<script>
+
+var jsTestIsAsync = true;
+
+description("This test verifies that both input-driven and programmatic " +
+    "smooth scrolls serviced by the compositor thread go to the correct " +
+    "scroll position on RTL pages with horizontal overflow.");
+
+onload = function() {
+    if (!window.eventSender) {
+        debug("This test requires window.eventSender.")
+        finishJSTest();
+        return;
+    }
+
+    // Start scrolled due to http://crbug.com/592799.
+    scrollBy(-120, 0);
+
+    // Click scrollbar stepper.
+    eventSender.mouseMoveTo(795, 595);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+
+    shouldBecomeEqual("scrollX", "-80", function() {
+        scrollBy({left: 40, behavior: "smooth"});
+        shouldBecomeEqual("scrollX", "-40", finishJSTest);
+    });
+};
+
+</script>
diff --git a/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp b/third_party/WebKit/Source/platform/scroll/ProgrammaticScrollAnimator.cpp
@@ -56,7 +56,7 @@ void ProgrammaticScrollAnimator::animateToOffset(FloatPoint offset)
     m_startTime = 0.0;
     m_targetOffset = offset;
     m_animationCurve = adoptPtr(CompositorFactory::current().createScrollOffsetAnimationCurve(
-        m_targetOffset,
+        compositorOffsetFromBlinkOffset(m_targetOffset),
         CompositorAnimationCurve::TimingFunctionTypeEaseInOut,
         CompositorScrollOffsetAnimationCurve::ScrollDurationDeltaBased));
 
@@ -83,7 +83,7 @@ void ProgrammaticScrollAnimator::tickAnimation(double monotonicTime)
         m_startTime = monotonicTime;
     double elapsedTime = monotonicTime - m_startTime;
     bool isFinished = (elapsedTime > m_animationCurve->duration());
-    FloatPoint offset = m_animationCurve->getValue(elapsedTime);
+    FloatPoint offset = blinkOffsetFromCompositorOffset(m_animationCurve->getValue(elapsedTime));
     notifyPositionChanged(IntPoint(offset.x(), offset.y()));
 
     if (isFinished) {
@@ -139,7 +139,8 @@ void ProgrammaticScrollAnimator::updateCompositorAnimations()
 
         if (!sentToCompositor) {
             m_runState = RunState::RunningOnMainThread;
-            m_animationCurve->setInitialValue(FloatPoint(m_scrollableArea->scrollPosition()));
+            m_animationCurve->setInitialValue(compositorOffsetFromBlinkOffset(
+                FloatPoint(m_scrollableArea->scrollPosition())));
             if (!m_scrollableArea->scheduleAnimation()) {
                 notifyPositionChanged(IntPoint(m_targetOffset.x(), m_targetOffset.y()));
                 resetAnimationState();
@@ -158,7 +159,8 @@ void ProgrammaticScrollAnimator::layerForCompositedScrollingDidChange(Compositor
         m_runState = RunState::RunningOnMainThread;
         m_compositorAnimationId = 0;
         m_compositorAnimationGroupId = 0;
-        m_animationCurve->setInitialValue(FloatPoint(m_scrollableArea->scrollPosition()));
+        m_animationCurve->setInitialValue(compositorOffsetFromBlinkOffset(
+            FloatPoint(m_scrollableArea->scrollPosition())));
         m_scrollableArea->registerForAnimation();
         if (!m_scrollableArea->scheduleAnimation()) {
             resetAnimationState();

diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp b/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp
@@ -149,7 +149,8 @@ bool ScrollAnimator::willAnimateToOffset(const FloatPoint& targetPos)
         // Running on the main thread, simply update the target offset instead
         // of sending to the compositor.
         if (m_runState == RunState::RunningOnMainThread) {
-            m_animationCurve->updateTarget(m_timeFunction() - m_startTime, targetPos);
+            m_animationCurve->updateTarget(m_timeFunction() - m_startTime,
+                compositorOffsetFromBlinkOffset(targetPos));
             return true;
         }
 
@@ -187,8 +188,9 @@ void ScrollAnimator::tickAnimation(double monotonicTime)
     double elapsedTime = monotonicTime - m_startTime;
 
     bool isFinished = (elapsedTime > m_animationCurve->duration());
-    FloatPoint offset = isFinished ? m_animationCurve->targetValue()
-        : m_animationCurve->getValue(elapsedTime);
+    FloatPoint offset = blinkOffsetFromCompositorOffset(isFinished
+        ? m_animationCurve->targetValue()
+        : m_animationCurve->getValue(elapsedTime));
 
     offset = FloatPoint(m_scrollableArea->clampScrollPosition(offset));
 
@@ -286,18 +288,18 @@ void ScrollAnimator::updateCompositorAnimations()
             m_compositorAnimationGroupId = 0;
 
             m_animationCurve->updateTarget(m_timeFunction() - m_startTime,
-                m_targetOffset);
+                compositorOffsetFromBlinkOffset(m_targetOffset));
             m_runState = RunState::WaitingToSendToCompositor;
         }
 
         if (!m_animationCurve) {
             m_animationCurve = adoptPtr(CompositorFactory::current().createScrollOffsetAnimationCurve(
-                m_targetOffset,
+                compositorOffsetFromBlinkOffset(m_targetOffset),
                 CompositorAnimationCurve::TimingFunctionTypeEaseInOut,
                 m_lastGranularity == ScrollByPixel ?
                     CompositorScrollOffsetAnimationCurve::ScrollDurationInverseDelta :
                     CompositorScrollOffsetAnimationCurve::ScrollDurationConstant));
-            m_animationCurve->setInitialValue(currentPosition());
+            m_animationCurve->setInitialValue(compositorOffsetFromBlinkOffset(currentPosition()));
         }
 
         bool runningOnMainThread = false;

diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.cpp
@@ -227,4 +227,16 @@ CompositorAnimationPlayer* ScrollAnimatorCompositorCoordinator::compositorPlayer
     return m_compositorPlayer.get();
 }
 
+FloatPoint ScrollAnimatorCompositorCoordinator::compositorOffsetFromBlinkOffset(FloatPoint offset)
+{
+    offset.moveBy(scrollableArea()->scrollOrigin());
+    return offset;
+}
+
+FloatPoint ScrollAnimatorCompositorCoordinator::blinkOffsetFromCompositorOffset(FloatPoint offset)
+{
+    offset.moveBy(-scrollableArea()->scrollOrigin());
+    return offset;
+}
+
 } // namespace blink
diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.h b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorCompositorCoordinator.h
@@ -10,6 +10,7 @@
 #include "platform/PlatformExport.h"
 #include "platform/animation/CompositorAnimationDelegate.h"
 #include "platform/animation/CompositorAnimationPlayerClient.h"
+#include "platform/geometry/FloatPoint.h"
 #include "platform/heap/Handle.h"
 #include "wtf/Allocator.h"
 #include "wtf/Noncopyable.h"
@@ -57,6 +58,9 @@ class PLATFORM_EXPORT ScrollAnimatorCompositorCoordinator : public NoBaseWillBeG
     void removeAnimation();
     virtual void abortAnimation();
 
+    FloatPoint compositorOffsetFromBlinkOffset(FloatPoint);
+    FloatPoint blinkOffsetFromCompositorOffset(FloatPoint);
+
     void compositorAnimationFinished(int groupId);
     // Returns true if the compositor player was attached to a new layer.
     bool reattachCompositorPlayerIfNeeded(CompositorAnimationTimeline*);