| Summary: | [Cocoa] Broker access to the PowerManagement API | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> | ||||||||||||
| Component: | WebKit Misc. | Assignee: | Per Arne Vollan <pvollan> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | bfulgham, commit-queue, darin, ggaren, sam, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Per Arne Vollan
2020-01-16 14:16:03 PST
Created attachment 387961 [details]
Patch
Created attachment 387964 [details]
Patch
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. Created attachment 388070 [details]
Patch
(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! Created attachment 388120 [details]
Patch
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. (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. Created attachment 388471 [details]
Patch
Comment on attachment 388471 [details] Patch Clearing flags on attachment: 388471 Committed r254995: <https://trac.webkit.org/changeset/254995> |