| Summary: | Free FontFaceSets may include fonts that were never actually added to them | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||
| Component: | Text | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | commit-queue, dino, ews-watchlist, jonlee, koivisto, mmaxfield, simon.fraser, thorton, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | Other | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 180062 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Myles C. Maxfield
2017-11-29 13:18:43 PST
Created attachment 327892 [details]
Reproduction. Should alert("0")
Looks like neither Firefox nor Chrome support constructing free FontFaceSets? at least, "new FontFaceSet([])" gives an error in those browsers. Created attachment 327950 [details]
WIP
Attachment 327950 [details] did not pass style-queue:
ERROR: Source/WebCore/ChangeLog:8: 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 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 328011 [details]
Patch
Comment on attachment 328011 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328011&action=review > Source/WebCore/css/CSSFontFaceSet.cpp:169 > + if (m_owningFontSelector) Why the if check if it's always non-null? > Source/WebCore/css/CSSFontFaceSet.h:110 > + CSSFontSelector* m_owningFontSelector; If this is always non-null why not use a ref? Comment on attachment 328011 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328011&action=review >> Source/WebCore/css/CSSFontFaceSet.h:110 >> + CSSFontSelector* m_owningFontSelector; > > If this is always non-null why not use a ref? One client calls CSSFontFaceSet::create() with no arguments, and the (only) other client calls it with (this) as an argument. Comment on attachment 328011 [details] Patch Clearing flags on attachment: 328011 Committed r225414: <https://trac.webkit.org/changeset/225414> All reviewed patches have been landed. Closing bug. |