Skip paint property update and visual rect update if no geometry change

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

Index: third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
diff --git a/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp b/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
index bf339c75be25aa053e7c3c8cf958451a9c1f6024..4f463c5d721e75553b7999f0f666f31a50c90aac 100644
--- a/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
+++ b/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
@@ -49,8 +49,8 @@ void PrePaintTreeWalk::walk(FrameView& rootFrame) {
 
   // GeometryMapper depends on paint properties.
   if (rootFrame.needsPaintPropertyUpdate() ||
-      (rootFrame.layoutView() &&
-       !shouldEndWalkBefore(*rootFrame.layoutView(), initialContext)))
+      (rootFrame.layoutView() && needsWalkForPaintPropertyUpdate(
+                                     *rootFrame.layoutView(), initialContext)))
     m_geometryMapper.clearCache();
 
   walk(rootFrame, initialContext);
@@ -72,6 +72,10 @@ void PrePaintTreeWalk::walk(FrameView& frameView,
                                              context.paintInvalidatorContext);
 
   if (LayoutView* view = frameView.layoutView()) {
+#if DCHECK_IS_ON()
+    context.treeBuilderContext.updatedForSelf = true;
+    context.treeBuilderContext.updatedForChildren = true;
+#endif
     walk(*view, context);
 #if DCHECK_IS_ON()
     view->assertSubtreeClearedPaintInvalidationFlags();
@@ -143,6 +147,10 @@ void PrePaintTreeWalk::invalidatePaintLayerOptimizationsIfNeeded(
   if (!object.hasLayer())
     return;
 
+#if DCHECK_IS_ON()
+  DCHECK(context.treeBuilderContext.updatedForChildren);
+#endif
+
   PaintLayer& paintLayer = *toLayoutBoxModelObject(object).layer();
   if (object.styleRef().hasTransform() ||
       &object == context.paintInvalidatorContext.paintInvalidationContainer) {
@@ -215,27 +223,32 @@ void PrePaintTreeWalk::invalidatePaintLayerOptimizationsIfNeeded(
     // All subsequences which are contained below this paintLayer must also
     // be checked.
     context.paintInvalidatorContext.forcedSubtreeInvalidationFlags |=
-        PaintInvalidatorContext::ForcedSubtreeInvalidationRectUpdate;
+        PaintInvalidatorContext::ForcedSubtreeVisualRectUpdate;
   }
 
   paintLayer.setPreviousPaintingClipRects(*clipRects);
 }
 
-bool PrePaintTreeWalk::shouldEndWalkBefore(
+bool PrePaintTreeWalk::needsWalkForPaintPropertyUpdate(
     const LayoutObject& object,
     const PrePaintTreeWalkContext& context) {
-  return (
-      !object.needsPaintPropertyUpdate() &&
-      !object.descendantNeedsPaintPropertyUpdate() &&
-      !context.treeBuilderContext.forceSubtreeUpdate &&
-      !context.paintInvalidatorContext.forcedSubtreeInvalidationFlags &&
-      !object
-           .shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState());
+  return object.needsPaintPropertyUpdate() ||
+         object.descendantNeedsPaintPropertyUpdate() ||
+         context.treeBuilderContext.forceSubtreeUpdate ||
+         // If the object needs visual rect update, we should update paint
+         // properties which are needed by visual rect update.
+         m_paintInvalidator.needsVisualRectUpdate(
+             object, context.paintInvalidatorContext);
 }
 
 void PrePaintTreeWalk::walk(const LayoutObject& object,
                             const PrePaintTreeWalkContext& parentContext) {
-  if (shouldEndWalkBefore(object, parentContext))
+  // Early out from the treewalk if possible.
+  bool needsWalkForPaintPropertyUpdate =
+      this->needsWalkForPaintPropertyUpdate(object, parentContext);
+  if (!needsWalkForPaintPropertyUpdate &&
+      !object
+           .shouldCheckForPaintInvalidationRegardlessOfPaintInvalidationState())
     return;
 
   // PrePaintTreeWalkContext is large and can lead to stack overflows when
@@ -248,14 +261,47 @@ void PrePaintTreeWalk::walk(const LayoutObject& object,
   // some of the state computed here.
   updateAuxiliaryObjectProperties(object, *context);
 
-  m_propertyTreeBuilder.updatePropertiesForSelf(object,
-                                                context->treeBuilderContext);
+  if (needsWalkForPaintPropertyUpdate) {

[pdr., 2017/03/10 06:21:39]

Trying to think of ways to simplify these DCHECKS... What if we split out the
heap-allocated prepaint context into two heap-allocated objects (paint inval
context and paint prop context). If there is no paint property update, just
don't allocate the paint prop context at all. A null ptr makes the invalid
property tree context even more obvious, and this approach might save on
allocations too since the unneeded property tree context stops being allocated
at all.

[Xianzhu, 2017/03/10 17:43:05]

This looks a good idea. Will try.

[Xianzhu, 2017/03/13 19:08:09]

I tried the method, but found that we still need the flags for checking
under-invalidation of setNeedsPaintOffsetAndVisualRectUpdate().

It works as follows when DCHECK_IS_ON():

- Even if !needsWalkForPaintPropertyUpdate(), we still create
treeBuilderContext, and update paint offset, paint properties and visual rect,
for checking.

- However, the forced updates don't affect the 'updated' flags so that we can
still check for prerequisites.

Without the 'updated' flags, we either can't check for changes, or can't check
for prerequisites.

+#if DCHECK_IS_ON()
+    DCHECK(context->treeBuilderContext.parentUpdated);
+#endif
+    m_propertyTreeBuilder.updatePropertiesForSelf(object,
+                                                  context->treeBuilderContext);
+#if DCHECK_IS_ON()
+    context->treeBuilderContext.updatedForSelf = true;
+#endif
+  } else {
+#if DCHECK_IS_ON()
+    // When DCHECK_IS_ON, always update paint properties so that paint
+    // invalidator can update visual rect for checking missing flags.
+    LayoutPoint oldPaintOffset = object.paintOffset();
+    m_propertyTreeBuilder.updatePropertiesForSelf(object,
+                                                  context->treeBuilderContext);
+    DCHECK(object.paintOffset() == oldPaintOffset)
+        << "Paint offset changed without needsPaintOffsetAndVisualRectUpdate"
+        << " old=" << oldPaintOffset.toString()
+        << " new=" << object.paintOffset().toString();
+#endif
+  }
+
   m_paintInvalidator.invalidatePaintIfNeeded(object,
                                              context->paintInvalidatorContext);
-  m_propertyTreeBuilder.updatePropertiesForChildren(
-      object, context->treeBuilderContext);
 
-  invalidatePaintLayerOptimizationsIfNeeded(object, *context);
+  if (needsWalkForPaintPropertyUpdate) {
+    m_propertyTreeBuilder.updatePropertiesForChildren(
+        object, context->treeBuilderContext);
+#if DCHECK_IS_ON()
+    context->treeBuilderContext.updatedForChildren = true;
+#endif
+    invalidatePaintLayerOptimizationsIfNeeded(object, *context);
+  } else {
+#if DCHECK_IS_ON()
+    // When DCHECK_IS_ON, always update paint properties so that paint
+    // invalidator can update visual rect for checking missing flags.
+    m_propertyTreeBuilder.updatePropertiesForChildren(
+        object, context->treeBuilderContext);
+#endif
+  }
 
   for (const LayoutObject* child = object.slowFirstChild(); child;
        child = child->nextSibling()) {