[webkit-reviews] review granted: [Bug 211457] Add the feature flag plist file parser : [Attachment 398531] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 211457: Add the feature flag plist file parser
https://bugs.webkit.org/show_bug.cgi?id=211457

Attachment 398531: Patch

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




--- Comment #3 from Tim Horton <thorton at apple.com> ---
Comment on attachment 398531
  --> https://bugs.webkit.org/attachment.cgi?id=398531
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?