Avoid updateFromElement() usage in SVG.
Created attachment 144195 [details] Patch
Comment on attachment 144195 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144195&action=review > Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:-119 > -void RenderSVGModelObject::updateFromElement() > -{ > - RenderObject::updateFromElement(); > - SVGResourcesCache::clientUpdatedFromElement(this, style()); > -} > - Don't we need to add calls to clientWasAddedToTree and clientWillBeRemovedFromTree here? RenderSVGShape, RenderSVGImage, etc... directly inherit from RenderSVGModelObject.
Created attachment 144197 [details] Patch v2
(In reply to comment #2) > Don't we need to add calls to clientWasAddedToTree and clientWillBeRemovedFromTree here? > RenderSVGShape, RenderSVGImage, etc... directly inherit from RenderSVGModelObject. As discussed on IRC, RenderSVGModelObject doesn't have children, so it doesn't need this logic.
Comment on attachment 144195 [details] Patch I think this refactoring is done right (had a lengthy discussion with WildFox on IRC)., and new approach makes the removal and addition of SVG resource much more apparent when constructing or destructing the render tree.
Comment on attachment 144197 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=144197&action=review > Source/WebCore/rendering/svg/SVGResourcesCache.cpp:140 > - if (diff == StyleDifferenceEqual) > + if (diff == StyleDifferenceEqual || !renderer->parent()) > return; > > // In this case the proper SVGFE*Element will decide whether the modified CSS properties require a relayout or repaint. > if (renderer->isSVGResourceFilterPrimitive() && diff == StyleDifferenceRepaint) > return; > > - clientUpdatedFromElement(renderer, newStyle); > + SVGResourcesCache* cache = resourcesCacheFromRenderObject(renderer); > + cache->removeResourcesFromRenderObject(renderer); > + cache->addResourcesFromRenderObject(renderer, newStyle); I think this function deserves a little clarifying why we would want to remove/add the resource here.
(In reply to comment #5) > (From update of attachment 144195 [details]) > I think this refactoring is done right (had a lengthy discussion with WildFox on IRC)., and new approach makes the removal and addition of SVG resource much more apparent when constructing or destructing the render tree. Thanks! The discussion was very helpful. I left a new comment there: // Dynamic changes of CSS properties like 'clip-path' may require us to recompute the associated resources for a renderer. // FIXME: Avoid passing in a useless StyleDifference, but instead compare oldStyle/newStyle to see which resources changed // to be able to selectively rebuild individual resources, instead of all of them.
Committed r118608: <http://trac.webkit.org/changeset/118608>