Bug 210296

Summary: [Cocoa] The function WebCore::systemHasBattery() should cache the result.
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: WebKit Misc.Assignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, darin, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Per Arne Vollan 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.
Comment 1 Per Arne Vollan 2020-04-09 12:40:18 PDT 
rdar://problem/61331536
Comment 2 Per Arne Vollan 2020-04-09 12:42:46 PDT 
Created attachment 395991 [details]
Patch
Comment 3 Darin Adler 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;
Comment 4 Per Arne Vollan 2020-04-09 12:58:41 PDT 
Created attachment 395992 [details]
Patch
Comment 5 Per Arne Vollan 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!
Comment 6 EWS 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].