Clean up Linux zygote creation code

content/browser/child_process_launcher_helper_linux.cc

Index: content/browser/child_process_launcher_helper_linux.cc
diff --git a/content/browser/child_process_launcher_helper_linux.cc b/content/browser/child_process_launcher_helper_linux.cc
index 5763547b1f6d38fc4db4a1b015a34578bbc6ffdf..b7afd29ff131c9b27fb8df7939212a2eb7aee9ad 100644
--- a/content/browser/child_process_launcher_helper_linux.cc
+++ b/content/browser/child_process_launcher_helper_linux.cc
@@ -66,23 +66,20 @@ ChildProcessLauncherHelper::LaunchProcessOnLauncherThread(
     int* launch_result) {
   *is_synchronous_launch = true;
 
-  ZygoteHandle* zygote_handle =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoZygote) ?
-      nullptr : delegate_->GetZygote();
+  ZygoteHandle zygote_handle =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoZygote)
+          ? nullptr
+          : delegate_->GetZygote();
   if (zygote_handle) {
-    // This code runs on the PROCESS_LAUNCHER thread so race conditions are not
-    // an issue with the lazy initialization.
-    if (*zygote_handle == nullptr) {

[James Cook, 2017/04/28 20:07:52]

This bit is a little subtle, and is the thing I'm worried could be causing the
zygote check failure I saw.

In the old code, delegate_->GetZygote() can return:
* null, which means "don't use a zygote"
* a pointer to a non-null ZygoteHandle (aka ZygoteCommunication*), which means
"use this zygote for launching". I think right now that ZygoteHandle is always
the "generic zygote"
* a pointer to a null ZygoteHandle, which means "lazily create a zygote for this
process". I don't think that is currently used. However, if it was used it could
result in extra zygotes being launched, which could cause the problem I saw.

Since the current intent of the code is to use a single zygote, I changed
GetZygote() to always return the handle, so the return value only has 2 possible
meanings.

-      *zygote_handle = CreateZygote();
-    }
-    base::ProcessHandle handle = (*zygote_handle)->ForkRequest(
-        command_line()->argv(),
-        std::move(files_to_register),
-        GetProcessType());
+    // TODO(crbug.com/569191): If chrome supported multiple zygotes they could
+    // be created lazily here, or in the delegate GetZygote() implementations.
+    // Additionally, the delegate could provide a UseGenericZygote() method.
+    base::ProcessHandle handle = zygote_handle->ForkRequest(
+        command_line()->argv(), std::move(files_to_register), GetProcessType());
     *launch_result = LAUNCH_RESULT_SUCCESS;
     Process process;
     process.process = base::Process(handle);
-    process.zygote = *zygote_handle;
+    process.zygote = zygote_handle;
     return process;
   }