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..771cd4f51843ab059f7e70e655bd252e781a22e2 100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -1135,21 +1135,31 @@ RenderLayer* RenderLayer::enclosingCompositingLayerForRepaint(IncludeSelfOrNot i
 
 RenderLayer* RenderLayer::ancestorCompositedScrollingLayer() const
 {
-    ASSERT(isAllowedToQueryCompositingState());
+    ASSERT(isInCompositingUpdate());

[Ian Vollick, 2014/03/20 19:26:48]

Changed the assert to enforce access only during comp-update.

 
     if (!renderer()->acceleratedCompositingForOverflowScrollEnabled())
         return 0;
 
+    if (m_ancestorDependentPropertyCache && m_ancestorDependentPropertyCache->containingBlock)
+        return m_ancestorDependentPropertyCache->ancestorCompositedScrollingLayer;

[Ian Vollick, 2014/03/20 19:26:48]

The old code made no use of the containing block. Use it here to avoid
recomputing the CB when the cache is fresh.

+
     RenderObject* containingBlock = renderer()->containingBlock();
     if (!containingBlock)
         return 0;
 
+    const_cast<RenderLayer*>(this)->m_ancestorDependentPropertyCache = adoptPtr(new AncestorDependentPropertyCache());

[abarth-chromium, 2014/03/20 20:04:54]

The pattern we usually use for this is a function called

ensureAncestorDependentPropertyCache()

That does this work.  You can mark m_ancestorDependentPropertyCache mutable to
remove the const_cast.  Then you can put the ASSERT(isInCompositingUpdate())
inside that function.

[Ian Vollick, 2014/03/20 21:32:21]

Done.

+    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;
+    m_ancestorDependentPropertyCache->ancestorCompositedScrollingLayer = ancestorCompositedScrollingLayer;
+    return ancestorCompositedScrollingLayer;
 }
 
 RenderLayer* RenderLayer::ancestorScrollingLayer() const
@@ -1669,6 +1679,8 @@ void RenderLayer::convertToLayerCoords(const RenderLayer* ancestorLayer, LayoutR
 
 RenderLayer* RenderLayer::scrollParent() const
 {
+    ASSERT(isInCompositingUpdate());

[Ian Vollick, 2014/03/20 19:26:48]

Again, enforce that we access scrollParent only during comp-update.

+
     if (!renderer()->compositorDrivenAcceleratedScrollingEnabled())
         return 0;
 
@@ -1689,8 +1701,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)
+        return m_ancestorDependentPropertyCache->scrollParent;

[abarth-chromium, 2014/03/20 20:04:54]

Isn't it possible for m_ancestorDependentPropertyCache to be created without
scrollParent being populated?  It seems like we need to check something more
here.

[Ian Vollick, 2014/03/20 21:32:21]

It is possible, yes. If we've computed a valid containingBlock, the scrollParent
should also be valid. I've added this check.

 
+    RenderLayer* scrollParent = ancestorCompositedScrollingLayer();
     if (!scrollParent || scrollParent->stackingNode()->isStackingContainer())
         return 0;
 
@@ -1699,8 +1713,15 @@ RenderLayer* RenderLayer::scrollParent() const
     for (RenderLayer* ancestor = parent(); ancestor && ancestor != scrollParent; ancestor = ancestor->parent()) {
         if (ancestor->stackingNode()->isStackingContainer())
             return 0;
+        if (AncestorDependentPropertyCache* ancestorCache = ancestor->m_ancestorDependentPropertyCache.get()) {
+            if (ancestorCache->ancestorCompositedScrollingLayer == scrollParent) {
+                scrollParent = ancestorCache->scrollParent;
+                break;
+            }
+        }
     }
 
+    m_ancestorDependentPropertyCache->scrollParent = scrollParent;
     return scrollParent;
 }
 
@@ -3520,6 +3541,13 @@ bool RenderLayer::isAllowedToQueryCompositingState() const
     return renderer()->document().lifecycle().state() >= DocumentLifecycle::InCompositingUpdate;
 }
 
+bool RenderLayer::isInCompositingUpdate() const
+{
+    if (gCompositingQueryMode == CompositingQueriesAreAllowed)
+        return true;

[abarth-chromium, 2014/03/20 20:04:54]

Doesn't this mean these functions could be called while assert disablers are on
the stack?  In that case, the cache could be stale, right?

[Ian Vollick, 2014/03/20 21:32:21]

Removed the ability to disable this. Since scrollParent() is accessed in the
wrong phase due to chicken and egg, I've also updated the code so as not to
memoize unless we're in the correct phase. I've also added ASSERTs to ensure
that if we even have a non-NULL cache, that we're in the comp-update phase.

+    return renderer()->document().lifecycle().state() == DocumentLifecycle::InCompositingUpdate;
+}
+
 CompositedLayerMappingPtr RenderLayer::compositedLayerMapping() const
 {
     ASSERT(isAllowedToQueryCompositingState());
@@ -4059,6 +4087,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