Adapt and reland old position sticky implementation

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

Index: third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
index 0c7af6cbd0553824d52db1650f1c05ee4ec47012..c217c9ec45eddb0108e1dd03e6c960ba1cd5ecee 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
@@ -178,6 +178,20 @@ void LayoutBoxModelObject::styleWillChange(StyleDifference diff, const ComputedS
     LayoutObject::styleWillChange(diff, newStyle);
 }
 
+static inline LayoutBox* findScrollAncestor(const LayoutObject* startObject)
+{
+    LayoutBox* curBox = startObject->containingBlock();
+
+    // Scrolling propagates along the containing block chain.
+    while (curBox && !curBox->isLayoutView()) {

[chrishtr, 2015/12/11 02:06:12]

Use enclosingLayer()->scrollParent() instead.

[flackr, 2016/01/19 15:40:48]

ancestorScrollingLayer (and scrollParent, which is something different) relies
on ancestorDependentCompositingInputs which are not available during layout when
we ask for this. I've added a check to see if we can query the compositing
inputs state and use that if so. This is the case during scrolling when we're
asking for the position from the compositing update step.

+        if (curBox->hasOverflowClip())
+            return curBox;
+        curBox = curBox->containingBlock();
+    }
+
+    return nullptr;
+}
+
 void LayoutBoxModelObject::styleDidChange(StyleDifference diff, const ComputedStyle* oldStyle)
 {
     bool hadTransform = hasTransformRelatedProperty();
@@ -268,14 +282,32 @@ void LayoutBoxModelObject::styleDidChange(StyleDifference diff, const ComputedSt
     }
 
     if (FrameView *frameView = view()->frameView()) {
-        bool newStyleIsViewportConstained = style()->hasViewportConstrainedPosition();
-        bool oldStyleIsViewportConstrained = oldStyle && oldStyle->hasViewportConstrainedPosition();
+        bool newStyleIsViewportConstained = style()->position() == FixedPosition;
+        bool oldStyleIsViewportConstrained = oldStyle && oldStyle->position() == FixedPosition;
+        bool newStyleIsSticky = style()->position() == StickyPosition;
+        bool oldStyleIsSticky = oldStyle && oldStyle->position() == StickyPosition;
+
+        // Sticky positioned elements are only viewport constrained if they have no ancestor scroller.
+        if (newStyleIsSticky || oldStyleIsSticky) {
+            if (!findScrollAncestor(this)) {
+                newStyleIsViewportConstained |= newStyleIsSticky;
+                oldStyleIsViewportConstrained |= oldStyleIsSticky;
+            }
+        }
+
         if (newStyleIsViewportConstained != oldStyleIsViewportConstrained) {
             if (newStyleIsViewportConstained && layer())
                 frameView->addViewportConstrainedObject(this);
             else
                 frameView->removeViewportConstrainedObject(this);
         }
+
+        if (newStyleIsSticky != oldStyleIsSticky) {
+            if (newStyleIsSticky)
+                frameView->addStickyPositionObject();
+            else
+                frameView->removeStickyPositionObject();
+        }
     }
 }
 
@@ -608,6 +640,118 @@ LayoutSize LayoutBoxModelObject::relativePositionOffset() const
     return offset;
 }
 
