Simplify ChildProcessLauncher

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 24fd73721846b21a9a9b63a390c4940ac8a472ac..28be72bb9c32eb853702adbdb0726379a3f9825c 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -4,12 +4,9 @@
 
 #include "content/browser/child_process_launcher.h"
 
-#include <utility>  // For std::pair.
-
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
-#include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
@@ -17,7 +14,6 @@
 #include "base/profiler/scoped_tracker.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
-#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/common/content_descriptors.h"
 #include "content/public/common/content_switches.h"
@@ -50,279 +46,71 @@
 
 namespace content {
 
-// Having the functionality of ChildProcessLauncher be in an internal
-// ref counted object allows us to automatically terminate the process when the
-// parent class destructs, while still holding on to state that we need.
-class ChildProcessLauncher::Context
-    : public base::RefCountedThreadSafe<ChildProcessLauncher::Context> {
- public:
-  Context();
-
-  // Posts a task to a dedicated thread to do the actual work.
-  // Must be called on the UI thread.
-  void Launch(SandboxedProcessLauncherDelegate* delegate,
-              base::CommandLine* cmd_line,
-              int child_process_id,
-              Client* client);
+namespace {
 
+typedef base::Callback<void(bool,
 #if defined(OS_ANDROID)
-  // Called on the UI thread with the operation result. Calls Notify().
-  static void OnChildProcessStarted(
-      // |this_object| is NOT thread safe. Only use it to post a task back.
-      scoped_refptr<Context> this_object,
-      BrowserThread::ID client_thread_id,
-      const base::TimeTicks begin_launch_time,
-      base::ProcessHandle handle);
+                            base::ScopedFD,
 #endif
+                            base::Process)> NotifyCallback;
 
-  // Resets the client (the client is going away).
-  void ResetClient();
-
-  bool starting() const { return starting_; }
-
-  const base::Process& process() const { return process_; }
-
-  int exit_code() const { return exit_code_; }
-
-  base::TerminationStatus termination_status() const {
-    return termination_status_;
-  }
-
-  void set_terminate_child_on_shutdown(bool terminate_on_shutdown) {
-    terminate_child_on_shutdown_ = terminate_on_shutdown;
-  }
-
-  void UpdateTerminationStatus(bool known_dead);
-
-  void Close() { process_.Close(); }
-
-  void SetProcessBackgrounded(bool background);
-
-  Client* ReplaceClientForTest(Client* client) {
-    Client* ret = client_;
-    client_ = client;
-    return ret;
-  }
-
- private:
-  friend class base::RefCountedThreadSafe<ChildProcessLauncher::Context>;
-
-  ~Context() {
-    Terminate();
+void RecordHistogramsOnLauncherThread(base::TimeDelta launch_time) {
+  DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
+  // Log the launch time, separating out the first one (which will likely be
+  // slower due to the rest of the browser initializing at the same time).
+  static bool done_first_launch = false;
+  if (done_first_launch) {
+    UMA_HISTOGRAM_TIMES("MPArch.ChildProcessLaunchSubsequent", launch_time);
+  } else {
+    UMA_HISTOGRAM_TIMES("MPArch.ChildProcessLaunchFirst", launch_time);
+    done_first_launch = true;
   }
-
-  static void RecordHistograms(base::TimeTicks begin_launch_time);
-  static void RecordLaunchHistograms(base::TimeDelta launch_time);
-
-  // Performs the actual work of launching the process.
-  // Runs on the PROCESS_LAUNCHER thread.
-  static void LaunchInternal(
-      // |this_object| is NOT thread safe. Only use it to post a task back.
-      scoped_refptr<Context> this_object,
-      BrowserThread::ID client_thread_id,
-      int child_process_id,
-      SandboxedProcessLauncherDelegate* delegate,
-      base::CommandLine* cmd_line);
-
-  // Notifies the client about the result of the operation.
-  // Runs on the UI thread.
-  void Notify(
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-      bool zygote,
-#endif
-      base::Process process);
-
-  void Terminate();
-
-  static void SetProcessBackgroundedInternal(base::Process process,
-                                             bool background);
-
-  static void TerminateInternal(
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-      bool zygote,
-#endif
-      base::Process process);
-
-  Client* client_;
-  BrowserThread::ID client_thread_id_;
-  base::Process process_;
-  base::TerminationStatus termination_status_;
-  int exit_code_;
-#if defined(OS_ANDROID)
-  // The fd to close after creating the process.
-  base::ScopedFD ipcfd_;
-#elif defined(OS_POSIX) && !defined(OS_MACOSX)
-  bool zygote_;
-#endif
-  bool starting_;
-  // Controls whether the child process should be terminated on browser
-  // shutdown. Default behavior is to terminate the child.
-  bool terminate_child_on_shutdown_;
-};
-
-ChildProcessLauncher::Context::Context()
-    : client_(NULL),
-      client_thread_id_(BrowserThread::UI),
-      termination_status_(base::TERMINATION_STATUS_NORMAL_TERMINATION),
-      exit_code_(RESULT_CODE_NORMAL_EXIT),
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-      zygote_(false),
-#endif
-      starting_(true),
-#if defined(ADDRESS_SANITIZER) || defined(LEAK_SANITIZER) || \
-    defined(MEMORY_SANITIZER) || defined(THREAD_SANITIZER) || \
-    defined(UNDEFINED_SANITIZER)
-      terminate_child_on_shutdown_(false) {
-#else
-      terminate_child_on_shutdown_(true) {
-#endif
 }
 
-void ChildProcessLauncher::Context::Launch(
-    SandboxedProcessLauncherDelegate* delegate,
-    base::CommandLine* cmd_line,
-    int child_process_id,
-    Client* client) {
-  CHECK(BrowserThread::GetCurrentThreadIdentifier(&client_thread_id_));
-  client_ = client;
-
 #if defined(OS_ANDROID)
-  // Android only supports renderer, sandboxed utility and gpu.
-  std::string process_type =
-      cmd_line->GetSwitchValueASCII(switches::kProcessType);
-  CHECK(process_type == switches::kGpuProcess ||
-        process_type == switches::kRendererProcess ||
-        process_type == switches::kUtilityProcess)
-      << "Unsupported process type: " << process_type;
-
-  // Non-sandboxed utility or renderer process are currently not supported.
-  DCHECK(process_type == switches::kGpuProcess ||
-         !cmd_line->HasSwitch(switches::kNoSandbox));
-
-  // We need to close the client end of the IPC channel to reliably detect
-  // child termination. We will close this fd after we create the child
-  // process which is asynchronous on Android.
-  ipcfd_.reset(delegate->TakeIpcFd().release());
-#endif
+// TODO(sievers): Remove this by defining better what happens on what
+// thread in the corresponding Java code.
+void OnChildProcessStartedAndroid(const NotifyCallback& callback,
+                                  BrowserThread::ID client_thread_id,
+                                  const base::TimeTicks begin_launch_time,
+                                  base::ScopedFD ipcfd,
+                                  base::ProcessHandle handle) {
+  // This can be called on the launcher thread or UI thread.
+  base::TimeDelta launch_time = base::TimeTicks::Now() - begin_launch_time;
   BrowserThread::PostTask(
       BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
-      base::Bind(&Context::LaunchInternal,
-                 make_scoped_refptr(this),
-                 client_thread_id_,
-                 child_process_id,
-                 delegate,
-                 cmd_line));
-}
+      base::Bind(&RecordHistogramsOnLauncherThread, launch_time));
 
-#if defined(OS_ANDROID)
-// static
-void ChildProcessLauncher::Context::OnChildProcessStarted(
-    // |this_object| is NOT thread safe. Only use it to post a task back.
-    scoped_refptr<Context> this_object,
-    BrowserThread::ID client_thread_id,
-    const base::TimeTicks begin_launch_time,
-    base::ProcessHandle handle) {
-  RecordHistograms(begin_launch_time);
+  base::Closure callback_on_client_thread(
+      base::Bind(callback, false, base::Passed(&ipcfd),
+                 base::Passed(base::Process(handle))));
   if (BrowserThread::CurrentlyOn(client_thread_id)) {
-    // This is always invoked on the UI thread which is commonly the
-    // |client_thread_id| so we can shortcut one PostTask.
-    this_object->Notify(base::Process(handle));
+    callback_on_client_thread.Run();
   } else {
     BrowserThread::PostTask(
-        client_thread_id, FROM_HERE,
-        base::Bind(&ChildProcessLauncher::Context::Notify,
-                   this_object,
-                   base::Passed(base::Process(handle))));
-  }
+        client_thread_id, FROM_HERE, callback_on_client_thread);
+ }
 }
 #endif
 
-void ChildProcessLauncher::Context::ResetClient() {
-  // No need for locking as this function gets called on the same thread that
-  // client_ would be used.
-  CHECK(BrowserThread::CurrentlyOn(client_thread_id_));
-  client_ = NULL;
-}
-
-void ChildProcessLauncher::Context::UpdateTerminationStatus(bool known_dead) {
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-  if (zygote_) {
-    termination_status_ = ZygoteHostImpl::GetInstance()->
-        GetTerminationStatus(process_.Handle(), known_dead, &exit_code_);
-  } else if (known_dead) {
-    termination_status_ =
-        base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
-  } else {
-#elif defined(OS_MACOSX)
-  if (known_dead) {
-    termination_status_ =
-        base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
-  } else {
-#elif defined(OS_ANDROID)
-  if (IsChildProcessOomProtected(process_.Handle())) {
-    termination_status_ = base::TERMINATION_STATUS_OOM_PROTECTED;
-  } else {
-#else
-  {
+void LaunchOnLauncherThread(const NotifyCallback& callback,
+                            BrowserThread::ID client_thread_id,
+                            int child_process_id,
+                            SandboxedProcessLauncherDelegate* delegate,
+#if defined(OS_ANDROID)
+                            base::ScopedFD ipcfd,
 #endif
-    termination_status_ =
-      base::GetTerminationStatus(process_.Handle(), &exit_code_);
-  }
-}
-
-void ChildProcessLauncher::Context::SetProcessBackgrounded(bool background) {
-  base::Process to_pass = process_.Duplicate();
-  BrowserThread::PostTask(
-      BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
-      base::Bind(&Context::SetProcessBackgroundedInternal,
-                 base::Passed(&to_pass), background));
-}
-
-// static
-void ChildProcessLauncher::Context::RecordHistograms(
-    base::TimeTicks begin_launch_time) {
-  base::TimeDelta launch_time = base::TimeTicks::Now() - begin_launch_time;
-  if (BrowserThread::CurrentlyOn(BrowserThread::PROCESS_LAUNCHER)) {
-    RecordLaunchHistograms(launch_time);
-  } else {
-    BrowserThread::PostTask(
-        BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
-        base::Bind(&ChildProcessLauncher::Context::RecordLaunchHistograms,
-                   launch_time));
-  }
-}
-
-// static
-void ChildProcessLauncher::Context::RecordLaunchHistograms(
-    base::TimeDelta launch_time) {
-  // Log the launch time, separating out the first one (which will likely be
-  // slower due to the rest of the browser initializing at the same time).
-  static bool done_first_launch = false;
-  if (done_first_launch) {
-    UMA_HISTOGRAM_TIMES("MPArch.ChildProcessLaunchSubsequent", launch_time);
-  } else {
-    UMA_HISTOGRAM_TIMES("MPArch.ChildProcessLaunchFirst", launch_time);
-    done_first_launch = true;
-  }
-}
-
-// static
-void ChildProcessLauncher::Context::LaunchInternal(
-    // |this_object| is NOT thread safe. Only use it to post a task back.
-    scoped_refptr<Context> this_object,
-    BrowserThread::ID client_thread_id,
-    int child_process_id,
-    SandboxedProcessLauncherDelegate* delegate,
-    base::CommandLine* cmd_line) {
+                            base::CommandLine* cmd_line) {
+  DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
   scoped_ptr<SandboxedProcessLauncherDelegate> delegate_deleter(delegate);
 #if defined(OS_WIN)
+  bool use_zygote = false;
   bool launch_elevated = delegate->ShouldLaunchElevated();
-#elif defined(OS_ANDROID)
-  // Uses |ipcfd_| instead of |ipcfd| on Android.
 #elif defined(OS_MACOSX)
+  bool use_zygote = false;
   base::EnvironmentMap env = delegate->GetEnvironment();
   base::ScopedFD ipcfd = delegate->TakeIpcFd();
-#elif defined(OS_POSIX)
+#elif defined(OS_POSIX) && !defined(OS_ANDROID)
   bool use_zygote = delegate->ShouldUseZygote();
   base::EnvironmentMap env = delegate->GetEnvironment();
   base::ScopedFD ipcfd = delegate->TakeIpcFd();
@@ -346,7 +134,7 @@ void ChildProcessLauncher::Context::LaunchInternal(
       FileDescriptorInfoImpl::Create());
 
 #if defined(OS_ANDROID)
-  files_to_register->Share(kPrimaryIPCChannel, this_object->ipcfd_.get());
+  files_to_register->Share(kPrimaryIPCChannel, ipcfd.get());
 #else
   files_to_register->Transfer(kPrimaryIPCChannel, ipcfd.Pass());
 #endif
@@ -361,13 +149,9 @@ void ChildProcessLauncher::Context::LaunchInternal(
       *cmd_line, child_process_id, files_to_register.get());
 
   StartChildProcess(
-      cmd_line->argv(),
-      child_process_id,
-      files_to_register.Pass(),
-      base::Bind(&ChildProcessLauncher::Context::OnChildProcessStarted,
-                 this_object,
-                 client_thread_id,
-                 begin_launch_time));
+      cmd_line->argv(), child_process_id, files_to_register.Pass(),
+      base::Bind(&OnChildProcessStartedAndroid, callback, client_thread_id,
+                 begin_launch_time, base::Passed(&ipcfd)));
 
 #elif defined(OS_POSIX)
   // We need to close the client end of the IPC channel to reliably detect
@@ -447,102 +231,19 @@ void ChildProcessLauncher::Context::LaunchInternal(
   }
 #endif  // else defined(OS_POSIX)
 #if !defined(OS_ANDROID)
-  if (process.IsValid())
-    RecordHistograms(begin_launch_time);
-  BrowserThread::PostTask(
-      client_thread_id, FROM_HERE,
-      base::Bind(&Context::Notify,
-                 this_object.get(),
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-                 use_zygote,
-#endif
-                 base::Passed(&process)));
-#endif  // !defined(OS_ANDROID)
-}
-
-void ChildProcessLauncher::Context::Notify(
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-    bool zygote,
-#endif
-    base::Process process) {
-  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
-  // is fixed.
-  tracked_objects::ScopedTracker tracking_profile1(
-      FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "465841 ChildProcessLauncher::Context::Notify::Start"));
-
-#if defined(OS_ANDROID)
-  // Finally close the ipcfd
-  base::ScopedFD ipcfd_closer = ipcfd_.Pass();
-#endif
-  starting_ = false;
-  process_ = process.Pass();
-  if (!process_.IsValid())
-    LOG(ERROR) << "Failed to launch child process";
-
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-  zygote_ = zygote;
-#endif
-  if (client_) {
-    if (process_.IsValid()) {
-      // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
-      // is fixed.
-      tracked_objects::ScopedTracker tracking_profile2(
-          FROM_HERE_WITH_EXPLICIT_FUNCTION(
-              "465841 ChildProcessLauncher::Context::Notify::ProcessLaunched"));
-      client_->OnProcessLaunched();
-    } else {
-      // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
-      // is fixed.
-      tracked_objects::ScopedTracker tracking_profile3(
-          FROM_HERE_WITH_EXPLICIT_FUNCTION(
-              "465841 ChildProcessLauncher::Context::Notify::ProcessFailed"));
-      client_->OnProcessLaunchFailed();
-    }
-  } else {
-    // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
-    // is fixed.
-    tracked_objects::ScopedTracker tracking_profile4(
-        FROM_HERE_WITH_EXPLICIT_FUNCTION(
-            "465841 ChildProcessLauncher::Context::Notify::ProcessTerminate"));
-    Terminate();
+  if (process.IsValid()) {
+    RecordHistogramsOnLauncherThread(base::TimeTicks::Now() -
+                                     begin_launch_time);
   }
+  BrowserThread::PostTask(client_thread_id, FROM_HERE,
+                          base::Bind(callback,
+                                     use_zygote,
+                                     base::Passed(&process)));
+#endif  // !defined(OS_ANDROID)
 }
 
-void ChildProcessLauncher::Context::Terminate() {
-  if (!process_.IsValid())
-    return;
-
-  if (!terminate_child_on_shutdown_)
-    return;
-
-  // On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep!  So
-  // don't this on the UI/IO threads.
-  BrowserThread::PostTask(
-      BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
-      base::Bind(&Context::TerminateInternal,
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-                zygote_,
-#endif
-                base::Passed(&process_)));
-}
-
-// static
-void ChildProcessLauncher::Context::SetProcessBackgroundedInternal(
-    base::Process process,
-    bool background) {
-  process.SetProcessBackgrounded(background);
-#if defined(OS_ANDROID)
-  SetChildProcessInForeground(process.Handle(), !background);
-#endif
-}
-
-// static
-void ChildProcessLauncher::Context::TerminateInternal(
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-    bool zygote,
-#endif
-    base::Process process) {
+void TerminateOnLauncherThread(bool zygote, base::Process process) {
+  DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
 #if defined(OS_ANDROID)
   VLOG(1) << "ChromeProcess: Stopping process with handle "
           << process.Handle();
@@ -565,47 +266,219 @@ void ChildProcessLauncher::Context::TerminateInternal(
 #endif  // defined(OS_ANDROID)
 }
 
-// -----------------------------------------------------------------------------
+void SetProcessBackgroundedOnLauncherThread(base::Process process,
+                                            bool background) {
+  DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
+  process.SetProcessBackgrounded(background);
+#if defined(OS_ANDROID)
+  SetChildProcessInForeground(process.Handle(), !background);
+#endif
+}
+
+}  // anonymous namespace
 
 ChildProcessLauncher::ChildProcessLauncher(
     SandboxedProcessLauncherDelegate* delegate,
     base::CommandLine* cmd_line,
     int child_process_id,
-    Client* client) {
-  context_ = new Context();
-  context_->Launch(
-      delegate,
-      cmd_line,
-      child_process_id,
-      client);
+    Client* client,
+    bool terminate_on_shutdown)
+    : client_(client),
+      termination_status_(base::TERMINATION_STATUS_NORMAL_TERMINATION),
+      exit_code_(RESULT_CODE_NORMAL_EXIT),
+      zygote_(false),
+      starting_(true),
+#if defined(ADDRESS_SANITIZER) || defined(LEAK_SANITIZER) ||  \
+    defined(MEMORY_SANITIZER) || defined(THREAD_SANITIZER) || \
+    defined(UNDEFINED_SANITIZER)
+      terminate_child_on_shutdown_(false),
+#else
+      terminate_child_on_shutdown_(terminate_on_shutdown),
+#endif
+      weak_factory_(this) {
+  DCHECK(CalledOnValidThread());
+  CHECK(BrowserThread::GetCurrentThreadIdentifier(&client_thread_id_));
+  Launch(delegate, cmd_line, child_process_id);
 }
 
 ChildProcessLauncher::~ChildProcessLauncher() {
-  context_->ResetClient();
+  DCHECK(CalledOnValidThread());
+  if (process_.IsValid() && terminate_child_on_shutdown_) {
+    // On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep!  So
+    // don't this on the UI/IO threads.
+    BrowserThread::PostTask(BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+                            base::Bind(&TerminateOnLauncherThread, zygote_,
+                                       base::Passed(&process_)));

[Will Harris, 2015/09/29 01:31:55]

This CL changes behavior of destructor to call TerminateOnLauncherThread where
previously it did not.

I think this could be the root-cause of crbug.com/493452 since the majority of
the crashes appear to be from this PostTask.

WDYT?

[no sievers, 2015/09/29 17:16:58]

There is no functional behavior here, or at least none intended.
The code here was previously performed through the Context destructor where it
calls Terminate(). Did you check if the crash previously happened with the old
signature? We can probably discuss in the bug.

+  }
+}
+
+void ChildProcessLauncher::Launch(
+    SandboxedProcessLauncherDelegate* delegate,
+    base::CommandLine* cmd_line,
+    int child_process_id) {
+  DCHECK(CalledOnValidThread());
+
+#if defined(OS_ANDROID)
+  // Android only supports renderer, sandboxed utility and gpu.
+  std::string process_type =
+      cmd_line->GetSwitchValueASCII(switches::kProcessType);
+  CHECK(process_type == switches::kGpuProcess ||
+        process_type == switches::kRendererProcess ||
+        process_type == switches::kUtilityProcess)
+      << "Unsupported process type: " << process_type;
+
+  // Non-sandboxed utility or renderer process are currently not supported.
+  DCHECK(process_type == switches::kGpuProcess ||
+         !cmd_line->HasSwitch(switches::kNoSandbox));
+
+  // We need to close the client end of the IPC channel to reliably detect
+  // child termination. We will close this fd after we create the child
+  // process which is asynchronous on Android.
+  base::ScopedFD ipcfd(delegate->TakeIpcFd().release());
+#endif
+  NotifyCallback reply_callback(base::Bind(&ChildProcessLauncher::DidLaunch,
+                                           weak_factory_.GetWeakPtr(),
+                                           terminate_child_on_shutdown_));
+  BrowserThread::PostTask(
+      BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+      base::Bind(&LaunchOnLauncherThread, reply_callback, client_thread_id_,
+                 child_process_id, delegate,
+#if defined(OS_ANDROID)
+                 base::Passed(&ipcfd),
+#endif
+                 cmd_line));
+}
+
+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_);
+  } else if (known_dead) {
+    termination_status_ =
+        base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
+  } else {
+#elif defined(OS_MACOSX)
+  if (known_dead) {
+    termination_status_ =
+        base::GetKnownDeadTerminationStatus(process_.Handle(), &exit_code_);
+  } else {
+#elif defined(OS_ANDROID)
+  if (IsChildProcessOomProtected(process_.Handle())) {
+    termination_status_ = base::TERMINATION_STATUS_OOM_PROTECTED;
+  } else {
+#else
+  {
+#endif
+    termination_status_ =
+      base::GetTerminationStatus(process_.Handle(), &exit_code_);
+  }
+}
+
+void ChildProcessLauncher::SetProcessBackgrounded(bool background) {
+  DCHECK(CalledOnValidThread());
+  base::Process to_pass = process_.Duplicate();
+  BrowserThread::PostTask(BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+                          base::Bind(&SetProcessBackgroundedOnLauncherThread,
+                                     base::Passed(&to_pass), background));
+}
+
+void ChildProcessLauncher::DidLaunch(
+    base::WeakPtr<ChildProcessLauncher> instance,
+    bool terminate_on_shutdown,
+    bool zygote,
+#if defined(OS_ANDROID)
+    base::ScopedFD ipcfd,
+#endif
+    base::Process process) {
+  if (!process.IsValid())
+    LOG(ERROR) << "Failed to launch child process";
+
+  // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
+  // is fixed.
+  tracked_objects::ScopedTracker tracking_profile1(
+      FROM_HERE_WITH_EXPLICIT_FUNCTION(
+          "465841 ChildProcessLauncher::Context::Notify::Start"));
+
+  if (instance.get()) {
+    instance->Notify(zygote,
+#if defined(OS_ANDROID)
+                     ipcfd.Pass(),
+#endif
+                     process.Pass());
+  } else {
+    // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
+    // is fixed.
+    tracked_objects::ScopedTracker tracking_profile4(
+        FROM_HERE_WITH_EXPLICIT_FUNCTION(
+            "465841 ChildProcessLauncher::Context::Notify::ProcessTerminate"));
+    if (process.IsValid() && terminate_on_shutdown) {

[rvargas (doing something else), 2015/03/27 22:01:22]

We should terminate the process regardless of terminate_on_shutdown because we
are not telling anyone about it.

[no sievers, 2015/03/27 22:23:04]

Isn't that a behavioral change though (which I've tried to avoid). Before we'd
have taken line 508 on the left, which would also have skipped calling
TerminateInternal() if |!terminate_child_on_shutdown_|?

Also, it looks like !terminate is only used for PluginProcessHost where it says
that it needs to shutdown gracefully. Wouldn't that be regardless of the client
going away or not?

[rvargas (doing something else), 2015/03/27 22:56:00]

You're right, that's the current behavior. Sounds like a bug to me, but it's
better not to change behavior in this CL.
I think the problem is that the delegate is never informed of the process being
created at all, so nobody will perform any cleanup and the process will just
leak. On the other hand, there may be some platform dependent behavior here
because I think on posix it's possible to start receiving IPCs before the
delegate is notified.

+      // On Posix, EnsureProcessTerminated can lead to 2 seconds of sleep!  So
+      // don't this on the UI/IO threads.
+      BrowserThread::PostTask(BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
+                              base::Bind(&TerminateOnLauncherThread, zygote,
+                                         base::Passed(&process)));
+    }
+  }
+}
+
+void ChildProcessLauncher::Notify(
+    bool zygote,
+#if defined(OS_ANDROID)
+    base::ScopedFD ipcfd,
+#endif
+    base::Process process) {
+  DCHECK(CalledOnValidThread());
+  starting_ = false;
+  process_ = process.Pass();
+
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
+  zygote_ = zygote;
+#endif
+  if (process_.IsValid()) {
+    // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
+    // is fixed.
+    tracked_objects::ScopedTracker tracking_profile2(
+        FROM_HERE_WITH_EXPLICIT_FUNCTION(
+            "465841 ChildProcessLauncher::Context::Notify::ProcessLaunched"));
+    client_->OnProcessLaunched();
+  } else {
+    // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/465841
+    // is fixed.
+    tracked_objects::ScopedTracker tracking_profile3(
+        FROM_HERE_WITH_EXPLICIT_FUNCTION(
+            "465841 ChildProcessLauncher::Context::Notify::ProcessFailed"));
+    client_->OnProcessLaunchFailed();
+  }
 }
 
 bool ChildProcessLauncher::IsStarting() {
-  return context_->starting();
+  // TODO(crbug.com/469248): This fails in some tests.
+  // DCHECK(CalledOnValidThread());
+  return starting_;
 }
 
 const base::Process& ChildProcessLauncher::GetProcess() const {
-  DCHECK(!context_->starting());
-  return context_->process();
+  // TODO(crbug.com/469248): This fails in some tests.
+  // DCHECK(CalledOnValidThread());
+  return process_;
 }
 
 base::TerminationStatus ChildProcessLauncher::GetChildTerminationStatus(
     bool known_dead,
     int* exit_code) {
-  if (!context_->process().IsValid()) {
+  DCHECK(CalledOnValidThread());
+  if (!process_.IsValid()) {
     // Process is already gone, so return the cached termination status.
     if (exit_code)
-      *exit_code = context_->exit_code();
-    return context_->termination_status();
+      *exit_code = exit_code_;
+    return termination_status_;
   }
 
-  context_->UpdateTerminationStatus(known_dead);
+  UpdateTerminationStatus(known_dead);
   if (exit_code)
-    *exit_code = context_->exit_code();
+    *exit_code = exit_code_;
 
   // POSIX: If the process crashed, then the kernel closed the socket
   // for it and so the child has already died by the time we get
@@ -613,25 +486,17 @@ base::TerminationStatus ChildProcessLauncher::GetChildTerminationStatus(
   // it'll reap the process.  However, if GetTerminationStatus didn't
   // reap the child (because it was still running), we'll need to
   // Terminate via ProcessWatcher. So we can't close the handle here.
-  if (context_->termination_status() != base::TERMINATION_STATUS_STILL_RUNNING)
-    context_->Close();
+  if (termination_status_ != base::TERMINATION_STATUS_STILL_RUNNING)
+    process_.Close();
 
-  return context_->termination_status();
-}
-
-void ChildProcessLauncher::SetProcessBackgrounded(bool background) {
-  context_->SetProcessBackgrounded(background);
-}
-
-void ChildProcessLauncher::SetTerminateChildOnShutdown(
-    bool terminate_on_shutdown) {
-  if (context_.get())
-    context_->set_terminate_child_on_shutdown(terminate_on_shutdown);
+  return termination_status_;
 }
 
 ChildProcessLauncher::Client* ChildProcessLauncher::ReplaceClientForTest(
     Client* client) {
-  return context_->ReplaceClientForTest(client);
+  Client* ret = client_;
+  client_ = client;
+  return ret;
 }
 
 }  // namespace content