Only update composited-scrolling state once after layout.

Source/core/rendering/RenderLayer.cpp

Index: Source/core/rendering/RenderLayer.cpp
diff --git a/Source/core/rendering/RenderLayer.cpp b/Source/core/rendering/RenderLayer.cpp
index 275b6e3a55f520bcb99e072fcf557cafde10ee70..ba365a674fb9280ca32df68a2283c70e0f42252e 100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -144,6 +144,7 @@ RenderLayer::RenderLayer(RenderLayerModelObject* renderer)
     , m_hasSelfPaintingLayerDescendantDirty(false)
     , m_hasOutOfFlowPositionedDescendant(false)
     , m_hasOutOfFlowPositionedDescendantDirty(true)
+    , m_hasUnclippedDescendant(false)
     , m_forceNeedsCompositedScrolling(DoNotForceCompositedScrolling)
     , m_needsCompositedScrolling(false)
     , m_descendantsAreContiguousInStackingOrder(false)
@@ -226,6 +227,9 @@ RenderLayer::~RenderLayer()
             toElement(node)->setSavedLayerScrollOffset(m_scrollOffset);
     }
 
+    if (!m_renderer->documentBeingDestroyed())
+        compositor()->removeOutOfFlowPositionedLayer(this);
+
     destroyScrollbar(HorizontalScrollbar);
     destroyScrollbar(VerticalScrollbar);
 
@@ -476,6 +480,35 @@ void RenderLayer::dirtyAncestorChainHasSelfPaintingLayerDescendantStatus()
     }
 }
 
+void RenderLayer::setAncestorChainHasOutOfFlowPositionedDescendant()
+{
+    for (RenderLayer* layer = this; layer; layer = layer->parent()) {
+        if (!layer->m_hasOutOfFlowPositionedDescendantDirty && layer->hasOutOfFlowPositionedDescendant())
+            break;
+
+        layer->m_hasOutOfFlowPositionedDescendantDirty = false;
+        layer->m_hasOutOfFlowPositionedDescendant = true;
+    }
+}
+
+void RenderLayer::dirtyAncestorChainHasOutOfFlowPositionedDescendantStatus()
+{
+    for (RenderLayer* layer = this; layer; layer = layer->parent()) {
+        layer->m_hasOutOfFlowPositionedDescendantDirty = true;
+
+        // We may or may not have an unclipped descendant. If we do, we'll reset
+        // this to true the next time composited scrolling state is updated.
+        layer->m_hasUnclippedDescendant = false;
+
+        // If we have reached an out of flow positioned layer, we know our parent should have an out-of-flow positioned descendant.
+        // In this case, there is no need to dirty our ancestors further.
+        if (layer->renderer()->isOutOfFlowPositioned()) {
+            ASSERT(!parent() || parent()->m_hasOutOfFlowPositionedDescendantDirty || parent()->hasOutOfFlowPositionedDescendant());
+            break;
+        }
+    }
+}
+
 bool RenderLayer::acceleratedCompositingForOverflowScrollEnabled() const
 {
     return renderer()->frame()
@@ -565,9 +598,6 @@ void RenderLayer::updateDescendantsAreContiguousInStackingOrder()
     if (!m_descendantsAreContiguousInStackingOrderDirty || !isStackingContext() || !acceleratedCompositingForOverflowScrollEnabled())
         return;
 
-    ASSERT(!m_normalFlowListDirty);
-    ASSERT(!m_zOrderListsDirty);
-
     OwnPtr<Vector<RenderLayer*> > posZOrderList;
     OwnPtr<Vector<RenderLayer*> > negZOrderList;
     rebuildZOrderLists(StopAtStackingContexts, posZOrderList, negZOrderList);
@@ -630,11 +660,8 @@ void RenderLayer::updateDescendantsAreContiguousInStackingOrderRecursive(const H
     }
 
     if (!isStackingContext()) {
-        bool newValue = maxIndex - minIndex == count;
-        bool didUpdate = newValue != m_descendantsAreContiguousInStackingOrder;
-        m_descendantsAreContiguousInStackingOrder = newValue;
-        if (didUpdate)
-            updateNeedsCompositedScrolling();
+        m_descendantsAreContiguousInStackingOrder = (maxIndex - minIndex) == count;
+        m_descendantsAreContiguousInStackingOrderDirty = false;
     }
 }
 
