position:sticky should stick for the enclosing overflow ancestor

Source/core/rendering/RenderLayerCompositor.cpp

 /*
  * Copyright (C) 2009, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 1778 matching lines @@
 bool RenderLayerCompositor::requiresCompositingForOverflowScrollingParent(const RenderLayer* layer) const
 {
     return !!layer->scrollParent();
 }
 
 bool RenderLayerCompositor::requiresCompositingForOutOfFlowClipping(const RenderLayer* layer) const
 {
     return layer->compositorDrivenAcceleratedScrollingEnabled() && layer->isUnclippedDescendant();
 }
 
+static bool isViewportConstrainedFixedOrStickyLayer(const RenderLayer* layer)
+{
+    if (layer->renderer()->isStickyPositioned())
+        return !layer->enclosingOverflowClipLayer(false);
+
+    if (layer->renderer()->style()->position() != FixedPosition)
+        return false;
+
+    for (const RenderLayerStackingNode* stackingContainer = layer->stackingNode(); stackingContainer;
+        stackingContainer = stackingContainer->ancestorStackingContainerNode()) {
+        if (stackingContainer->layer()->compositingState() != NotComposited

[Julien - ping for review, 2014/01/06 17:22:07]

The old code was checking
stackingContainer->layer()->hasCompositedLayerMapping(), I think this check
should be kept instead of changed.

[shawnsingh, 2014/01/06 20:02:48]

Actually what ostap@ has done here is reasonable.  We should be relying less on
hasCompositedLayerMapping() to indicate compositingState, and when we want to
query compositingState, do it directly.

A side comment - don't need to block this patch:
There is a red flag here, even before this patch.  This function is being used
within computeCompositingRequirements() - and we should not have any code in
computeCompositingRequirements that depends on any layer's compositingState() or
hasCompositedLayerMapping().  The compositingState() will be from a previous
update or be in an inconsistent state here.  compositingState() is only fully
correct after assignLayersToBackings().  I created bug
https://code.google.com/p/chromium/issues/detail?id=331866 for this.

I suspect the webkit code might have an error about doing this, too, but webkit
and blink are subtly different these days about the data flow in the compositing
code, so I'm not sure.

[ostap, 2014/01/08 01:46:40]

The original code in webkit patch calls stackingContainer->isComposited() .
Since in blink stacking container is separated from renderLayer and
isComposited() doesn't exist any more, I thought compositingState() check would
be valid replacement.

[Julien - ping for review, 2014/01/08 15:31:45]

I stand corrected by Shawn. I was more comfortable matching the original intent
of the code but we are moving away from that idiom so we may as well update the
code.

+            && stackingContainer->layer()->renderer()->style()->position() == FixedPosition)
+            return false;
+    }
+
+    return true;
+}
+
 bool RenderLayerCompositor::requiresCompositingForPosition(RenderObject* renderer, const RenderLayer* layer, RenderLayer::ViewportConstrainedNotCompositedReason* viewportConstrainedNotCompositedReason) const
 {
     // position:fixed elements that create their own stacking context (e.g. have an explicit z-index,
     // opacity, transform) can get their own composited layer. A stacking context is required otherwise
     // z-index and clipping will be broken.
     if (!renderer->isPositioned())
         return false;
 
     EPosition position = renderer->style()->position();
     bool isFixed = renderer->isOutOfFlowPositioned() && position == FixedPosition;
     if (isFixed && !layer->stackingNode()->isStackingContainer())
         return false;
 
     bool isSticky = renderer->isInFlowPositioned() && position == StickyPosition;
     if (!isFixed && !isSticky)
         return false;
 
     // FIXME: acceleratedCompositingForFixedPositionEnabled should probably be renamed acceleratedCompositingForViewportConstrainedPositionEnabled().
     if (Settings* settings = m_renderView->document().settings()) {
         if (!settings->acceleratedCompositingForFixedPositionEnabled())
             return false;
     }
 
     if (isSticky)
-        return true;
+        return isViewportConstrainedFixedOrStickyLayer(layer);
 
     RenderObject* container = renderer->container();
     // If the renderer is not hooked up yet then we have to wait until it is.
     if (!container) {
         m_needsToRecomputeCompositingRequirements = true;
         return false;
     }
 
     // Don't promote fixed position elements that are descendants of a non-view container, e.g. transformed elements.
     // They will stay fixed wrt the container rather than the enclosing frame.
@@ skipping 505 matching lines @@
 
     RenderLayerStackingNodeIterator iterator(*layer->stackingNode(), AllChildren);
     while (RenderLayerStackingNode* curNode = iterator.next()) {
         if (layerHas3DContent(curNode->layer()))
             return true;
     }
 
     return false;
 }
 
-static bool isRootmostFixedOrStickyLayer(RenderLayer* layer)
-{
-    if (layer->renderer()->isStickyPositioned())
-        return true;
-
-    if (layer->renderer()->style()->position() != FixedPosition)
-        return false;
-
-    for (RenderLayerStackingNode* stackingContainerNode = layer->stackingNode()->ancestorStackingContainerNode(); stackingContainerNode; stackingContainerNode = stackingContainerNode->ancestorStackingContainerNode()) {
-        if (stackingContainerNode->layer()->hasCompositedLayerMapping() && stackingContainerNode->layer()->renderer()->style()->position() == FixedPosition)
-            return false;
-    }
-
-    return true;
-}
-
 void RenderLayerCompositor::updateViewportConstraintStatus(RenderLayer* layer)
 {
-    if (isRootmostFixedOrStickyLayer(layer))
+    if (isViewportConstrainedFixedOrStickyLayer(layer))
         addViewportConstrainedLayer(layer);
     else
         removeViewportConstrainedLayer(layer);
 }
 
 void RenderLayerCompositor::addViewportConstrainedLayer(RenderLayer* layer)
 {
     m_viewportConstrainedLayers.add(layer);
 }
 
@@ skipping 39 matching lines @@
     // If top and bottom are auto, use top.
     if (style->top().isAuto() && style->bottom().isAuto())
         constraints.addAnchorEdge(ViewportConstraints::AnchorEdgeTop);
 
     return constraints;
 }
 
 StickyPositionViewportConstraints RenderLayerCompositor::computeStickyViewportConstraints(RenderLayer* layer) const
 {
     ASSERT(layer->hasCompositedLayerMapping());
+    // We should never get here for stickies constrained by an enclosing clipping layer.
+    ASSERT(!layer->enclosingOverflowClipLayer(false));
 
     FrameView* frameView = m_renderView->frameView();
     LayoutRect viewportRect = frameView->viewportConstrainedVisibleContentRect();
 
     StickyPositionViewportConstraints constraints;
 
     RenderBoxModelObject* renderer = toRenderBoxModelObject(layer->renderer());
 
     renderer->computeStickyPositionConstraints(constraints, viewportRect);
 
@@ skipping 47 matching lines @@
     } else if (graphicsLayer == m_scrollLayer.get()) {
         name = "Frame Scrolling Layer";
     } else {
         ASSERT_NOT_REACHED();
     }
 
     return name;
 }
 
 } // namespace WebCore