WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(5.40 KB, patch)
2021-06-03 23:12 PDT
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch
(5.38 KB, patch)
2021-06-03 23:13 PDT
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1
2021-06-03 22:17:10 PDT
Created
attachment 430539
[details]
Patch
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
Created
attachment 430547
[details]
Patch
Myles C. Maxfield
Comment 5
2021-06-03 23:13:02 PDT
Created
attachment 430548
[details]
Patch
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
<
rdar://problem/78872787
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug