Bug 82242

Summary: Remove Document::mappedElementSheet()
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: kling, macpherson, menard, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 82339    
Bug Blocks: 77745    
Attachments:
Description Flags
patch
webkit-ews: commit-queue-
Try to fix qt build
webkit-ews: commit-queue-
moved the map to SVGDocumentExtensions
webkit-ews: commit-queue-
test SVG_FONTS instead of SVG none

Antti Koivisto
Reported 2012-03-26 13:51:09 PDT
Only thing this is used for anymore is SVGFontFaceElement. That can be handled without confusing extra stylesheets.
Attachments
patch (11.87 KB, patch)
2012-03-26 14:05 PDT, Antti Koivisto 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
Try to fix qt build (12.19 KB, patch)
2012-03-27 02:51 PDT, Antti Koivisto 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
moved the map to SVGDocumentExtensions (14.58 KB, patch)
2012-03-27 04:02 PDT, Antti Koivisto 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
test SVG_FONTS instead of SVG (14.33 KB, patch)
2012-03-27 04:29 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2012-03-26 14:05:51 PDT
Created attachment 133889 [details] patch
Early Warning System Bot
Comment 2 2012-03-26 15:31:21 PDT
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 3 2012-03-26 15:46:43 PDT
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 4 2012-03-26 15:55:35 PDT
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
Comment 5 2012-03-27 02:51:19 PDT
Created attachment 134010 [details] Try to fix qt build
Early Warning System Bot
Comment 6 2012-03-27 03:03:45 PDT
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 7 2012-03-27 03:06:22 PDT
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 8 2012-03-27 03:06:57 PDT
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
Comment 9 2012-03-27 04:02:26 PDT
Created attachment 134017 [details] moved the map to SVGDocumentExtensions
Early Warning System Bot
Comment 10 2012-03-27 04:14:39 PDT
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 11 2012-03-27 04:18:22 PDT
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
Comment 12 2012-03-27 04:29:31 PDT
Created attachment 134021 [details] test SVG_FONTS instead of SVG
Andreas Kling
Comment 13 2012-03-27 04:35:40 PDT
Comment on attachment 134021 [details] test SVG_FONTS instead of SVG Me gusta.
Nikolas Zimmermann
Comment 14 2012-03-27 04:41:30 PDT
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
Comment 15 2012-03-27 05:41:10 PDT
http://trac.webkit.org/changeset/112258
Csaba Osztrogonác
Comment 16 2012-03-27 07:30:01 PDT
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
Comment 17 2012-03-27 08:03:16 PDT
Removed the assert in ~SVGDocumentExtensions in http://trac.webkit.org/changeset/112269. It seems insertedInto/removedFromDocument don't always pair.
Note You need to log in before you can comment on or make changes to this bug.