+void LayoutBoxModelObject::computeStickyPositionConstraints(StickyPositionViewportConstraints& constraints, const LayoutRect& constrainingRect) const
+{
+    LayoutBlock* containingBlock = this->containingBlock();
+
+    LayoutRect containerContentRect = containingBlock->contentBoxRect();
+    LayoutUnit maxWidth = containingBlock->availableLogicalWidth();
+
+    // Sticky positioned element ignore any override logical width on the containing block (as they don't call
+    // containingBlockLogicalWidthForContent). It's unclear whether this is totally fine.
+    // Compute the container-relative area within which the sticky element is allowed to move.
+    containerContentRect.contractEdges(
+        minimumValueForLength(style()->marginTop(), maxWidth),
+        minimumValueForLength(style()->marginRight(), maxWidth),
+        minimumValueForLength(style()->marginBottom(), maxWidth),
+        minimumValueForLength(style()->marginLeft(), maxWidth));
+
+    // Map to the view to avoid including page scale factor.
+    constraints.setAbsoluteContainingBlockRect(LayoutRect(containingBlock->localToContainerQuad(FloatRect(containerContentRect), view()).boundingBox()));
+
+    LayoutRect stickyBoxRect = isLayoutInline()
+        ? LayoutRect(toLayoutInline(this)->linesBoundingBox())
+        : toLayoutBox(this)->frameRect();
+    LayoutRect flippedStickyBoxRect = stickyBoxRect;
+    containingBlock->flipForWritingMode(flippedStickyBoxRect);
+    LayoutPoint stickyLocation = flippedStickyBoxRect.location();
+
+    // FIXME: sucks to call localToAbsolute again, but we can't just offset from the previously computed rect if there are transforms.
+    // Map to the view to avoid including page scale factor.
+    LayoutRect absContainerFrame = LayoutRect(containingBlock->localToContainerQuad(FloatRect(FloatPoint(), FloatSize(containingBlock->size())), view()).boundingBox());

[chrishtr, 2015/12/11 02:06:12]

I don't think this should be in LayoutView coordinates, it should be in the
coordinates of the scroller.
Do you know of a testcase where page scale comes into play?

[flackr, 2016/01/19 15:40:48]

So this is specifically excluding page scale, which is if I understand
correctly, above the view(), which means relative to the scroller should be just
fine. I've refactored this to be relative to the scroll ancestor.

+
+    if (containingBlock->hasOverflowClip()) {
+        LayoutSize scrollOffset(containingBlock->layer()->scrollableArea()->adjustedScrollOffset());
+        stickyLocation -= scrollOffset;
+    }
+
+    // We can't call localToAbsolute on |this| because that will recur. FIXME: For now, assume that |this| is not transformed.
+    constraints.setAbsoluteStickyBoxRect(LayoutRect(LayoutPoint(absContainerFrame.location()) + stickyLocation, flippedStickyBoxRect.size()));
+
+    LayoutUnit horizontalOffsets = minimumValueForLength(style()->right(), constrainingRect.width()) +
+        minimumValueForLength(style()->left(), constrainingRect.width());
+    bool skipRight = false;
+    bool skipLeft = false;
+    if (!style()->left().isAuto() && !style()->right().isAuto()) {
+        if (horizontalOffsets > containerContentRect.width()
+            || horizontalOffsets + containerContentRect.width() > constrainingRect.width()) {
+            skipRight = style()->isLeftToRightDirection();
+            skipLeft = !skipRight;
+        }
+    }
+
+    if (!style()->left().isAuto() && !skipLeft) {
+        constraints.setLeftOffset(minimumValueForLength(style()->left(), constrainingRect.width()));
+        constraints.addAnchorEdge(ViewportConstraints::AnchorEdgeLeft);
+    }
+
+    if (!style()->right().isAuto() && !skipRight) {
+        constraints.setRightOffset(minimumValueForLength(style()->right(), constrainingRect.width()));
+        constraints.addAnchorEdge(ViewportConstraints::AnchorEdgeRight);
+    }
+
+    bool skipBottom = false;
+    // FIXME(ostap): Exclude top or bottom edge offset depending on the writing mode when related
+    // sections are fixed in spec: http://lists.w3.org/Archives/Public/www-style/2014May/0286.html
+    LayoutUnit verticalOffsets = minimumValueForLength(style()->top(), constrainingRect.width()) +
+        minimumValueForLength(style()->bottom(), constrainingRect.width());
+    if (!style()->top().isAuto() && !style()->bottom().isAuto()) {
+        if (verticalOffsets > containerContentRect.height()
+            || verticalOffsets + containerContentRect.height() > constrainingRect.height()) {
+            skipBottom = true;
+        }
+    }
+
+    if (!style()->top().isAuto()) {
+        constraints.setTopOffset(minimumValueForLength(style()->top(), constrainingRect.height()));
+        constraints.addAnchorEdge(ViewportConstraints::AnchorEdgeTop);
+    }
+
+    if (!style()->bottom().isAuto() && !skipBottom) {
+        constraints.setBottomOffset(minimumValueForLength(style()->bottom(), constrainingRect.height()));
+        constraints.addAnchorEdge(ViewportConstraints::AnchorEdgeBottom);
+    }
+}
+
+LayoutRect LayoutBoxModelObject::computeStickyConstrainingRect() const
+{
+    LayoutRect constrainingRect;
+
+    ASSERT(hasLayer());
+    LayoutBox* enclosingClippingBox = findScrollAncestor(this);
+    if (enclosingClippingBox) {
+        LayoutRect clipRect = enclosingClippingBox->overflowClipRect(LayoutPoint());
+        clipRect.move(enclosingClippingBox->paddingLeft(), enclosingClippingBox->paddingTop());
+        clipRect.contract(LayoutSize(enclosingClippingBox->paddingLeft() + enclosingClippingBox->paddingRight(),
+            enclosingClippingBox->paddingTop() + enclosingClippingBox->paddingBottom()));
+        constrainingRect = LayoutRect(enclosingClippingBox->localToContainerQuad(FloatRect(clipRect), view()).boundingBox());
+    } else {
+        constrainingRect = LayoutRect(view()->frameView()->visibleContentRect());
+    }
+
+    return constrainingRect;
+}
+
+LayoutSize LayoutBoxModelObject::stickyPositionOffset() const
+{
+    LayoutRect constrainingRect = computeStickyConstrainingRect();

[chrishtr, 2015/12/11 02:06:12]

This will compute these rects from scratch on every scroll. You should be able
to get it set up so that the only thing that happens on each scroll is to add
the scroll offset to the sticky bounds and compare with the already-computed
containing block and scroller viewport.

[flackr, 2016/01/19 15:40:48]

I've been trying to wrap my head around how we can do this correctly, updating
at the right times.

It seems that we would need to make layout updates on the scroll ancestor /
containing block descend to sticky position children. One caveat is that
LayoutInline::layout() is not supposed to be reached but sticky objects can be
inline.

Once we did have the update of these rects happening at the right time, would I
then then add a rare data member to LayoutBoxModelObject to save these details?

[chrishtr, 2016/01/30 01:49:02]

What do you mean exactly that "LayoutInline::layout() is not supposed to be
reached but sticky objects can be inline" ?
Yes. It'd be: run the lifecycle code in FrameView up to compositing, then update
all sticky constraints for elements which
need it based on dirty bits. Store these constraints in their LayoutObject's
rare data.

This would make position sticky offsets something that is computed after layout
and just before paint, similar to how transforms are updated after layout and
are (mostly) not affecting layout. But this would mean that position:sticky
elements would have an updated in-flow offset that layout wouldn't know about,
so we need to make sure that doesn't change
anything about layout. I think it should not, in fact I don't know why these
constraints would have to be applied in that
way. Is there some reason?

Alternatively, we can think of it as a purely paint effect that has nothing to
do with layout, which is spec'ed to
work out. I hope the spec was set up to make this work?

[flackr, 2016/02/03 22:48:38]

My original thinking was that I may need to call layout on sticky objects to
update their saved sticky constraints, but LayoutInline::layout() has an
ASSERT_NOT_REACHED() so I assumed that this would not be correct.
So to be clear, this would be somewhere in PaintLayerCompositor::updateIfNeeded
after the compositing input update?

As for detecting dirtiness, I notice there is LayoutBox::frameRectChanged which
could be the right place to detect the containing block / ancestor scroller size
change and set some flag on sticky descendants at that point.

If the ancestor scroller changes I imagine this could be detected during the
compositing inputs update and mark the sticky object dirty then.
I don't think there's any problems with this. As long the offset will be correct
if JS queries element.offsetTop after updating some sticky property / making an
element sticky it should be fine. I had been assuming (perhaps incorrectly) that
querying offset from JS only advanced the lifecycle past the layout stage.
Is LayoutObject::invalidateTreeIfNeeded perhaps a good hook for when to dirty
sticky constraints? It seems like this happens whenever sizes change or things
move and that this should happen after compositing inputs have been updated:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[chrishtr, 2016/02/08 21:46:55]

No, in FrameView::updateLifecyclePhasesInternal, after compositing update. Just
before the call to scrollContentsIfNeededRecursive(), probably.
Updating layout which may affect the sticky-positioned element's positioning
should require re-computation of it. It's not just that the scroller's rect
changed; there could be other changes of intermediate elements which affect the
sticky element constraint rects.

The simplest way I can think of to be correct is to clear out the sticky
position constraints for a scroller if anything about it or its contents changes
(maybe if any style or layout on any element in the scroller, including itself,
changed?), and then in the part where you update those constraints, look to see
if they were cleared out, and if so recompute.
Element::offsetTop currently only advances to layout clean, and only for the
current frame, unfortunately. :( It's really spec'ed to require this?

Options I can think of:

1. Change the spec?
2. Change Element::offsetTop to do more work. This may make some microbenchmarks
slower.
3. #2, but only if there is a position:sticky element present. This hacks around
the benchmarks.
4. Run the computation of ancestor- and descendant-dependent PaintLayer
properties (i.e. CompositingInputsUpdater) inside of the sticky position update
code, if needed, in its own
lifecycle phases with or separate from sticky.

I think I like 4 or 1 the best. 4 shouldn't be super hard, I can walk you
through it as a preparatory CL.
See my comment above. I don't think it would be a good idea to conflate the
dirtiness you would be tracking here with invalidation.

+    StickyPositionViewportConstraints constraints;
+    computeStickyPositionConstraints(constraints, constrainingRect);
+
+    // The sticky offset is physical, so we can just return the delta computed in absolute coords (though it may be wrong with transforms).
+    return LayoutSize(constraints.computeStickyOffset(constrainingRect));
+}
+
 LayoutPoint LayoutBoxModelObject::adjustedPositionRelativeToOffsetParent(const LayoutPoint& startPoint) const
 {
     // If the element is the HTML body element or doesn't have a parent
@@ -630,7 +774,7 @@ LayoutPoint LayoutBoxModelObject::adjustedPositionRelativeToOffsetParent(const L
             referencePoint.move(-toLayoutBox(offsetParent)->borderLeft(), -toLayoutBox(offsetParent)->borderTop());
         if (!isOutOfFlowPositioned() || flowThreadContainingBlock()) {
             if (isInFlowPositioned())
-                referencePoint.move(relativePositionOffset());
+                referencePoint.move(offsetForInFlowPosition());
 
             LayoutObject* current;
             for (current = parent(); current != offsetParent && current->parent(); current = current->parent()) {
@@ -652,7 +796,13 @@ LayoutPoint LayoutBoxModelObject::adjustedPositionRelativeToOffsetParent(const L
 
 LayoutSize LayoutBoxModelObject::offsetForInFlowPosition() const
 {
-    return isRelPositioned() ? relativePositionOffset() : LayoutSize();
+    if (isRelPositioned())
+        return relativePositionOffset();
+
+    if (isStickyPositioned())
+        return stickyPositionOffset();
+
+    return LayoutSize();
 }
 
 LayoutUnit LayoutBoxModelObject::offsetLeft() const
@@ -975,7 +1125,7 @@ const LayoutObject* LayoutBoxModelObject::pushMappingToContainer(const LayoutBox
         return nullptr;
 
     bool isInline = isLayoutInline();
-    bool isFixedPos = !isInline && style()->position() == FixedPosition;
+    bool isFixedPosition = !isInline && style()->position() == FixedPosition;
     bool hasTransform = !isInline && hasLayer() && layer()->transform();
 
     LayoutSize adjustmentForSkippedAncestor;
@@ -987,16 +1137,19 @@ const LayoutObject* LayoutBoxModelObject::pushMappingToContainer(const LayoutBox
 
     bool offsetDependsOnPoint = false;
     LayoutSize containerOffset = offsetFromContainer(container, LayoutPoint(), &offsetDependsOnPoint);
+    LayoutSize stickyOffset;
+    if (style()->position() == StickyPosition)
+        stickyOffset = offsetForInFlowPosition();
 
     bool preserve3D = container->style()->preserves3D() || style()->preserves3D();
     if (shouldUseTransformFromContainer(container)) {
         TransformationMatrix t;
         getTransformFromContainer(container, containerOffset, t);
         t.translateRight(adjustmentForSkippedAncestor.width().toFloat(), adjustmentForSkippedAncestor.height().toFloat());
-        geometryMap.push(this, t, preserve3D, offsetDependsOnPoint, isFixedPos, hasTransform);
+        geometryMap.push(this, t, preserve3D, offsetDependsOnPoint, isFixedPosition, hasTransform, LayoutSize(), stickyOffset);
     } else {
         containerOffset += adjustmentForSkippedAncestor;
-        geometryMap.push(this, containerOffset, preserve3D, offsetDependsOnPoint, isFixedPos, hasTransform);
+        geometryMap.push(this, containerOffset, preserve3D, offsetDependsOnPoint, isFixedPosition, hasTransform, LayoutSize(), stickyOffset);
     }
 
     return ancestorSkipped ? ancestorToStopAt : container;