Preserve fractional scroll offset for JS scrolling API

Source/core/rendering/RenderLayerScrollableArea.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  *
  * Portions are Copyright (C) 1998 Netscape Communications Corporation.
  *
  * Other contributors:
  *   Robert O'Callahan <roc+@cs.cmu.edu>
  *   David Baron <dbaron@fas.harvard.edu>
  *   Christian Biesinger <cbiesinger@web.de>
  *   Randall Jesup <rjesup@wgate.com>
@@ skipping 108 matching lines @@
     }
 
     if (box().frame() && box().frame()->page()) {
         if (ScrollingCoordinator* scrollingCoordinator = box().frame()->page()->scrollingCoordinator())
             scrollingCoordinator->willDestroyScrollableArea(this);
     }
 
     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()) {
         if (FrameView* frameView = frame->view())
             frameView->removeResizerArea(box());
     }
 
     destroyScrollbar(HorizontalScrollbar);
     destroyScrollbar(VerticalScrollbar);
 
@@ skipping 197 matching lines @@
 }
 
 int RenderLayerScrollableArea::scrollSize(ScrollbarOrientation orientation) const
 {
     IntSize scrollDimensions = maximumScrollPosition() - minimumScrollPosition();
     return (orientation == HorizontalScrollbar) ? scrollDimensions.width() : scrollDimensions.height();
 }
 
 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);
 
     RefPtr<FrameView> frameView = box().frameView();
 
     TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "ScrollLayer", "data", InspectorScrollLayerEvent::data(&box()));
     // FIXME(361045): remove InspectorInstrumentation calls once DevTools Timeline migrates to tracing.
     InspectorInstrumentation::willScrollLayer(&box());
 
@@ skipping 43 matching lines @@
         box().node()->document().enqueueScrollEventForNode(box().node());
 
     if (AXObjectCache* cache = box().document().existingAXObjectCache())
         cache->handleScrollPositionChanged(&box());
 
     InspectorInstrumentation::didScrollLayer(&box());
 }
 
 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
 {
     return -scrollOrigin();
 }
 
 IntPoint RenderLayerScrollableArea::maximumScrollPosition() const
 {
     if (!box().hasOverflowClip())
@@ skipping 124 matching lines @@
     m_scrollDimensionsDirty = false;
 
     m_overflowRect = box().layoutOverflowRect();
     box().flipForWritingMode(m_overflowRect);
 
     int scrollableLeftOverflow = m_overflowRect.x() - box().borderLeft() - (box().style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft() ? box().verticalScrollbarWidth() : 0);
     int scrollableTopOverflow = m_overflowRect.y() - box().borderTop();
     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();
 
     {
         // Hits in compositing/overflow/automatically-opt-into-composited-scrolling-after-style-change.html.
         DisableCompositingQueryAsserts disabler;
 
         // overflow:scroll should just enable/disable.
         if (box().style()->overflowX() == OSCROLL)
@@ skipping 169 matching lines @@
     }
 
     bool hasHorizontalOverflow = this->hasHorizontalOverflow();
     bool hasVerticalOverflow = this->hasVerticalOverflow();
     bool autoHorizontalScrollBarChanged = box().hasAutoHorizontalScrollbar() && (hasHorizontalScrollbar() != hasHorizontalOverflow);
     bool autoVerticalScrollBarChanged = box().hasAutoVerticalScrollbar() && (hasVerticalScrollbar() != hasVerticalOverflow);
     if (autoHorizontalScrollBarChanged || autoVerticalScrollBarChanged)
         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
 {
     if (!m_hBar)
         return IntRect();
 
     const IntRect& scrollCorner = scrollCornerRect();
 
     return IntRect(horizontalScrollbarStart(borderBoxRect.x()),
@@ skipping 582 matching lines @@
 
     // FIXME (Radar 4118564): We should also autoscroll the window as necessary to keep the point under the cursor in view.
 }
 
 LayoutRect RenderLayerScrollableArea::exposeRect(const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
 {
     LayoutRect localExposeRect(box().absoluteToLocalQuad(FloatQuad(FloatRect(rect)), UseTransforms).boundingBox());
     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());
 }
 
 void RenderLayerScrollableArea::updateScrollableAreaSet(bool hasOverflow)
 {
     LocalFrame* frame = box().frame();
     if (!frame)
         return;
 
     FrameView* frameView = frame->view();
@@ skipping 64 matching lines @@
 void RenderLayerScrollableArea::setTopmostScrollChild(RenderLayer* scrollChild)
 {
     // We only want to track the topmost scroll child for scrollable areas with
     // overlay scrollbars.
     if (!hasOverlayScrollbars())
         return;
     m_nextTopmostScrollChild = scrollChild;
 }
 
 } // namespace blink