| Summary: | Leak of SVGFontFaceElement when RenderStyle holds onto a FontRanges which uses it | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||||||
| Component: | SVG | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | ews-watchlist, koivisto, mmaxfield, rniwa, sabouhallawa, simon.fraser, zalan, zimmermann | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Ryosuke Niwa
2019-03-20 23:14:05 PDT
Created attachment 365497 [details]
WIP (missing a test)
The patch works but still working on a test.
Created attachment 365498 [details]
test (WIP)
Somehow this always passes... I suspect RenderStyle isn't storing things as expected. Gonna look into it tomorrow.
Attachment 365497 [details] did not pass style-queue:
ERROR: Source/WebCore/ChangeLog:17: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 1 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 365497 [details] WIP (missing a test) Attachment 365497 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11595109 New failing tests: http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html Created attachment 365519 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Alright, writing a test here is impossible. We need to trigger select-all in a SVG document followed by a top-level navigation, etc... to trigger a leak. Created attachment 365676 [details]
font-leak-1.svg
Created attachment 365677 [details]
font-leak-2.html
./Tools/Scripts/run-webkit-tests --debug svg/dom/font-leak-1.svg svg/dom/font-leak-2.html --leaks --no-retry --no-show-results --child-processes=1 would reproduce the issue. Unfortunately, navigating via location.href seems to work once but then managing the state between navigations & detecting the actual leak within a single test is basically impossible. Created attachment 365678 [details]
Fixes the bug
*** Bug 189460 has been marked as a duplicate of this bug. *** Comment on attachment 365678 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=365678&action=review > Source/WebCore/ChangeLog:18 > + No new tests. Unfortunately, writing a test proved to be intractable. The leak can be reproduced by running > + svg/text/text-text-05-t.svg then svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html consecutively. Does running with --world-leaks reveal the leak (and prove the fix)? (In reply to Simon Fraser (smfr) from comment #12) > Comment on attachment 365678 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=365678&action=review > > > Source/WebCore/ChangeLog:18 > > + No new tests. Unfortunately, writing a test proved to be intractable. The leak can be reproduced by running > > + svg/text/text-text-05-t.svg then svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html consecutively. > > Does running with --world-leaks reveal the leak (and prove the fix)? I didn't try --world-leaks but --leaks definitely detects this leak and the fix works. Confirmed that I can detect a leak with ./Tools/Scripts/run-webkit-tests --debug --world-leaks svg/text/text-text-05-t.svg and the leak is gone with my patch! Committed r243483: <https://trac.webkit.org/changeset/243483> |