From 954e37aad499a68771f3c517d1e4760e38986e0e Mon Sep 17 00:00:00 2001 From: sievers Date: Fri, 27 Mar 2015 18:50:24 -0700 Subject: [PATCH] Simplify ChildProcessLauncher Remove the refcounted internal state object (nested class 'Context'). This refactor makes it more obvious what happens on what thread, and avoids the need to pass refptrs around. TBR=bradnelson@chromium.org BUG=469248 Review URL: https://codereview.chromium.org/1022703007 Cr-Commit-Position: refs/heads/master@{#322695} --- components/nacl/browser/nacl_broker_host_win.cc | 3 +- components/nacl/browser/nacl_process_host.cc | 3 +- content/browser/browser_child_process_host_impl.cc | 12 +- content/browser/browser_child_process_host_impl.h | 7 +- content/browser/child_process_launcher.cc | 635 ++++++++------------- content/browser/child_process_launcher.h | 53 +- content/browser/gpu/gpu_process_host.cc | 3 +- content/browser/plugin_process_host.cc | 10 +- content/browser/ppapi_plugin_process_host.cc | 3 +- content/browser/utility_process_host_impl.cc | 3 +- .../public/browser/browser_child_process_host.h | 3 +- 11 files changed, 317 insertions(+), 418 deletions(-) diff --git a/components/nacl/browser/nacl_broker_host_win.cc b/components/nacl/browser/nacl_broker_host_win.cc index a8c5350144bf..17c7ed0b44b8 100644 --- a/components/nacl/browser/nacl_broker_host_win.cc +++ b/components/nacl/browser/nacl_broker_host_win.cc @@ -67,7 +67,8 @@ bool NaClBrokerHost::Init() { cmd_line->AppendSwitch(switches::kNoErrorDialogs); process_->Launch(new NaClBrokerSandboxedProcessLauncherDelegate, - cmd_line); + cmd_line, + true); return true; } diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc index a0c8e56c9a35..28a05374a0d8 100644 --- a/components/nacl/browser/nacl_process_host.cc +++ b/components/nacl/browser/nacl_process_host.cc @@ -626,7 +626,8 @@ bool NaClProcessHost::LaunchSelLdr() { #endif process_->Launch( new NaClSandboxedProcessLauncherDelegate(process_->GetHost()), - cmd_line.release()); + cmd_line.release(), + true); return true; } diff --git a/content/browser/browser_child_process_host_impl.cc b/content/browser/browser_child_process_host_impl.cc index 6ace7de4aba1..c8d0641d7711 100644 --- a/content/browser/browser_child_process_host_impl.cc +++ b/content/browser/browser_child_process_host_impl.cc @@ -128,7 +128,8 @@ void BrowserChildProcessHostImpl::TerminateAll() { void BrowserChildProcessHostImpl::Launch( SandboxedProcessLauncherDelegate* delegate, - base::CommandLine* cmd_line) { + base::CommandLine* cmd_line, + bool terminate_on_shutdown) { DCHECK_CURRENTLY_ON(BrowserThread::IO); GetContentClient()->browser()->AppendExtraCommandLineSwitches( @@ -152,7 +153,8 @@ void BrowserChildProcessHostImpl::Launch( delegate, cmd_line, data_.id, - this)); + this, + terminate_on_shutdown)); } const ChildProcessData& BrowserChildProcessHostImpl::GetData() const { @@ -194,12 +196,6 @@ void BrowserChildProcessHostImpl::SetBackgrounded(bool backgrounded) { child_process_->SetProcessBackgrounded(backgrounded); } -void BrowserChildProcessHostImpl::SetTerminateChildOnShutdown( - bool terminate_on_shutdown) { - DCHECK_CURRENTLY_ON(BrowserThread::IO); - child_process_->SetTerminateChildOnShutdown(terminate_on_shutdown); -} - void BrowserChildProcessHostImpl::AddFilter(BrowserMessageFilter* filter) { child_process_host_->AddFilter(filter->GetFilter()); } diff --git a/content/browser/browser_child_process_host_impl.h b/content/browser/browser_child_process_host_impl.h index 778b276cbeca..c563410d9865 100644 --- a/content/browser/browser_child_process_host_impl.h +++ b/content/browser/browser_child_process_host_impl.h @@ -54,7 +54,8 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl // BrowserChildProcessHost implementation: bool Send(IPC::Message* message) override; void Launch(SandboxedProcessLauncherDelegate* delegate, - base::CommandLine* cmd_line) override; + base::CommandLine* cmd_line, + bool terminate_on_shutdown) override; const ChildProcessData& GetData() const override; ChildProcessHost* GetHost() const override; base::TerminationStatus GetTerminationStatus(bool known_dead, @@ -77,10 +78,6 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl // Callers can reduce the BrowserChildProcess' priority. void SetBackgrounded(bool backgrounded); - // Controls whether the child process should be terminated on browser - // shutdown. Default is to always terminate. - void SetTerminateChildOnShutdown(bool terminate_on_shutdown); - // Adds an IPC message filter. void AddFilter(BrowserMessageFilter* filter); diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc index 24fd73721846..28be72bb9c32 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 // 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 { - 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 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; - - ~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 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 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 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 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_))); + } +} + +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 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) { + // 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 diff --git a/content/browser/child_process_launcher.h b/content/browser/child_process_launcher.h index bd923b97a524..2d647f408776 100644 --- a/content/browser/child_process_launcher.h +++ b/content/browser/child_process_launcher.h @@ -6,11 +6,14 @@ #define CONTENT_BROWSER_CHILD_PROCESS_LAUNCHER_H_ #include "base/basictypes.h" -#include "base/memory/ref_counted.h" +#include "base/files/scoped_file.h" +#include "base/memory/weak_ptr.h" #include "base/process/kill.h" #include "base/process/launch.h" #include "base/process/process.h" +#include "base/threading/non_thread_safe.h" #include "content/common/content_export.h" +#include "content/public/browser/browser_thread.h" namespace base { class CommandLine; @@ -22,7 +25,7 @@ class SandboxedProcessLauncherDelegate; // Launches a process asynchronously and notifies the client of the process // handle when it's available. It's used to avoid blocking the calling thread // on the OS since often it can take > 100 ms to create the process. -class CONTENT_EXPORT ChildProcessLauncher { +class CONTENT_EXPORT ChildProcessLauncher : public base::NonThreadSafe { public: class CONTENT_EXPORT Client { public: @@ -45,7 +48,8 @@ class CONTENT_EXPORT ChildProcessLauncher { SandboxedProcessLauncherDelegate* delegate, base::CommandLine* cmd_line, int child_process_id, - Client* client); + Client* client, + bool terminate_on_shutdown = true); ~ChildProcessLauncher(); // True if the process is being launched and so the handle isn't available. @@ -72,18 +76,49 @@ class CONTENT_EXPORT ChildProcessLauncher { // this after the process has started. void SetProcessBackgrounded(bool background); - // Controls whether the child process should be terminated on browser - // shutdown. - void SetTerminateChildOnShutdown(bool terminate_on_shutdown); - // Replaces the ChildProcessLauncher::Client for testing purposes. Returns the // previous client. Client* ReplaceClientForTest(Client* client); private: - class Context; + // Posts a task to the launcher thread to do the actual work. + void Launch(SandboxedProcessLauncherDelegate* delegate, + base::CommandLine* cmd_line, + int child_process_id); + + void UpdateTerminationStatus(bool known_dead); + + // This is always called on the client thread after an attempt + // to launch the child process on the launcher thread. + // It makes sure we always perform the necessary cleanup if the + // client went away. + static void DidLaunch(base::WeakPtr instance, + bool terminate_on_shutdown, + bool zygote, +#if defined(OS_ANDROID) + base::ScopedFD ipcfd, +#endif + base::Process process); + + // Notifies the client about the result of the operation. + void Notify(bool zygote, +#if defined(OS_ANDROID) + base::ScopedFD ipcfd, +#endif + base::Process process); + + Client* client_; + BrowserThread::ID client_thread_id_; + base::Process process_; + base::TerminationStatus termination_status_; + int exit_code_; + bool zygote_; + bool starting_; + // Controls whether the child process should be terminated on browser + // shutdown. Default behavior is to terminate the child. + const bool terminate_child_on_shutdown_; - scoped_refptr context_; + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(ChildProcessLauncher); }; diff --git a/content/browser/gpu/gpu_process_host.cc b/content/browser/gpu/gpu_process_host.cc index 18b51fefbba5..be1437451b93 100644 --- a/content/browser/gpu/gpu_process_host.cc +++ b/content/browser/gpu/gpu_process_host.cc @@ -938,7 +938,8 @@ bool GpuProcessHost::LaunchGpuProcess(const std::string& channel_id) { process_->Launch( new GpuSandboxedProcessLauncherDelegate(cmd_line, process_->GetHost()), - cmd_line); + cmd_line, + true); process_launched_ = true; UMA_HISTOGRAM_ENUMERATION("GPU.GPUProcessLifetimeEvents", diff --git a/content/browser/plugin_process_host.cc b/content/browser/plugin_process_host.cc index 9bb8cf8ff5ff..4b6f361c7aea 100644 --- a/content/browser/plugin_process_host.cc +++ b/content/browser/plugin_process_host.cc @@ -248,15 +248,15 @@ bool PluginProcessHost::Init(const WebPluginInfo& info) { cmd_line->AppendSwitchASCII(switches::kProcessChannelID, channel_id); - process_->Launch( - new PluginSandboxedProcessLauncherDelegate(process_->GetHost()), - cmd_line); - // The plugin needs to be shutdown gracefully, i.e. NP_Shutdown needs to be // called on the plugin. The plugin process exits when it receives the // OnChannelError notification indicating that the browser plugin channel has // been destroyed. - process_->SetTerminateChildOnShutdown(false); + bool terminate_on_shutdown = false; + process_->Launch( + new PluginSandboxedProcessLauncherDelegate(process_->GetHost()), + cmd_line, + terminate_on_shutdown); ResourceMessageFilter::GetContextsCallback get_contexts_callback( base::Bind(&PluginProcessHost::GetContexts, diff --git a/content/browser/ppapi_plugin_process_host.cc b/content/browser/ppapi_plugin_process_host.cc index 480296fd0313..f5756b1aa0e1 100644 --- a/content/browser/ppapi_plugin_process_host.cc +++ b/content/browser/ppapi_plugin_process_host.cc @@ -388,7 +388,8 @@ bool PpapiPluginProcessHost::Init(const PepperPluginInfo& info) { new PpapiPluginSandboxedProcessLauncherDelegate(is_broker_, info, process_->GetHost()), - cmd_line); + cmd_line, + true); return true; } diff --git a/content/browser/utility_process_host_impl.cc b/content/browser/utility_process_host_impl.cc index d2e43dbc19fe..ac016d274603 100644 --- a/content/browser/utility_process_host_impl.cc +++ b/content/browser/utility_process_host_impl.cc @@ -292,7 +292,8 @@ bool UtilityProcessHostImpl::StartProcess() { run_elevated_, no_sandbox_, env_, process_->GetHost()), - cmd_line); + cmd_line, + true); } return true; diff --git a/content/public/browser/browser_child_process_host.h b/content/public/browser/browser_child_process_host.h index 695d27c7cca0..2cd989660851 100644 --- a/content/public/browser/browser_child_process_host.h +++ b/content/public/browser/browser_child_process_host.h @@ -44,7 +44,8 @@ class CONTENT_EXPORT BrowserChildProcessHost : public IPC::Sender { // Takes ownership of |cmd_line| and |delegate|. virtual void Launch( SandboxedProcessLauncherDelegate* delegate, - base::CommandLine* cmd_line) = 0; + base::CommandLine* cmd_line, + bool terminate_on_shutdown) = 0; virtual const ChildProcessData& GetData() const = 0; -- 2.11.4.GIT