Fix incorrect invalidation of foreign objects under window zoom.

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 393 matching lines @@
 
     // TODO(wangxianzhu): This is a workaround for crbug.com/490725. We don't have enough saved information to do accurate check
     // of clipping change. Will remove when we remove rect-based paint invalidation.
     if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()
         && previousPaintInvalidationRect != this->previousPaintInvalidationRect()
         && !usesCompositedScrolling()
         // Note that isLayoutView() below becomes unnecessary after the launch of root layer scrolling.
         && (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);
 }
 
 void LayoutBoxModelObject::setBackingNeedsPaintInvalidationInRect(const LayoutRect& r, PaintInvalidationReason invalidationReason, const LayoutObject& object) const
 {
     // TODO(wangxianzhu): Enable the following assert after paint invalidation for spv2 is ready.
     // ASSERT(!RuntimeEnabledFeatures::slimmingPaintV2Enabled());
 
     // https://bugs.webkit.org/show_bug.cgi?id=61159 describes an unreproducible crash here,
@@ skipping 714 matching lines @@
     if (rootElementStyle->hasBackground())
         return false;
 
     if (node() != document().firstBodyElement())
         return false;
 
     return true;
 }
 
 } // namespace blink