Remove build-webkit's notion of feature flags having a default value

https://bugs.webkit.org/show_bug.cgi?id=177338

Reviewed by Alex Christensen.

.:

Add an ENABLE_EXPERIMENTAL_FEATURES flag and use it to enable certain features.

* Source/cmake/OptionsGTK.cmake:
* Source/cmake/OptionsWPE.cmake:
* Source/cmake/WebKitFeatures.cmake:

Source/WebKit:

Use ENABLE_EXPERIMENTAL_FEATURES instead of ENABLE_DEVELOPER_MODE to enable runtime
experimental features.

* Shared/WebPreferencesDefinitions.h:

Tools:

Delegate feature flag default values to the build system.
(FeatureDefines.xcconfig for Xcode, WebKitFeatures.cmake and Options*.cmake for CMake, and FeatureDefines.h for everyone)

* BuildSlaveSupport/build.webkit.org-config/config.json:
* BuildSlaveSupport/build.webkit.org-config/master.cfg:
(Factory.__init__):
Update GTK and WPE buildbot configuration to use --no-experimental-features on old stable
bots instead of --default-cmake-features.

* Scripts/build-webkit:
(cMakeArgsFromFeatures):
Default feature values to 'undef'.
Remove the --default-cmake-features argument; it is now the default.
Add --no-experimental-features to replace it.
Stop printing the default feature flag value in the help.
Avoid propagating undefined values to the build system;
only mention flags that are overridden on the command line.

This has the nice side effect of using the default CMake features
for CMake builds, but still allowing toggling of them using the
prettier --FEATURE and --no-FEATURE arguments.

* Scripts/webkitdirs.pm:
(generateBuildSystemFromCMakeProject):
No longer need to suppress CMake warnings about unused arguments.

* Scripts/webkitperl/FeatureList.pm:
Remove the default values from the feature list.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@222394 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

ChangeLog

@@ -1,3 +1,16 @@
+2017-09-22  Tim Horton <[email protected]> and Michael Catanzaro <[email protected]>
+
+        Remove build-webkit's notion of feature flags having a default value
+        https://bugs.webkit.org/show_bug.cgi?id=177338
+
+        Reviewed by Alex Christensen.
+
+        Add an ENABLE_EXPERIMENTAL_FEATURES flag and use it to enable certain features.
+
+        * Source/cmake/OptionsGTK.cmake:
+        * Source/cmake/OptionsWPE.cmake:
+        * Source/cmake/WebKitFeatures.cmake:
+
 2017-09-20  Keith Miller  <[email protected]>
 
         JSC should use unified sources for platform specific files.

Source/WebKit/ChangeLog

@@ -1,3 +1,15 @@
+2017-09-22  Tim Horton <[email protected]> and Michael Catanzaro <[email protected]>
+
+        Remove build-webkit's notion of feature flags having a default value
+        https://bugs.webkit.org/show_bug.cgi?id=177338
+
+        Reviewed by Alex Christensen.
+
+        Use ENABLE_EXPERIMENTAL_FEATURES instead of ENABLE_DEVELOPER_MODE to enable runtime
+        experimental features.
+
+        * Shared/WebPreferencesDefinitions.h:
+
 2017-09-22  Chris Dumez  <[email protected]>
 
         Use high resolution timestamp for event time

Source/WebKit/Shared/WebPreferencesDefinitions.h

@@ -348,23 +348,25 @@
 #define FOR_EACH_WEBKIT_DEBUG_UINT32_PREFERENCE(macro) \
     macro(VisibleDebugOverlayRegions, visibleDebugOverlayRegions, UInt32, uint32_t, 0, "", "")
 
-// Our Xcode build system does not currently have any concept of DEVELOPER_MODE.
 // Cocoa ports must disable experimental features on release branches for now.
-#if ENABLE(DEVELOPER_MODE) || PLATFORM(COCOA)
+#if ENABLE(EXPERIMENTAL_FEATURES) || PLATFORM(COCOA)
 #define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED true
 #else
 #define DEFAULT_EXPERIMENTAL_FEATURES_ENABLED false
 #endif
 
 // For experimental features:
 // - The type should be boolean.
