Re-land: Make V8 extras a separate type of native

Instead of making them an extra option that gets passed in and compiled
at the end of the natives file for a given run of js2c, we now make them a
separate run of js2c with a separate natives file output.

This natives file output is then compiled in the bootstrapper. It is not part
of the snapshot (yet), but instead is treated similar to the experimental
natives, just without any of the complexity that comes from tieing the
behavior to flags. We also don't need counterparts to
InitializeExperimentalGlobal and InstallExperimentalNativeFunctions (yet?).

This fixes the issue with https://codereview.chromium.org/1129743003 by making
the dummy file that is generated for snapshots with no extras (or no experimental
features) nonempty.

R=yangguo@chromium.org, jochen@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1130993003

Cr-Commit-Position: refs/heads/master@{#28311}

BUILD.gn

@@ -296,6 +296,36 @@ action("js2c_experimental") {
   }
 }
 
+action("js2c_extras") {
+  visibility = [ ":*" ]  # Only targets in this file can depend on this.
+
+  script = "tools/js2c.py"
+
+  # The script depends on this other script, this rule causes a rebuild if it
+  # changes.
+  inputs = [ "tools/jsmin.py" ]
+
+  sources = v8_extra_library_files
+
+  outputs = [
+    "$target_gen_dir/extras-libraries.cc",
+  ]
+
+  args = [
+           rebase_path("$target_gen_dir/extras-libraries.cc",
+                       root_build_dir),
+           "EXTRAS",
+         ] + rebase_path(sources, root_build_dir)
+
+  if (v8_use_external_startup_data) {
+    outputs += [ "$target_gen_dir/libraries_extras.bin" ]
+    args += [
+      "--startup_blob",
+      rebase_path("$target_gen_dir/libraries_extras.bin", root_build_dir),
+    ]
+  }
+}
+
 action("d8_js2c") {
   visibility = [ ":*" ]  # Only targets in this file can depend on this.
 
@@ -321,11 +351,13 @@ if (v8_use_external_startup_data) {
     deps = [
       ":js2c",
       ":js2c_experimental",
+      ":js2c_extras",
     ]
 
     sources = [
       "$target_gen_dir/libraries.bin",
       "$target_gen_dir/libraries_experimental.bin",
+      "$target_gen_dir/libraries_extras.bin",
     ]
 
     outputs = [
@@ -410,12 +442,14 @@ source_set("v8_nosnapshot") {
   deps = [
     ":js2c",
     ":js2c_experimental",
+    ":js2c_extras",
     ":v8_base",
   ]
 
   sources = [
     "$target_gen_dir/libraries.cc",
     "$target_gen_dir/experimental-libraries.cc",
+    "$target_gen_dir/extras-libraries.cc",
     "src/snapshot/snapshot-empty.cc",
   ]
 
@@ -439,6 +473,7 @@ source_set("v8_snapshot") {
   deps = [
     ":js2c",
     ":js2c_experimental",
+    ":js2c_extras",
     ":run_mksnapshot",
     ":v8_base",
   ]
@@ -446,6 +481,7 @@ source_set("v8_snapshot") {
   sources = [
     "$target_gen_dir/libraries.cc",
     "$target_gen_dir/experimental-libraries.cc",
+    "$target_gen_dir/extras-libraries.cc",
     "$target_gen_dir/snapshot.cc",
   ]
 
@@ -465,6 +501,7 @@ if (v8_use_external_startup_data) {
     deps = [
       ":js2c",
       ":js2c_experimental",
+      ":js2c_extras",
       ":run_mksnapshot",
       ":v8_base",
       ":natives_blob",

src/bootstrapper.cc

@@ -42,6 +42,12 @@ FixedArray* GetCache<ExperimentalNatives>(Heap* heap) {
 }
 
 
+template <>
+FixedArray* GetCache<ExtraNatives>(Heap* heap) {
+  return heap->extra_natives_source_cache();
+}
+
+
 template <class Source>
 Handle<String> Bootstrapper::SourceLookup(int index) {
   DCHECK(0 <= index && index < Source::GetBuiltinsCount());
@@ -67,6 +73,7 @@ Handle<String> Bootstrapper::SourceLookup(int index) {
 template Handle<String> Bootstrapper::SourceLookup<Natives>(int index);
 template Handle<String> Bootstrapper::SourceLookup<ExperimentalNatives>(
     int index);
+template Handle<String> Bootstrapper::SourceLookup<ExtraNatives>(int index);
 
 
 void Bootstrapper::Initialize(bool create_heap_objects) {
@@ -134,6 +141,7 @@ void DeleteNativeSources(Object* maybe_array) {
 void Bootstrapper::TearDown() {
   DeleteNativeSources(isolate_->heap()->natives_source_cache());
   DeleteNativeSources(isolate_->heap()->experimental_natives_source_cache());
+  DeleteNativeSources(isolate_->heap()->extra_natives_source_cache());
   extensions_cache_.Initialize(isolate_, false);  // Yes, symmetrical
 }
 
@@ -220,6 +228,7 @@ class Genesis BASE_EMBEDDED {
       Handle<JSFunction>* fun,
       Handle<Map>* external_map);
   bool InstallExperimentalNatives();
+  bool InstallExtraNatives();
   void InstallBuiltinFunctionIds();
   void InstallJSFunctionResultCaches();
   void InitializeNormalizedMapCaches();
@@ -300,6 +309,7 @@ class Genesis BASE_EMBEDDED {
 
   static bool CompileBuiltin(Isolate* isolate, int index);
   static bool CompileExperimentalBuiltin(Isolate* isolate, int index);
+  static bool CompileExtraBuiltin(Isolate* isolate, int index);
   static bool CompileNative(Isolate* isolate,
                             Vector<const char> name,
                             Handle<String> source);
@@ -1447,6 +1457,14 @@ bool Genesis::CompileExperimentalBuiltin(Isolate* isolate, int index) {
 }
 
 
+bool Genesis::CompileExtraBuiltin(Isolate* isolate, int index) {
+  Vector<const char> name = ExtraNatives::GetScriptName(index);
+  Handle<String> source_code =
+      isolate->bootstrapper()->SourceLookup<ExtraNatives>(index);
+  return CompileNative(isolate, name, source_code);
+}
+
+
 bool Genesis::CompileNative(Isolate* isolate,
                             Vector<const char> name,
                             Handle<String> source) {
@@ -2356,6 +2374,16 @@ bool Genesis::InstallExperimentalNatives() {
 }
 
 
+bool Genesis::InstallExtraNatives() {
+  for (int i = ExtraNatives::GetDebuggerCount();
+       i < ExtraNatives::GetBuiltinsCount(); i++) {
+    if (!CompileExtraBuiltin(isolate(), i)) return false;
+  }
+
+  return true;
+}
+
+
 static void InstallBuiltinFunctionId(Handle<JSObject> holder,
                                      const char* function_name,
                                      BuiltinFunctionId id) {
@@ -2929,12 +2957,13 @@ Genesis::Genesis(Isolate* isolate,
     isolate->counters()->contexts_created_from_scratch()->Increment();
   }
 
-  // Install experimental natives. Do not include them into the snapshot as we
-  // should be able to turn them off at runtime. Re-installing them after
-  // they have already been deserialized would also fail.
+  // Install experimental and extra natives. Do not include them into the
+  // snapshot as we should be able to turn them off at runtime. Re-installing
+  // them after they have already been deserialized would also fail.
   if (!isolate->serializer_enabled()) {
     InitializeExperimentalGlobal();
     if (!InstallExperimentalNatives()) return;
+    if (!InstallExtraNatives()) return;
   }
 
   // The serializer cannot serialize typed arrays. Reset those typed arrays

src/heap/heap.cc

@@ -3080,6 +3080,9 @@ void Heap::CreateInitialObjects() {
   set_experimental_natives_source_cache(
       *factory->NewFixedArray(ExperimentalNatives::GetBuiltinsCount()));
 
+  set_extra_natives_source_cache(
+      *factory->NewFixedArray(ExtraNatives::GetBuiltinsCount()));
+
   set_undefined_cell(*factory->NewCell(factory->undefined_value()));
 
   // The symbol registry is initialized lazily.

src/heap/heap.h

@@ -175,6 +175,7 @@ namespace internal {
   V(FixedArray, natives_source_cache, NativesSourceCache)                      \
   V(FixedArray, experimental_natives_source_cache,                             \
     ExperimentalNativesSourceCache)                                            \
+  V(FixedArray, extra_natives_source_cache, ExtraNativesSourceCache)           \
   V(Script, empty_script, EmptyScript)                                         \
   V(NameDictionary, intrinsic_function_names, IntrinsicFunctionNames)          \
   V(Cell, undefined_cell, UndefinedCell)                                       \

src/snapshot/natives-external.cc

@@ -159,6 +159,7 @@ void ReadNatives() {
     NativesHolder<CORE>::set(NativesStore::MakeFromScriptsSource(&bytes));
     NativesHolder<EXPERIMENTAL>::set(
         NativesStore::MakeFromScriptsSource(&bytes));
+    NativesHolder<EXTRAS>::set(NativesStore::MakeFromScriptsSource(&bytes));
     DCHECK(!bytes.HasMore());
   }
 }
@@ -185,6 +186,7 @@ void SetNativesFromFile(StartupData* natives_blob) {
 void DisposeNatives() {
   NativesHolder<CORE>::Dispose();
   NativesHolder<EXPERIMENTAL>::Dispose();
+  NativesHolder<EXTRAS>::Dispose();
 }
 
 
@@ -229,6 +231,7 @@ Vector<const char> NativesCollection<type>::GetScriptsSource() {
 // my choice to elide them. This we'll explicitly instantiate these.
 template class NativesCollection<CORE>;
 template class NativesCollection<EXPERIMENTAL>;
+template class NativesCollection<EXTRAS>;
 
 }  // namespace v8::internal
 }  // namespace v8

src/snapshot/natives.h

@@ -12,9 +12,7 @@ namespace v8 { class StartupData; }  // Forward declaration.
 namespace v8 {
 namespace internal {
 
-enum NativeType {
-  CORE, EXPERIMENTAL, D8, TEST
-};
+enum NativeType { CORE, EXPERIMENTAL, EXTRAS, D8, TEST };
 
 template <NativeType type>
 class NativesCollection {
@@ -36,6 +34,7 @@ class NativesCollection {
 
 typedef NativesCollection<CORE> Natives;
 typedef NativesCollection<EXPERIMENTAL> ExperimentalNatives;
+typedef NativesCollection<EXTRAS> ExtraNatives;
 
 #ifdef V8_USE_EXTERNAL_STARTUP_DATA
 // Used for reading the natives at runtime. Implementation in natives-empty.cc

tools/gyp/v8.gyp

@@ -180,6 +180,7 @@
       'sources': [
         '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
         '<(SHARED_INTERMEDIATE_DIR)/experimental-libraries.cc',
+        '<(SHARED_INTERMEDIATE_DIR)/extras-libraries.cc',
         '<(INTERMEDIATE_DIR)/snapshot.cc',
       ],
       'actions': [
@@ -224,6 +225,7 @@
       'sources': [
         '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
         '<(SHARED_INTERMEDIATE_DIR)/experimental-libraries.cc',
+        '<(SHARED_INTERMEDIATE_DIR)/extras-libraries.cc',
         '../../src/snapshot/snapshot-empty.cc',
       ],
       'conditions': [
@@ -1639,6 +1641,7 @@
               '../../tools/concatenate-files.py',
               '<(SHARED_INTERMEDIATE_DIR)/libraries.bin',
               '<(SHARED_INTERMEDIATE_DIR)/libraries-experimental.bin',
+              '<(SHARED_INTERMEDIATE_DIR)/libraries-extras.bin',
             ],
             'conditions': [
               ['want_separate_host_toolset==1', {
@@ -1745,6 +1748,7 @@
         ],
         'libraries_bin_file': '<(SHARED_INTERMEDIATE_DIR)/libraries.bin',
         'libraries_experimental_bin_file': '<(SHARED_INTERMEDIATE_DIR)/libraries-experimental.bin',
+        'libraries_extras_bin_file': '<(SHARED_INTERMEDIATE_DIR)/libraries-extras.bin',
       },
       'actions': [
         {
@@ -1752,8 +1756,7 @@
           'inputs': [
             '../../tools/js2c.py',
             '<@(library_files)',
-            '<@(i18n_library_files)',
-            '<@(v8_extra_library_files)',
+            '<@(i18n_library_files)'
           ],
           'outputs': [
             '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
@@ -1764,9 +1767,7 @@
             '<(SHARED_INTERMEDIATE_DIR)/libraries.cc',
             'CORE',
             '<@(library_files)',
-            '<@(i18n_library_files)',
-            '--extra',
-            '<@(v8_extra_library_files)',
+            '<@(i18n_library_files)'
           ],
           'conditions': [
             [ 'v8_use_external_startup_data==1', {
@@ -1802,6 +1803,31 @@
             }],
           ],
         },
+        {
+          'action_name': 'js2c_extras',
+          'inputs': [
+            '../../tools/js2c.py',
+            '<@(v8_extra_library_files)',
+          ],
+          'outputs': [
+            '<(SHARED_INTERMEDIATE_DIR)/extras-libraries.cc',
+          ],
+          'action': [
+            'python',
+            '../../tools/js2c.py',
+            '<(SHARED_INTERMEDIATE_DIR)/extras-libraries.cc',
+            'EXTRAS',
+            '<@(v8_extra_library_files)',
+          ],
+          'conditions': [
+            [ 'v8_use_external_startup_data==1', {
+              'outputs': ['<@(libraries_extras_bin_file)'],
+              'action': [
+                '--startup_blob', '<@(libraries_extras_bin_file)',
+              ],
+            }],
+          ],
+        },
       ],
     },
     {

tools/js2c.py

@@ -340,9 +340,9 @@ def BuildFilterChain(macro_filename, message_template_file):
     macro_filename: Name of the macro file, if any.
 
   Returns:
-    A function (string -> string) that reads a source file and processes it.
+    A function (string -> string) that processes a source file.
   """
-  filter_chain = [ReadFile]
+  filter_chain = []
 
   if macro_filename:
     (consts, macros) = ReadMacros(ReadFile(macro_filename))
@@ -367,7 +367,7 @@ def BuildFilterChain(macro_filename, message_template_file):
   return reduce(chain, filter_chain)
 
 def BuildExtraFilterChain():
-  return lambda x: RemoveCommentsAndTrailingWhitespace(Validate(ReadFile(x)))
+  return lambda x: RemoveCommentsAndTrailingWhitespace(Validate(x))
 
 class Sources:
   def __init__(self):
@@ -386,14 +386,14 @@ def IsMessageTemplateFile(filename):
   return filename.endswith("messages.h")
 
 
-def PrepareSources(source_files, extra_files, emit_js):
+def PrepareSources(source_files, native_type, emit_js):
   """Read, prepare and assemble the list of source files.
 
   Args:
     source_files: List of JavaScript-ish source files. A file named macros.py
         will be treated as a list of macros.
-    extra_files: List of JavaScript-ish extra source files, passed in
-        externally from V8. Will not be minified or macro-ified.
+    native_type: String corresponding to a NativeType enum value, allowing us
+        to treat different types of sources differently.
     emit_js: True if we should skip the byte conversion and just leave the
         sources as JS strings.
 
@@ -414,18 +414,30 @@ def PrepareSources(source_files, extra_files, emit_js):
     source_files.remove(message_template_files[0])
     message_template_file = message_template_files[0]
 
-  filters = BuildFilterChain(macro_file, message_template_file)
-  extra_filters = BuildExtraFilterChain()
+  filters = None
+  if native_type == "EXTRA":
+    filters = BuildExtraFilterChain()
+  else:
+    filters = BuildFilterChain(macro_file, message_template_file)
 
   # Sort 'debugger' sources first.
   source_files = sorted(source_files,
                         lambda l,r: IsDebuggerFile(r) - IsDebuggerFile(l))
 
+  source_files_and_contents = [(f, ReadFile(f)) for f in source_files]
+
+  # Have a single not-quite-empty source file if there are none present;
+  # otherwise you get errors trying to compile an empty C++ array.
+  # It cannot be empty (or whitespace, which gets trimmed to empty), as
+  # the deserialization code assumes each file is nonempty.
+  if not source_files_and_contents:
+    source_files_and_contents = [("dummy.js", "void 0;")]
+
   result = Sources()
 
-  for source in source_files:
+  for (source, contents) in source_files_and_contents:
     try:
-      lines = filters(source)
+      lines = filters(contents)
     except Error as e:
       raise Error("In file %s:\n%s" % (source, str(e)))
 
@@ -437,16 +449,6 @@ def PrepareSources(source_files, extra_files, emit_js):
     name = os.path.basename(source)[:-3]
     result.names.append(name if not is_debugger else name[:-9])
 
-  for extra in extra_files:
-    try:
-      lines = extra_filters(extra)
-    except Error as e:
-      raise Error("In file %s:\n%s" % (extra, str(e)))
-
-    result.modules.append(lines)
-    name = os.path.basename(extra)[:-3]
-    result.names.append(name)
-
   return result
 
 
@@ -550,8 +552,8 @@ def WriteStartupBlob(sources, startup_blob):
   output.close()
 
 
-def JS2C(sources, extra_sources, target, native_type, raw_file, startup_blob, emitJS):
-  prepared_sources = PrepareSources(sources, extra_sources, emitJS)
+def JS2C(sources, target, native_type, raw_file, startup_blob, emit_js):
+  prepared_sources = PrepareSources(sources, native_type, emit_js)
   sources_output = "".join(prepared_sources.modules)
   metadata = BuildMetadata(prepared_sources, sources_output, native_type)
 
@@ -566,7 +568,7 @@ def JS2C(sources, extra_sources, target, native_type, raw_file, startup_blob, em
 
   # Emit resulting source file.
   output = open(target, "w")
-  if emitJS:
+  if emit_js:
     output.write(sources_output)
   else:
     output.write(HEADER_TEMPLATE % metadata)
@@ -578,24 +580,21 @@ def main():
   parser.add_argument("out.cc",
                       help="output filename")
   parser.add_argument("type",
-                      help="type parameter for NativesCollection template")
+                      help="type parameter for NativesCollection template " +
+                           "(see NativeType enum)")
   parser.add_argument("sources.js",
                       help="JS internal sources or macros.py.",
-                      nargs="+")
+                      nargs="*")
   parser.add_argument("--raw",
                       help="file to write the processed sources array to.")
   parser.add_argument("--startup_blob",
                       help="file to write the startup blob to.")
-  parser.add_argument("--extra",
-                      help="extra JS sources.",
-                      nargs="*")
   parser.add_argument("--js",
                       help="writes a JS file output instead of a C file",
                       action="store_true")
 
   args = vars(parser.parse_args())
-  JS2C(args["sources.js"], args["extra"] or [], args["out.cc"], args["type"], args["raw"], args["startup_blob"],
-       args["js"])
+  JS2C(args["sources.js"], args["out.cc"], args["type"], args["raw"], args["startup_blob"], args["js"])
 
 
 if __name__ == "__main__":