Don't invalidate rect if it's covered by fully invalidated parent's visual rect

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

Index: third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
diff --git a/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
index eedca8d87b2e4e5ed0df5364011f1a75c4de5a89..8f72299d5337f38a32fd209e0f2509e45b1f7d3e 100644
--- a/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
+++ b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
@@ -338,6 +338,16 @@ void ObjectPaintInvalidator::setBackingNeedsPaintInvalidationInRect(
   }
 }
 
+static bool parentIsContainer(const LayoutObject& object) {
+  if (object.isTextOrSVGChild() ||

[wkorman, 2017/02/22 21:52:22]

Is there a relevant spec section or something we could link here in a comment so
that future eng will know how we decided on the below conditionals as being the
right ones? This is not rhetorical, I ask because I myself pondered while
reading. The out-of-flow and floating made immediate sense, I assume
column-span-all is a proxy for is-multicol and there's some multicol spec. In
any case a comment likely helpful for future others.

[Xianzhu, 2017/02/22 23:20:59]

The conditions are the same conditions that LayoutObject::container() will
surely return parent().

However, your question made me think of better heuristics.

I uploaded a new patch that uses heuristics that are closer to what we want: the
parent and the child are on the same backing and the parent's visual rect covers
child's. The new patch is more efficient because the condition covers more cases
that are suitable for the optimization.

The new patch also avoids the optimization if the invalidation is not during
paint invalidation phase (e.g. invalidatePaintRectangle or
invalidatePaintOfPreviousVisualRect) and the optimization might be incorrect.

+      (!object.isOutOfFlowPositioned() && !object.isColumnSpanAll() &&
+       !object.isFloating())) {
+    DCHECK(object.parent() == object.container());
+    return true;
+  }
+  return false;
+}
+
 void ObjectPaintInvalidator::invalidatePaintUsingContainer(
     const LayoutBoxModelObject& paintInvalidationContainer,
     const LayoutRect& dirtyRect,
@@ -358,7 +368,15 @@ void ObjectPaintInvalidator::invalidatePaintUsingContainer(
   if (dirtyRect.isEmpty())
     return;
 
-  CHECK(m_object.isRooted());
+  DCHECK(m_object.isRooted());

[wkorman, 2017/02/22 21:52:22]

Just curious, why shift to DCHECK?

[Xianzhu, 2017/02/22 23:20:59]

isRooted() is slow because it traverse the layout/layer tree.

However, in the new patch I'll leave invalidatePaintUsingContainer() unchanged,
so I would like to leave this change to a follow-up.

+
+  if (m_object != paintInvalidationContainer && m_object.parent() &&

[wkorman, 2017/02/22 21:52:22]

Could be worth adding a comment to explain background re: this optimization in
prose. Reading all of the conditionals below alone makes it kind of obtuse.

+      m_object.parent() != paintInvalidationContainer &&
+      isImmediateFullPaintInvalidationReason(
+          m_object.parent()->fullPaintInvalidationReason()) &&
+      parentIsContainer(m_object) &&
+      m_object.parent()->visualRect().contains(dirtyRect))
+    return;
 
   // FIXME: Unify "devtools.timeline.invalidationTracking" and
   // "blink.invalidation". crbug.com/413527.