Move paint invalidation after compositing update

Source/core/frame/FrameView.cpp

Index: Source/core/frame/FrameView.cpp
diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp
index 3410bfce246a70beae4c21c393caadb42315efa9..cb6fc35b3449a99088403719b0bc4461d54bfee1 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -958,6 +958,12 @@ void FrameView::layout(bool allowSubtree)
         performLayout(rootForThisLayout, inSubtreeLayout);
 
         m_layoutSubtreeRoot = 0;
+        // The following loop ensures that we mark up all renderers up to the RenderView
+        // for paint invalidation. This simplifies our code as we just always do a full
+        // tree walk. FIXME: This loop will probably make us over-mark as we don't skip
+        // containers and we can't use containingBlock as it skips some cases (see markContainingBlocksForLayout).
+        for (RenderObject* container = rootForThisLayout->container(); container; container = container->container())
+            container->setMayNeedPaintInvalidation(true);

[abarth-chromium, 2014/07/01 20:02:19]

    void setMayNeedPaintInvalidation(bool b)
    {
        m_bitfields.setMayNeedPaintInvalidation(b);

        // Make sure our parent is marked as needing invalidation.
        if (b && parent() && !parent()->mayNeedPaintInvalidation())
            parent()->setMayNeedPaintInvalidation(b);
    }

[esprehn, 2014/07/01 20:03:34]

This shouldn't be needed, setMayNeedPaintInvalidation already walks up through
all the parent()'s and the container() is going to be one of those.

[Julien - ping for review, 2014/07/01 21:44:36]

I do need to call
rootForThisLayout->container()->setMayNeedPaintInvalidation(true) as we may not
have called it inside the subtree (and the containers' chain may not have been
marked for layout too). As you both pointed out, we don't need the for loop
though. Here is some better code that I will use for the final patch:

if (RenderObject* container = rootForThisLayout->container())
   container->setMayNeedPaintInvalidation(true);

     } // Reset m_layoutSchedulingEnabled to its previous value.
 
     if (!inSubtreeLayout && !toRenderView(rootForThisLayout)->document().printing())
@@ -990,10 +996,6 @@ void FrameView::layout(bool allowSubtree)
     if (m_nestedLayoutCount)
         return;
 
-    invalidateTree(rootForThisLayout);
-
-    m_doFullPaintInvalidation = false;
-
 #ifndef NDEBUG
     // Post-layout assert that nobody was re-marked as needing layout during layout.
     document->renderView()->assertSubtreeIsLaidOut();
@@ -1013,22 +1015,16 @@ void FrameView::layout(bool allowSubtree)
 // method would setNeedsRedraw on the GraphicsLayers with invalidations and
 // let the compositor pick which to actually draw.
 // See http://crbug.com/306706
-void FrameView::invalidateTree(RenderObject* root)
+void FrameView::invalidateTreeIfNeeded()
 {
-    ASSERT(!root->needsLayout());
-    // We should only invalidate paints for the outer most layout. This works as
-    // we continue to track paint invalidation rects until this function is called.
-    ASSERT(!m_nestedLayoutCount);
-
-    TRACE_EVENT1("blink", "FrameView::invalidateTree", "root", root->debugName().ascii());
+    RenderObject* rootForPaintInvalidation = renderView();
+    ASSERT(!rootForPaintInvalidation->needsLayout());
 
-    // FIXME: really, we're in the paint invalidation phase here, and the compositing queries are legal.
-    // Until those states are fully fledged, I'll just disable the ASSERTS.
-    DisableCompositingQueryAsserts compositingQueryAssertsDisabler;
+    TRACE_EVENT1("blink", "FrameView::invalidateTree", "root", rootForPaintInvalidation->debugName().ascii());
 
-    LayoutState rootLayoutState(*root);
+    LayoutState rootLayoutState(*rootForPaintInvalidation);
 
-    root->invalidateTreeAfterLayout(*root->containerForPaintInvalidation());
+    rootForPaintInvalidation->invalidateTreeAfterLayout(*rootForPaintInvalidation->containerForPaintInvalidation());
 
     // Invalidate the paint of the frameviews scrollbars if needed
     if (hasVerticalBarDamage())
@@ -1036,6 +1032,8 @@ void FrameView::invalidateTree(RenderObject* root)
     if (hasHorizontalBarDamage())
         invalidateRect(horizontalBarDamage());
     resetScrollbarDamage();
+
+    m_doFullPaintInvalidation = false;
 }
 
 DocumentLifecycle& FrameView::lifecycle() const
@@ -2826,6 +2824,8 @@ void FrameView::updateLayoutAndStyleForPainting()
             m_frame->page()->scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
 
         InspectorInstrumentation::didUpdateLayerTree(view->frame());
+
+        invalidateTreeIfNeededRecursive();
     }
 
     scrollContentsIfNeededRecursive();
@@ -2881,6 +2881,19 @@ void FrameView::updateLayoutAndStyleIfNeededRecursive()
 
 }
 
+void FrameView::invalidateTreeIfNeededRecursive()
+{
+    // FIXME: We should be more aggressive at cutting tree traversals.
+    invalidateTreeIfNeeded();
+
+    for (Frame* child = m_frame->tree().firstChild(); child; child = child->tree().nextSibling()) {
+        if (!child->isLocalFrame())
+            continue;
+
+        toLocalFrame(child)->view()->invalidateTreeIfNeededRecursive();
+    }
+}
+
 void FrameView::enableAutoSizeMode(bool enable, const IntSize& minSize, const IntSize& maxSize)
 {
     ASSERT(!enable || !minSize.isEmpty());