Bug 200405 - Consistently use Obj-C boolean literals
Summary: Consistently use Obj-C boolean literals
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Rollin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-02 14:33 PDT by Keith Rollin
Modified: 2019-08-04 11:11 PDT (History)
9 users (show)

See Also:


Attachments
Patch (44.14 KB, patch)
2019-08-02 14:35 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff
Patch (55.96 KB, patch)
2019-08-02 15:56 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Rollin 2019-08-02 14:33:05 PDT
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.
Comment 1 Radar WebKit Bug Importer 2019-08-02 14:33:22 PDT
<rdar://problem/53880043>
Comment 2 Simon Fraser (smfr) 2019-08-02 14:33:49 PDT
Yes please!
Comment 3 Keith Rollin 2019-08-02 14:35:46 PDT
Created attachment 375457 [details]
Patch
Comment 4 EWS Watchlist 2019-08-02 14:38:06 PDT
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.
Comment 5 Joseph Pecoraro 2019-08-02 14:48:22 PDT
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!
Comment 6 Keith Rollin 2019-08-02 14:51:49 PDT
(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. :-)
Comment 7 Keith Rollin 2019-08-02 15:56:43 PDT
Created attachment 375463 [details]
Patch
Comment 8 EWS Watchlist 2019-08-02 15:59:40 PDT
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.
Comment 9 WebKit Commit Bot 2019-08-02 16:44:53 PDT
Comment on attachment 375463 [details]
Patch

Clearing flags on attachment: 375463

Committed r248190: <https://trac.webkit.org/changeset/248190>
Comment 10 WebKit Commit Bot 2019-08-02 16:44:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Darin Adler 2019-08-04 11:11:41 PDT
Very satisfying: There may even be some performance benefit to making this change.