Have each SandboxedProcessLauncherDelegate maintain a zygote.

content/browser/child_process_launcher.cc

Index: content/browser/child_process_launcher.cc
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc
index 40d78e0cae0d3ceb1dbb215e067282cf0b777955..41a297095d2d86cfdb1d3d53ba89877591577f64 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -39,6 +39,7 @@
 #elif defined(OS_POSIX)
 #include "base/memory/singleton.h"
 #include "content/browser/renderer_host/render_sandbox_host_linux.h"
+#include "content/browser/zygote_host/zygote_communication_linux.h"
 #include "content/browser/zygote_host/zygote_host_impl_linux.h"
 #include "content/common/child_process_sandbox_support_impl_linux.h"
 #endif
@@ -53,7 +54,7 @@ namespace content {
 
 namespace {
 
-typedef base::Callback<void(bool,
+typedef base::Callback<void(ZygoteHandle,
 #if defined(OS_ANDROID)
                             base::ScopedFD,
 #endif
@@ -87,7 +88,7 @@ void OnChildProcessStartedAndroid(const NotifyCallback& callback,
       base::Bind(&RecordHistogramsOnLauncherThread, launch_time));
 
   base::Closure callback_on_client_thread(
-      base::Bind(callback, false, base::Passed(&ipcfd),
+      base::Bind(callback, nullptr, base::Passed(&ipcfd),
                  base::Passed(base::Process(handle))));
   if (BrowserThread::CurrentlyOn(client_thread_id)) {
     callback_on_client_thread.Run();
@@ -108,15 +109,15 @@ void LaunchOnLauncherThread(const NotifyCallback& callback,
                             base::CommandLine* cmd_line) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
   scoped_ptr<SandboxedProcessLauncherDelegate> delegate_deleter(delegate);
+#if !defined(OS_ANDROID)
+  ZygoteHandle zygote = nullptr;
+#endif
 #if defined(OS_WIN)
-  bool use_zygote = false;
   bool launch_elevated = delegate->ShouldLaunchElevated();
 #elif defined(OS_MACOSX)
-  bool use_zygote = false;
   base::EnvironmentMap env = delegate->GetEnvironment();
   base::ScopedFD ipcfd = delegate->TakeIpcFd();
 #elif defined(OS_POSIX) && !defined(OS_ANDROID)
-  bool use_zygote = delegate->ShouldUseZygote();
   base::EnvironmentMap env = delegate->GetEnvironment();
   base::ScopedFD ipcfd = delegate->TakeIpcFd();
 #endif
@@ -199,8 +200,17 @@ void LaunchOnLauncherThread(const NotifyCallback& callback,
   // child termination.
 
 #if !defined(OS_MACOSX)
-  if (use_zygote) {
-    base::ProcessHandle handle = ZygoteHostImpl::GetInstance()->ForkRequest(
+  ZygoteHandle* zygote_handle = delegate->GetZygote();
+  // If |zygote_handle| is null, a zygote should not be used.
+  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) {

[Mark Seaborn, 2016/01/19 21:06:43]

So there is still this code for launching the zygote process on demand, then? 
Is that still used (e.g. for plugins)?

Does this leave a small race condition?  chrome/browser/chrome_browser_main.cc
uses PostTask() to run nacl::NaClProcessHost::EarlyZygoteLaunch().  If that task
hasn't run by the time we try to use the zygote, then one zygote would get
launched here, and then a second zygote would get launched by
EarlyZygoteLaunch() (or the DCHECK in EarlyZygoteLaunch() would fail).

Maybe EarlyZygoteLaunch() should check whether the zygote has been launched
already?

Or maybe this would never be an issue in practice because anything that launches
a NaCl process would occur from a task run by PostTask() that is queued later
than EarlyZygoteLaunch()?

[mdempsky, 2016/01/19 21:26:43]

It's not currently used, but we'll start using it once/if we switch to lazily
starting zygotes.
My understanding is it shouldn't be racy: EarlyZygoteLaunch and
LaunchOnLauncherThread are both posted to the PROCESS_LAUNCHER thread, which
should ensure they're ordered.


kerrnel: This does make me realize we should use CreateZygote() here too, and
that CreateZygote() should include
DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);

[Greg K, 2016/01/19 22:20:16]

I talked to Matt, except for NaCl, the zygotes are currently launched in the
main thread initialization, which should be complete before the PROCESS_LAUNCHER
thread is receiving any requests to create rendererers, etc.

+      *zygote_handle = new ZygoteCommunication();
+      (*zygote_handle)->Init();
+    }
+    zygote = *zygote_handle;
+    base::ProcessHandle handle = zygote->ForkRequest(
         cmd_line->argv(), std::move(files_to_register), process_type);
     process = base::Process(handle);
   } else
@@ -279,13 +289,11 @@ void LaunchOnLauncherThread(const NotifyCallback& callback,
                                      begin_launch_time);
   }
   BrowserThread::PostTask(client_thread_id, FROM_HERE,
-                          base::Bind(callback,
-                                     use_zygote,
-                                     base::Passed(&process)));
+                          base::Bind(callback, zygote, base::Passed(&process)));
 #endif  // !defined(OS_ANDROID)
 }
 
-void TerminateOnLauncherThread(bool zygote, base::Process process) {
+void TerminateOnLauncherThread(ZygoteHandle zygote, base::Process process) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
 #if defined(OS_ANDROID)
   VLOG(1) << "ChromeProcess: Stopping process with handle "
@@ -301,7 +309,7 @@ void TerminateOnLauncherThread(bool zygote, base::Process process) {
   if (zygote) {
     // If the renderer was created via a zygote, we have to proxy the reaping
     // through the zygote process.
-    ZygoteHostImpl::GetInstance()->EnsureProcessTerminated(process.Handle());
+    zygote->EnsureProcessTerminated(process.Handle());
   } else
 #endif  // !OS_MACOSX
     base::EnsureProcessTerminated(std::move(process));
@@ -331,7 +339,7 @@ ChildProcessLauncher::ChildProcessLauncher(
     : client_(client),
       termination_status_(base::TERMINATION_STATUS_NORMAL_TERMINATION),
       exit_code_(RESULT_CODE_NORMAL_EXIT),
-      zygote_(false),
+      zygote_(nullptr),
       starting_(true),
 #if defined(ADDRESS_SANITIZER) || defined(LEAK_SANITIZER) ||  \
     defined(MEMORY_SANITIZER) || defined(THREAD_SANITIZER) || \
@@ -401,8 +409,8 @@ void ChildProcessLauncher::UpdateTerminationStatus(bool known_dead) {
   DCHECK(CalledOnValidThread());
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   if (zygote_) {
-    termination_status_ = ZygoteHostImpl::GetInstance()->
-        GetTerminationStatus(process_.Handle(), known_dead, &exit_code_);
+    termination_status_ = zygote_->GetTerminationStatus(
+        process_.Handle(), known_dead, &exit_code_);
   } else if (known_dead) {
     termination_status_ =
         base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
@@ -435,7 +443,7 @@ void ChildProcessLauncher::SetProcessBackgrounded(bool background) {
 void ChildProcessLauncher::DidLaunch(
     base::WeakPtr<ChildProcessLauncher> instance,
     bool terminate_on_shutdown,
-    bool zygote,
+    ZygoteHandle zygote,
 #if defined(OS_ANDROID)
     base::ScopedFD ipcfd,
 #endif
@@ -460,12 +468,11 @@ void ChildProcessLauncher::DidLaunch(
   }
 }
 
-void ChildProcessLauncher::Notify(
-    bool zygote,
+void ChildProcessLauncher::Notify(ZygoteHandle zygote,
 #if defined(OS_ANDROID)
-    base::ScopedFD ipcfd,
+                                  base::ScopedFD ipcfd,
 #endif
-    base::Process process) {
+                                  base::Process process) {
   DCHECK(CalledOnValidThread());
   starting_ = false;
   process_ = std::move(process);