-// - You must provide the last two parameters for all experimental features. They
-//   are the text exposed to the user from the WebKit client.
-// - They should be alphabetically ordered by the human readable text (the first string).
-// - The default value may be either false (for unstable features) or
+// - You must provide the last two parameters for all experimental features.
+//   They are the text exposed to the user from the WebKit client.
+// - They should be *alphabetically ordered* by the human readable text (the
+//   first string).
+// - The default value may be either false (for really unstable features) or
 //   DEFAULT_EXPERIMENTAL_FEATURES_ENABLED (for features that are ready for
-//   wider testing).
-
+//   wider testing). *The default value may not be true*. That would no longer
+//   be experimental.
+//
+// Actually read the comment above before modifying this list!
 #define FOR_EACH_WEBKIT_EXPERIMENTAL_FEATURE_PREFERENCE(macro) \
     macro(ConstantPropertiesEnabled, constantPropertiesEnabled, Bool, bool, true, "Constant Properties", "Enable CSS constant() properties") \
     macro(DisplayContentsEnabled, displayContentsEnabled, Bool, bool, false, "CSS display: contents", "Enable CSS display: contents support") \

Source/cmake/OptionsGTK.cmake

@@ -157,6 +157,8 @@ WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_INDEXED_DATABASE PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_INDEXED_DATABASE_IN_WORKERS PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_INPUT_TYPE_COLOR PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_CONTROLS_SCRIPT PRIVATE ON)
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_SOURCE PRIVATE ${ENABLE_EXPERIMENTAL_FEATURES})
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_STREAM PRIVATE ${ENABLE_EXPERIMENTAL_FEATURES})
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MHTML PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_NOTIFICATIONS PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_PUBLIC_SUFFIX_LIST PRIVATE ON)
@@ -166,6 +168,7 @@ WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_USERSELECT_ALL PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_USER_MESSAGE_HANDLERS PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SUBTLE_CRYPTO PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WEBGL PRIVATE ON)
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WEB_RTC PRIVATE ${ENABLE_EXPERIMENTAL_FEATURES})
 
 include(GStreamerDefinitions)
 

Source/cmake/OptionsWPE.cmake

@@ -14,7 +14,7 @@ include(GStreamerDefinitions)
 WEBKIT_OPTION_DEFINE(EXPORT_DEPRECATED_WEBKIT2_C_API "Whether to export the WebKit2 C API" PRIVATE OFF)
 
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_3D_TRANSFORMS PUBLIC ON)
-WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_ACCELERATED_2D_CANVAS PUBLIC OFF)
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_ACCELERATED_2D_CANVAS PUBLIC ${ENABLE_EXPERIMENTAL_FEATURES})
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS_REGIONS PUBLIC OFF)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS_SELECTORS_LEVEL4 PUBLIC ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_DEVICE_ORIENTATION PUBLIC OFF)
@@ -24,7 +24,7 @@ WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_GEOLOCATION PUBLIC OFF)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_INDEXED_DATABASE PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_INDEXED_DATABASE_IN_WORKERS PRIVATE OFF)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_CONTROLS_SCRIPT PUBLIC ON)
-WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_SOURCE PUBLIC OFF)
+WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MEDIA_SOURCE PUBLIC ${ENABLE_EXPERIMENTAL_FEATURES})
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_MHTML PRIVATE ON)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_NETSCAPE_PLUGIN_API PRIVATE OFF)
 WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_PUBLIC_SUFFIX_LIST PRIVATE ON)

Source/cmake/WebKitFeatures.cmake

@@ -343,3 +343,6 @@ macro(CREATE_CONFIGURATION_HEADER)
     )
     file(REMOVE "${CMAKE_BINARY_DIR}/cmakeconfig.h.tmp")
 endmacro()
+
+option(ENABLE_EXPERIMENTAL_FEATURES "Enable experimental features" OFF)
+SET_AND_EXPOSE_TO_BUILD(ENABLE_EXPERIMENTAL_FEATURES ${ENABLE_EXPERIMENTAL_FEATURES})

Tools/BuildSlaveSupport/build.webkit.org-config/config.json

