Fix paint invalidation of paintInvalidationContainer itself

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

Index: third_party/WebKit/Source/core/layout/LayoutBlock.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
index d58373e6a124239bc25b92d4913befe51f5dd5e1..ec9e9d55081338809455e98d697ffa158904c8c2 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
@@ -356,7 +356,7 @@ void LayoutBlock::invalidatePaintOfSubtreesIfNeeded(PaintInvalidationState& chil
             // If it's a new paint invalidation container, we won't have properly accumulated the offset into the
             // PaintInvalidationState.
             // FIXME: Teach PaintInvalidationState to handle this case. crbug.com/371485
-            if (paintInvalidationContainerForChild != childPaintInvalidationState.paintInvalidationContainer()) {
+            if (&paintInvalidationContainerForChild != box && paintInvalidationContainerForChild != childPaintInvalidationState.paintInvalidationContainer()) {

[chrishtr, 2016/03/11 18:19:59]

If box establishes a paint invalidation container, why is it that the paint
offsets are still correct? In other words, consider a box that leads to this
code path and
is not composited. In that case, we have to force the slow mapping because its
paint invalidation container is not correctly tracked. Adding compositing means
we
are sure the paint invalidation container is tracked, but why is the offset
correct?

The comment in line 356 seems to indicate otherwise.

I also don't remember the cases when this code path would be hit. Can you remind
me of one?

[Xianzhu, 2016/03/11 18:30:32]

In the case the paint offset won't be used, for either old code or with this
patch:
- The old code used ForceHorriblySlowRectMapping;
- The new code will early return in mapToVisibleRectInAncestorSpace() without
using PaintInvalidationState.
 
This code was intended for the case that the actual paint invalidation container
is not in the containing block chain. For example:
<div style="composited-stacking-context">
  <div style="fixed-not-composited"></div>
</div>
At here, childPaintInvalidationState.paintInvalidationContainer() is the
LayoutView, but the actual paintInvalidationContainer of the fixed-position div
is the composited-stacking-context.

However, this path was also used when the box itself establishes a paint
invalidation container, forcing slow rect mapping for the whole subtree.

The above issues will be fixed in new paint invalidation for spv1. For the
self-paint-invalidation-container case, LayoutObject::invalidatePaintIfNeeded()
will get a PaintInvalidationState with correct paintOffset (zero) and
paintInvalidationContainer (itself).

[chrishtr, 2016/03/11 21:17:16]

Is there a test for this case?

[Xianzhu, 2016/03/11 21:39:56]

Many existing layout tests cover the code here and the cases for correctness of
paint invalidation, but not for whether ForceHorriblySlowRectMapping is used. It
seems not worth testing the latter as the code here is about to be obsoleted by
slimmingPaintInvalidation.

                 ForceHorriblySlowRectMapping slowRectMapping(&childPaintInvalidationState);
                 PaintInvalidationState disabledPaintInvalidationState(childPaintInvalidationState, *this, paintInvalidationContainerForChild);
                 box->invalidateTreeIfNeeded(disabledPaintInvalidationState);