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 a89ffe2b63c98cc73973e47ea679af86463c81c0..e2f6d939ad4a495e9c706ba2eb7a2624ca80cf07 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -195,6 +195,7 @@ void FrameView::reset()
     m_contentIsOpaque = false;
     m_hasPendingLayout = false;
     m_layoutSubtreeRoot = 0;
+    m_paintInvalidationSubtreeRoot = 0;
     m_doFullPaintInvalidation = false;
     m_layoutSchedulingEnabled = true;
     m_inPerformLayout = false;
@@ -958,6 +959,7 @@ void FrameView::layout(bool allowSubtree)
         performLayout(rootForThisLayout, inSubtreeLayout);
 
         m_layoutSubtreeRoot = 0;
+        m_paintInvalidationSubtreeRoot = rootForThisLayout;

[abarth-chromium, 2014/06/24 22:57:26]

What happens if there are multiple layouts between two compositing updates? 
Will the last one overwrite this value and cause us to potentially miss
invalidations?

[Julien - ping for review, 2014/06/25 22:34:30]

That's indeed a bug. I will fix it and land a test case for that.

     } // Reset m_layoutSchedulingEnabled to its previous value.
 
     if (!inSubtreeLayout && !toRenderView(rootForThisLayout)->document().printing())
@@ -990,10 +992,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 +1011,19 @@ 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);
+    if (!m_paintInvalidationSubtreeRoot)
+        return;
 
-    TRACE_EVENT1("blink", "FrameView::invalidateTree", "root", root->debugName().ascii());
+    RenderObject* rootForPaintInvalidation = m_paintInvalidationSubtreeRoot;
+    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 +1031,9 @@ void FrameView::invalidateTree(RenderObject* root)
     if (hasHorizontalBarDamage())
         invalidateRect(horizontalBarDamage());
     resetScrollbarDamage();
+
+    m_paintInvalidationSubtreeRoot = 0;
+    m_doFullPaintInvalidation = false;
 }
 
 DocumentLifecycle& FrameView::lifecycle() const
@@ -2828,6 +2826,8 @@ void FrameView::updateLayoutAndStyleForPainting()
             m_frame->page()->scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
 
         InspectorInstrumentation::didUpdateLayerTree(view->frame());
+
+        invalidateTreeIfNeededRecursive();
     }
 
     scrollContentsIfNeededRecursive();
@@ -2883,6 +2883,19 @@ void FrameView::updateLayoutAndStyleIfNeededRecursive()
 
 }
 
+void FrameView::invalidateTreeIfNeededRecursive()
+{
+    // FIXME: We should be more aggressive cutting sub-branches 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());