RESOLVED FIXED 226622
Use references in font loading code instead of pointers which can never be null
https://bugs.webkit.org/show_bug.cgi?id=226622
Summary Use references in font loading code instead of pointers which can never be null
Myles C. Maxfield
Reported 2021-06-03 21:01:11 PDT
Use references instead of pointers which can never be null
Attachments
Patch (9.71 KB, patch)
2021-06-03 22:17 PDT, Myles C. Maxfield
no flags
Patch (5.40 KB, patch)
2021-06-03 23:12 PDT, Myles C. Maxfield
no flags
Patch (5.38 KB, patch)
2021-06-03 23:13 PDT, Myles C. Maxfield
no flags
Myles C. Maxfield
Comment 1 2021-06-03 22:17:10 PDT
Chris Dumez
Comment 2 2021-06-03 22:57:02 PDT
Comment on attachment 430539 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430539&action=review > Source/WebCore/css/CSSFontFaceSet.cpp:121 > + Ref<CSSFontFace> face = CSSFontFace::create(*m_owningFontSelector.get(), nullptr, nullptr, true); .get() should not be needed. > Source/WebCore/css/FontFaceSet.cpp:169 > + ASSERT(scriptExecutionContext); I don't see how this is safe. FontFaceSet is an ActiveDOMObject and an ActiveDOMObject's scriptExecutionContext can definitely become null. I would imagine it is able to hit the assertion by: 1. Having the top frame get a FontFaceSet from a subframe 2. Remove that subframe from the document 3. Have the top frame call load() on that FontFaceSet
Myles C. Maxfield
Comment 3 2021-06-03 23:07:43 PDT
(In reply to Chris Dumez from comment #2) > Comment on attachment 430539 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=430539&action=review > > > Source/WebCore/css/FontFaceSet.cpp:169 > > + ASSERT(scriptExecutionContext); > > I don't see how this is safe. FontFaceSet is an ActiveDOMObject and an > ActiveDOMObject's scriptExecutionContext can definitely become null. I would > imagine it is able to hit the assertion by: > 1. Having the top frame get a FontFaceSet from a subframe > 2. Remove that subframe from the document > 3. Have the top frame call load() on that FontFaceSet Aha! You're right. When I was reading ContextDestructionObserver I missed the implementation of the contextDestroyed() function.
Myles C. Maxfield
Comment 4 2021-06-03 23:12:18 PDT
Myles C. Maxfield
Comment 5 2021-06-03 23:13:02 PDT
Chris Dumez
Comment 6 2021-06-03 23:15:50 PDT
Comment on attachment 430548 [details] Patch r=me
EWS
Comment 7 2021-06-04 09:34:22 PDT
Committed r278466 (238486@main): <https://commits.webkit.org/238486@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430548 [details].
Radar WebKit Bug Importer
Comment 8 2021-06-04 09:35:16 PDT
Note You need to log in before you can comment on or make changes to this bug.