Properly handle unmanaged processes in mojo::shell:ChildProcessHost

mojo/shell/runner/host/child_process_host.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "mojo/shell/runner/host/child_process_host.h"
 
 #include <stdint.h>
 
 #include <utility>
 
@@ skipping 37 matching lines @@
       start_child_process_event_(false, false),
       weak_factory_(this) {
   node_channel_.reset(new edk::PlatformChannelPair);
   primordial_pipe_token_ = edk::GenerateRandomToken();
   controller_.Bind(
       InterfacePtrInfo<mojom::ChildController>(
           edk::CreateParentMessagePipe(primordial_pipe_token_), 0u));
 }
 
 ChildProcessHost::ChildProcessHost(ScopedHandle channel)
-    : launch_process_runner_(nullptr),
+    : external_process_(true),
+      launch_process_runner_(nullptr),
       delegate_(nullptr),
       start_sandboxed_(false),
       start_child_process_event_(false, false),
       weak_factory_(this) {
   CHECK(channel.is_valid());
   ScopedMessagePipeHandle handle(MessagePipeHandle(channel.release().value()));
   controller_.Bind(
       InterfacePtrInfo<mojom::ChildController>(std::move(handle), 0u));
 }
 
 ChildProcessHost::~ChildProcessHost() {
   if (!app_path_.empty())
     CHECK(!controller_) << "Destroying ChildProcessHost before calling Join";
 }
 
 void ChildProcessHost::Start(const ProcessReadyCallback& callback) {
+  DCHECK(!external_process_);
   DCHECK(!child_process_.IsValid());
   launch_process_runner_->PostTaskAndReply(
       FROM_HERE,
       base::Bind(&ChildProcessHost::DoLaunch, base::Unretained(this)),
       base::Bind(&ChildProcessHost::DidStart,
                  weak_factory_.GetWeakPtr(), callback));
 }
 
 int ChildProcessHost::Join() {
-  if (controller_)  // We use this as a signal that Start was called.
+  if (controller_ && !external_process_)
     start_child_process_event_.Wait();
 
   controller_ = mojom::ChildControllerPtr();
+
   // This host may be hosting a child process whose lifetime is controlled
   // elsewhere. In this case we have no known process handle to wait on.
   if (child_process_.IsValid()) {
     int rv = -1;
     LOG_IF(ERROR, !child_process_.WaitForExit(&rv))
         << "Failed to wait for child process";
 
     child_process_.Close();
     return rv;
   }
 
   return 0;
 }
 
 void ChildProcessHost::StartApp(
     InterfaceRequest<mojom::ShellClient> request,
     const mojom::ChildController::StartAppCallback& on_app_complete) {
   DCHECK(controller_);
-
-  // In this case the process must have already been launched.
-  start_child_process_event_.Signal();
-
   on_app_complete_ = on_app_complete;
   controller_->StartApp(
       std::move(request),
       base::Bind(&ChildProcessHost::AppCompleted, weak_factory_.GetWeakPtr()));
 }
 
 void ChildProcessHost::ExitNow(int32_t exit_code) {
   DCHECK(controller_);
 
   controller_->ExitNow(exit_code);
 }
 
 void ChildProcessHost::DidStart(const ProcessReadyCallback& callback) {
   DVLOG(2) << "ChildProcessHost::DidStart()";
 
   if (child_process_.IsValid()) {
     callback.Run(child_process_.Pid());
   } else {
     LOG(ERROR) << "Failed to start child process";
     AppCompleted(MOJO_RESULT_UNKNOWN);
   }
 }
 
 void ChildProcessHost::DoLaunch() {
+  DCHECK(!external_process_);
+
   const base::CommandLine* parent_command_line =
       base::CommandLine::ForCurrentProcess();
   base::FilePath target_path = parent_command_line->GetProgram();
   // |app_path_| can be empty in tests.
   if (!app_path_.MatchesExtension(FILE_PATH_LITERAL(".mojo")) &&
       !app_path_.empty()) {
     target_path = app_path_;
   }
 
   base::CommandLine child_command_line(target_path);
@@ skipping 87 matching lines @@
 void ChildProcessHost::AppCompleted(int32_t result) {
   if (!on_app_complete_.is_null()) {
     auto on_app_complete = on_app_complete_;
     on_app_complete_.reset();
     on_app_complete.Run(result);
   }
 }
 
 }  // namespace shell
 }  // namespace mojo