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 934 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

[ojan, 2014/07/02 03:33:58]

This comment is out of data now that this isn't a loop anymore.

[Julien - ping for review, 2014/07/02 18:27:06]

oops, got catch. I have amended the comment to avoid mentioning the loop. Also
removed the comment as it's a more general issue.

+        // 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).
+        if (RenderObject* container = rootForThisLayout->container())
+            container->setMayNeedPaintInvalidation(true);

[ojan, 2014/07/02 03:33:58]

Why not just have this be rootForThisLayout->setMayNeedPaintInvalidation(true)?

[Julien - ping for review, 2014/07/02 18:27:06]

It's not needed to mark rootForThisLayout for mayNeedPaintInvalidation as it was
laid out and it's in this spirit that I have started at the container. Marking
it with setMayNeedPaintInvalidation will skip some optimization we have now so I
would rather not do it. Also conceptually it doesn't need it and
mayNeedPaintInvalidation is a giant hammer we have to force paint invalidation
on a renderer.

     } // 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