[webkit-reviews] review granted: [Bug 65643] Repaint issues with -webkit-svg-shadow used on a container : [Attachment 172860] patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Tim Horton
<timothy_horton at apple.com>'s request for review:
Bug 65643: Repaint issues with -webkit-svg-shadow used on a container
https://bugs.webkit.org/show_bug.cgi?id=65643

Attachment 172860: patch
https://bugs.webkit.org/attachment.cgi?id=172860&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=172860&action=review


> Source/WebCore/rendering/RenderObject.cpp:289
> +#if ENABLE(SVG)
> +    SVGRenderSupport::setRendererHasSVGShadow(newChild,
SVGRenderSupport::rendererHasSVGShadow(this) ||
SVGRenderSupport::rendererHasSVGShadow(newChild));
> +#endif

This could be slightly less gross by having SVGRenderSupport::childAdded(RO*,
RO*) and burying this inside it.

> Source/WebCore/rendering/RenderObject.cpp:1923
> +    SVGRenderSupport::setRendererHasSVGShadow(this, (m_parent &&
SVGRenderSupport::rendererHasSVGShadow(m_parent)) || (style()->svgStyle() &&
style()->svgStyle()->shadow()));

And SVGRenderSupport::styleChanged().

> Source/WebCore/rendering/svg/SVGRenderSupport.cpp:332
> +	   ASSERT(style);

Not much point asserting there; you'll crash on the next line.