Fix clip and offset issues of slimmingPaintInvalidation for SPv1

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

Index: third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
index d6f93077f10445e3e80dffc43128877addad0fd2..d30a142cd29bc6525cb085b8b325e6d151538602 100644
--- a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
@@ -20,6 +20,14 @@
 
 namespace blink {
 
+static void slowMapToVisualRectInAncestorSpace(const LayoutObject& object, const LayoutBoxModelObject& ancestor, LayoutRect& rect)
+{
+    if (object.isLayoutView())
+        toLayoutView(object).mapToVisualRectInAncestorSpace(&ancestor, rect, InputIsInFrameCoordinates, DefaultVisualRectFlags);
+    else
+        object.mapToVisualRectInAncestorSpace(&ancestor, rect);
+}
+
 // TODO(wangxianzhu): Combine this into PaintInvalidator::mapLocalRectToPaintInvalidationBacking() when removing PaintInvalidationState.
 static LayoutRect mapLocalRectToPaintInvalidationBacking(GeometryMapper& geometryMapper, const LayoutObject& object, const FloatRect& localRect, const PaintInvalidatorContext& context)
 {
@@ -32,17 +40,22 @@ static LayoutRect mapLocalRectToPaintInvalidationBacking(GeometryMapper& geometr
         toLayoutBox(object).flipForWritingMode(rect);
 
     LayoutRect result;
-    if (object == context.paintInvalidationContainer) {
+    if (context.forcedSubtreeInvalidationFlags & PaintInvalidatorContext::ForcedSubtreeSlowPathRect) {
+        result = LayoutRect(rect);
+        slowMapToVisualRectInAncestorSpace(object, *context.paintInvalidationContainer, result);
+    } else if (object == context.paintInvalidationContainer) {
         result = LayoutRect(rect);
     } else {
-        rect.moveBy(FloatPoint(context.treeBuilderContext.current.paintOffset));
-
-        bool success = false;
         PropertyTreeState currentTreeState(context.treeBuilderContext.current.transform, context.treeBuilderContext.current.clip, context.treeBuilderContext.currentEffect);
         PropertyTreeState containerTreeState;
-        context.paintInvalidationContainer->objectPaintProperties()->getContentsProperties(containerTreeState);
+        const ObjectPaintProperties* containerPaintProperties = context.paintInvalidationContainer->objectPaintProperties();
+        containerPaintProperties->getContentsProperties(containerTreeState);
+
+        rect.moveBy(FloatPoint(context.treeBuilderContext.current.paintOffset));
+        bool success = false;
         result = LayoutRect(geometryMapper.mapToVisualRectInDestinationSpace(rect, currentTreeState, containerTreeState, success));
         DCHECK(success);
+        result.moveBy(-containerPaintProperties->localBorderBoxProperties()->paintOffset);

[chrishtr, 2016/09/13 23:30:03]

This was a straight-up bug in slimmingPaintInvalidation? Why are you computing
the diff of the paint
offsets?

[Xianzhu, 2016/09/13 23:47:15]

Yes, this was a bug, but didn't notice it because there are too many failures on
spv2. 

Now with it running on spv1 I discovered this from failures of a much smaller
number.

After Line 57 we get a rect in the space of container's PaintTreeStatus. The
rect still includes container's paint offset of the container which should be
subtracted from the rect to get a rect in container's space.

[chrishtr, 2016/09/13 23:54:25]

Ah yes, makes sense.

     }
 
     if (context.paintInvalidationContainer->layer()->groupedMapping())
@@ -161,6 +174,10 @@ void PaintInvalidator::updateContext(const LayoutObject& object, PaintInvalidato
     if (object.mayNeedPaintInvalidationSubtree())
         context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
 
+    // TODO(crbug.com/637313): This is temporary before we support filters in paint property tree.
+    if (object.hasFilterInducingProperty())
+        context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeSlowPathRect;
+
     context.oldBounds = object.previousPaintInvalidationRect();
     context.oldLocation = object.previousPositionFromPaintInvalidationBacking();
     context.newBounds = computePaintInvalidationRectInBacking(object, context);
@@ -195,6 +212,17 @@ void PaintInvalidator::invalidatePaintIfNeeded(FrameView& frameView, PaintInvali
     layoutView->sendMediaPositionChangeNotifications(visibleRect);
 }
 
+static bool hasPercentageTransform(const ComputedStyle& style)
+{
+    if (TransformOperation* translate = style.translate()) {
+        if (translate->dependsOnBoxSize())
+            return true;
+    }
+    return style.transform().dependsOnBoxSize()
+        || (style.transformOriginX() != Length(50, Percent) && style.transformOriginX().isPercentOrCalc())

[chrishtr, 2016/09/13 23:30:03]

There's no canonical way to detect non-default transform origin?

[Xianzhu, 2016/09/13 23:47:15]

We only need to check non-default percentage transform origin here.

This code is copied from LayoutBoxModelObject.cpp and is a workaround of
crbug.com/533277 (see line 265 below) and will be removed when we implement
paint offset change detection.

+        || (style.transformOriginY() != Length(50, Percent) && style.transformOriginY().isPercentOrCalc());
+}
+
 void PaintInvalidator::invalidatePaintIfNeeded(const LayoutObject& object, PaintInvalidatorContext& context)
 {
     object.getMutableForPainting().ensureIsReadyForPaintInvalidation();
@@ -216,7 +244,8 @@ void PaintInvalidator::invalidatePaintIfNeeded(const LayoutObject& object, Paint
         return;
     }
 
-    switch (object.invalidatePaintIfNeeded(context)) {
+    PaintInvalidationReason reason = object.invalidatePaintIfNeeded(context);
+    switch (reason) {
     case PaintInvalidationDelayedFull:
         m_pendingDelayedPaintInvalidations.append(&object);
         break;
@@ -233,6 +262,19 @@ void PaintInvalidator::invalidatePaintIfNeeded(const LayoutObject& object, Paint
     if (context.oldLocation != context.newLocation)
         context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
 
+    // TODO(crbug.com/533277): This is a workaround for the bug. Remove when we detect paint offset change.
+    if (reason != PaintInvalidationNone && hasPercentageTransform(object.styleRef()))
+        context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationChecking;
+
+    // TODO(crbug.com/490725): This is a workaround for the bug, to force descendant to update paint invalidation
+    // rects on clipping change.
+    if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()
+        && context.oldBounds != context.newBounds
+        // Note that isLayoutView() below becomes unnecessary after the launch of root layer scrolling.
+        && (object.hasOverflowClip() || object.isLayoutView())
+        && !toLayoutBox(object).usesCompositedScrolling())
+        context.forcedSubtreeInvalidationFlags |= PaintInvalidatorContext::ForcedSubtreeInvalidationRectUpdate;
+
     object.getMutableForPainting().clearPaintInvalidationFlags();
 }