update3dRenderingContext is 13.5% of GraphicsLayerUpdater::rebuildTree

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 20f618d211c61b254a05595db9ba40aaa1bb68a3..73d92be2aaaeac404f7d803b5af959360a795885 100644
--- a/Source/core/rendering/RenderLayer.cpp
+++ b/Source/core/rendering/RenderLayer.cpp
@@ -136,7 +136,6 @@ RenderLayer::RenderLayer(RenderLayerModelObject* renderer, LayerType type)
     , m_staticInlinePosition(0)
     , m_staticBlockPosition(0)
     , m_enclosingPaginationLayer(0)
-    , m_3dRenderingContextRoot(0)
     , m_groupedMapping(0)
     , m_repainter(renderer)
     , m_clipper(renderer)
@@ -562,20 +561,20 @@ void RenderLayer::updateTransform()
         dirty3DTransformedDescendantStatus();
 }
 
-// Note: this function assumes that all ancestors have an updated 3d rendering context root.
-void RenderLayer::update3dRenderingContext()
+RenderLayer* RenderLayer::renderingContextRoot()
 {
-    m_3dRenderingContextRoot = 0;
-
     if (!shouldFlattenTransform())

[esprehn, 2014/03/25 02:39:35]

I don't think this is correct. The previous code would reset if this was false
here, but then still look at the ancestor below.

[abarth-chromium, 2014/03/25 02:59:39]

I see.  Yes, I screwed this up.  I'll work on a test case that would have caught
this issue tomorrow.

Thanks!

[ojan, 2014/03/25 04:16:27]

Ick. Good catch. The next question is why this code is this way? This seems
super weird to me. 

[Ian Vollick, 2014/03/25 14:40:01]

Some possibly-useful background: the code was set up like this to enforce these
3d rendering context rules:
 - If your parent doesn't flatten the transform, you're in its 3d rendering
context, so you need to have its id. (this is what the 2nd if block is for --
and it's crucial that the parent's data be up to date if you want to use its
values rather than walking all the way up the tree).
 - Else, if you don't flatten the transform, you establish a new 3d rendering
context, so you get a new id. (that's what the 1st if block is for).
 - Otherwise, you're not in a 3d rendering context, so your id is zero.

The code could likely have been laid out more clearly, but that was the intent.

-        m_3dRenderingContextRoot = this;
+        return this;
 
+    // FIXME: Why doesn't this need to be a loop?

[Ian Vollick, 2014/03/25 14:40:01]

The 3d rendering context was meant to bubble down. The code was set up so that
values on the parent were always updated before visiting the children. The
children could then grab the bubbled values from their parent without ascending
higher in the tree.

This was what I was trying to communicate (probably badly) with the deleted
comment above this function.

     if (RenderObject* containingBlock = renderer()->containingBlock()) {
         if (RenderLayer* ancestorLayer = containingBlock->enclosingLayer()) {
             if (!ancestorLayer->shouldFlattenTransform())
-                m_3dRenderingContextRoot = ancestorLayer->renderingContextRoot();
+                return ancestorLayer;
         }
     }
+
+    return 0;
 }
 
 TransformationMatrix RenderLayer::currentTransform(RenderStyle::ApplyTransformOrigin applyOrigin) const