@@ -307,13 +307,13 @@
                     {
                       "name": "GTK Linux 64-bit Release Debian Stable (Build)", "type": "Build", "builddir": "gtk-linux-64-release-debian",
                       "platform": "gtk", "configuration": "release", "architectures": ["x86_64"],
-                      "additionalArguments": ["--default-cmake-features"],
+                      "additionalArguments": ["--no-experimental-features"],
                       "slavenames": ["gtk-linux-slave-10"]
                     },
                     {
                       "name": "GTK Linux 64-bit Release Ubuntu LTS (Build)", "type": "Build", "builddir": "gtk-linux-64-release-ubuntu",
                       "platform": "gtk", "configuration": "release", "architectures": ["x86_64"],
-                      "additionalArguments": ["--default-cmake-features"],
+                      "additionalArguments": ["--no-experimental-features"],
                       "slavenames": ["gtk-linux-slave-11"]
                     },
                     {

Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg

@@ -871,7 +871,7 @@ class Factory(factory.BuildFactory):
         self.addStep(DeleteStaleBuildFiles())
         if platform == "win":
             self.addStep(InstallWin32Dependencies())
-        if platform == "gtk" and additionalArguments != ["--default-cmake-features"]:
+        if platform == "gtk" and additionalArguments != ["--no-experimental-features"]:
             self.addStep(InstallGtkDependencies())
         if platform == "wpe":
             self.addStep(InstallWpeDependencies())

Tools/ChangeLog

@@ -1,3 +1,39 @@
+2017-09-22  Tim Horton <[email protected]> and Michael Catanzaro <[email protected]>
+
+        Remove build-webkit's notion of feature flags having a default value
+        https://bugs.webkit.org/show_bug.cgi?id=177338
+
+        Reviewed by Alex Christensen.
+
+        Delegate feature flag default values to the build system.
+        (FeatureDefines.xcconfig for Xcode, WebKitFeatures.cmake and Options*.cmake for CMake, and FeatureDefines.h for everyone)
+
+        * BuildSlaveSupport/build.webkit.org-config/config.json:
+        * BuildSlaveSupport/build.webkit.org-config/master.cfg:
+        (Factory.__init__):
+        Update GTK and WPE buildbot configuration to use --no-experimental-features on old stable
+        bots instead of --default-cmake-features.
+
+        * Scripts/build-webkit:
+        (cMakeArgsFromFeatures):
+        Default feature values to 'undef'.
+        Remove the --default-cmake-features argument; it is now the default.
+        Add --no-experimental-features to replace it.
+        Stop printing the default feature flag value in the help.
+        Avoid propagating undefined values to the build system;
+        only mention flags that are overridden on the command line.
+
+        This has the nice side effect of using the default CMake features
+        for CMake builds, but still allowing toggling of them using the
+        prettier --FEATURE and --no-FEATURE arguments.
+
+        * Scripts/webkitdirs.pm:
+        (generateBuildSystemFromCMakeProject):
+        No longer need to suppress CMake warnings about unused arguments.
+
+        * Scripts/webkitperl/FeatureList.pm:
+        Remove the default values from the feature list.
+
 2017-09-21  Joseph Pecoraro  <[email protected]>
 
         Unreviewed, add the ability to skip a test262 test.
@@ -393,7 +429,6 @@
         (-[RedirectedDownloadDelegate _downloadDidFinish:]):
         (TEST):
 
->>>>>>> .r222309
 2017-09-20  Alex Christensen  <[email protected]>
 
         Add infrastructure for adding custom headers to requests per website

Tools/Scripts/build-webkit

@@ -52,7 +52,6 @@ chdirWebKit();
 
 my $showHelp = 0;
 my $clean = 0;
-my $defaultCMakeFeatures = 0;
 my $minimal = 0;
 my $installHeaders;
 my $installLibs;
@@ -62,6 +61,7 @@ my @cmakeArgs;
 my $onlyWebKitProject = 0;
 my $coverageSupport = 0;
 my $shouldRunStaticAnalyzer = 0;
+my $noExperimentalFeatures = 0;
 my $startTime = time();
 my $archs32bit = 0;
 
@@ -79,9 +79,10 @@ foreach (@ARGV) {
     }
 }
 
-# Initialize values from defaults
+# Feature flags default to undefined, where they will inherit the default value
+# specified by the build system, or to 'off' if --minimal is specified.
 foreach (@features) {
-    ${$_->{value}} = ($minimal ? 0 : $_->{default});
+    ${$_->{value}} = ($minimal ? 0 : undef);
 }
 
 my $programName = basename($0);
