[webkit-reviews] review granted: [Bug 213608] Upstream macOS 11 additions to RenderThemeMac.mm and ThemeMac.mm : [Attachment 402740] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 213608: Upstream macOS 11 additions to RenderThemeMac.mm and ThemeMac.mm
https://bugs.webkit.org/show_bug.cgi?id=213608

Attachment 402740: Patch

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




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 402740
  --> https://bugs.webkit.org/attachment.cgi?id=402740
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.