Moved logic for mapping child process FDs for ICU and V8 into child_process_launcher.cc

gin/v8_initializer.cc

Index: gin/v8_initializer.cc
diff --git a/gin/v8_initializer.cc b/gin/v8_initializer.cc
index 0568fe77db9300785c3830a0461028d93e89b47c..d3cfbecb39313e9a22c9017290eb36bbc9af1336 100644
--- a/gin/v8_initializer.cc
+++ b/gin/v8_initializer.cc
@@ -26,11 +26,15 @@
 namespace gin {
 
 namespace {
-
 base::MemoryMappedFile* g_mapped_natives = nullptr;
 base::MemoryMappedFile* g_mapped_snapshot = nullptr;
 
 #if defined(V8_USE_EXTERNAL_STARTUP_DATA)
+base::File* g_natives_file = nullptr;
+base::File* g_snapshot_file = nullptr;
+base::MemoryMappedFile::Region g_natives_region;
+base::MemoryMappedFile::Region g_snapshot_region;
+
 #if !defined(OS_MACOSX)
 const int kV8SnapshotBasePathKey =
 #if defined(OS_ANDROID)
@@ -80,9 +84,8 @@ static bool MapV8File(base::File file,
   return true;
 }
 
-static bool OpenV8File(const base::FilePath& path,
-                       int flags,
-                       base::File& file) {
+base::File OpenV8File(const char* file_name,
+                      base::MemoryMappedFile::Region* region_out) {
   // Re-try logic here is motivated by http://crbug.com/479537
   // for A/V on Windows (https://support.microsoft.com/en-us/kb/316609).
 
@@ -95,10 +98,16 @@ static bool OpenV8File(const base::FilePath& path,
     MAX_VALUE
   };
 
+  base::FilePath path;
+  GetV8FilePath(file_name, &path);
+
   OpenV8FileResult result = OpenV8FileResult::FAILED_IN_USE;
+  int flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
+  base::File file;
   for (int attempt = 0; attempt < kMaxOpenAttempts; attempt++) {
     file.Initialize(path, flags);
     if (file.IsValid()) {
+      *region_out = base::MemoryMappedFile::Region::kWholeFile;
       if (attempt == 0) {
         result = OpenV8FileResult::OPENED;
         break;
@@ -118,9 +127,21 @@ static bool OpenV8File(const base::FilePath& path,
   UMA_HISTOGRAM_ENUMERATION("V8.Initializer.OpenV8File.Result",
                             result,
                             OpenV8FileResult::MAX_VALUE);
+  return file.Pass();
+}
+
+void OpenNativesFileIfNecessary() {
+  if (g_natives_file == nullptr) {
+    g_natives_file =
+        new base::File(OpenV8File(kNativesFileName, &g_natives_region));
+  }
+}
 
-  return result == OpenV8FileResult::OPENED
-         || result == OpenV8FileResult::OPENED_RETRY;
+void OpenSnapshotFileIfNecessary() {
+  if (g_snapshot_file == nullptr) {
+    g_snapshot_file =
+        new base::File(OpenV8File(kSnapshotFileName, &g_snapshot_region));
+  }
 }
 
 #if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
@@ -163,22 +184,17 @@ enum LoadV8FileResult {
   V8_LOAD_MAX_VALUE
 };
 
-static LoadV8FileResult OpenMapVerify(
-    const char* file_name,
+static LoadV8FileResult MapVerify(base::File& file,
+                                  const base::MemoryMappedFile::Region& region,
 #if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
-    const unsigned char* fingerprint,
+                                  const unsigned char* fingerprint,
 #endif
-    base::MemoryMappedFile** mmapped_file_out) {
-  base::FilePath path;
-  GetV8FilePath(file_name, &path);
-
-  base::File file;
-  int flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
-
-  if (!OpenV8File(path, flags, file))
+                                  base::MemoryMappedFile** mmapped_file_out) {
+  if (!file.IsValid())
     return V8_LOAD_FAILED_OPEN;
-  if (!MapV8File(file.Pass(), base::MemoryMappedFile::Region::kWholeFile,
-                 mmapped_file_out))
+  // It would be more correct to use File::Duplicate() here, but since
+  // the file is never closed, sharing it is fine.
+  if (!MapV8File(base::File(file.GetPlatformFile()), region, mmapped_file_out))
     return V8_LOAD_FAILED_MAP;
 #if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
   if (!VerifyV8StartupFile(mmapped_file_out, fingerprint))
@@ -192,11 +208,14 @@ void V8Initializer::LoadV8Snapshot() {
   if (g_mapped_snapshot)
     return;
 
-  LoadV8FileResult result = OpenMapVerify(kSnapshotFileName,
+  OpenSnapshotFileIfNecessary();
+  LoadV8FileResult result = MapVerify(*g_snapshot_file, g_snapshot_region,
 #if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
-                                          g_snapshot_fingerprint,
+                                      g_snapshot_fingerprint,
 #endif
-                                          &g_mapped_snapshot);
+                                      &g_mapped_snapshot);
+  // V8 can't start up without the source of the natives, but it can
+  // start up (slower) without the snapshot.
   UMA_HISTOGRAM_ENUMERATION("V8.Initializer.LoadV8Snapshot.Result", result,
                             V8_LOAD_MAX_VALUE);
 }
@@ -205,11 +224,12 @@ void V8Initializer::LoadV8Natives() {
   if (g_mapped_natives)
     return;
 
-  LoadV8FileResult result = OpenMapVerify(kNativesFileName,
+  OpenNativesFileIfNecessary();
+  LoadV8FileResult result = MapVerify(*g_natives_file, g_natives_region,
 #if defined(V8_VERIFY_EXTERNAL_STARTUP_DATA)
-                                          g_natives_fingerprint,
+                                      g_natives_fingerprint,
 #endif
-                                          &g_mapped_natives);
+                                      &g_mapped_natives);
   if (result != V8_LOAD_SUCCESS) {
     LOG(FATAL) << "Couldn't mmap v8 natives data file, status code is "
                << static_cast<int>(result);
@@ -271,33 +291,21 @@ void V8Initializer::LoadV8NativesFromFD(base::PlatformFile natives_pf,
 }
 
 // static
-bool V8Initializer::OpenV8FilesForChildProcesses(
-    base::PlatformFile* natives_fd_out,
-    base::PlatformFile* snapshot_fd_out) {
-  base::FilePath natives_data_path;
-  base::FilePath snapshot_data_path;
-  GetV8FilePath(kNativesFileName, &natives_data_path);
-  GetV8FilePath(kSnapshotFileName, &snapshot_data_path);
-
-  base::File natives_data_file;
-  base::File snapshot_data_file;
-  int file_flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
-
-  bool natives_success =
-      OpenV8File(natives_data_path, file_flags, natives_data_file);
-  if (natives_success) {
-    *natives_fd_out = natives_data_file.TakePlatformFile();
-  }
-  bool snapshot_success =
-      OpenV8File(snapshot_data_path, file_flags, snapshot_data_file);
-  if (snapshot_success) {
-    *snapshot_fd_out = snapshot_data_file.TakePlatformFile();
-  }
-  // We can start up without the snapshot file, but not without the natives.
-  return natives_success;
+base::PlatformFile V8Initializer::OpenNativesFileForChildProcesses(
+    base::MemoryMappedFile::Region* region_out) {
+  OpenNativesFileIfNecessary();
+  *region_out = g_natives_region;
+  return g_natives_file->GetPlatformFile();
 }
 
-#endif  // V8_USE_EXTERNAL_STARTUP_DATA
+// static
+base::PlatformFile V8Initializer::OpenSnapshotFileForChildProcesses(

[rmcilroy, 2015/06/12 22:09:25]

How about GetOpenSnapshotFileForChildProcess (to make it clear this doesn't open
the file each time).

+    base::MemoryMappedFile::Region* region_out) {
+  OpenSnapshotFileIfNecessary();
+  *region_out = g_snapshot_region;
+  return g_snapshot_file->GetPlatformFile();
+}
+#endif  // defined(V8_USE_EXTERNAL_STARTUP_DATA)
 
 // static
 void V8Initializer::Initialize(gin::IsolateHolder::ScriptMode mode) {