@@ -106,12 +107,12 @@ Usage: $programName [options] [options to pass to build system]
 
   --inspector-frontend              Copy Web Inspector user interface resources to the build directory
 
-  --prefix=<path>                   Set installation prefix to the given path (Gtk/Efl only)
+  --prefix=<path>                   Set installation prefix to the given path (CMake only, except Windows)
   --makeargs=<arguments>            Optional Makefile flags
   --cmakeargs=<arguments>           One or more optional CMake flags (e.g. --cmakeargs="-DFOO=bar -DCMAKE_PREFIX_PATH=/usr/local")
 
   --minimal                         No optional features, unless explicitly enabled
-  --default-cmake-features          Use the default CMake features enabled for the port (CMake based ports)
+  --no-experimental-features        No experimental features, unless explicitly enabled (CMake only)
 
   --only-webkit                     Build only the WebKit project
 
@@ -129,13 +130,13 @@ my %options = (
     'only-webkit' => \$onlyWebKitProject,
     'coverage' => \$coverageSupport,
     'analyze' => \$shouldRunStaticAnalyzer,
-    'default-cmake-features' => \$defaultCMakeFeatures,
+    'no-experimental-features' => \$noExperimentalFeatures,
 );
 
 # Build usage text and options list from features
 foreach (@features) {
     my $opt = sprintf("%-35s", "  --[no-]$_->{option}");
-    $usage .= "$opt $_->{desc} (default: $_->{default})\n";
+    $usage .= "$opt $_->{desc}\n";
     $options{"$_->{option}!"} = $_->{value};
 }
 
@@ -176,15 +177,15 @@ my @options = ();
 
 if (isAppleCocoaWebKit()) {
     push @options, XcodeOptions();
-    sub option($$$)
+    sub option($$)
     {
-        my ($feature, $isEnabled, $defaultValue) = @_;
-        return "" if $defaultValue == $isEnabled;
+        my ($feature, $isEnabled) = @_;
+        return "" if not defined $isEnabled;
         return $feature . "=" . ($isEnabled ? $feature : "");
     }
 
     foreach (@features) {
-        my $option = option($_->{define}, ${$_->{value}}, $_->{default});
+        my $option = option($_->{define}, ${$_->{value}});
         push @options, $option unless $option eq "";
     }
 
@@ -249,7 +250,7 @@ if (isCMakeBuild() && !isAnyWindows()) {
     my $maxCPULoad = maxCPULoad() if $makeArgs !~ /-l\s*\d+\.?\d*/;
     $makeArgs .= " -l" . maxCPULoad() if defined $maxCPULoad;
 
-    # We remove CMakeCache to avoid the bots to reuse cached flags when
+    # We remove CMakeCache to avoid the bots reusing cached flags when
     # we enable new features. This forces a reconfiguration.
     my @featureArgs = cMakeArgsFromFeatures();
     removeCMakeCache(@featureArgs);
@@ -346,15 +347,22 @@ exit 0;
 sub cMakeArgsFromFeatures()
 {
     my @args;
-    if (!$defaultCMakeFeatures) {
-        foreach (@features) {
-            my $featureName = $_->{define};
-            if ($featureName) {
-                my $featureEnabled = ${$_->{value}} ? "ON" : "OFF";
+
+    if (!$noExperimentalFeatures) {
+        push @args, "-DENABLE_EXPERIMENTAL_FEATURES=ON";
+    }
+
+    foreach (@features) {
+        my $featureName = $_->{define};
+        if ($featureName) {
+            my $featureValue = ${$_->{value}};
+            if (defined $featureValue) {
+                my $featureEnabled = $featureValue ? "ON" : "OFF";
                 push @args, "-D$featureName=$featureEnabled";
             }
         }
     }
+
     return @args;
 }
 

Tools/Scripts/webkitdirs.pm

@@ -2072,8 +2072,6 @@ sub generateBuildSystemFromCMakeProject
     # Some ports have production mode, but build-webkit should always use developer mode.
     push @args, "-DDEVELOPER_MODE=ON" if isGtk() || isJSCOnly() || isWPE();
 
-    # Don't warn variables which aren't used by cmake ports.
-    push @args, "--no-warn-unused-cli";
     push @args, @cmakeArgs if @cmakeArgs;
 
     my $cmakeSourceDir = isCygwin() ? windowsSourceDir() : sourceDir();