We currently have a very fragile system where we have a flag enabling experimental features for our development builds and Safari Technology Preview, but do not want to ship this not-ready-for-prime-time features in releases. This generally involves landing a branch-specific hard-coded flag to turn these features off. This is silly! We know what we are building. Let's let our build system do this work for us.
<rdar://problem/40782992>
Created attachment 341946 [details] Patch
Created attachment 341949 [details] Patch
Comment on attachment 341949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341949&action=review > Source/WebKit/ChangeLog:11 > + Use the WK_RELOCATABLE_FRAMEWORKS flag (which is always defined for non-production builds) > + to define ENABLE(EXPERIMENTAL_FEATURES) so that we do not need to manually > + change this flag when preparing for a production release. This seems like a reasonable plan, but then what you did below is strangely different and unnecessarily complicated. > Source/WebKit/Configurations/BaseTarget.xcconfig:39 > +GCC_PREPROCESSOR_DEFINITIONS = $(DEBUG_DEFINES) $(FEATURE_DEFINES) $(WK_MANUAL_SANDBOXING_DEFINES) $(WK_CORE_PREDICTION_DEFINES) U_DISABLE_RENAMING=1 U_SHOW_CPLUSPLUS_API=0 WK_RELOCATABLE_FRAMEWORKS=$(WK_RELOCATABLE_FRAMEWORKS) FRAMEWORK_NAME=WebKit; Instead of doing this, just make sure that ENABLE_EXPERIMENTAL_FEATURES is defined correctly. You can do this by adding something like this to (every copy of) FeatureDefines.xcconfig: ENABLE_EXPERIMENTAL_FEATURES = $(ENABLE_EXPERIMENTAL_FEATURES_$(WK_RELOCATABLE_FRAMEWORKS)); ENABLE_EXPERIMENTAL_FEATURES_YES = ENABLE_EXPERIMENTAL_FEATURES; And adding $(ENABLE_EXPERIMENTAL_FEATURES) to FEATURE_DEFINES in that file. > Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:69 > +bool enableExperimentalFeatures() > +{ > +#if PLATFORM(COCOA) && defined(WK_RELOCATABLE_FRAMEWORKS) > + return STRINGIZE_VALUE_OF(WK_RELOCATABLE_FRAMEWORKS) == "YES"; > +#elif ENABLE(EXPERIMENTAL_FEATURES) > + return true; > +#else > + return false; > +#endif > +} Then you won’t need this. > Source/WebKit/Shared/WebPreferencesDefaultValues.h:191 > -// Cocoa ports must disable experimental features on release branches for now. > -#if ENABLE(EXPERIMENTAL_FEATURES) || PLATFORM(COCOA) > -#define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED true > -#else > -#define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED false > -#endif > +#define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED enableExperimentalFeatures() And here you’ll just need to remove the “|| PLATFORM(COCOA)” from the first #if.
Comment on attachment 341949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341949&action=review >> Source/WebKit/Configurations/BaseTarget.xcconfig:39 >> +GCC_PREPROCESSOR_DEFINITIONS = $(DEBUG_DEFINES) $(FEATURE_DEFINES) $(WK_MANUAL_SANDBOXING_DEFINES) $(WK_CORE_PREDICTION_DEFINES) U_DISABLE_RENAMING=1 U_SHOW_CPLUSPLUS_API=0 WK_RELOCATABLE_FRAMEWORKS=$(WK_RELOCATABLE_FRAMEWORKS) FRAMEWORK_NAME=WebKit; > > Instead of doing this, just make sure that ENABLE_EXPERIMENTAL_FEATURES is defined correctly. You can do this by adding something like this to (every copy of) FeatureDefines.xcconfig: > > ENABLE_EXPERIMENTAL_FEATURES = $(ENABLE_EXPERIMENTAL_FEATURES_$(WK_RELOCATABLE_FRAMEWORKS)); > ENABLE_EXPERIMENTAL_FEATURES_YES = ENABLE_EXPERIMENTAL_FEATURES; > > And adding $(ENABLE_EXPERIMENTAL_FEATURES) to FEATURE_DEFINES in that file. Makes sense. I'll take that approach. >> Source/WebKit/Shared/WebPreferencesDefaultValues.cpp:69 >> +} > > Then you won’t need this. Good! >> Source/WebKit/Shared/WebPreferencesDefaultValues.h:191 >> +#define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED enableExperimentalFeatures() > > And here you’ll just need to remove the “|| PLATFORM(COCOA)” from the first #if. Great!
Created attachment 341973 [details] Patch
Committed r232515: <https://trac.webkit.org/changeset/232515>