[webkit-reviews] review denied: [Bug 61183] SVG svgAttributeChanged/synchronizeProperty/parseMappedAttribute should be optimized : [Attachment 94227] Patch v5

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 61183: SVG svgAttributeChanged/synchronizeProperty/parseMappedAttribute
should be optimized
https://bugs.webkit.org/show_bug.cgi?id=61183

Attachment 94227: Patch v5
https://bugs.webkit.org/attachment.cgi?id=94227&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=94227&action=review

I like the code changes. Once some of my questions and ChangeLog suggestions
are cleared up this can be r+ed quickly I think, until then r-.

> Source/WebCore/ChangeLog:18
> +	   Switch to a more efficient pattern, by providing a "bool
isSupportedAttribute(const QualifiedName&);" function for all SVG*Elements.

Typo: hierarchy

> Source/WebCore/ChangeLog:31
> +	       // Note for eg. SVGNames::transformAttr, the call from
SVGRectElement::svgAttributeChanged, would be immediately forwarded to the base
class, which handles transformAttr changes)

You mean by the if section above? If so the Note can be moved above it to
indicate what the if section does.

> Source/WebCore/ChangeLog:47
> +

dance sounds a bit frivolous :) Better reword

> Source/WebCore/ChangeLog:48
> +	   Add "SVGElementInstance::InvalidationGuard guard(this)" statments in
all svgAttributeChanged implementations, that calls
invalidateAllInstancesOfElement(this)

Typo: statements

> Source/WebCore/ChangeLog:53
> +

how? Better, worse, the same?