Move paint invalidation after compositing update

Source/core/frame/FrameView.cpp

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Dirk Mueller <mueller@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
@@ skipping 940 matching lines @@
             // We need to set m_doFullPaintInvalidation before triggering layout as RenderObject::checkForPaintInvalidation
             // checks the boolean to disable local paint invalidations.
             m_doFullPaintInvalidation |= renderView()->shouldDoFullRepaintForNextLayout();
         }
 
         layer = rootForThisLayout->enclosingLayer();
 
         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())
         adjustViewSize();
 
     layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, inSubtreeLayout, m_doFullPaintInvalidation));
     renderView()->compositor()->didLayout();
 
     m_layoutCount++;
 
@@ skipping 12 matching lines @@
     scheduleOrPerformPostLayoutTasks();
 
     TRACE_EVENT_END1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "Layout", "endData", InspectorLayoutEvent::endData(rootForThisLayout));
     // FIXME(361045): remove InspectorInstrumentation calls once DevTools Timeline migrates to tracing.
     InspectorInstrumentation::didLayout(cookie, rootForThisLayout);
 
     m_nestedLayoutCount--;
     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();
 #endif
 
     // FIXME: It should be not possible to remove the FrameView from the frame/page during layout
     // however m_inPerformLayout is not set for most of this function, so none of our RELEASE_ASSERTS
     // in LocalFrame/Page will fire. One of the post-layout tasks is disconnecting the LocalFrame from
     // the page in fast/frames/crash-remove-iframe-during-object-beforeload-2.html
     // necessitating this check here.
     // ASSERT(frame()->page());
     if (frame().page())
         frame().page()->chrome().client().layoutUpdated(m_frame.get());
 }
 
 // The plan is to move to compositor-queried paint invalidation, in which case this
 // 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);
+    RenderObject* rootForPaintInvalidation = renderView();
+    ASSERT(!rootForPaintInvalidation->needsLayout());
 
-    TRACE_EVENT1("blink", "FrameView::invalidateTree", "root", root->debugName().ascii());
+    TRACE_EVENT1("blink", "FrameView::invalidateTree", "root", rootForPaintInvalidation->debugName().ascii());
 
-    // 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;
+    LayoutState rootLayoutState(*rootForPaintInvalidation);
 
-    LayoutState rootLayoutState(*root);
-
-    root->invalidateTreeAfterLayout(*root->containerForPaintInvalidation());
+    rootForPaintInvalidation->invalidateTreeAfterLayout(*rootForPaintInvalidation->containerForPaintInvalidation());
 
     // Invalidate the paint of the frameviews scrollbars if needed
     if (hasVerticalBarDamage())
         invalidateRect(verticalBarDamage());
     if (hasHorizontalBarDamage())
         invalidateRect(horizontalBarDamage());
     resetScrollbarDamage();
+
+    m_doFullPaintInvalidation = false;
 }
 
 DocumentLifecycle& FrameView::lifecycle() const
 {
     return m_frame->document()->lifecycle();
 }
 
 void FrameView::gatherDebugLayoutRects(RenderObject* layoutRoot)
 {
     bool isTracing;
@@ skipping 1770 matching lines @@
 
     if (RenderView* view = renderView()) {
         InspectorInstrumentation::willUpdateLayerTree(view->frame());
 
         view->compositor()->updateIfNeededRecursive();
 
         if (view->compositor()->inCompositingMode() && m_frame->isMainFrame())
             m_frame->page()->scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
 
         InspectorInstrumentation::didUpdateLayerTree(view->frame());
+
+        invalidateTreeIfNeededRecursive();
     }
 
     scrollContentsIfNeededRecursive();
 }
 
 void FrameView::updateLayoutAndStyleIfNeededRecursive()
 {
     // We have to crawl our entire tree looking for any FrameViews that need
     // layout and make sure they are up to date.
     // Mac actually tests for intersection with the dirty region and tries not to
@@ skipping 35 matching lines @@
 
     // These asserts ensure that parent frames are clean, when child frames finished updating layout and style.
     ASSERT(!needsLayout());
     ASSERT(!m_frame->document()->hasSVGFilterElementsRequiringLayerUpdate());
 #ifndef NDEBUG
     m_frame->document()->renderView()->assertRendererLaidOut();
 #endif
 
 }
 
+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());
     ASSERT(minSize.width() <= maxSize.width());
     ASSERT(minSize.height() <= maxSize.height());
 
     if (m_shouldAutoSize == enable && m_minAutoSize == minSize && m_maxAutoSize == maxSize)
         return;
 
 
@@ skipping 371 matching lines @@
 void FrameView::willRemoveScrollbar(Scrollbar* scrollbar, ScrollbarOrientation orientation)
 {
     ScrollableArea::willRemoveScrollbar(scrollbar, orientation);
     if (AXObjectCache* cache = axObjectCache()) {
         cache->remove(scrollbar);
         cache->handleScrollbarUpdate(this);
     }
 }
 
 } // namespace WebCore