@@ -1063,6 +1090,7 @@ bool RenderLayer::canBeStackingContainer() const
     if (isStackingContext() || !ancestorStackingContainer())
         return true;
 
+    ASSERT(!m_descendantsAreContiguousInStackingOrderDirty);
     return m_descendantsAreContiguousInStackingOrder;
 }
 
@@ -1119,56 +1147,62 @@ void RenderLayer::setAncestorChainHasVisibleDescendant()
     }
 }
 
-void RenderLayer::updateDescendantDependentFlags(HashSet<const RenderObject*>* outOfFlowDescendantContainingBlocks)
+// This method is called on all out of flow positioned layers. It simply walks
+// up to its containing block marking every layer it passes as having an
+// 'unclipped' descendant. The map is used to prevent wasted walking. If we
+// have already passed layer A heading towards containing block B, there's no
+// need to do it again; we can early out.
+void RenderLayer::updateHasUnclippedDescendant(RenderLayer::AncestorContainingBlockMap& map)

[Julien - ping for review, 2013/05/06 22:38:26]

Wouldn't checking ancestor->m_hasUnclippedDescendant in the loop would alleviate
the need for this map?

Also based on the average render tree depth (~30 I heard), it seems like an
overkill to add this map. Did this function show up on your profile?

It's also misnamed as it's not really a containing block map but an ancestor
map.

[Ian Vollick, 2013/05/08 16:00:07]

