Fix incorrect invalidation of foreign objects under window zoom.

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

Index: third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
index d83f1633504c0f04c338ede0370b2395be6c27fd..97ca0109a4b73d1ff9319a710179ff80c480fca2 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
@@ -411,6 +411,20 @@ void LayoutBoxModelObject::invalidateTreeIfNeeded(const PaintInvalidationState&
         && (hasOverflowClip() || isLayoutView()))
         newPaintInvalidationState.setForceSubtreeInvalidationRectUpdateWithinContainer();
 
+    // SVGForeignObject clips descendants and needs to over-invalidate to ensure
+    // newly exposed content repaints.
+    // TODO(pdr): SVGForeignObject supports overflow clipping using a different
+    // approach than LayoutBlock (see LayoutSVGBlock::allowsOverflowClip). This
+    // over-invalidation should be removed when overflow is unified.
+    if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()

[wkorman, 2016/08/23 23:13:10]

This is just a minor adaptation of your patch:
https://codereview.chromium.org/1835843002/patch/1/10036

Per my change description however, I find that on window zoom we see this called
with the new and previous paint invalidation rect matching, thus we won't force
subtree invalidation, and it appears that we do need to do so in order for all
contents of the foreign object to be invalidated.

So, I've removed:

&& previousPaintInvalidationRect != this->previousPaintInvalidationRect()

which could mean we'll be over-invalidating more frequently than we would,
ideally. Do you have alternate suggestions or insight?

[pdr., 2016/08/24 18:12:19]

I am not sure if my original patch was on the right track. Looking back at it, I
think I was off in the weeds.

Do you know how paint invalidation works for zoom changes in svg in general? The
root SVG layout object should apply zoom globally (see
LayoutSVGRoot::buildLocalToBorderBoxTransform and
SVGRootPainter::transformToPixelSnappedBorderBox). When the root's zoom changes,
how do regular SVG objects get invalidated?

I've added WangXianzhu and Fs in case they spot anything obvious here.

[Xianzhu, 2016/08/24 18:19:48]

I think the zoom case works because we layout the whole subtree and then
invalidate the objects which changed layout. Clip is different because it
doesn't affect subtree layout.

[fs, 2016/08/24 19:11:44]

Like Xianzhu say, zoom ought to trigger layout for the whole subtree (the fancy
screen-scale-changed flag doohickey), which then in turn will invalidate the
objects. Maybe that breaks down at the fO boundary. Can think of obvious stuff
wrt clipping (although I rather suspect we don't apply it at all in these
cases...)

[pdr., 2016/08/24 19:55:23]

I see, that makes sense.

Would something like the following work?
    if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()
        && previousPaintInvalidationRect !=
this->previousPaintInvalidationRect()
        && !usesCompositedScrolling()
        // Note that isLayoutView() below becomes unnecessary after the launch
of root layer scrolling.
        && (hasOverflowClip() || isLayoutView() || isSVGRoot()))
       
newPaintInvalidationState.setForceSubtreeInvalidationRectUpdateWithinContainer();

+        && isSVGForeignObject()
+        && !styleRef().isOverflowVisible()) {
+        // We'd like to only force subtree invalidation if the paint
+        // invalidation rect changes, but on window zoom for foreign objects we
+        // have to invalidate regardless.
+        newPaintInvalidationState.setForceSubtreeInvalidationRectUpdateWithinContainer();
+    }
+
     newPaintInvalidationState.updateForChildren(reason);
     invalidatePaintOfSubtreesIfNeeded(newPaintInvalidationState);
 }