Preserve fractional scroll offset for JS scrolling API

Source/core/rendering/RenderLayerScrollableArea.cpp

Index: Source/core/rendering/RenderLayerScrollableArea.cpp
diff --git a/Source/core/rendering/RenderLayerScrollableArea.cpp b/Source/core/rendering/RenderLayerScrollableArea.cpp
index 806fdaf31e92388deb1fa0b23ddf41b13abfe004..4dc43c38ed52df643407b83dfd76ef66c702089e 100644
--- a/Source/core/rendering/RenderLayerScrollableArea.cpp
+++ b/Source/core/rendering/RenderLayerScrollableArea.cpp
@@ -126,7 +126,7 @@ RenderLayerScrollableArea::~RenderLayerScrollableArea()
     if (!box().documentBeingDestroyed()) {
         Node* node = box().node();
         if (node && node->isElementNode())
-            toElement(node)->setSavedLayerScrollOffset(m_scrollOffset);
+            toElement(node)->setSavedLayerScrollOffset(flooredIntSize(m_scrollOffset));

[Rick Byers, 2014/10/02 21:44:06]

What's the implication of flooring here?  Do we need a FIXME?

[Yufeng Shen (Slow to review), 2014/10/02 23:34:34]

No documentation about what this scrolloffet save/restore is about.
I will just add a FIXME here.

     }
 
     if (LocalFrame* frame = box().frame()) {
@@ -344,16 +344,21 @@ int RenderLayerScrollableArea::scrollSize(ScrollbarOrientation orientation) cons
 
 void RenderLayerScrollableArea::setScrollOffset(const IntPoint& newScrollOffset)
 {
+    setScrollOffset(DoublePoint(newScrollOffset));
+}
+
+void RenderLayerScrollableArea::setScrollOffset(const DoublePoint& newScrollOffset)

[Rick Byers, 2014/10/02 21:44:06]

Didn't you guys agree that you'd drive to move towards using sizes for "offsets"
and points only for "positions"?  I.e. the callers should be converting to
DoubleSize here if necessary, not the implementation of this function, right?

[Yufeng Shen (Slow to review), 2014/10/02 23:34:34]

Ideally yes. Just that ScrollableArea::setScrollOffset is overriden by so many
subclasses that changing its signature and calling sites will touch too many
files. I added a FIXME in  ScrollableArea::setScrollOffset to make the
conversion
in a separate CL.

[Rick Byers, 2014/10/03 16:42:46]

Sounds good, thanks.

+{
     if (!box().isMarquee()) {
         // Ensure that the dimensions will be computed if they need to be (for overflow:hidden blocks).
         if (m_scrollDimensionsDirty)
             computeScrollDimensions();
     }
 
-    if (scrollOffset() == toIntSize(newScrollOffset))
+    if (scrollOffset() == toDoubleSize(newScrollOffset))
         return;
 
-    m_scrollOffset = toIntSize(newScrollOffset);
+    m_scrollOffset = toDoubleSize(newScrollOffset);
 
     LocalFrame* frame = box().frame();
     ASSERT(frame);
@@ -417,7 +422,12 @@ void RenderLayerScrollableArea::setScrollOffset(const IntPoint& newScrollOffset)
 
 IntPoint RenderLayerScrollableArea::scrollPosition() const
 {
-    return IntPoint(m_scrollOffset);
+    return IntPoint(flooredIntSize(m_scrollOffset));
+}
+
+DoublePoint RenderLayerScrollableArea::scrollPositionDouble() const
+{
+    return DoublePoint(m_scrollOffset);
 }
 
 IntPoint RenderLayerScrollableArea::minimumScrollPosition() const
@@ -562,30 +572,34 @@ void RenderLayerScrollableArea::computeScrollDimensions()
     setScrollOrigin(IntPoint(-scrollableLeftOverflow, -scrollableTopOverflow));
 }
 
-void RenderLayerScrollableArea::scrollToOffset(const IntSize& scrollOffset, ScrollOffsetClamping clamp)
+void RenderLayerScrollableArea::scrollToOffset(const DoubleSize& scrollOffset, ScrollOffsetClamping clamp)
 {
-    IntSize newScrollOffset = clamp == ScrollOffsetClamped ? clampScrollOffset(scrollOffset) : scrollOffset;
-    if (newScrollOffset != adjustedScrollOffset())
-        scrollToOffsetWithoutAnimation(-scrollOrigin() + newScrollOffset);
+    DoubleSize newScrollOffset = clamp == ScrollOffsetClamped ? clampScrollOffset(scrollOffset) : scrollOffset;
+    if (newScrollOffset != adjustedScrollOffset()) {
+        DoublePoint origin(scrollOrigin());
+        scrollToOffsetWithoutAnimation(toFloatPoint(-origin + newScrollOffset));

[Rick Byers, 2014/10/02 21:44:06]

scrollToOffsetWithoutAnimation should be updated (here or in a future CL) to
take a DoublePoint instead of a FloatPoint, right?

[Yufeng Shen (Slow to review), 2014/10/02 23:34:34]

FIXME added.

+    }
 }
 
 void RenderLayerScrollableArea::updateAfterLayout()
 {
     m_scrollDimensionsDirty = true;
-    IntSize originalScrollOffset = adjustedScrollOffset();
+    DoubleSize originalScrollOffset = adjustedScrollOffset();
 
     computeScrollDimensions();
 
     if (!box().isMarquee()) {
         // Layout may cause us to be at an invalid scroll position. In this case we need
         // to pull our scroll offsets back to the max (or push them up to the min).
-        IntSize clampedScrollOffset = clampScrollOffset(adjustedScrollOffset());
+        DoubleSize clampedScrollOffset = clampScrollOffset(adjustedScrollOffset());
         if (clampedScrollOffset != adjustedScrollOffset())
             scrollToOffset(clampedScrollOffset);
     }
 
-    if (originalScrollOffset != adjustedScrollOffset())
-        scrollToOffsetWithoutAnimation(-scrollOrigin() + adjustedScrollOffset());
+    if (originalScrollOffset != adjustedScrollOffset()) {
+        DoublePoint origin(scrollOrigin());
+        scrollToOffsetWithoutAnimation(toFloatPoint(-origin + adjustedScrollOffset()));
+    }
 
     bool hasHorizontalOverflow = this->hasHorizontalOverflow();
     bool hasVerticalOverflow = this->hasVerticalOverflow();
@@ -775,14 +789,14 @@ void RenderLayerScrollableArea::updateAfterOverflowRecalc()
         box().setNeedsLayoutAndFullPaintInvalidation();
 }
 
-IntSize RenderLayerScrollableArea::clampScrollOffset(const IntSize& scrollOffset) const
+DoubleSize RenderLayerScrollableArea::clampScrollOffset(const DoubleSize& scrollOffset) const
 {
     int maxX = scrollWidth() - box().pixelSnappedClientWidth();
     int maxY = scrollHeight() - box().pixelSnappedClientHeight();
 
-    int x = std::max(std::min(scrollOffset.width(), maxX), 0);
-    int y = std::max(std::min(scrollOffset.height(), maxY), 0);
-    return IntSize(x, y);
+    double x = std::max(std::min(scrollOffset.width(), static_cast<double>(maxX)), 0.0);
+    double y = std::max(std::min(scrollOffset.height(), static_cast<double>(maxY)), 0.0);
+    return DoubleSize(x, y);
 }
 
 IntRect RenderLayerScrollableArea::rectForHorizontalScrollbar(const IntRect& borderBoxRect) const
@@ -1385,14 +1399,14 @@ LayoutRect RenderLayerScrollableArea::exposeRect(const LayoutRect& rect, const S
     LayoutRect layerBounds(0, 0, box().clientWidth(), box().clientHeight());
     LayoutRect r = ScrollAlignment::getRectToExpose(layerBounds, localExposeRect, alignX, alignY);
 
-    IntSize clampedScrollOffset = clampScrollOffset(adjustedScrollOffset() + toIntSize(roundedIntRect(r).location()));
+    DoubleSize clampedScrollOffset = clampScrollOffset(adjustedScrollOffset() + toIntSize(roundedIntRect(r).location()));
     if (clampedScrollOffset == adjustedScrollOffset())
         return rect;
 
-    IntSize oldScrollOffset = adjustedScrollOffset();
+    DoubleSize oldScrollOffset = adjustedScrollOffset();
     scrollToOffset(clampedScrollOffset);
-    IntSize scrollOffsetDifference = adjustedScrollOffset() - oldScrollOffset;
-    localExposeRect.move(-scrollOffsetDifference);
+    DoubleSize scrollOffsetDifference = adjustedScrollOffset() - oldScrollOffset;
+    localExposeRect.move(-LayoutSize(scrollOffsetDifference));
     return LayoutRect(box().localToAbsoluteQuad(FloatQuad(FloatRect(localExposeRect)), UseTransforms).boundingBox());
 }