Propagate resource-triggered repaints to text node children of <text>

Source/core/layout/svg/LayoutSVGResourceContainer.cpp

 /*
  * Copyright (C) Research In Motion Limited 2010. 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
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 146 matching lines @@
 void LayoutSVGResourceContainer::markClientForInvalidation(LayoutObject* client, InvalidationMode mode)
 {
     ASSERT(client);
     ASSERT(!m_clients.isEmpty());
 
     switch (mode) {
     case LayoutAndBoundariesInvalidation:
     case BoundariesInvalidation:
         client->setNeedsBoundariesUpdate();
         break;
-    case PaintInvalidation:
-        client->setShouldDoFullPaintInvalidation();
+    case PaintInvalidation: {
+        PaintInvalidationReason reason = PaintInvalidationFull;
+        // Since LayoutSVGInlineTexts don't have SVGResources (they use their
+        // parent's), they will not be notified of changes to paint servers. So
+        // if the client is one that could have a LayoutSVGInlineText use a
+        // paint invalidation reason that will force paint invalidation of the
+        // entire <text>/<tspan>/... subtree.
+        // TODO(fs): Allowing LayoutSVGInlineTexts to have should allow them to

[fs, 2015/09/10 10:41:13]

Unless you can tell me up-front that this won't work out, I'll see if I can make
it work this way. I think it'll be a more involved fix though (so may be worse
for backporting if needed).

[pdr., 2015/09/11 06:22:04]

This seems reasonable to me.

Ultra nit: missing a word in the comment.

[fs, 2015/09/11 16:32:16]

I accidentally an entire word! Will fix if we end up actually landing this part.

+        // get direct invalidation notifications (while getting redundant ones
+        // for the parent text content element's.)
+        if (client->isSVGText() || client->isSVGInline())
+            reason = PaintInvalidationLocationChange;

[pdr., 2015/09/11 06:22:04]

This is pragmatic but I worry it could confuse us in the future. WDYT about
adding a new invalidation reason for this case and adding the over-invalidation
infrastructure, similar to PaintInvalidationLocationChange? This suggestion may
be as much work as your larger fix above :/

If this is just for merging into M45 and M46, I'd be okay with just making this
change (as-is) directly on the branches.

[fs, 2015/09/11 16:32:16]

Yes, I was pondering that, and decided against it because of the reason you
suggest below. It could be localized to LayoutSVGText/LayoutSVGInline, but yeah
quite possibly larger than the "real, proper fix"...
 
TBH I'd feel a bit bad about backporting something "completely different", but
if you.think it's ok.

(Unfortunately I've been unable to get to this today =/, but I'll try to do it
on Monday...)

+        client->setShouldDoFullPaintInvalidation(reason);
         break;
+    }
     case ParentOnlyInvalidation:
         break;
     }
 }
 
 void LayoutSVGResourceContainer::addClient(LayoutObject* client)
 {
     ASSERT(client);
     m_clients.add(client);
     clearInvalidationMask();
@@ skipping 134 matching lines @@
             // This will process the rest of the ancestors.
             toLayoutSVGResourceContainer(current)->removeAllClientsFromCache();
             break;
         }
 
         current = current->parent();
     }
 }
 
 }