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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 22 03:10:14 PDT 2014


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





--- Comment #10 from Jeongeun Kim <je_julie.kim at samsung.com>  2014-08-22 03:10:17 PST ---
(In reply to comment #9)
> (From update of attachment 235879 [details])
> 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.
It's a great idea. I'll try the latter approach. Thank you for you suggestion.

> 
> > 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?
Actually, that is one of my worries and the part that I'd like to hear your opinion. 
I changed it to non-const and it's according to change of  'svgExtensions()' but I think it's not good.
Do I need to keep 'const SVGDocumentExtensions* svgExtensions()' for this part?

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



More information about the webkit-unassigned mailing list