Defer the real work in updateCompositingLayers until it's really needed.

Source/core/rendering/RenderLayerCompositor.cpp

Index: Source/core/rendering/RenderLayerCompositor.cpp
diff --git a/Source/core/rendering/RenderLayerCompositor.cpp b/Source/core/rendering/RenderLayerCompositor.cpp
index 1ea9f0d6cac234fff5d3b12110d82d54db11b137..eaf9d325a3519f924f6a9859bacdc9701edaa4c1 100644
--- a/Source/core/rendering/RenderLayerCompositor.cpp
+++ b/Source/core/rendering/RenderLayerCompositor.cpp
@@ -225,7 +225,8 @@ RenderLayerCompositor::RenderLayerCompositor(RenderView* renderView)
     , m_compositingTriggers(static_cast<ChromeClient::CompositingTriggerFlags>(ChromeClient::AllTriggers))
     , m_compositedLayerCount(0)
     , m_showRepaintCounter(false)
-    , m_reevaluateCompositingAfterLayout(false)
+    , m_needsToRecomputeCompositingRequirements(false)
+    , m_needsToUpdateLayerTreeGeometry(false)
     , m_compositing(false)
     , m_compositingLayersNeedRebuild(false)
     , m_forceCompositingMode(false)
@@ -380,42 +381,52 @@ void RenderLayerCompositor::updateCompositingLayers(CompositingUpdateType update
     if (m_forceCompositingMode && !m_compositing)
         enableCompositingMode(true);
 
-    if (!m_reevaluateCompositingAfterLayout && !m_compositing)
+    if (!m_needsToRecomputeCompositingRequirements && !m_compositing)
         return;
 
     AnimationUpdateBlock animationUpdateBlock(m_renderView->frameView()->frame().animation());
 
     TemporaryChange<bool> postLayoutChange(m_inPostLayoutUpdate, true);
 
-    bool checkForHierarchyUpdate = m_reevaluateCompositingAfterLayout;
+    bool checkForHierarchyUpdate = false;
     bool needGeometryUpdate = false;
 
+    // CompositingUpdateOnBeginDrawingFrame is the only updateType that will actually do any work in this function.
+    // All other updateTypes will simply mark that something needed updating, and defer the actual update.
+    // This way we only need to compute all compositing state once for every frame drawn (if needed).

[Ian Vollick, 2013/10/08 18:32:21]

I love this. :)

     switch (updateType) {
     case CompositingUpdateAfterStyleChange:
     case CompositingUpdateAfterLayout:
-        checkForHierarchyUpdate = true;
+        m_needsToRecomputeCompositingRequirements = true;
         break;
     case CompositingUpdateOnScroll:
-        checkForHierarchyUpdate = true; // Overlap can change with scrolling, so need to check for hierarchy updates.
-        needGeometryUpdate = true;
+        m_needsToRecomputeCompositingRequirements = true; // Overlap can change with scrolling, so need to check for hierarchy updates.
+        m_needsToUpdateLayerTreeGeometry = true;
         break;
     case CompositingUpdateOnCompositedScroll:
-        needGeometryUpdate = true;
+        m_needsToUpdateLayerTreeGeometry = true;
+        break;
+    case CompositingUpdateOnBeginDrawingFrame:
+        checkForHierarchyUpdate = m_needsToRecomputeCompositingRequirements;
+        needGeometryUpdate = m_needsToUpdateLayerTreeGeometry;
         break;
     }
 
+    // FIXME: why isn't needHierarchyUpdate part of this?

[Ian Vollick, 2013/10/08 18:32:21]

Whoa. No good reason I can see. If we need to rebuild the composited layer tree,
we'd better not bail, right?

     if (!checkForHierarchyUpdate && !needGeometryUpdate)
         return;
 
     bool needHierarchyUpdate = m_compositingLayersNeedRebuild;
     bool isFullUpdate = !updateRoot;
 
-    // Only clear the flag if we're updating the entire hierarchy.
+    // Only clear the flags if we're updating the entire hierarchy.
     m_compositingLayersNeedRebuild = false;
