Bug 206373 – [Cocoa] Broker access to the PowerManagement API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Per Arne Vollan]

Power management calls should be performed in the UI process.

[Per Arne Vollan]

rdar://problem/34722450

[Per Arne Vollan]

Created attachment 387961 [details]
Patch

[Per Arne Vollan]

Created attachment 387964 [details]
Patch

[Sam Weinig]

Comment on attachment 387964 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetTrackUtilities.h:40
> +WEBCORE_EXPORT void setSystemHasBattery(bool);
> +WEBCORE_EXPORT bool systemHasBattery();

This does not seem like the right file for these. If they need to be exposed outside of the .mm file, please move them to a more appropriate place or create a new file for them.

[Per Arne Vollan]

Created attachment 388070 [details]
Patch

[Per Arne Vollan]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 387964 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387964&action=review
> 
> > Source/WebCore/platform/graphics/avfoundation/objc/AVAssetTrackUtilities.h:40
> > +WEBCORE_EXPORT void setSystemHasBattery(bool);
> > +WEBCORE_EXPORT bool systemHasBattery();
> 
> This does not seem like the right file for these. If they need to be exposed
> outside of the .mm file, please move them to a more appropriate place or
> create a new file for them.

Good point! I have moved this to a new file in the new patch.

Thanks for reviewing, Sam!

[Per Arne Vollan]

Created attachment 388120 [details]
Patch

[Darin Adler]

Comment on attachment 388120 [details]
Patch

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

> Source/WebCore/platform/cocoa/PowerUtils.h:2
> +* Copyright (C) 2020 Apple Inc. All rights reserved.

File naming: A file named "XXXUtils" is against WebKit coding style. Not sure if we wrote it down or not.

I suggest the name SystemBattery.h/mm. But there are probably other good names to consider.

The vague programmer word "utilities" should be avoided, even though this rule has not been followed consistently. Lets not make another header with a name like this. Even just "Power.h" might be OK.

The abbreviation "utils" goes against our stance against unnecessary abbreviation.

[Per Arne Vollan]

(In reply to Darin Adler from comment #8)
> Comment on attachment 388120 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388120&action=review
> 
> > Source/WebCore/platform/cocoa/PowerUtils.h:2
> > +* Copyright (C) 2020 Apple Inc. All rights reserved.
> 
> File naming: A file named "XXXUtils" is against WebKit coding style. Not
> sure if we wrote it down or not.
> 
> I suggest the name SystemBattery.h/mm. But there are probably other good
> names to consider.
> 
> The vague programmer word "utilities" should be avoided, even though this
> rule has not been followed consistently. Lets not make another header with a
> name like this. Even just "Power.h" might be OK.
> 
> The abbreviation "utils" goes against our stance against unnecessary
> abbreviation.

Thanks for reviewing! I will update the patch according to the comments :) I think I will go with SystemBattery.h/mm as you suggested.

[Per Arne Vollan]

Created attachment 388471 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 388471 [details]
Patch

Clearing flags on attachment: 388471

Committed r254995: <https://trac.webkit.org/changeset/254995>