[webkit-reviews] review denied: [Bug 134186] Make it clear to get m_svgExtensions using svgExtensions() : [Attachment 235879] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 21 23:21:57 PDT 2014

Daniel Bates <dbates at webkit.org> has denied Jeongeun Kim
<je_julie.kim at samsung.com>'s request for review:
Bug 134186: Make it clear to get m_svgExtensions using svgExtensions()

Attachment 235879: Patch

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=235879&action=review

> Source/WebCore/dom/Document.h:1088
> -    const SVGDocumentExtensions* svgExtensions();
> +    SVGDocumentExtensions* svgExtensions();
>      SVGDocumentExtensions* accessSVGExtensions();

What do you envision we do with accessSVGExtensions() given its similarity in
signature to to svgExtensions()? I mean, it's unclear which function should be
called to ensure we get a non-null SVGDocumentExtensions without looking at the
implementation of each function, looking at other call sites and reasoning
about their usage. One way to resolve this is to remove accessSVGExtensions()
entirely. Another way is to take an approach similar to
Node::ensureEventTargetData() and rename accessSVGExtensions() to
ensureSVGExtensions() and have it return a reference to SVGDocumentExtensions.
If we choose to take the latter approach then I suggest we rename
svgExtensions() to svgExtensionsIfExists() to better describe the
conditionality of its return value.

> Source/WebCore/svg/SVGDocumentExtensions.h:76
> -    const HashSet<SVGFontFaceElement*>& svgFontFaceElements() const { return
m_svgFontFaceElements; }
> +    const HashSet<SVGFontFaceElement*>& svgFontFaceElements() { return
m_svgFontFaceElements; }

How did you come to the decision to make this function non-const?

More information about the webkit-reviews mailing list