Bug 200405 – Consistently use Obj-C boolean literals

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

[Keith Rollin]

There are places where we use equivalent but different expressions for Obj-C boolean objects. For example, we use both [NSNumber numberWithBool:YES] and @YES. There are places where both are used in the same function, such as -[WebPreferences initialize]. The boolean literal is in greater use and is more succinct, so standardize on that.

[Radar WebKit Bug Importer]

<rdar://problem/53880043>

[Simon Fraser (smfr)]

Yes please!

[Keith Rollin]

Created attachment 375457 [details]
Patch

[EWS Watchlist]

Attachment 375457 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/mac/mainMac.mm:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/WebKitLauncher/WebKitNightlyEnabler.m:160:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/mac/InjectedBundleControllerMac.mm:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/mac/InjectedBundleControllerMac.mm:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/mainIOS.mm:41:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Examples/NetscapeCoreAnimationMoviePlugin/main.m:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Examples/NetscapeCoreAnimationMoviePlugin/main.m:367:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 7 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joseph Pecoraro]

Comment on attachment 375457 [details]
Patch

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

r=me

> Source/ThirdParty/libwebrtc/ChangeLog:9
> +        There are places where we use equivalent but different expressions for

Nice!

Can we also translate these forms `@(YES)` => `@YES`:

    WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
    991:        configuration._socketStreamProperties = @{ @"kCFStreamPropertyAutoErrorOnSystemChange" : @(NO) };

    WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm
    101:        [mutableRequest _setProperty:@(YES) forKey:(NSString *)kCFURLRequestContentDecoderSkipURLCheck];
    105:        [mutableRequest _setProperty:@(NO) forKey:(NSString *)_kCFURLConnectionPropertyShouldSniff];

    WebKit/UIProcess/WebAuthentication/Cocoa/LocalConnection.mm
    106:        kMAOptionsBAAIgnoreExistingKeychainItems: @(YES),
    109:        kMAOptionsBAASCRTAttestation: @(NO),

    WebCore/platform/network/mac/ResourceHandleMac.mm
    127:        [mutableRequest _setProperty:@(YES) forKey:(__bridge NSString *)kCFURLRequestContentDecoderSkipURLCheck];
    131:        [mutableRequest _setProperty:@(NO) forKey:(__bridge NSString *)_kCFURLConnectionPropertyShouldSniff];

    WebCore/platform/mediastream/mac/ScreenDisplayCaptureSourceMac.mm
    152:            (__bridge NSString *)kCGDisplayStreamShowCursor : @(YES),

    WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm
    103:            (__bridge NSString *)kCVPixelBufferCGImageCompatibilityKey: @(NO),
    105:            (__bridge NSString *)kCVPixelFormatOpenGLESCompatibility : @(YES),
    107:            (__bridge NSString *)kCVPixelBufferOpenGLCompatibilityKey : @(YES),

    WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm
    1134:                CMSetAttachment(sampleCopy.get(), kCMSampleBufferAttachmentKey_PostNotificationWhenConsumed, (__bridge CFDictionaryRef)@{ (__bridge NSString *)kCMSampleBufferAttachmentKey_PostNotificationWhenConsumed : @(YES) }, kCMAttachmentMode_ShouldNotPropagate);

    WebCore/platform/graphics/cv/ImageTransferSessionVT.mm
    66:    status = VTSessionSetProperty(transferSession, kVTPixelTransferPropertyKey_EnableHighSpeedTransfer, @(YES));
    70:    status = VTSessionSetProperty(transferSession, kVTPixelTransferPropertyKey_RealTime, @(YES));
    75:    status = VTSessionSetProperty(transferSession, kVTPixelTransferPropertyKey_EnableHardwareAcceleratedTransfer, @(YES));
    92:        (__bridge NSString *)cvPixelFormatOpenGLKey() : @(YES),
    283:        (__bridge NSString *)cvPixelFormatOpenGLKey() : @(YES),
    298:                (__bridge NSString *)kCVPixelBufferOpenGLCompatibilityKey : @(YES),

> Source/ThirdParty/libwebrtc/ChangeLog:17
> +        * Source/webrtc/examples/objc/AppRTCMobile/third_party/SocketRocket/SRWebSocket.m:
> +        (-[SRWebSocket _initializeStreams]):

Do we normally update webrtc sources? (Youenn would know)

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:669
> +        @NO,                            WebKitAdClickAttributionEnabledPreferenceKey,
>  #if ENABLE(INTERSECTION_OBSERVER)
>          @NO, WebKitIntersectionObserverEnabledPreferenceKey,

Now that we've shortened these to just a few characters, I think we probably don't need to line them up. Looking at all the new features down below, those down have whitespace alignment.

> Tools/TestWebKitAPI/mac/mainMac.mm:42
>      NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:

Ugh these are just asking for @{} now!

[Keith Rollin]

(In reply to Joseph Pecoraro from comment #5)
> Comment on attachment 375457 [details]
> Patch
> 
> Can we also translate these forms `@(YES)` => `@YES`:

Sure.

> > Source/ThirdParty/libwebrtc/ChangeLog:17
> > +        * Source/webrtc/examples/objc/AppRTCMobile/third_party/SocketRocket/SRWebSocket.m:
> > +        (-[SRWebSocket _initializeStreams]):
> 
> Do we normally update webrtc sources? (Youenn would know)

Probably not. I'll revert.

> > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:669
> > +        @NO,                            WebKitAdClickAttributionEnabledPreferenceKey,
> >  #if ENABLE(INTERSECTION_OBSERVER)
> >          @NO, WebKitIntersectionObserverEnabledPreferenceKey,
> 
> Now that we've shortened these to just a few characters, I think we probably
> don't need to line them up. Looking at all the new features down below,
> those down have whitespace alignment.

OK.

> > Tools/TestWebKitAPI/mac/mainMac.mm:42
> >      NSDictionary *dict = [NSDictionary dictionaryWithObjectsAndKeys:
> 
> Ugh these are just asking for @{} now!

Yeah, I'm thinking of some follow-on patches. :-)

[Keith Rollin]

Created attachment 375463 [details]
Patch

[EWS Watchlist]

Attachment 375463 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/mac/InjectedBundleControllerMac.mm:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/mac/InjectedBundleControllerMac.mm:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/mac/mainMac.mm:43:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Examples/NetscapeCoreAnimationMoviePlugin/main.m:0:  No copyright message found.  You should have a line: "Copyright [year] <Copyright Owner>"  [legal/copyright] [5]
ERROR: Examples/NetscapeCoreAnimationMoviePlugin/main.m:367:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/WebKitLauncher/WebKitNightlyEnabler.m:160:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/ios/mainIOS.mm:41:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 7 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 375463 [details]
Patch

Clearing flags on attachment: 375463

Committed r248190: <https://trac.webkit.org/changeset/248190>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Very satisfying: There may even be some performance benefit to making this change.