[webkit-reviews] review granted: [Bug 56906] CSS related SVG*Element changes doesn't require relayout : [Attachment 88929] Proposed patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has granted Renata Hodovan
<reni at webkit.org>'s request for review:
Bug 56906: CSS related SVG*Element changes doesn't require relayout
https://bugs.webkit.org/show_bug.cgi?id=56906

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

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88929&action=review

r=me, some minor tweaks:

> Source/WebCore/rendering/svg/SVGResourcesCache.cpp:129
> +    // In this case the proper SVGFE*Element will deceide whether the
modifided CSS properties implies a relayout or repaint.

typo: decide.
typo: imply,

> Source/WebCore/svg/SVGFEDiffuseLightingElement.cpp:92
> +	   return
diffuseLighting->setLightingColor(this->renderer()->style()->svgStyle()->lighti
ngColor());

Is this->renderer() and this->renderer()->style() guaranteed to be non null
here?
I'd like to see two asserts here.

> Source/WebCore/svg/SVGFEFloodElement.cpp:46
> +    RenderStyle* style = this->renderer()->style();

I'd like to see this->renderer*( stored in a local variable, and two
assertions.

> Source/WebCore/svg/SVGFESpecularLightingElement.cpp:97
> +    if (attrName == SVGNames::lighting_colorAttr)
> +	   return
specularLighting->setLightingColor(this->renderer()->style()->svgStyle()->light
ingColor());

Ditto.