Bug 211457 – Add the feature flag plist file parser

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Peng Liu]

Add the feature flag plist file parser

[Peng Liu]

<rdar://problem/60562564>

[Peng Liu]

Created attachment 398531 [details]
Patch

[Tim Horton]

Comment on attachment 398531 [details]
Patch

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

> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:41
> +bool featureFlagEnabled(const String& featureName)

maybe a leading is-? I'm not sure

> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:43
> +    BOOL isWebKitBundleFromStageFramework = [[[NSBundle mainBundle] bundlePath] hasPrefix:@"/Library/Apple/System/Library/StagedFrameworks/WebKit"];

"staged", not "stage"

> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:46
> +        return _os_feature_enabled_impl("WebKit", (const char*)featureName.characters8());

I think you want `.utf8().data()`, no?

> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:50
> +    if ([[dictionary objectForKey:featureName] objectForKey:@"Enabled"] == nil)

normally we use if (!x) instead of if (x == nil)

> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:53
> +    return [[[dictionary objectForKey:featureName] objectForKey:@"Enabled"] boolValue];

Do you want to make sure it's an NSNumber before calling -boolValue?

[Peng Liu]

Created attachment 399233 [details]
Revised patch based on Tim's comments

[Peng Liu]

Comment on attachment 398531 [details]
Patch

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

>> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:41
>> +bool featureFlagEnabled(const String& featureName)
> 
> maybe a leading is-? I'm not sure

Renamed to isFeatureFlagEnabled().

>> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:43
>> +    BOOL isWebKitBundleFromStageFramework = [[[NSBundle mainBundle] bundlePath] hasPrefix:@"/Library/Apple/System/Library/StagedFrameworks/WebKit"];
> 
> "staged", not "stage"

Fixed.

>> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:46
>> +        return _os_feature_enabled_impl("WebKit", (const char*)featureName.characters8());
> 
> I think you want `.utf8().data()`, no?

Right, fixed.

>> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:50
>> +    if ([[dictionary objectForKey:featureName] objectForKey:@"Enabled"] == nil)
> 
> normally we use if (!x) instead of if (x == nil)

Fixed.

>> Source/WebKit/Shared/Cocoa/WebPreferencesDefaultValuesCocoa.mm:53
>> +    return [[[dictionary objectForKey:featureName] objectForKey:@"Enabled"] boolValue];
> 
> Do you want to make sure it's an NSNumber before calling -boolValue?

Fixed.

[EWS]

Committed r261599: <https://trac.webkit.org/changeset/261599>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399233 [details].