Conditionally convert V8 build overrides to declare_args.

BUILD.gn

Index: BUILD.gn
diff --git a/BUILD.gn b/BUILD.gn
index 2f065a0504bc5416fe89898850d5f67c049ce662..6340cc50d28b31e6e094b4430d396219f427dc05 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -30,7 +30,15 @@ declare_args() {
   v8_deprecation_warnings = false
 
   # Enable compiler warnings when using V8_DEPRECATE_SOON apis.
-  v8_imminent_deprecation_warnings = ""
+  #
+  # TODO(brettw) http://crbug.com/684096 Remove the define condition and the
+  # v8_imminent_deprecation_warnings_default variable when the build_override
+  # conversion is complete. This value should just default to true.
+  if (defined(v8_imminent_deprecation_warnings_default)) {
+    v8_imminent_deprecation_warnings = v8_imminent_deprecation_warnings_default
+  } else {
+    v8_imminent_deprecation_warnings = false
+  }
 
   # Embeds the given script into the snapshot.
   v8_embed_script = ""
@@ -76,22 +84,38 @@ declare_args() {
 
   # Similar to the ARM hard float ABI but on MIPS.
   v8_use_mips_abi_hardfloat = true
-}
 
-# Set project-specific defaults for some args if not provided in args.gn. The
-# defaults can be set in the respective build_overrides files.
-if (v8_imminent_deprecation_warnings == "") {
-  if (defined(v8_imminent_deprecation_warnings_default)) {
-    v8_imminent_deprecation_warnings = v8_imminent_deprecation_warnings_default
-  } else {
-    v8_imminent_deprecation_warnings = false
+  # List of extra files to snapshot. They will be snapshotted in order so
+  # if files export symbols used by later files, they should go first.
+  #
+  # This default is used by cctests. Projects using V8 will want to override.
+  #
+  # TODO(brettw) http://crbug.com/684096 Remove the define condition when the
+  # build_override conversion is complete.
+  if (!defined(v8_extra_library_files)) {
+    v8_extra_library_files = [

[Michael Achenbach, 2017/01/24 20:16:27]

So, the v8_extra_library_files and the var below are args now? They will then
also show up in the user's list of declarable args, right? Which wasn't the case
before as they were just variables that hold different values in chromium and
v8.

[brettw, 2017/01/24 20:35:52]

Correct, this isn't always the greatest thing (as in this case) but it seems to
make the most sense overall in terms of configuring repositories with different
arguments.

+      "//test/cctest/test-extra.js"
+    ]
   }
-}
-if (v8_enable_gdbjit == "") {
+
+  # Like v8_extra_library_files but for experimental features.
+  #
+  # This default is used by cctests. Projects using V8 will want to override.
+  #
+  # TODO(brettw) http://crbug.com/684096 Remove the define condition when the
+  # build_override conversion is complete.
+  if (!defined(v8_experimental_extra_library_files)) {
+    v8_experimental_extra_library_files = [
+      "//test/cctest/test-experimental-extra.js",
+    ]
+  }
+
   if (defined(v8_enable_gdbjit_default)) {
     v8_enable_gdbjit = v8_enable_gdbjit_default
   } else {
-    v8_enable_gdbjit = false
+    v8_enable_gdbjit = ((v8_current_cpu == "x86" || v8_current_cpu == "x64" ||

[Michael Achenbach, 2017/01/24 20:16:28]

I think this is different now. We want this to be off in chromium. Before, we'd
run into the else case in chromium, where it is false, as the default value is
never defined in chromium. Now, I think the if case is dead code and in the else
case we switch it on in some configurations, also in chromium.

The logic before was: If the user specifies it, do what the user specified. If
not, use the default in the v8 case and switch it off in chromium (unless
chromium specified a default, which it didn't). The default is to switch it on
with the configurations that are able to use it.

[brettw, 2017/01/24 20:35:52]

This is what I meant to address in https://codereview.chromium.org/2648383003/
but I realized I needed to call it v8_enable_gdbjit_default to trigger the above
arm and Chrome should be in the "false" case above. I'll do an update on that
patch (https://codereview.chromium.org/2652083002/ TBR'ed to you) but I think
this patch should still be OK to go once that lands.

+        v8_current_cpu == "x87") && (is_linux || is_mac)) ||
+      (v8_current_cpu == "ppc64" && is_linux)
   }
 }