Bug 186286 – Revise DEFAULT_EXPERIMENTAL_FEATURES_ENABLED to work properly on Apple builds

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

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.

[Radar WebKit Bug Importer]

<rdar://problem/40782992>

[Brent Fulgham]

Created attachment 341946 [details]
Patch

[Brent Fulgham]

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.

[Brent Fulgham]

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!

[Brent Fulgham]

Created attachment 341973 [details]
Patch

[Brent Fulgham]

Committed r232515: <https://trac.webkit.org/changeset/232515>