Bug 211457

Summary: Add the feature flag plist file parser
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: New BugsAssignee: Peng Liu <peng.liu6>
Status: RESOLVED FIXED    
Severity: Normal CC: jer.noble, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
thorton: review+
Revised patch based on Tim's comments none

Peng Liu
Reported 2020-05-05 11:22:38 PDT
Add the feature flag plist file parser
Attachments
Patch (11.48 KB, patch)
2020-05-05 11:28 PDT, Peng Liu 		thorton: review+
DetailsFormatted DiffDiff
Revised patch based on Tim's comments (14.81 KB, patch)
2020-05-12 19:42 PDT, Peng Liu 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Peng Liu
Comment 1 2020-05-05 11:23:17 PDT
<rdar://problem/60562564>
Peng Liu
Comment 2 2020-05-05 11:28:18 PDT
Created attachment 398531 [details] Patch
Tim Horton
Comment 3 2020-05-12 17:50:28 PDT
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
Comment 4 2020-05-12 19:42:37 PDT
Created attachment 399233 [details] Revised patch based on Tim's comments
Peng Liu
Comment 5 2020-05-12 22:21:00 PDT
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
Comment 6 2020-05-12 22:35:44 PDT
Committed r261599: <https://trac.webkit.org/changeset/261599> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399233 [details].
Note You need to log in before you can comment on or make changes to this bug.