WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
213608
Upstream macOS 11 additions to RenderThemeMac.mm and ThemeMac.mm
https://bugs.webkit.org/show_bug.cgi?id=213608
Summary
Upstream macOS 11 additions to RenderThemeMac.mm and ThemeMac.mm
Wenson Hsieh
Reported
2020-06-25 09:02:39 PDT
<
rdar://problem/64758299
>
Attachments
Patch
(6.60 KB, patch)
2020-06-25 09:18 PDT
,
Wenson Hsieh
darin
: review+
Details
Formatted Diff
Diff
Patch for landing
(6.44 KB, patch)
2020-06-25 10:13 PDT
,
Wenson Hsieh
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Wenson Hsieh
Comment 1
2020-06-25 09:18:10 PDT
Created
attachment 402740
[details]
Patch
Darin Adler
Comment 2
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.
Wenson Hsieh
Comment 3
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.
Wenson Hsieh
Comment 4
2020-06-25 10:13:52 PDT
Created
attachment 402742
[details]
Patch for landing
EWS
Comment 5
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]
.
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