Invalidate previous paint rect in RenderObject::paintInvalidationForWholeRenderer()

Source/core/rendering/RenderObject.cpp

Index: Source/core/rendering/RenderObject.cpp
diff --git a/Source/core/rendering/RenderObject.cpp b/Source/core/rendering/RenderObject.cpp
index 612025741947dfc4a0327918231b98b6bb27393f..b70a74f19e4ee7af2bfcb3f2c5b3d6e53a79a3c7 100644
--- a/Source/core/rendering/RenderObject.cpp
+++ b/Source/core/rendering/RenderObject.cpp
@@ -1531,7 +1531,10 @@ void RenderObject::paintInvalidationForWholeRenderer() const
     // Until those states are fully fledged, I'll just disable the ASSERTS.
     DisableCompositingQueryAsserts disabler;
     const RenderLayerModelObject* paintInvalidationContainer = containerForPaintInvalidation();
-    LayoutRect paintInvalidationRect = boundsRectForPaintInvalidation(paintInvalidationContainer);
+
+    // FIXME: We should invalidate previousPaintInvalidationRect, but for objects that don't save previousPaintInvalidationRect,
+    // we have to use the current paint invalidation rect. crbug.com/394133.
+    LayoutRect paintInvalidationRect = savesPreviousPaintInvalidationRect() ? previousPaintInvalidationRect() : boundsRectForPaintInvalidation(paintInvalidationContainer);
     invalidatePaintUsingContainer(paintInvalidationContainer, paintInvalidationRect, InvalidationPaint);
 }
 
@@ -2061,10 +2064,13 @@ void RenderObject::setStyle(PassRefPtr<RenderStyle> style)
 
         // The paintInvalidationForWholeRenderer() call is needed for non-layout changes to style. See the corresponding
         // comment in RenderObject::styleWillChange for why.
-        if (needsLayout())
+        if (needsLayout()) {
             setShouldDoFullPaintInvalidationAfterLayout(true);
-        else if (!selfNeedsLayout())
+        } else {
+            if (savesPreviousPaintInvalidationRect())
+                setPreviousPaintInvalidationRect(boundsRectForPaintInvalidation(containerForPaintInvalidation()));

[leviw_travelin_and_unemployed, 2014/07/16 23:06:48]

Isn't this redundant with the change you're making to
paintInvalidationForWholeRenderer()?

[Xianzhu, 2014/07/16 23:47:52]

No. Actually this is paired with the change in paintInvalidationForWholeRenderer
because here the caller wants to invalidate the new location.

With unified invalidation path (I'm working on), the above code won't be needed.

[leviw_travelin_and_unemployed, 2014/07/22 21:42:29]

If we've ever put up a frame with the thing at the new location, we should have
the right location saved, no? Can you help me understand why this case would be
different and we'd want the "new" location?

[Xianzhu, 2014/07/22 21:53:52]

This change has been abandoned. The above code was only needed before we go
unified invalidation path (https://codereview.chromium.org/398343003). Will
continue the new change after landing of depended changes.

             paintInvalidationForWholeRenderer();
+        }
     }
 }