[SPInvalidation] Ensure paint invalidation flags are cleared

third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/paint/PrePaintTreeWalk.h"
 
 #include "core/dom/DocumentLifecycle.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/layout/LayoutMultiColumnSpannerPlaceholder.h"
@@ skipping 26 matching lines @@
 
 void PrePaintTreeWalk::walk(FrameView& frameView,
                             const PrePaintTreeWalkContext& context) {
   if (frameView.shouldThrottleRendering())
     return;
 
   PrePaintTreeWalkContext localContext(context);
   m_propertyTreeBuilder.buildTreeNodes(frameView,
                                        localContext.treeBuilderContext);
 
-  if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
+  if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled()) {
     m_paintInvalidator.invalidatePaintIfNeeded(
         frameView, localContext.paintInvalidatorContext);
+  }
 
   if (LayoutView* layoutView = frameView.layoutView())
     walk(*layoutView, localContext);
+
+#if DCHECK_IS_ON()
+  if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
+    frameView.layoutView()->assertSubtreeClearedPaintInvalidationFlags();
+#endif
 }
 
 void PrePaintTreeWalk::walk(const LayoutObject& object,
                             const PrePaintTreeWalkContext& context) {
   PrePaintTreeWalkContext localContext(context);
 
   if (object.isLayoutMultiColumnSpannerPlaceholder()) {
     // Walk multi-column spanner as if it replaces the placeholder.
     // Set the flag so that the tree builder can specially handle out-of-flow
     // positioned descendants if their containers are between the multi-column
     // container and the spanner. See PaintPropertyTreeBuilder for details.
     localContext.treeBuilderContext.isUnderMultiColumnSpanner = true;
+    object.getMutableForPainting().clearPaintInvalidationFlags();

[chrishtr, 2016/10/11 22:12:42]

The incorrect flags would have been set by parents which are not really the
parents?

Also, could you point me to the corresponding code in SPv1?

[Xianzhu, 2016/10/11 22:37:21]

No, we just missed clearing paint invalidation flags of the placeholder.
In SPv1, we don't have special handling about placeholders and spanners. We just
walk them in normal layout tree order. For placeholder, the normal
LayoutObject::invalidatePaintIfNeeded() is called which issues no paint
invalidation and clear the paint invalidation flags. For spanner, we force slow
path rect mapping so don't need to care about the paint offset.

For slimmingPaintInvalidation, we need to follow the logical tree order
(placeholder -> spanner) and skip the placeholder to avoid the paint offset from
being calculated twice (because the placeholder and the spanner has the same
geometries, and the placeholder's parent is actually the logical parent of the
spanner). We should not skip clearing paint invalidation flags anyway.

     walk(*toLayoutMultiColumnSpannerPlaceholder(object)
               .layoutObjectInFlowThread(),
          localContext);
     return;
   }
 
   m_propertyTreeBuilder.buildTreeNodesForSelf(object,
                                               localContext.treeBuilderContext);
   if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled())
     m_paintInvalidator.invalidatePaintIfNeeded(
@@ skipping 18 matching lines @@
           widget->frameRect().location();
       localContext.treeBuilderContext.current.paintOffset =
           roundedIntPoint(localContext.treeBuilderContext.current.paintOffset);
       walk(*toFrameView(widget), localContext);
     }
     // TODO(pdr): Investigate RemoteFrameView (crbug.com/579281).
   }
 }
 
 }  // namespace blink