I did some measurements and this is indeed premature optimization. On gmail load
without the map we spend 10.88ms in the function (1810 invocations). With the
map we spend 10.73ms in the function (1686 invocations). I've removed the map
and updated the CL description. Sadly, this makes the worst case O(n^2), but I
really don't expect us to get hit this hard in practice.

 {
-    if (m_visibleDescendantStatusDirty || m_hasSelfPaintingLayerDescendantDirty || m_hasOutOfFlowPositionedDescendantDirty) {
-        const bool hadVisibleDescendant = m_hasVisibleDescendant;
-        const bool hadOutOfFlowPositionedDescendant = m_hasOutOfFlowPositionedDescendant;
+    ASSERT(renderer()->isOutOfFlowPositioned());
+    if (!m_hasVisibleContent && !m_hasVisibleDescendant)
+        return;
 
+    const RenderObject* containingBlock = renderer()->containingBlock();
+    for (RenderLayer* ancestor = parent(); ancestor && ancestor->renderer() != containingBlock; ancestor = ancestor->parent()) {
+        AncestorContainingBlockMap::iterator it = map.find(ancestor);
+        if (it == map.end()) {
+            OwnPtr<HashSet<const RenderObject*> > containingBlocks = adoptPtr(new HashSet<const RenderObject*>);
+            containingBlocks->add(containingBlock);
+            map.add(ancestor, containingBlocks.release());
+        } else if (!it->value->contains(containingBlock)) {
+            it->value->add(containingBlock);
+        } else {
+            break;
+        }
+
+        ancestor->m_hasUnclippedDescendant = true;
+    }
+}
+
+void RenderLayer::updateDescendantDependentFlags()
+{
+    if (m_visibleDescendantStatusDirty || m_hasSelfPaintingLayerDescendantDirty || m_hasOutOfFlowPositionedDescendantDirty) {
         m_hasVisibleDescendant = false;
         m_hasSelfPaintingLayerDescendant = false;
         m_hasOutOfFlowPositionedDescendant = false;
 
-        HashSet<const RenderObject*> childOutOfFlowDescendantContainingBlocks;
         for (RenderLayer* child = firstChild(); child; child = child->nextSibling()) {
-            childOutOfFlowDescendantContainingBlocks.clear();
-            child->updateDescendantDependentFlags(&childOutOfFlowDescendantContainingBlocks);
-
-            bool childIsOutOfFlowPositioned = child->renderer() && child->renderer()->isOutOfFlowPositioned();
-            if (childIsOutOfFlowPositioned)
-                childOutOfFlowDescendantContainingBlocks.add(child->renderer()->containingBlock());
-
-            if (outOfFlowDescendantContainingBlocks) {
-                HashSet<const RenderObject*>::const_iterator it = childOutOfFlowDescendantContainingBlocks.begin();
-                for (; it != childOutOfFlowDescendantContainingBlocks.end(); ++it)
-                    outOfFlowDescendantContainingBlocks->add(*it);
-            }
+            child->updateDescendantDependentFlags();
 
             bool hasVisibleDescendant = child->m_hasVisibleContent || child->m_hasVisibleDescendant;
             bool hasSelfPaintingLayerDescendant = child->isSelfPaintingLayer() || child->hasSelfPaintingLayerDescendant();
-            bool hasOutOfFlowPositionedDescendant = hasVisibleDescendant && (!childOutOfFlowDescendantContainingBlocks.isEmpty() || child->hasOutOfFlowPositionedDescendant());
+            bool hasOutOfFlowPositionedDescendant = child->renderer()->isOutOfFlowPositioned() || child->hasOutOfFlowPositionedDescendant();
 
             m_hasVisibleDescendant |= hasVisibleDescendant;
             m_hasSelfPaintingLayerDescendant |= hasSelfPaintingLayerDescendant;
             m_hasOutOfFlowPositionedDescendant |= hasOutOfFlowPositionedDescendant;
 
-            if (m_hasVisibleDescendant && m_hasSelfPaintingLayerDescendant && m_hasOutOfFlowPositionedDescendant)
+            if (m_hasVisibleDescendant && m_hasSelfPaintingLayerDescendant && hasOutOfFlowPositionedDescendant)
                 break;
         }
 
-        if (outOfFlowDescendantContainingBlocks && renderer())
-            outOfFlowDescendantContainingBlocks->remove(renderer());
-
         m_visibleDescendantStatusDirty = false;
         m_hasSelfPaintingLayerDescendantDirty = false;
         m_hasOutOfFlowPositionedDescendantDirty = false;
-
-        if (m_hasVisibleDescendant != hadVisibleDescendant || m_hasOutOfFlowPositionedDescendant != hadOutOfFlowPositionedDescendant)
-            updateNeedsCompositedScrolling();
     }
 
     if (m_visibleContentStatusDirty) {
-        const bool hadVisibleContent = m_hasVisibleContent;
         if (renderer()->style()->visibility() == VISIBLE)
             m_hasVisibleContent = true;
         else {
@@ -1194,10 +1228,8 @@ void RenderLayer::updateDescendantDependentFlags(HashSet<const RenderObject*>* o
                         r = r->nextSibling();
                 }
             }
-        }    
-        m_visibleContentStatusDirty = false; 
-        if (hadVisibleContent != m_hasVisibleContent)
-            updateNeedsCompositedScrolling();
+        }
+        m_visibleContentStatusDirty = false;
     }
 }
 
@@ -1768,16 +1800,6 @@ void RenderLayer::addChild(RenderLayer* child, RenderLayer* beforeChild)
         // case where we're building up generated content layers. This is ok, since the lists will start
         // off dirty in that case anyway.
         child->dirtyStackingContainerZOrderLists();
-
-        // Adding an out of flow positioned descendant can only affect
-        // the opt-in decision for layers beneath and including our
-        // containing block.
-        RenderObject* containingBlock = child->renderer()->containingBlock();
-        for (RenderLayer* layer = child; layer; layer = layer->parent()) {
-            layer->updateNeedsCompositedScrolling();
-            if (layer->renderer() == containingBlock)
-                break;
-        }
     }
 
     child->updateDescendantDependentFlags();
@@ -1787,8 +1809,14 @@ void RenderLayer::addChild(RenderLayer* child, RenderLayer* beforeChild)
     if (child->isSelfPaintingLayer() || child->hasSelfPaintingLayerDescendant())
         setAncestorChainHasSelfPaintingLayerDescendant();
 
