Handle layer clients in LayoutSVGResourceContainer::registerResource

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 205 matching lines @@
 
     // Update cached resources of pending clients.
     const SVGDocumentExtensions::SVGPendingElements::const_iterator end = clients->end();
     for (SVGDocumentExtensions::SVGPendingElements::const_iterator it = clients->begin(); it != end; ++it) {
         ASSERT((*it)->hasPendingResources());
         extensions.clearHasPendingResourcesIfPossible(*it);
         LayoutObject* layoutObject = (*it)->layoutObject();
         if (!layoutObject)
             continue;
 
+        // If the client is has a layer (is a non-SVGElement) we need to signal

[kouhei (in TOK), 2015/04/08 17:14:41]

s/is has/has/

[fs, 2015/04/08 17:26:04]

Done.

+        // invalidation in the same way as is done in markAllClientLayersForInvalidation above.
+        if (layoutObject->hasLayer() && resourceType() == FilterResourceType)
+            toLayoutBoxModelObject(layoutObject)->layer()->filterNeedsPaintInvalidation();

[kouhei (in TOK), 2015/04/08 17:14:41]

Would it make sense to do a early "continue" here, instead of removing the
assert on SVGResourcesCache?

[fs, 2015/04/08 17:26:04]

I believe we still want to do the setNeedsLayoutAndFullPaintInvalidation, so
something like:

if (layoutObject->hasLayer() &&...) {
    ...filterNeedsPaintInval
     idation();
} else {
   ...
   ...clientStyleChanged();
}
layoutObject->setNeeds...

might work I suppose. I wouldn't think it's fundamentally wrong to call
clientStyleChanged with a non-SVG element though.

[kouhei (in TOK), 2015/04/08 17:28:50]

Thanks for the explanation. No strong opinion here. lgtm either way.

[fs, 2015/04/08 17:34:16]

Hmm, it's quite possible that the "filter update hack" will be enough though (it
triggers a style-recalc IIRC.)

[fs, 2015/04/09 10:21:33]

Ended up doing like this suggests, because I concluded that it should be enough
(and is more consistent with the rest of the code.)

+
         StyleDifference diff;
         diff.setNeedsFullLayout();
         SVGResourcesCache::clientStyleChanged(layoutObject, diff, layoutObject->styleRef());
         layoutObject->setNeedsLayoutAndFullPaintInvalidation(LayoutInvalidationReason::SvgResourceInvalidated);
     }
 }
 
 static inline void removeFromCacheAndInvalidateDependencies(LayoutObject* object, bool needsLayout)
 {
     ASSERT(object);
@@ skipping 54 matching lines @@
             // This will process the rest of the ancestors.
             toLayoutSVGResourceContainer(current)->removeAllClientsFromCache();
             break;
         }
 
         current = current->parent();
     }
 }
 
 }