Created attachment 402740 [details]
Patch

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 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.

Created attachment 402742 [details]
Patch for landing

Committed r263519: <https://trac.webkit.org/changeset/263519>

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