-    if (child->renderer() && (child->renderer()->isOutOfFlowPositioned() || child->hasOutOfFlowPositionedDescendant()))
-        setAncestorChainHasOutOfFlowPositionedDescendant(child->renderer()->containingBlock());
+    if (child->renderer() && (child->renderer()->isOutOfFlowPositioned() || child->hasOutOfFlowPositionedDescendant())) {

[Julien - ping for review, 2013/05/06 22:38:26]

child->renderer()->isOutOfFlowPositioned() ||
child->hasOutOfFlowPositionedDescendant() is repeated several time. It's
probably time to add a helper function.

[Ian Vollick, 2013/05/08 16:00:07]

Done.

+        // Now that the out of flow positioned descendant is in the tree, we
+        // need to tell the compositor to reevaluate the compositing
+        // requirements since we may be able to mark more layers as having
+        // an 'unclipped' descendant.
+        compositor()->setNeedsUpdateCompositingRequirementsState();
+        setAncestorChainHasOutOfFlowPositionedDescendant();
+    }
 
     compositor()->layerWasAdded(this, child);
 }
@@ -1816,28 +1844,21 @@ RenderLayer* RenderLayer::removeChild(RenderLayer* oldChild)
         // reattachment process in removeOnlyThisLayer, the layer may already be disconnected
         // from the main layer tree, so we need to null-check the |stackingContainer| value.
         oldChild->dirtyStackingContainerZOrderLists();
-
-        // This could affect whether or not a layer has an out of flow
-        // positioned descendant so we need to schedule some updates.
-        // Removing an out of flow positioned descendant can only affect
-        // the opt-in decision for layers beneath and including the old child's
-        // containing block.
-        RenderObject* containingBlock = oldChild->renderer()->containingBlock();
-        for (RenderLayer* layer = this; layer; layer = layer->parent()) {
-            layer->updateNeedsCompositedScrolling();
-            if (layer->renderer() == containingBlock)
-                break;
-        }
     }
 
-    if ((oldChild->renderer() && oldChild->renderer()->isOutOfFlowPositioned()) || oldChild->hasOutOfFlowPositionedDescendant())
-        dirtyAncestorChainHasOutOfFlowPositionedDescendantStatus();
-
     oldChild->setPreviousSibling(0);
     oldChild->setNextSibling(0);
     oldChild->setParent(0);
 
     oldChild->updateDescendantDependentFlags();
+    if ((oldChild->renderer() && oldChild->renderer()->isOutOfFlowPositioned()) || oldChild->hasOutOfFlowPositionedDescendant()) {
+        // It may now be the case that a layer no longer has an unclipped
+        // descendant. Let the compositor know that it needs to reevaluate
+        // its compositing requirements to check this.
+        compositor()->setNeedsUpdateCompositingRequirementsState();
+        dirtyAncestorChainHasOutOfFlowPositionedDescendantStatus();
+    }
+
     if (oldChild->m_hasVisibleContent || oldChild->m_hasVisibleDescendant)
         dirtyAncestorChainVisibleDescendantStatus();
 
@@ -2066,6 +2087,9 @@ bool RenderLayer::needsCompositedScrolling() const
 
 void RenderLayer::updateNeedsCompositedScrolling()
 {
+    if (RenderLayer* ancestor = ancestorStackingContext())
+        ancestor->updateDescendantsAreContiguousInStackingOrder();
+
     bool needsCompositedScrolling = false;
 
     FrameView* frameView = renderer()->view()->frameView();
@@ -2074,7 +2098,7 @@ void RenderLayer::updateNeedsCompositedScrolling()
 
         bool forceUseCompositedScrolling = acceleratedCompositingForOverflowScrollEnabled()
             && canBeStackingContainer()
-            && !hasOutOfFlowPositionedDescendant();
+            && !hasUnclippedDescendant();
 
 #if ENABLE(ACCELERATED_OVERFLOW_SCROLLING)
         needsCompositedScrolling = forceUseCompositedScrolling || renderer()->style()->useTouchOverflowScrolling();
@@ -2089,6 +2113,11 @@ void RenderLayer::updateNeedsCompositedScrolling()
             HistogramSupport::histogramEnumeration("Renderer.NeedsCompositedScrolling", needsCompositedScrolling, 2);
     }
 
