[Webkit-unassigned] [Bug 134186] Make it clear to get m_svgExtensions using svgExtensions()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=134186


Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #235879|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #9 from Daniel Bates <dbates at webkit.org>  2014-08-21 23:22:06 PST ---
(From update of attachment 235879)
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?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.