+    m_needsToUpdateLayerTreeGeometry = false;
     updateRoot = rootRenderLayer();
 
-    if (isFullUpdate && updateType == CompositingUpdateAfterLayout)
-        m_reevaluateCompositingAfterLayout = false;
+    // FIXME, I think we can remove this if-condition?

[Ian Vollick, 2013/10/08 18:32:21]

I don't think so. If the update root is non-NULL, then I think there be layers
in the tree we don't visit when computing compositing requirements (and will
therefore have stale state).

+    if (isFullUpdate)
+        m_needsToRecomputeCompositingRequirements = false;
 
 #if !LOG_DISABLED
     double startTime = 0;
@@ -1932,7 +1943,8 @@ bool RenderLayerCompositor::requiresCompositingForPlugin(RenderObject* renderer)
     if (!composite)
         return false;
 
-    m_reevaluateCompositingAfterLayout = true;
+    // FIXME: this seems bogus. If we don't know the layout position/size of the plugin yet, would't that be handled elsewhere?

[Ian Vollick, 2013/10/08 18:32:21]

Totally fishy. Doesn't this mean that if we have a potentially composited
plugin, we'll _always_ need to recompute compositing requirements?

If, as you suggested offline, the original intent of this code had been to
indicate that we need to defer our determination here until post-layer, could we
remove this and the similar deferrals below (since they'll all happen
post-layout)?

[Vangelis Kokkevis, 2013/10/08 23:22:33]

I think the reason this code existed is that there was a special condition to
not switch to compositing mode for zero-sized plugins. Back in the day sites
were using Flash for audio and WebKit didn't want to incur the cost of switching
over to accelerated compositing for it. Maybe we could special-case it for
non-FCM or remove it altogether.

+    m_needsToRecomputeCompositingRequirements = true;
 
     RenderWidget* pluginRenderer = toRenderWidget(renderer);
     // If we can't reliably know the size of the plugin yet, don't change compositing state.
@@ -1954,7 +1966,8 @@ bool RenderLayerCompositor::requiresCompositingForFrame(RenderObject* renderer)
     if (!frameRenderer->requiresAcceleratedCompositing())
         return false;
 
-    m_reevaluateCompositingAfterLayout = true;
+    // FIXME: this seems bogus. If we don't know the layout position/size of the frame yet, wouldn't that be handled elsehwere?

[Ian Vollick, 2013/10/08 18:32:21]

Ditto.

+    m_needsToRecomputeCompositingRequirements = true;
 
     RenderLayerCompositor* innerCompositor = frameContentsCompositor(frameRenderer);
     if (!innerCompositor)
@@ -2091,7 +2104,7 @@ bool RenderLayerCompositor::requiresCompositingForPosition(RenderObject* rendere
     RenderObject* container = renderer->container();
     // If the renderer is not hooked up yet then we have to wait until it is.
     if (!container) {
-        m_reevaluateCompositingAfterLayout = true;
+        m_needsToRecomputeCompositingRequirements = true;
         return false;
     }
 
@@ -2131,7 +2144,7 @@ bool RenderLayerCompositor::requiresCompositingForPosition(RenderObject* rendere
 
     // Subsequent tests depend on layout. If we can't tell now, just keep things the way they are until layout is done.
     if (!m_inPostLayoutUpdate) {
-        m_reevaluateCompositingAfterLayout = true;
+        m_needsToRecomputeCompositingRequirements = true;
         return layer->compositedLayerMapping();
     }
 
@@ -2150,7 +2163,7 @@ bool RenderLayerCompositor::requiresCompositingForPosition(RenderObject* rendere
         if (!viewBounds.intersects(enclosingIntRect(layerBounds))) {
             if (viewportConstrainedNotCompositedReason) {
                 *viewportConstrainedNotCompositedReason = RenderLayer::NotCompositedForBoundsOutOfView;
-                m_reevaluateCompositingAfterLayout = true;
+                m_needsToRecomputeCompositingRequirements = true;
             }
             return false;
         }