Bug 82242 – Remove Document::mappedElementSheet()

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

Only thing this is used for anymore is SVGFontFaceElement. That can be handled without confusing extra stylesheets.

[Antti Koivisto]

Created attachment 133889 [details]
patch

[Early Warning System Bot]

Comment on attachment 133889 [details]
patch

Attachment 133889 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12141126

[Early Warning System Bot]

Comment on attachment 133889 [details]
patch

Attachment 133889 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12076033

[Andreas Kling]

Comment on attachment 133889 [details]
patch

TD;DR but it looks pretty good. Fix the build and slap me for an r+ tomorrow.

[Antti Koivisto]

Created attachment 134010 [details]
Try to fix qt build

[Early Warning System Bot]

Comment on attachment 134010 [details]
Try to fix qt build 

Attachment 134010 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12147392

[Early Warning System Bot]

Comment on attachment 134010 [details]
Try to fix qt build 

Attachment 134010 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12142409

[Nikolas Zimmermann]

Comment on attachment 134010 [details]
Try to fix qt build 

View in context: https://bugs.webkit.org/attachment.cgi?id=134010&action=review

> Source/WebCore/dom/Document.h:638
> +    HashSet<SVGFontFaceElement*>& svgFontFaceElements();

As discussed on IRC, this needs ENABLE(SVG) guards, and should be moved right into SVGDocumentExtensions.

[Antti Koivisto]

Created attachment 134017 [details]
moved the map to SVGDocumentExtensions

[Early Warning System Bot]

Comment on attachment 134017 [details]
moved the map to SVGDocumentExtensions

Attachment 134017 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12147423

[Early Warning System Bot]

Comment on attachment 134017 [details]
moved the map to SVGDocumentExtensions

Attachment 134017 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12142434

[Antti Koivisto]

Created attachment 134021 [details]
test SVG_FONTS instead of SVG

[Andreas Kling]

Comment on attachment 134021 [details]
test SVG_FONTS instead of SVG

Me gusta.

[Nikolas Zimmermann]

Comment on attachment 134021 [details]
test SVG_FONTS instead of SVG

View in context: https://bugs.webkit.org/attachment.cgi?id=134021&action=review

r=me with comments:

> Source/WebCore/svg/SVGDocumentExtensions.cpp:421
> +void SVGDocumentExtensions::registerSVGFontFaceElement(SVGFontFaceElement* element)

How about checking in the SVGDocExt dtor that this set is empty? I'm not sure we do it for the others, but we should do so to catch missing unregister() calls.
I hope removedFromDocument is guaranteed to be called :-)

[Antti Koivisto]

http://trac.webkit.org/changeset/112258

[Csaba Osztrogonác]

Reopen, because it broke debug builds on Qt:
../../../../Source/WebCore/svg/SVGDocumentExtensions.cpp: In destructor ‘WebCore::SVGDocumentExtensions::~SVGDocumentExtensions()’:
../../../../Source/WebCore/svg/SVGDocumentExtensions.cpp:53: error: ‘m_svgFontFaceElements’ was not declared in this scope

Additionally it made almost all tests assert:
STDERR: ASSERTION FAILED: m_svgFontFaceElements.isEmpty()
STDERR: ../../Source/WebCore/svg/SVGDocumentExtensions.cpp(53) : WebCore::SVGDocumentExtensions::~SVGDocumentExtensions()

[Antti Koivisto]

Removed the assert in ~SVGDocumentExtensions in http://trac.webkit.org/changeset/112269. It seems insertedInto/removedFromDocument don't always pair.