Let synchronized painting generate correct paint invalidation rects

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 15fa2e07ca658736df4a6257d6befc6bdf59c775..1a9e7e31d7f289c5b1b3e7ec2fdf56b1c7d1f710 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -1435,13 +1435,18 @@ PaintInvalidationReason LayoutBox::invalidatePaintIfNeeded(PaintInvalidationStat
 
         // Issue paint invalidations for any scrollbars if there is a scrollable area for this layoutObject.
         if (ScrollableArea* area = scrollableArea()) {
-            // In slimming paint mode, we already invalidated the display item clients of the scrollbars
-            // during PaintLayerScrollableArea::invalidateScrollbarRect(). However, for now we still need to
-            // invalidate the rectangles to trigger repaints.
-            if (area->hasVerticalBarDamage())
-                invalidatePaintRectangleNotInvalidatingDisplayItemClients(LayoutRect(area->verticalBarDamage()));
-            if (area->hasHorizontalBarDamage())
-                invalidatePaintRectangleNotInvalidatingDisplayItemClients(LayoutRect(area->horizontalBarDamage()));
+            if (area->hasVerticalBarDamage()) {
+                if (Scrollbar* verticalScrollbar = area->verticalScrollbar())

[chrishtr, 2015/10/22 00:02:41]

Why is this conditional needed now but not before?

[Xianzhu, 2015/10/22 01:17:04]

Previously we invalidate the rect directly to GraphicsLayer, but with sync
painting, the rect is lost. Here we just need to pass the rect along with 'some
display item client' through PaintController::invalidate(), but it requires us
to also pass a display item client. The following code tries to pass the actual
client (scrollbar) if it's still there, or pass the box as the client. Perhaps
this logic is unneeded because we actually want to associate the rects to
PaintChunk not individual display item client, but that needs a change of the
PaintController interface. Would like to address this during spv2 paint
invalidation.

+                    invalidatePaintRectangleForDisplayItemClient(*verticalScrollbar, LayoutRect(area->verticalBarDamage()));
+                else
+                    invalidatePaintRectangle(LayoutRect(area->verticalBarDamage()));
+            }
+            if (area->hasHorizontalBarDamage()) {
+                if (Scrollbar* horizontalScrollbar = area->horizontalScrollbar())
+                    invalidatePaintRectangleForDisplayItemClient(*horizontalScrollbar, LayoutRect(area->horizontalBarDamage()));
+                else
+                    invalidatePaintRectangle(LayoutRect(area->horizontalBarDamage()));
+            }
         }
     }
 
@@ -4098,9 +4103,9 @@ PaintInvalidationReason LayoutBox::paintInvalidationReason(const LayoutBoxModelO
     return PaintInvalidationIncremental;
 }
 
-void LayoutBox::incrementallyInvalidatePaint(const LayoutBoxModelObject& paintInvalidationContainer, const LayoutRect& oldBounds, const LayoutRect& newBounds, const LayoutPoint& positionFromPaintInvalidationBacking)
+void LayoutBox::incrementallyInvalidatePaint(const LayoutBoxModelObject& paintInvalidationContainer, const LayoutRect& oldBounds, const LayoutRect& newBounds, const LayoutPoint& positionFromPaintInvalidationBacking, Vector<LayoutRect>& paintInvalidationRects)
 {
-    LayoutObject::incrementallyInvalidatePaint(paintInvalidationContainer, oldBounds, newBounds, positionFromPaintInvalidationBacking);
+    LayoutObject::incrementallyInvalidatePaint(paintInvalidationContainer, oldBounds, newBounds, positionFromPaintInvalidationBacking, paintInvalidationRects);
 
     bool hasBoxDecorations = style()->hasBoxDecorations();
     if (!style()->hasBackground() && !hasBoxDecorations)
@@ -4133,7 +4138,7 @@ void LayoutBox::incrementallyInvalidatePaint(const LayoutBoxModelObject& paintIn
             positionFromPaintInvalidationBacking.y(),
             deltaWidth + borderWidth,
             std::max(oldBorderBoxSize.height(), newBorderBoxSize.height()));
-        invalidatePaintRectClippedByOldAndNewBounds(paintInvalidationContainer, rightDeltaRect, oldBounds, newBounds);
+        invalidatePaintRectClippedByOldAndNewBounds(paintInvalidationContainer, rightDeltaRect, oldBounds, newBounds, paintInvalidationRects);
     }
 
     // Invalidate the bottom delta part and the bottom border of the old or new box which has smaller height.
@@ -4147,11 +4152,11 @@ void LayoutBox::incrementallyInvalidatePaint(const LayoutBoxModelObject& paintIn
             positionFromPaintInvalidationBacking.y() + smallerHeight - borderHeight,
             std::max(oldBorderBoxSize.width(), newBorderBoxSize.width()),
             deltaHeight + borderHeight);
-        invalidatePaintRectClippedByOldAndNewBounds(paintInvalidationContainer, bottomDeltaRect, oldBounds, newBounds);
+        invalidatePaintRectClippedByOldAndNewBounds(paintInvalidationContainer, bottomDeltaRect, oldBounds, newBounds, paintInvalidationRects);
     }
 }
 
-void LayoutBox::invalidatePaintRectClippedByOldAndNewBounds(const LayoutBoxModelObject& paintInvalidationContainer, const LayoutRect& rect, const LayoutRect& oldBounds, const LayoutRect& newBounds)
+void LayoutBox::invalidatePaintRectClippedByOldAndNewBounds(const LayoutBoxModelObject& paintInvalidationContainer, const LayoutRect& rect, const LayoutRect& oldBounds, const LayoutRect& newBounds, Vector<LayoutRect>& paintInvalidationRects)
 {
     if (rect.isEmpty())
         return;
@@ -4159,14 +4164,14 @@ void LayoutBox::invalidatePaintRectClippedByOldAndNewBounds(const LayoutBoxModel
     LayoutRect rectClippedByNewBounds = intersection(rect, newBounds);
     // Invalidate only once if the clipped rects equal.
     if (rectClippedByOldBounds == rectClippedByNewBounds) {
-        invalidatePaintUsingContainer(paintInvalidationContainer, rectClippedByOldBounds, PaintInvalidationIncremental);
+        invalidatePaintUsingContainer(paintInvalidationContainer, rectClippedByOldBounds, PaintInvalidationIncremental, paintInvalidationRects);
         return;
     }
     // Invalidate the bigger one if one contains another. Otherwise invalidate both.
     if (!rectClippedByNewBounds.contains(rectClippedByOldBounds))
-        invalidatePaintUsingContainer(paintInvalidationContainer, rectClippedByOldBounds, PaintInvalidationIncremental);
+        invalidatePaintUsingContainer(paintInvalidationContainer, rectClippedByOldBounds, PaintInvalidationIncremental, paintInvalidationRects);
     if (!rectClippedByOldBounds.contains(rectClippedByNewBounds))
-        invalidatePaintUsingContainer(paintInvalidationContainer, rectClippedByNewBounds, PaintInvalidationIncremental);
+        invalidatePaintUsingContainer(paintInvalidationContainer, rectClippedByNewBounds, PaintInvalidationIncremental, paintInvalidationRects);
 }
 
 void LayoutBox::markForPaginationRelayoutIfNeeded(SubtreeLayoutScope& layoutScope)