RESOLVED FIXED 210296
[Cocoa] The function WebCore::systemHasBattery() should cache the result.
https://bugs.webkit.org/show_bug.cgi?id=210296
Summary [Cocoa] The function WebCore::systemHasBattery() should cache the result.
Per Arne Vollan
Reported 2020-04-09 12:39:49 PDT
The function WebCore::systemHasBattery() should cache the result, since the return value of this function will be the same on a specific device.
Attachments
Patch (2.78 KB, patch)
2020-04-09 12:42 PDT, Per Arne Vollan
no flags
Patch (2.93 KB, patch)
2020-04-09 12:58 PDT, Per Arne Vollan
no flags
Per Arne Vollan
Comment 1 2020-04-09 12:40:18 PDT
Per Arne Vollan
Comment 2 2020-04-09 12:42:46 PDT
Darin Adler
Comment 3 2020-04-09 12:49:47 PDT
Comment on attachment 395991 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395991&action=review > Source/WebCore/platform/cocoa/SystemBattery.mm:45 > + static bool hasBattery = [] { Very confusing that we have two different global booleans of different types and scopes, but the same names. I suggest not defining a second boolean here, and just assigning a value to the first one. > Source/WebCore/platform/cocoa/SystemBattery.mm:61 > + return hasBattery; This would then be: return *hasBattery; Or you could write the function as: if (!hasBattery) { hasBattery = [] { <....> }(); } return *hasBattery;
Per Arne Vollan
Comment 4 2020-04-09 12:58:41 PDT
Per Arne Vollan
Comment 5 2020-04-09 12:59:22 PDT
(In reply to Darin Adler from comment #3) > Comment on attachment 395991 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=395991&action=review > > > Source/WebCore/platform/cocoa/SystemBattery.mm:45 > > + static bool hasBattery = [] { > > Very confusing that we have two different global booleans of different types > and scopes, but the same names. > > I suggest not defining a second boolean here, and just assigning a value to > the first one. > > > Source/WebCore/platform/cocoa/SystemBattery.mm:61 > > + return hasBattery; > > This would then be: > > return *hasBattery; > > Or you could write the function as: > > if (!hasBattery) { > hasBattery = [] { > <....> > }(); > } > return *hasBattery; Very good point, thanks for reviewing!
EWS
Comment 6 2020-04-09 14:04:52 PDT
Committed r259827: <https://trac.webkit.org/changeset/259827> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395992 [details].
Note You need to log in before you can comment on or make changes to this bug.