Round anchor offsets to DIPs in ScrollAnchor::restore.

third_party/WebKit/Source/core/layout/ScrollAnchor.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/layout/ScrollAnchor.h"
 
 #include "core/frame/FrameView.h"
 #include "core/frame/UseCounter.h"
 #include "core/layout/LayoutView.h"
 #include "core/layout/line/InlineTextBox.h"
@@ skipping 71 matching lines @@
     } else if (layoutObject->isText()) {
         // TODO(skobes): Use first and last InlineTextBox only?
         for (InlineTextBox* box = toLayoutText(layoutObject)->firstTextBox(); box; box = box->nextTextBox())
             localBounds.unite(box->calculateBoundaries());
     } else {
         // Only LayoutBox and LayoutText are supported.
         ASSERT_NOT_REACHED();
     }
     LayoutRect relativeBounds = LayoutRect(layoutObject->localToAncestorQuad(
         FloatRect(localBounds), scrollerLayoutBox(scroller)).boundingBox());
+    // When the scroller is the FrameView, localToAncestorQuad returns document coords,
+    // so we must subtract scroll offset to get viewport coords. We discard the fractional
+    // part of the scroll offset so that the rounding in restore() matches the snapping of
+    // the anchor node to the pixel grid of the layer it paints into. For non-FrameView
+    // scrollers, we rely on the flooring behavior of LayoutBox::scrolledContentOffset.
     if (scroller->isFrameView())
-        relativeBounds.moveBy(-LayoutPoint(scroller->scrollPositionDouble()));
+        relativeBounds.moveBy(-flooredIntPoint(scroller->scrollPositionDouble()));

[eae, 2016/05/10 23:09:18]

We round scroll offsets in PaintLayerScrollableArea and floor them in
RootFrameViewport. Logically rounding seems more correct but as long as we're
consistent it doesn't really matter.

[skobes, 2016/05/10 23:19:00]

It's somewhat more subtle.

For the return value of relativeBounds the important thing is that the
fractional part matches the fractional position the paint code will see when it
paints the anchor node into the GraphicsLayer that holds the scrolling content. 
The way the ScrollableArea treats the fractional part of the scroll offset is
irrelevant.

If scroller is a PaintLayerScrollableArea, localToAncestorQuad has already given
us coordinates relative to the scroller's viewport based on a floored scroll
offset (in LayoutBox::scrolledContentOffset).  Flooring the FrameView's offset
is consistent with that.

     return relativeBounds;
 }
 
 static LayoutPoint computeRelativeOffset(const LayoutObject* layoutObject, const ScrollableArea* scroller, Corner corner)
 {
     return cornerPointOfRect(relativeBounds(layoutObject, scroller), corner);
 }
 
 static bool candidateMovesWithScroller(const LayoutObject* candidate, const ScrollableArea* scroller)
 {
@@ skipping 70 matching lines @@
 
     m_anchorObject->setIsScrollAnchorObject();
     m_savedRelativeOffset = computeRelativeOffset(m_anchorObject, m_scroller, m_corner);
 }
 
 void ScrollAnchor::restore()
 {
     if (!m_anchorObject)
         return;
 
-    LayoutSize adjustment = computeRelativeOffset(m_anchorObject, m_scroller, m_corner) - m_savedRelativeOffset;
+    // The anchor node can report fractional positions, but it is DIP-snapped when
+    // painting (crbug.com/610805), so we must round the offsets to determine the
+    // visual delta. If we scroll by the delta in LayoutUnits, the snapping of the
+    // anchor node may round differently from the snapping of the scroll position.
+    // (For example, anchor moving from 2.4px -> 2.6px is really 2px -> 3px, so we
+    // should scroll by 1px instead of 0.2px.) This is true regardless of whether
+    // the ScrollableArea actually uses fractional scroll positions.
+    IntSize adjustment =
+        roundedIntSize(computeRelativeOffset(m_anchorObject, m_scroller, m_corner)) -
+        roundedIntSize(m_savedRelativeOffset);
     if (!adjustment.isZero()) {
+        DoublePoint desiredPos = m_scroller->scrollPositionDouble() + adjustment;
         ScrollAnimatorBase* animator = m_scroller->existingScrollAnimator();
         if (!animator || !animator->hasRunningAnimation()) {
-            m_scroller->setScrollPosition(
-                m_scroller->scrollPositionDouble() + DoubleSize(adjustment),
-                AnchoringScroll);
+            m_scroller->setScrollPosition(desiredPos, AnchoringScroll);
         } else {
             // If in the middle of a scroll animation, stop the animation, make
             // the adjustment, and continue the animation on the pending delta.
             FloatSize pendingDelta = animator->desiredTargetPosition() - FloatPoint(m_scroller->scrollPositionDouble());
             animator->cancelAnimation();
-            m_scroller->setScrollPosition(
-                m_scroller->scrollPositionDouble() + DoubleSize(adjustment),
-                AnchoringScroll);
+            m_scroller->setScrollPosition(desiredPos, AnchoringScroll);
             animator->userScroll(ScrollByPixel, pendingDelta);
         }
         // Update UMA metric.
         DEFINE_STATIC_LOCAL(EnumerationHistogram, adjustedOffsetHistogram,
             ("Layout.ScrollAnchor.AdjustedScrollOffset", 2));
         adjustedOffsetHistogram.count(1);
         UseCounter::count(scrollerLayoutBox(m_scroller)->document(), UseCounter::ScrollAnchored);
     }
 }
 
 void ScrollAnchor::clear()
 {
     LayoutObject* anchorObject = m_anchorObject;
     m_anchorObject = nullptr;
 
     if (anchorObject)
         anchorObject->maybeClearIsScrollAnchorObject();
 }
 
 } // namespace blink