Bug 230187

Summary: [Cocoa] Drawing the rounded system ui font into canvas causes a crash
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: jjorgenson, levindixon, luonan.666, mouad.debbar, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=229633
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
wenson_hsieh: review+
Fix leak mmaxfield: review+

Description Myles C. Maxfield 2021-09-11 01:56:51 PDT 
[Cocoa] Drawing the rounded system ui font into canvas causes a crash
Comment 1 Myles C. Maxfield 2021-09-11 01:59:36 PDT 
Created attachment 437945 [details]
Patch
Comment 2 Myles C. Maxfield 2021-09-11 01:59:38 PDT 
<rdar://problem/81436658>
Comment 3 Myles C. Maxfield 2021-09-11 16:52:36 PDT 
Created attachment 437972 [details]
Patch
Comment 4 Myles C. Maxfield 2021-09-11 18:15:42 PDT 
Created attachment 437974 [details]
Patch
Comment 5 Myles C. Maxfield 2021-09-11 18:16:36 PDT 
Created attachment 437975 [details]
Patch
Comment 6 Wenson Hsieh 2021-09-12 11:43:51 PDT 
Comment on attachment 437975 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437975&action=review

> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:517
>      if (!fontDescriptor)
>          return nullptr;

What do you think about using `CTFontDescriptorCreateLastResort()` here as a sort of failsafe (perhaps with an `ASSERT_NOT_REACHED` so that we're still able to catch instances where we would've otherwise failed to decode)?
Comment 7 Myles C. Maxfield 2021-09-12 19:24:26 PDT 
Comment on attachment 437975 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437975&action=review

>> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:517
>>          return nullptr;
> 
> What do you think about using `CTFontDescriptorCreateLastResort()` here as a sort of failsafe (perhaps with an `ASSERT_NOT_REACHED` so that we're still able to catch instances where we would've otherwise failed to decode)?

That is a good idea!
Comment 8 Myles C. Maxfield 2021-09-12 20:23:08 PDT 
Committed r282320 (241590@main): <https://commits.webkit.org/241590@main>
Comment 9 Wenson Hsieh 2021-09-13 09:57:55 PDT 
Reopening to attach new patch.
Comment 10 Wenson Hsieh 2021-09-13 09:57:56 PDT 
Created attachment 438049 [details]
Fix leak
Comment 11 Myles C. Maxfield 2021-09-13 12:41:26 PDT 
Committed r282349 (241615@main): <https://commits.webkit.org/241615@main>
Comment 12 Wenson Hsieh 2021-10-11 09:42:08 PDT 
*** Bug 231495 has been marked as a duplicate of this bug. ***
Comment 13 Myles C. Maxfield 2021-10-14 23:05:49 PDT 
*** Bug 231686 has been marked as a duplicate of this bug. ***
Comment 14 Alexey Proskuryakov 2021-10-19 17:47:37 PDT 
*** Bug 231988 has been marked as a duplicate of this bug. ***
Comment 15 Sam Sneddon [:gsnedders] 2021-10-21 02:11:57 PDT 
*** Bug 232043 has been marked as a duplicate of this bug. ***