Avoid tree walks when computing RenderLayer::scrollParent

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 ef40889fb15879f48508ec0638e98382c9c74ccb..14e839b94de296470df144d1a8c095d615803d45 100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -1133,23 +1133,48 @@ RenderLayer* RenderLayer::enclosingCompositingLayerForRepaint(IncludeSelfOrNot i
     return 0;
 }
 
-RenderLayer* RenderLayer::ancestorCompositedScrollingLayer() const
+void RenderLayer::clearAncestorDependentPropertyCache()
 {
-    ASSERT(isAllowedToQueryCompositingState());
+    ASSERT(isInCompositingUpdate());
+    m_ancestorDependentPropertyCache.clear();
+}
 
+void RenderLayer::ensureAncestorDependentPropertyCache() const
+{
+    ASSERT(isInCompositingUpdate());
+    m_ancestorDependentPropertyCache = adoptPtr(new AncestorDependentPropertyCache());

[abarth-chromium, 2014/03/20 21:36:45]

Should we just return if the cache already exists?  In this version, we'll blow
it away if it already exists.

[Ian Vollick, 2014/03/20 21:57:00]

Done.

+}
+
+RenderLayer* RenderLayer::ancestorCompositedScrollingLayer() const
+{
     if (!renderer()->acceleratedCompositingForOverflowScrollEnabled())
         return 0;
 
+    ASSERT(isInCompositingUpdate() || !m_ancestorDependentPropertyCache);
+
+    if (m_ancestorDependentPropertyCache && m_ancestorDependentPropertyCache->containingBlock)
+        return m_ancestorDependentPropertyCache->ancestorCompositedScrollingLayer;
+
     RenderObject* containingBlock = renderer()->containingBlock();
     if (!containingBlock)
         return 0;
 
+    if (isInCompositingUpdate()) {
+        ensureAncestorDependentPropertyCache();
+        m_ancestorDependentPropertyCache->containingBlock = containingBlock;
+    }
+
+    RenderLayer* ancestorCompositedScrollingLayer = 0;
     for (RenderLayer* ancestorLayer = containingBlock->enclosingLayer(); ancestorLayer; ancestorLayer = ancestorLayer->parent()) {
-        if (ancestorLayer->needsCompositedScrolling())
-            return ancestorLayer;
+        if (ancestorLayer->needsCompositedScrolling()) {
+            ancestorCompositedScrollingLayer = ancestorLayer;
+            break;
+        }
     }
 
-    return 0;
+    if (m_ancestorDependentPropertyCache)
+        m_ancestorDependentPropertyCache->ancestorCompositedScrollingLayer = ancestorCompositedScrollingLayer;
+    return ancestorCompositedScrollingLayer;
 }
 
 RenderLayer* RenderLayer::ancestorScrollingLayer() const
@@ -1677,6 +1702,10 @@ RenderLayer* RenderLayer::scrollParent() const
     if (stackingNode()->isNormalFlowOnly())
         return 0;
 
+    // We should never have an ancestor dependent property cache outside of the
+    // compositing update phase.
+    ASSERT(isInCompositingUpdate() || !m_ancestorDependentPropertyCache);
+
     // A layer scrolls with its containing block. So to find the overflow scrolling layer
     // that we scroll with respect to, we must ascend the layer tree until we reach the
     // first overflow scrolling div at or above our containing block. I will refer to this
@@ -1689,8 +1718,10 @@ RenderLayer* RenderLayer::scrollParent() const
     // our scrolling ancestor, and we will therefore not scroll with it. In this case, we must
     // be a composited layer since the compositor will need to take special measures to ensure
     // that we scroll with our scrolling ancestor and it cannot do this if we do not promote.
-    RenderLayer* scrollParent = ancestorCompositedScrollingLayer();
+    if (m_ancestorDependentPropertyCache && m_ancestorDependentPropertyCache->containingBlock)

[abarth-chromium, 2014/03/20 21:36:45]

Why is checking for containingBlock correct here?  containingBlock could be
populated in RenderLayer::ancestorCompositedScrollingLayer, which doesn't set
scrollParent...

[Ian Vollick, 2014/03/20 21:57:00]

Oof. It isn't! It was only correct if you call scrollParent before
ancestorCompositedScrollLayer (if you did that, scrollParent would invoke
ancestorCompositedScrollLayer, indirectly creating the cache, and the rest of
the scrollParent function would ensure that the cached scrollParent was corret).
If you called ancestorCompositedScrollLayer first, then this is busted.

I was using containingBlock essentially as a dirty bit, but this is cryptic and
misleading. I've switched this to two, clearly named dirty bits.

Hopefully this time the dirty bit checking is correct. Thanks for catching this.

+        return m_ancestorDependentPropertyCache->scrollParent;
 
+    RenderLayer* scrollParent = ancestorCompositedScrollingLayer();
     if (!scrollParent || scrollParent->stackingNode()->isStackingContainer())
         return 0;
 
@@ -1699,8 +1730,19 @@ RenderLayer* RenderLayer::scrollParent() const
     for (RenderLayer* ancestor = parent(); ancestor && ancestor != scrollParent; ancestor = ancestor->parent()) {
         if (ancestor->stackingNode()->isStackingContainer())
             return 0;
+        if (!isInCompositingUpdate())
+            continue;
+        if (AncestorDependentPropertyCache* ancestorCache = ancestor->m_ancestorDependentPropertyCache.get()) {
+            if (ancestorCache->ancestorCompositedScrollingLayer == scrollParent) {
+                scrollParent = ancestorCache->scrollParent;
+                break;
+            }
+        }
     }
 
+    if (m_ancestorDependentPropertyCache)
+        m_ancestorDependentPropertyCache->scrollParent = scrollParent;
+
     return scrollParent;
 }
 
@@ -3520,6 +3562,11 @@ bool RenderLayer::isAllowedToQueryCompositingState() const
     return renderer()->document().lifecycle().state() >= DocumentLifecycle::InCompositingUpdate;
 }
 
+bool RenderLayer::isInCompositingUpdate() const
+{
+    return renderer()->document().lifecycle().state() == DocumentLifecycle::InCompositingUpdate;
+}
+
 CompositedLayerMappingPtr RenderLayer::compositedLayerMapping() const
 {
     ASSERT(isAllowedToQueryCompositingState());
@@ -4059,6 +4106,11 @@ DisableCompositingQueryAsserts::DisableCompositingQueryAsserts()
 
 COMPILE_ASSERT(1 << RenderLayer::ViewportConstrainedNotCompositedReasonBits >= RenderLayer::NumNotCompositedReasons, too_many_viewport_constrained_not_compositing_reasons);
 
+RenderLayer::AncestorDependentPropertyCache::AncestorDependentPropertyCache()
+    : containingBlock(0)
+    , ancestorCompositedScrollingLayer(0)
+    , scrollParent(0) { }
+
 } // namespace WebCore
 
 #ifndef NDEBUG