Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect

third_party/WebKit/Source/core/layout/LayoutBox.cpp

Index: third_party/WebKit/Source/core/layout/LayoutBox.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
index 89b23b078658cced2b4731d5f485539a9a6f9bb3..0726a9cb3cb002b9d79f8302606d61afe9a8ded9 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -2034,6 +2034,26 @@ bool LayoutBox::needsForcedBreakBefore(EBreak previousBreakAfterValue) const
     return isForcedFragmentainerBreakValue(classABreakPointValue(previousBreakAfterValue));
 }
 
+bool LayoutBox::canSkipUnforcedPaintInvalidation() const

[chrishtr, 2016/08/03 23:13:15]

In what case are the conditions in this method different than "this object
doesn't paint anything"? There
most be something, but don't know yet..

[Xianzhu, 2016/08/04 00:02:14]

For now there is no other cases.

However, clip is a case not to skip paint invalidation even if the object paints
nothing.

[chrishtr, 2016/08/04 00:57:47]

Right. Then is it correct to say that this method could be named
"doesNotAffectPaintedOutput"? Then this optimization would
be reframed as "skip paint invalidation for objects whose painted output has no
effect". And given that definition, why
invalidate paint for it ever? Why not early out from painting it also?

[Xianzhu, 2016/08/04 16:27:02]

The name "doesNotAffectPaintedOutput" seems not clear about what does not affect
painted output. In the context, it's "geometry change without forced paint
invalidation".

Also we should state "unforced paint invalidation". We can't skip forced paint
invalidation even if currently the object will paint nothing, in case that the
object painted something in the previous frame.

So canSkipUnforcedPaintInvalidation() seems better describe the result (but not
the reason).
Previously the optimization existed in
LayoutObject::skipInvalidationWhenLaidOutChildren(). This CL make the
optimization not depend on whether we laid-out children. Created this CL because
I felt skipInvalidationWhenLaidOutChildren() is not a good function to export
from layout/ to paint/ when I was moving paint invalidation code under paint/.
We do early out from painting, but the conditions are not at a single place in
painting code. For example, we early out from background painting if there is no
background. Actually this function is a conservative collection of early-out
conditions in painting.

+{
+    // In case scrollbars got repositioned (which will typically happen if the box got
+    // resized), we cannot skip invalidation.
+    if (hasNonCompositedScrollbars())
+        return false;
+
+    // Cannot skip paint invalidation if the box has real things to paint.
+    if (getSelectionState() != SelectionNone || hasBoxDecorationBackground() || styleRef().hasVisualOverflowingEffect())
+        return false;
+
+    // If the box has clip, we need to repaint the changed part when the box got resized,

[chrishtr, 2016/08/03 23:13:15]

Changed part of children

[chrishtr, 2016/08/03 23:13:43]

Meaning: it's the childrens' paint output that would be clipped.

[Xianzhu, 2016/08/04 00:02:14]

Done.

(I think there is a bug about ancestor clip change. In theory the children
should invalidate themselves on ancestor clip change, but this is missing for
now. The condition hides the bug in most cases, but the bug still occur in the
following scenario:

  Frame 1: Ancestor expands clip which should a child's visual rect.
  Frame 2: The child moves.

There will be under-invalidation in Frame 2 for the child's exposed area in
Frame 1 because the child's previousPaintInvalidationRect is not updated during
Frame 1.

I believe the bug has existed since the first of repaint-after-layout, but it's
so rare that no one reported it and no layout test covers it.

It seems not easy to fix it in spv1 without hurting performance, so I would like
to treat it as a "fundamental paint invalidation bug" for spv1 and not to fix it
for now.

This will be very different in spv2 because we'll use unclipped visual rects,
apply clip in later stages, and also handle rasterization invalidation on clip
property change differently.)

+    // so cannot skip invalidation.
+    // TODO(wangxianzhu): Paint invalidation for clip change will be different in spv2.
+    if (hasClipRelatedProperty() || hasControlClip())
+        return false;
+
+    return true;
+}
+
 LayoutRect LayoutBox::localOverflowRectForPaintInvalidation() const
 {
     if (style()->visibility() != VISIBLE)