Bug 213608

Summary: Upstream macOS 11 additions to RenderThemeMac.mm and ThemeMac.mm
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: FormsAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, cdumez, changseok, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, macpherson, megan_gardner, menard, pdr, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
darin: review+
Patch for landing none

Description Wenson Hsieh 2020-06-25 09:02:39 PDT 
<rdar://problem/64758299>
Comment 1 Wenson Hsieh 2020-06-25 09:18:10 PDT 
Created attachment 402740 [details]
Patch
Comment 2 Darin Adler 2020-06-25 09:23:28 PDT 
Comment on attachment 402740 [details]
Patch

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

> Source/WebCore/platform/mac/ThemeMac.mm:994
> +    static bool hasSupport = false;
> +    static dispatch_once_t onceToken;
> +    dispatch_once(&onceToken, ^{
> +        hasSupport = [[NSAppearance currentAppearance] _usesMetricsAppearance];
> +    });
> +    return hasSupport;

Overkill to use dispatch_once here; this is not called concurrently on multiple threads. Should just be:

    static bool hasSupport = [[NSAppearance currentAppearance] _usesMetricsAppearance];

> Source/WebCore/rendering/RenderThemeMac.mm:2738
> +    auto image = retainPtr([NSImage _imageWithSystemSymbolName:@"arrow.down.circle"]);

No need for this retainPtr; this is autoreleased and should work fine with a raw pointer.

> Source/WebCore/rendering/RenderThemeMac.mm:2743
> +    auto cgImage = retainPtr([image CGImageForProposedRect:&imageRect context:nil hints:@{

Ditto.
Comment 3 Wenson Hsieh 2020-06-25 10:13:16 PDT 
Comment on attachment 402740 [details]
Patch

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

Thanks for the review!

>> Source/WebCore/platform/mac/ThemeMac.mm:994
>> +    return hasSupport;
> 
> Overkill to use dispatch_once here; this is not called concurrently on multiple threads. Should just be:
> 
>     static bool hasSupport = [[NSAppearance currentAppearance] _usesMetricsAppearance];

Sounds good — removed the dispatch_once.

>> Source/WebCore/rendering/RenderThemeMac.mm:2738
>> +    auto image = retainPtr([NSImage _imageWithSystemSymbolName:@"arrow.down.circle"]);
> 
> No need for this retainPtr; this is autoreleased and should work fine with a raw pointer.

Fixed.

>> Source/WebCore/rendering/RenderThemeMac.mm:2743
>> +    auto cgImage = retainPtr([image CGImageForProposedRect:&imageRect context:nil hints:@{
> 
> Ditto.

Fixed.
Comment 4 Wenson Hsieh 2020-06-25 10:13:52 PDT 
Created attachment 402742 [details]
Patch for landing
Comment 5 EWS 2020-06-25 10:39:48 PDT 
Committed r263519: <https://trac.webkit.org/changeset/263519>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402742 [details].