+    setNeedsCompositedScrolling(needsCompositedScrolling);
+}
+
+void RenderLayer::setNeedsCompositedScrolling(bool needsCompositedScrolling)
+{
     if (m_needsCompositedScrolling == needsCompositedScrolling)
         return;
 
@@ -5616,6 +5645,7 @@ void RenderLayer::dirtyZOrderLists()
     m_descendantsAreContiguousInStackingOrderDirty = true;
 
     if (!renderer()->documentBeingDestroyed()) {
+        compositor()->setNeedsUpdateCompositingRequirementsState();
         compositor()->setCompositingLayersNeedRebuild();
         if (acceleratedCompositingForOverflowScrollEnabled())
             compositor()->setShouldReevaluateCompositingAfterLayout();
@@ -5754,7 +5784,6 @@ void RenderLayer::collectLayers(bool includeHiddenLayers, CollectLayersBehavior
 
 void RenderLayer::updateLayerListsIfNeeded()
 {
-    bool shouldUpdateDescendantsAreContiguousInStackingOrder = acceleratedCompositingForOverflowScrollEnabled() && isStackingContext() && (m_zOrderListsDirty || m_normalFlowListDirty) && m_descendantsAreContiguousInStackingOrderDirty;
     updateZOrderLists();
     updateNormalFlowList();
 
@@ -5762,14 +5791,6 @@ void RenderLayer::updateLayerListsIfNeeded()
         reflectionLayer->updateZOrderLists();
         reflectionLayer->updateNormalFlowList();
     }
-
-    if (shouldUpdateDescendantsAreContiguousInStackingOrder) {
-        updateDescendantsAreContiguousInStackingOrder();
-        // The above function can cause us to update m_needsCompositedScrolling
-        // and dirty our layer lists. Refresh them if necessary.
-        updateZOrderLists();
-        updateNormalFlowList();
-    }
 }
 
 void RenderLayer::repaintIncludingDescendants()
@@ -5942,14 +5963,8 @@ bool RenderLayer::isVisuallyNonEmpty() const
 
 void RenderLayer::updateVisibilityAfterStyleChange(const RenderStyle* oldStyle)
 {
-    EVisibility oldVisibility = oldStyle ? oldStyle->visibility() : VISIBLE;
-    if (oldVisibility == renderer()->style()->visibility() || !renderer()->isOutOfFlowPositioned())
-        return;
-
-    if (renderer()->style()->visibility() == VISIBLE)
-        setAncestorChainHasOutOfFlowPositionedDescendant(renderer()->containingBlock());
-    else
-        dirtyAncestorChainHasOutOfFlowPositionedDescendantStatus();
+    if (!oldStyle || (oldStyle->visibility() != renderer()->style()->visibility()))
+        compositor()->setNeedsUpdateCompositingRequirementsState();
 }
 
 void RenderLayer::updateStackingContextsAfterStyleChange(const RenderStyle* oldStyle)
@@ -5971,7 +5986,7 @@ void RenderLayer::updateStackingContextsAfterStyleChange(const RenderStyle* oldS
     else
         clearZOrderLists();
 
-    updateNeedsCompositedScrolling();
+    compositor()->setNeedsUpdateCompositingRequirementsState();
 }
 
 static bool overflowRequiresScrollbar(EOverflow overflow)
@@ -6020,45 +6035,34 @@ void RenderLayer::updateScrollbarsAfterStyleChange(const RenderStyle* oldStyle)
         updateScrollableAreaSet(hasScrollableHorizontalOverflow() || hasScrollableVerticalOverflow());
 }
 
-void RenderLayer::setAncestorChainHasOutOfFlowPositionedDescendant(RenderObject* containingBlock)
+void RenderLayer::updateOutOfFlowPositioned(const RenderStyle* oldStyle)
 {
-    for (RenderLayer* layer = this; layer; layer = layer->parent()) {
-        if (!layer->m_hasOutOfFlowPositionedDescendantDirty && layer->hasOutOfFlowPositionedDescendant())
-            break;
+    if (oldStyle && (renderer()->style()->position() == oldStyle->position()))
+        return;
 
-        layer->m_hasOutOfFlowPositionedDescendantDirty = false;
-        layer->m_hasOutOfFlowPositionedDescendant = true;
-        layer->updateNeedsCompositedScrolling();
+    bool wasOutOfFlowPositioned = oldStyle && (oldStyle->position() == AbsolutePosition || oldStyle->position() == FixedPosition);
+    bool isOutOfFlowPositioned = renderer()->isOutOfFlowPositioned();
+    if (!wasOutOfFlowPositioned && !isOutOfFlowPositioned)
+        return;
 
-        if (layer->renderer() && layer->renderer() == containingBlock)
-            break;
-    }
-}
+    // Even if the layer remains out-of-flow, a change to this property
+    // will likely change its containing block. We must clear these bits
+    // so that they can be set properly by the RenderLayerCompositor.
+    for (RenderLayer* ancestor = parent(); ancestor; ancestor = ancestor->parent())
+        ancestor->m_hasUnclippedDescendant = false;
 
-void RenderLayer::dirtyAncestorChainHasOutOfFlowPositionedDescendantStatus()
-{
-    if (m_hasOutOfFlowPositionedDescendant) {
-        m_hasOutOfFlowPositionedDescendantDirty = true;
-        // FIXME It would be nice to avoid this when we clean up render layer
-        // updating. We shouldn't have to update the composited scrolling state
-        // nearly as frequently if all the updates happen in a single, well
-        // defined phase.
-        updateNeedsCompositedScrolling();
-    }
+    // Ensures that we reset the above bits correctly.
+    compositor()->setNeedsUpdateCompositingRequirementsState();
 
-    if (parent())
-        parent()->dirtyAncestorChainHasOutOfFlowPositionedDescendantStatus();
-}
+    if (wasOutOfFlowPositioned && isOutOfFlowPositioned)
+        return;
 
-void RenderLayer::updateOutOfFlowPositioned(const RenderStyle* oldStyle)
-{
-    bool wasOutOfFlowPositioned = oldStyle && (oldStyle->position() == AbsolutePosition || oldStyle->position() == FixedPosition);
-    bool isOutOfFlowPositioned = renderer()->isOutOfFlowPositioned();
-    if (parent() && isOutOfFlowPositioned != wasOutOfFlowPositioned) {
-        if (isOutOfFlowPositioned)
-            parent()->setAncestorChainHasOutOfFlowPositionedDescendant(renderer()->containingBlock());
-        else
-            parent()->dirtyAncestorChainHasOutOfFlowPositionedDescendantStatus();
+    if (isOutOfFlowPositioned) {
+        setAncestorChainHasOutOfFlowPositionedDescendant();
+        compositor()->addOutOfFlowPositionedLayer(this);
+    } else {
+        dirtyAncestorChainHasSelfPaintingLayerDescendantStatus();
+        compositor()->removeOutOfFlowPositionedLayer(this);
     }
 }
 
@@ -6211,13 +6215,13 @@ void RenderLayer::updateScrollableAreaSet(bool hasOverflow)
         isVisibleToHitTest &= owner->renderer() && owner->renderer()->visibleToHitTesting();
 
     bool updatedScrollableAreaSet = false;
-    if (hasOverflow && isVisibleToHitTest)
+    if (hasOverflow && isVisibleToHitTest) {
         updatedScrollableAreaSet = frameView->addScrollableArea(this);
-    else
+        compositor()->setNeedsUpdateCompositingRequirementsState();
+    } else {
         updatedScrollableAreaSet = frameView->removeScrollableArea(this);
-
-    if (updatedScrollableAreaSet)
-        updateNeedsCompositedScrolling();
+        setNeedsCompositedScrolling(false);
+    }
 }
 
 void RenderLayer::updateScrollCornerStyle()