Change primordial pipes to ShellClient

Changes shell client startup pipes to be ShellClient instead
of ShellClientFactory. This simplifies a lot of startup code.

Due to this change, apps must now take responsibility for
shutting themselves down when they feel it's appropriate. For
now, all relevant apps are force-exited to mimic the old
behavior.

Once apptests are deleted, we should expose a clean quit
closure to apps so they have something to run when they want
to quit. Hard-exiting the process is definitely not the right
thing to do.

BUG=

Review URL: https://codereview.chromium.org/1801963002

Cr-Commit-Position: refs/heads/master@{#381114}

32 files changed, 323 insertions, 646 deletions

diff --git a/mojo/mojo_shell.gyp b/mojo/mojo_shell.gyp
index 9e2e5ee..0b95228 100644
--- a/mojo/mojo_shell.gyp
+++ b/mojo/mojo_shell.gyp
@@ -74,35 +74,28 @@
       'mojom_bindings_generator_explicit.gypi',
     ],
   }, {
-    'target_name': 'mojo_runner_connection_lib',
+    'target_name': 'mojo_runner_common_lib',
     'type': 'static_library',
     'sources': [
-      'shell/runner/child/runner_connection.cc',
-      'shell/runner/child/runner_connection.h',
+      'shell/runner/common/client_util.cc',
+      'shell/runner/common/client_util.h',
+      'shell/runner/common/switches.cc',
+      'shell/runner/common/switches.h',
+    ],
+    'include_dirs': [
+      '..',
     ],
     'dependencies': [
-      'mojo_runner_common_lib',
       '<(DEPTH)/base/base.gyp:base',
-      '<(DEPTH)/mojo/mojo_base.gyp:mojo_application_base',
       '<(DEPTH)/mojo/mojo_base.gyp:mojo_application_bindings',
       '<(DEPTH)/mojo/mojo_edk.gyp:mojo_system_impl',
-      '<(DEPTH)/mojo/mojo_platform_handle.gyp:platform_handle',
-      '<(DEPTH)/mojo/mojo_public.gyp:mojo_message_pump_lib',
+      '<(DEPTH)/mojo/mojo_public.gyp:mojo_cpp_bindings',
+      '<(DEPTH)/mojo/mojo_public.gyp:mojo_cpp_system',
     ],
     'export_dependent_settings': [
       '<(DEPTH)/mojo/mojo_base.gyp:mojo_application_bindings',
     ],
   }, {
-    'target_name': 'mojo_runner_common_lib',
-    'type': 'static_library',
-    'sources': [
-      'shell/runner/common/switches.cc',
-      'shell/runner/common/switches.h',
-    ],
-    'include_dirs': [
-      '..',
-    ],
-  }, {
     'target_name': 'mojo_runner_host_lib',
     'type': 'static_library',
     'sources': [
diff --git a/mojo/shell/native_runner.h b/mojo/shell/native_runner.h
index 92c1adc..bf28755 100644
--- a/mojo/shell/native_runner.h
+++ b/mojo/shell/native_runner.h
@@ -10,7 +10,6 @@
 #include "base/process/process_handle.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "mojo/shell/public/interfaces/shell_client.mojom.h"
-#include "mojo/shell/public/interfaces/shell_client_factory.mojom.h"
 
 namespace base {
 class FilePath;
@@ -28,12 +27,12 @@ class NativeRunner {
   virtual ~NativeRunner() {}
 
   // Loads the app in the file at |app_path| and runs it on some other
-  // thread/process.
-  virtual void Start(
+  // thread/process. Returns a ShellClient handle the shell can use to connect
+  // to the the app.
+  virtual mojom::ShellClientPtr Start(
       const base::FilePath& app_path,
       const Identity& target,
       bool start_sandboxed,
-      InterfaceRequest<mojom::ShellClient> request,
       const base::Callback<void(base::ProcessId)>& pid_available_callback,
       const base::Closure& app_completed_callback) = 0;
 };
diff --git a/mojo/shell/public/cpp/application_runner.h b/mojo/shell/public/cpp/application_runner.h
index 3863923..2fb28a2 100644
--- a/mojo/shell/public/cpp/application_runner.h
+++ b/mojo/shell/public/cpp/application_runner.h
@@ -27,7 +27,7 @@ class ShellConnection;
 // ultimately Quit().
 class ApplicationRunner {
  public:
-  // Takes ownership of |delegate|.
+  // Takes ownership of |client|.
   explicit ApplicationRunner(ShellClient* client);
   ~ApplicationRunner();
 
@@ -53,6 +53,10 @@ class ApplicationRunner {
   // requests from others.
   void DestroyShellConnection();
 
+  // Allows the caller to explicitly quit the application. Must be called from
+  // the thread which created the ApplicationRunner.
+  void Quit();
+
  private:
   scoped_ptr<ShellConnection> connection_;
   scoped_ptr<ShellClient> client_;
diff --git a/mojo/shell/public/cpp/connector.h b/mojo/shell/public/cpp/connector.h
index 73a6ef4..b615535 100644
--- a/mojo/shell/public/cpp/connector.h
+++ b/mojo/shell/public/cpp/connector.h
@@ -9,7 +9,7 @@
 #include "mojo/shell/public/cpp/identity.h"
 #include "mojo/shell/public/interfaces/connector.mojom.h"
 #include "mojo/shell/public/interfaces/shell.mojom.h"
-#include "mojo/shell/public/interfaces/shell_client_factory.mojom.h"
+#include "mojo/shell/public/interfaces/shell_client.mojom.h"
 
 namespace mojo {
 
@@ -40,21 +40,21 @@ class Connector {
     const Identity& target() { return target_; }
     void set_target(const Identity& target) { target_ = target; }
     void set_client_process_connection(
-        shell::mojom::ShellClientFactoryPtr shell_client_factory,
+        shell::mojom::ShellClientPtr shell_client,
         shell::mojom::PIDReceiverRequest pid_receiver_request) {
-      shell_client_factory_ = std::move(shell_client_factory);
+      shell_client_ = std::move(shell_client);
       pid_receiver_request_ = std::move(pid_receiver_request);
     }
     void TakeClientProcessConnection(
-        shell::mojom::ShellClientFactoryPtr* shell_client_factory,
+        shell::mojom::ShellClientPtr* shell_client,
         shell::mojom::PIDReceiverRequest* pid_receiver_request) {
-      *shell_client_factory = std::move(shell_client_factory_);
+      *shell_client = std::move(shell_client_);
       *pid_receiver_request = std::move(pid_receiver_request_);
     }
 
    private:
     Identity target_;
-    shell::mojom::ShellClientFactoryPtr shell_client_factory_;
+    shell::mojom::ShellClientPtr shell_client_;
     shell::mojom::PIDReceiverRequest pid_receiver_request_;
 
     DISALLOW_COPY_AND_ASSIGN(ConnectParams);
diff --git a/mojo/shell/public/cpp/lib/application_runner.cc b/mojo/shell/public/cpp/lib/application_runner.cc
index dd9be48..5253053 100644
--- a/mojo/shell/public/cpp/lib/application_runner.cc
+++ b/mojo/shell/public/cpp/lib/application_runner.cc
@@ -81,4 +81,8 @@ void ApplicationRunner::DestroyShellConnection() {
   connection_.reset();
 }
 
+void ApplicationRunner::Quit() {
+  base::MessageLoop::current()->QuitWhenIdle();
+}
+
 }  // namespace mojo
diff --git a/mojo/shell/public/cpp/lib/application_test_base.cc b/mojo/shell/public/cpp/lib/application_test_base.cc
index c03f7f72..a52a193 100644
--- a/mojo/shell/public/cpp/lib/application_test_base.cc
+++ b/mojo/shell/public/cpp/lib/application_test_base.cc
@@ -36,7 +36,9 @@ shell::mojom::ConnectorPtr g_connector;
 class ShellGrabber : public shell::mojom::ShellClient {
  public:
   explicit ShellGrabber(InterfaceRequest<shell::mojom::ShellClient> request)
-      : binding_(this, std::move(request)) {}
+      : binding_(this, std::move(request)) {
+    binding_.set_connection_error_handler([] { _exit(1); });
+  }
 
   void WaitForInitialize() {
     // Initialize is always the first call made on ShellClient.
diff --git a/mojo/shell/public/cpp/lib/connector_impl.cc b/mojo/shell/public/cpp/lib/connector_impl.cc
index 7ff56d4..3b16a0c 100644
--- a/mojo/shell/public/cpp/lib/connector_impl.cc
+++ b/mojo/shell/public/cpp/lib/connector_impl.cc
@@ -59,21 +59,19 @@ scoped_ptr<Connection> ConnectorImpl::Connect(ConnectParams* params) {
       shell::mojom::kInvalidInstanceID, std::move(remote_interfaces),
       std::move(local_request), request, Connection::State::PENDING));
 
-  shell::mojom::ShellClientFactoryPtr shell_client_factory;
+  shell::mojom::ShellClientPtr shell_client;
   shell::mojom::PIDReceiverRequest pid_receiver_request;
-  params->TakeClientProcessConnection(&shell_client_factory,
-                                      &pid_receiver_request);
+  params->TakeClientProcessConnection(&shell_client, &pid_receiver_request);
   shell::mojom::ClientProcessConnectionPtr client_process_connection;
-  if (shell_client_factory.is_bound() && pid_receiver_request.is_pending()) {
+  if (shell_client.is_bound() && pid_receiver_request.is_pending()) {
     client_process_connection = shell::mojom::ClientProcessConnection::New();
-    client_process_connection->shell_client_factory =
-        shell_client_factory.PassInterface().PassHandle();
+    client_process_connection->shell_client =
+        shell_client.PassInterface().PassHandle();
     client_process_connection->pid_receiver_request =
         pid_receiver_request.PassMessagePipe();
-  } else if (shell_client_factory.is_bound() ||
-             pid_receiver_request.is_pending()) {
-    NOTREACHED() << "If one of shell_client_factory or pid_receiver_request is"
-                 << "valid, both must be valid.";
+  } else if (shell_client.is_bound() || pid_receiver_request.is_pending()) {
+    NOTREACHED() << "If one of shell_client or pid_receiver_request is valid, "
+                 << "both must be valid.";
     return std::move(registry);
   }
   connector_->Connect(
diff --git a/mojo/shell/public/cpp/lib/shell_connection.cc b/mojo/shell/public/cpp/lib/shell_connection.cc
index 24210ad..7c8b032 100644
--- a/mojo/shell/public/cpp/lib/shell_connection.cc
+++ b/mojo/shell/public/cpp/lib/shell_connection.cc
@@ -19,9 +19,6 @@ namespace mojo {
 ////////////////////////////////////////////////////////////////////////////////
 // ShellConnection, public:
 
-ShellConnection::ShellConnection(mojo::ShellClient* client)
-    : ShellConnection(client, nullptr) {}
-
 ShellConnection::ShellConnection(mojo::ShellClient* client,
                                  shell::mojom::ShellClientRequest request)
     : client_(client), binding_(this) {
@@ -29,17 +26,12 @@ ShellConnection::ShellConnection(mojo::ShellClient* client,
   pending_connector_request_ = GetProxy(&connector);
   connector_.reset(new ConnectorImpl(std::move(connector)));
 
-  if (request.is_pending())
-    BindToRequest(std::move(request));
+  DCHECK(request.is_pending());
+  binding_.Bind(std::move(request));
 }
 
 ShellConnection::~ShellConnection() {}
 
-void ShellConnection::BindToRequest(shell::mojom::ShellClientRequest request) {
-  DCHECK(!binding_.is_bound());
-  binding_.Bind(std::move(request));
-}
-
 void ShellConnection::SetAppTestConnectorForTesting(
     shell::mojom::ConnectorPtr connector) {
   pending_connector_request_ = nullptr;
diff --git a/mojo/shell/public/cpp/names.h b/mojo/shell/public/cpp/names.h
index 9815c87..9fbabf2 100644
--- a/mojo/shell/public/cpp/names.h
+++ b/mojo/shell/public/cpp/names.h
@@ -28,9 +28,9 @@ namespace mojo {
 //   Represents a native executable on the host platform, expected to live
 //   alongside the shell executable. Executables launched via this mechanism are
 //   passed a handle to the shell on the command line and are expected to bind
-//   a ShellClientFactory via which they will receive a ShellClientRequest
-//   enabling further communication with the shell. The path component contains
-//   the executable name, minus any platform-specific extension.
+//   a ShellClientRequest enabling further communication with the shell. The
+//   path component contains the executable name, minus any platform-specific
+//   extension.
 //
 // Other types may be supplied but are not recognized by any of the
 // NativeRunners, and as such custom loaders must be specified for such names.
diff --git a/mojo/shell/public/cpp/shell_connection.h b/mojo/shell/public/cpp/shell_connection.h
index 216ff93..8c90ec7 100644
--- a/mojo/shell/public/cpp/shell_connection.h
+++ b/mojo/shell/public/cpp/shell_connection.h
@@ -43,15 +43,10 @@ class Connector;
 //
 class ShellConnection : public shell::mojom::ShellClient {
  public:
-  // Creates a new ShellConnection to eventually be bound to a
-  // ShellClientRequest (see BindToRequest()). This connection may be used
-  // immediately to begin making outgoing connections via connector().
-  //
-  // Does not take ownership of |client|, which must remain valid for the
-  // lifetime of ShellConnection.
-  explicit ShellConnection(mojo::ShellClient* client);
-
-  // Like above but binds to |request| upon construction.
+  // Creates a new ShellConnection bound to |request|. This connection may be
+  // used immediately to make outgoing connections via connector().  Does not
+  // take ownership of |client|, which must remain valid for the lifetime of
+  // ShellConnection.
   ShellConnection(mojo::ShellClient* client,
                   shell::mojom::ShellClientRequest request);
 
@@ -59,10 +54,6 @@ class ShellConnection : public shell::mojom::ShellClient {
 
   Connector* connector() { return connector_.get(); }
 
-  // Binds this ShellConnection to a client request if one was not available at
-  // construction time.
-  void BindToRequest(shell::mojom::ShellClientRequest request);
-
   // TODO(rockot): Remove this once we get rid of app tests.
   void SetAppTestConnectorForTesting(shell::mojom::ConnectorPtr connector);
 
diff --git a/mojo/shell/public/interfaces/connector.mojom b/mojo/shell/public/interfaces/connector.mojom
index 4dff63da..b48440a 100644
--- a/mojo/shell/public/interfaces/connector.mojom
+++ b/mojo/shell/public/interfaces/connector.mojom
@@ -18,7 +18,7 @@ enum ConnectResult {
   // The name or user id supplied was malformed, or the application specified
   // by |name| could not be loaded.
   INVALID_ARGUMENT,
-  
+
   // The connection was blocked by policy. Either connections to |name| are
   // forbidden from this app by the CapabilityFilter, or the application
   // attempted to connect using a user id other than its own,
@@ -69,12 +69,12 @@ interface PIDReceiver {
 // the process itself and provide the shell the pipes it needs to communicate
 // with it. When an instance of this struct is supplied to Connect(), the client
 // owns the lifetime of the child process, not the shell. The shell binds the
-// |shell_client_factory| pipe, and when it closes destroys the associated
-// instance but the process stays alive.
+// |shell_client| pipe, and when it closes destroys the associated  instance but
+// the process stays alive.
 struct ClientProcessConnection {
-  // Provides the shell the ability to bind a ShellClientRequest from the client
+  // Provides the shell the ability to bind a ShellClient from the client
   // process to the instance it creates.
-  handle<message_pipe> shell_client_factory;
+  handle<message_pipe> shell_client;
 
   // Allows the client process launcher to tell the shell the PID of the process
   // it created (the pid isn't supplied directly here as the process may not
diff --git a/mojo/shell/runner/child/BUILD.gn b/mojo/shell/runner/child/BUILD.gn
index e8c63bd..0276b3d 100644
--- a/mojo/shell/runner/child/BUILD.gn
+++ b/mojo/shell/runner/child/BUILD.gn
@@ -10,24 +10,6 @@ group("child") {
   testonly = true
   deps = [
     ":apptests",
-    ":lib",
-  ]
-}
-
-source_set("lib") {
-  sources = [
-    "runner_connection.cc",
-    "runner_connection.h",
-  ]
-
-  deps = [
-    "//base",
-    "//mojo/edk/system",
-    "//mojo/message_pump",
-    "//mojo/platform_handle:platform_handle_impl",
-    "//mojo/shell/public/cpp:sources",
-    "//mojo/shell/public/interfaces",
-    "//mojo/shell/runner/common",
   ]
 }
 
@@ -46,7 +28,7 @@ source_set("test_native_main") {
     "//mojo/edk/system",
     "//mojo/message_pump",
     "//mojo/shell/public/cpp",
-    "//mojo/shell/runner/child:lib",
+    "//mojo/shell/runner/common",
   ]
 }
 
@@ -100,7 +82,6 @@ executable("native_target") {
 
   deps = [
     ":apptest_interfaces",
-    ":lib",
     ":test_native_main",
     "//base",
     "//build/config/sanitizers:deps",
diff --git a/mojo/shell/runner/child/runner_connection.cc b/mojo/shell/runner/child/runner_connection.cc
deleted file mode 100644
index f1478d5..0000000
--- a/mojo/shell/runner/child/runner_connection.cc
+++ /dev/null
@@ -1,86 +0,0 @@
-// Copyright 2015 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/child/runner_connection.h"
-
-#include <stdint.h>
-
-#include <utility>
-
-#include "base/bind.h"
-#include "base/callback.h"
-#include "base/command_line.h"
-#include "base/macros.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-#include "mojo/edk/embedder/embedder.h"
-#include "mojo/edk/embedder/platform_channel_pair.h"
-#include "mojo/edk/embedder/scoped_platform_handle.h"
-#include "mojo/public/cpp/bindings/binding.h"
-#include "mojo/shell/public/interfaces/shell_client_factory.mojom.h"
-#include "mojo/shell/runner/common/switches.h"
-
-namespace mojo {
-namespace shell {
-namespace {
-
-class RunnerConnectionImpl : public RunnerConnection,
-                             public mojom::ShellClientFactory {
- public:
-  RunnerConnectionImpl(mojo::ShellConnection* shell_connetion,
-                       mojom::ShellClientFactoryRequest client_factory_request,
-                       bool exit_on_error)
-      : shell_connection_(shell_connetion),
-        binding_(this, std::move(client_factory_request)),
-        exit_on_error_(exit_on_error) {
-    binding_.set_connection_error_handler([this] { OnConnectionError(); });
-  }
-
-  ~RunnerConnectionImpl() override {}
-
- private:
-  // mojom::ShellClientFactory:
-  void CreateShellClient(mojom::ShellClientRequest client_request,
-                         const String& name) override {
-    shell_connection_->BindToRequest(std::move(client_request));
-  }
-
-  void OnConnectionError() {
-    // A connection error means the connection to the shell is lost. This is not
-    // recoverable.
-    DLOG(ERROR) << "Connection error to the shell.";
-    if (exit_on_error_)
-      _exit(1);
-  }
-
-  mojo::ShellConnection* const shell_connection_;
-  Binding<mojom::ShellClientFactory> binding_;
-  const bool exit_on_error_;
-
-  DISALLOW_COPY_AND_ASSIGN(RunnerConnectionImpl);
-};
-
-}  // namespace
-
-RunnerConnection::~RunnerConnection() {}
-
-// static
-scoped_ptr<RunnerConnection> RunnerConnection::Create(
-    mojo::ShellConnection* shell_connection,
-    bool exit_on_error) {
-  std::string primordial_pipe_token =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          switches::kPrimordialPipeToken);
-  mojom::ShellClientFactoryRequest client_factory_request;
-  client_factory_request.Bind(
-      edk::CreateChildMessagePipe(primordial_pipe_token));
-
-  return make_scoped_ptr(new RunnerConnectionImpl(
-      shell_connection, std::move(client_factory_request), exit_on_error));
-}
-
-RunnerConnection::RunnerConnection() {}
-
-}  // namespace shell
-}  // namespace mojo
diff --git a/mojo/shell/runner/child/runner_connection.h b/mojo/shell/runner/child/runner_connection.h
deleted file mode 100644
index e1829c1..0000000
--- a/mojo/shell/runner/child/runner_connection.h
+++ /dev/null
@@ -1,55 +0,0 @@
-// Copyright 2015 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.
-
-#ifndef MOJO_SHELL_RUNNER_CHILD_RUNNER_CONNECTION_H_
-#define MOJO_SHELL_RUNNER_CHILD_RUNNER_CONNECTION_H_
-
-#include "base/callback.h"
-#include "base/macros.h"
-#include "base/memory/scoped_ptr.h"
-#include "mojo/shell/public/cpp/shell_connection.h"
-#include "mojo/shell/public/interfaces/connector.mojom.h"
-#include "mojo/shell/public/interfaces/shell_client.mojom.h"
-#include "mojo/shell/public/interfaces/shell_client_factory.mojom.h"
-
-namespace mojo {
-namespace shell {
-
-// Encapsulates a connection to a runner process.
-class RunnerConnection {
- public:
-  virtual ~RunnerConnection();
-
-  // Establishes a new runner connection using a ShellClientFactory request pipe
-  // from the command line.
-  //
-  // |shell_connection| is a ShellConnection which will be used to service
-  // incoming connection requests from other clients. This may be null, in which
-  // case no incoming connection requests will be serviced. This is not owned
-  // and must out-live the RunnerConnection.
-  //
-  // If |exit_on_error| is true, the calling process will be terminated in the
-  // event of an error on the ShellClientFactory pipe.
-  //
-  // The RunnerConnection returned by this call bounds the lifetime of the
-  // connection. If connection fails (which can happen if there is no
-  // ShellClientFactory request pipe on the command line) this returns null.
-  //
-  // TODO(rockot): Remove |exit_on_error|. We shouldn't be killing processes
-  // abruptly on pipe errors, but some tests currently depend on this behavior.
-  static scoped_ptr<RunnerConnection> Create(
-      mojo::ShellConnection* shell_connection,
-      bool exit_on_error = true);
-
- protected:
-  RunnerConnection();
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(RunnerConnection);
-};
-
-}  // namespace shell
-}  // namespace mojo
-
-#endif  // MOJO_SHELL_RUNNER_CHILD_RUNNER_CONNECTION_H_
diff --git a/mojo/shell/runner/child/test_native_main.cc b/mojo/shell/runner/child/test_native_main.cc
index 4caddc3..cc54215 100644
--- a/mojo/shell/runner/child/test_native_main.cc
+++ b/mojo/shell/runner/child/test_native_main.cc
@@ -16,7 +16,7 @@
 #include "mojo/edk/embedder/process_delegate.h"
 #include "mojo/shell/public/cpp/shell_client.h"
 #include "mojo/shell/public/cpp/shell_connection.h"
-#include "mojo/shell/runner/child/runner_connection.h"
+#include "mojo/shell/runner/common/client_util.h"
 #include "mojo/shell/runner/init.h"
 
 namespace mojo {
@@ -58,9 +58,8 @@ int TestNativeMain(mojo::ShellClient* shell_client) {
     mojo::edk::SetParentPipeHandleFromCommandLine();
 
     base::MessageLoop loop;
-    mojo::ShellConnection impl(shell_client);;
-    scoped_ptr<mojo::shell::RunnerConnection> connection =
-        mojo::shell::RunnerConnection::Create(&impl);
+    mojo::ShellConnection impl(
+        shell_client, mojo::shell::GetShellClientRequestFromCommandLine());
     loop.Run();
 
     mojo::edk::ShutdownIPCSupport();
diff --git a/mojo/shell/runner/common/BUILD.gn b/mojo/shell/runner/common/BUILD.gn
index 2fe8f65..f780070 100644
--- a/mojo/shell/runner/common/BUILD.gn
+++ b/mojo/shell/runner/common/BUILD.gn
@@ -4,7 +4,20 @@
 
 source_set("common") {
   sources = [
+    "client_util.cc",
+    "client_util.h",
     "switches.cc",
     "switches.h",
   ]
+
+  deps = [
+    "//base",
+    "//mojo/edk/system",
+    "//mojo/public/cpp/bindings",
+    "//mojo/public/cpp/system",
+  ]
+
+  public_deps = [
+    "//mojo/shell/public/interfaces",
+  ]
 }
diff --git a/mojo/shell/runner/common/client_util.cc b/mojo/shell/runner/common/client_util.cc
new file mode 100644
index 0000000..2c264bd
--- /dev/null
+++ b/mojo/shell/runner/common/client_util.cc
@@ -0,0 +1,38 @@
+// Copyright 2016 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/common/client_util.h"
+
+#include <string>
+
+#include "base/command_line.h"
+#include "mojo/edk/embedder/embedder.h"
+#include "mojo/shell/runner/common/switches.h"
+
+namespace mojo {
+namespace shell {
+
+mojom::ShellClientPtr PassShellClientRequestOnCommandLine(
+    base::CommandLine* command_line) {
+  std::string token = edk::GenerateRandomToken();
+  command_line->AppendSwitchASCII(switches::kPrimordialPipeToken, token);
+
+  mojom::ShellClientPtr client;
+  client.Bind(
+      mojom::ShellClientPtrInfo(edk::CreateParentMessagePipe(token), 0));
+  return client;
+}
+
+mojom::ShellClientRequest GetShellClientRequestFromCommandLine() {
+  std::string token =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kPrimordialPipeToken);
+  mojom::ShellClientRequest request;
+  if (!token.empty())
+    request.Bind(edk::CreateChildMessagePipe(token));
+  return request;
+}
+
+}  // namespace shell
+}  // namespace mojo
diff --git a/mojo/shell/runner/common/client_util.h b/mojo/shell/runner/common/client_util.h
new file mode 100644
index 0000000..ac61c77
--- /dev/null
+++ b/mojo/shell/runner/common/client_util.h
@@ -0,0 +1,32 @@
+// Copyright 2016 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.
+
+#ifndef MOJO_SHELL_RUNNER_COMMON_CLIENT_UTIL_H_
+#define MOJO_SHELL_RUNNER_COMMON_CLIENT_UTIL_H_
+
+#include "mojo/shell/public/interfaces/shell_client.mojom.h"
+
+namespace base {
+class CommandLine;
+}
+
+namespace mojo {
+namespace shell {
+
+// Creates a new ShellClient pipe and returns one end of it. The other end is
+// passed via a token in |command_line|. A child of the calling process may
+// extract a ShellClientRequest from this by calling
+// GetShellClientRequestFromCommandLine().
+mojom::ShellClientPtr PassShellClientRequestOnCommandLine(
+    base::CommandLine* command_line);
+
+// Extracts a ShellClientRequest from the command line of the current process.
+// The parent of this process should have passed a request using
+// PassShellClientRequestOnCommandLine().
+mojom::ShellClientRequest GetShellClientRequestFromCommandLine();
+
+}  // namespace shell
+}  // namespace mojo
+
+#endif  // MOJO_SHELL_RUNNER_COMMON_CLIENT_UTIL_H_
diff --git a/mojo/shell/runner/host/child_process_base.cc b/mojo/shell/runner/host/child_process_base.cc
index 538cf7d..e6b9a25 100644
--- a/mojo/shell/runner/host/child_process_base.cc
+++ b/mojo/shell/runner/host/child_process_base.cc
@@ -4,99 +4,28 @@
 
 #include "mojo/shell/runner/host/child_process_base.h"
 
-#include <stdint.h>
-
-#include <utility>
-
-#include "base/base_switches.h"
-#include "base/bind.h"
-#include "base/callback_helpers.h"
-#include "base/command_line.h"
-#include "base/debug/stack_trace.h"
-#include "base/files/file_path.h"
-#include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/waitable_event.h"
-#include "base/thread_task_runner_handle.h"
 #include "base/threading/thread.h"
-#include "base/threading/thread_checker.h"
 #include "mojo/edk/embedder/embedder.h"
-#include "mojo/edk/embedder/platform_channel_pair.h"
 #include "mojo/edk/embedder/process_delegate.h"
-#include "mojo/edk/embedder/scoped_platform_handle.h"
-#include "mojo/message_pump/message_pump_mojo.h"
-#include "mojo/public/cpp/bindings/binding.h"
-#include "mojo/public/cpp/system/core.h"
-#include "mojo/shell/public/interfaces/shell_client_factory.mojom.h"
-#include "mojo/shell/runner/common/switches.h"
-#include "mojo/shell/runner/init.h"
+#include "mojo/shell/runner/common/client_util.h"
 
 namespace mojo {
 namespace shell {
 
 namespace {
 
-// Blocker ---------------------------------------------------------------------
-
-// Blocks a thread until another thread unblocks it, at which point it unblocks
-// and runs a closure provided by that thread.
-class Blocker {
+// Should be created and initialized on the main thread and kept alive as long
+// a Mojo application is running in the current process.
+class ScopedAppContext : public edk::ProcessDelegate {
  public:
-  class Unblocker {
-   public:
-    explicit Unblocker(Blocker* blocker = nullptr) : blocker_(blocker) {}
-    ~Unblocker() {}
-
-    void Unblock(base::Closure run_after) {
-      DCHECK(blocker_);
-      DCHECK(blocker_->run_after_.is_null());
-      blocker_->run_after_ = run_after;
-      blocker_->event_.Signal();
-      blocker_ = nullptr;
-    }
-
-   private:
-    Blocker* blocker_;
-
-    // Copy and assign allowed.
-  };
-
-  Blocker() : event_(true, false) {}
-  ~Blocker() {}
-
-  void Block() {
-    DCHECK(run_after_.is_null());
-    event_.Wait();
-    if (!run_after_.is_null())
-      run_after_.Run();
-  }
-
-  Unblocker GetUnblocker() { return Unblocker(this); }
-
- private:
-  base::WaitableEvent event_;
-  base::Closure run_after_;
-
-  DISALLOW_COPY_AND_ASSIGN(Blocker);
-};
-
-// AppContext ------------------------------------------------------------------
-
-class ChildControllerImpl;
-
-// Should be created and initialized on the main thread.
-class AppContext : public edk::ProcessDelegate {
- public:
-  AppContext()
-      : io_thread_("io_thread"), controller_thread_("controller_thread") {}
-  ~AppContext() override {}
-
-  void Init() {
+  ScopedAppContext()
+      : io_thread_("io_thread"), wait_for_shutdown_event_(true, false) {
     // Initialize Mojo before starting any threads.
     edk::Init();
 
@@ -106,170 +35,37 @@ class AppContext : public edk::ProcessDelegate {
     io_runner_ = io_thread_.task_runner().get();
     CHECK(io_runner_.get());
 
-    // This must be created before controller_thread_ since MessagePumpMojo will
-    // create a message pipe which requires this code to be run first.
     edk::InitIPCSupport(this, io_runner_);
+    edk::SetParentPipeHandleFromCommandLine();
   }
 
-  void StartControllerThread() {
-    // Create and start our controller thread.
-    base::Thread::Options controller_thread_options;
-    controller_thread_options.message_loop_type =
-        base::MessageLoop::TYPE_CUSTOM;
-    controller_thread_options.message_pump_factory =
-        base::Bind(&common::MessagePumpMojo::Create);
-    CHECK(controller_thread_.StartWithOptions(controller_thread_options));
-    controller_runner_ = controller_thread_.task_runner().get();
-    CHECK(controller_runner_.get());
-  }
-
-  void Shutdown() {
-    Blocker blocker;
-    shutdown_unblocker_ = blocker.GetUnblocker();
-    controller_runner_->PostTask(
-        FROM_HERE, base::Bind(&AppContext::ShutdownOnControllerThread,
-                              base::Unretained(this)));
-    blocker.Block();
-  }
-
-  base::SingleThreadTaskRunner* io_runner() const { return io_runner_.get(); }
-
-  base::SingleThreadTaskRunner* controller_runner() const {
-    return controller_runner_.get();
-  }
-
-  ChildControllerImpl* controller() const { return controller_.get(); }
-
-  void set_controller(scoped_ptr<ChildControllerImpl> controller) {
-    controller_ = std::move(controller);
-  }
-
- private:
-  void ShutdownOnControllerThread() {
-    // First, destroy the controller.
-    controller_.reset();
-
-    // Next shutdown IPC. We'll unblock the main thread in OnShutdownComplete().
+  ~ScopedAppContext() override {
     edk::ShutdownIPCSupport();
+    wait_for_shutdown_event_.Wait();
   }
 
+ private:
   // ProcessDelegate implementation.
   void OnShutdownComplete() override {
-    shutdown_unblocker_.Unblock(base::Closure());
+    wait_for_shutdown_event_.Signal();
   }
 
   base::Thread io_thread_;
   scoped_refptr<base::SingleThreadTaskRunner> io_runner_;
 
-  base::Thread controller_thread_;
-  scoped_refptr<base::SingleThreadTaskRunner> controller_runner_;
-
-  // Accessed only on the controller thread.
-  scoped_ptr<ChildControllerImpl> controller_;
-
   // Used to unblock the main thread on shutdown.
-  Blocker::Unblocker shutdown_unblocker_;
+  base::WaitableEvent wait_for_shutdown_event_;
 
-  DISALLOW_COPY_AND_ASSIGN(AppContext);
+  DISALLOW_COPY_AND_ASSIGN(ScopedAppContext);
 };
 
-// ChildControllerImpl ------------------------------------------------------
-
-class ChildControllerImpl : public mojom::ShellClientFactory {
- public:
-  ~ChildControllerImpl() override {
-    DCHECK(thread_checker_.CalledOnValidThread());
-  }
-
-  // To be executed on the controller thread. Creates the |ChildController|,
-  // etc.
-  static void Init(AppContext* app_context,
-                   const RunCallback& run_callback,
-                   ScopedMessagePipeHandle host_message_pipe,
-                   const Blocker::Unblocker& unblocker) {
-    DCHECK(app_context);
-    DCHECK(host_message_pipe.is_valid());
-
-    DCHECK(!app_context->controller());
-
-    scoped_ptr<ChildControllerImpl> impl(
-        new ChildControllerImpl(app_context, run_callback, unblocker));
-
-    impl->Bind(std::move(host_message_pipe));
-
-    app_context->set_controller(std::move(impl));
-  }
-
-  void Bind(ScopedMessagePipeHandle handle) {
-    binding_.Bind(std::move(handle));
-    binding_.set_connection_error_handler([this]() { OnConnectionError(); });
-  }
-
-  void OnConnectionError() {
-    // A connection error means the connection to the shell is lost. This is not
-    // recoverable.
-    LOG(ERROR) << "Connection error to the shell.";
-    _exit(1);
-  }
-
-  // |mojom::ShellClientFactory| methods:
-  void CreateShellClient(mojom::ShellClientRequest request,
-                         const String& name) override {
-    DCHECK(thread_checker_.CalledOnValidThread());
-    unblocker_.Unblock(base::Bind(run_callback_, base::Passed(&request)));
-  }
-
- private:
-  ChildControllerImpl(AppContext* app_context,
-                      const RunCallback& run_callback,
-                      const Blocker::Unblocker& unblocker)
-      : run_callback_(run_callback), unblocker_(unblocker), binding_(this) {}
-
-  base::ThreadChecker thread_checker_;
-  RunCallback run_callback_;
-  Blocker::Unblocker unblocker_;
-
-  Binding<mojom::ShellClientFactory> binding_;
-
-  DISALLOW_COPY_AND_ASSIGN(ChildControllerImpl);
-};
-
-ScopedMessagePipeHandle InitializeHostMessagePipe(
-    edk::ScopedPlatformHandle platform_channel,
-    scoped_refptr<base::TaskRunner> io_task_runner) {
-  edk::SetParentPipeHandle(std::move(platform_channel));
-  std::string primordial_pipe_token =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          switches::kPrimordialPipeToken);
-  return edk::CreateChildMessagePipe(primordial_pipe_token);
-}
-
 }  // namespace
 
 void ChildProcessMain(const RunCallback& callback) {
-  const base::CommandLine& command_line =
-      *base::CommandLine::ForCurrentProcess();
-  edk::ScopedPlatformHandle platform_channel =
-      edk::PlatformChannelPair::PassClientHandleFromParentProcess(command_line);
-  CHECK(platform_channel.is_valid());
-
   DCHECK(!base::MessageLoop::current());
 
-  Blocker blocker;
-  AppContext app_context;
-  app_context.Init();
-  app_context.StartControllerThread();
-
-  ScopedMessagePipeHandle host_pipe = InitializeHostMessagePipe(
-      std::move(platform_channel), app_context.io_runner());
-  app_context.controller_runner()->PostTask(
-      FROM_HERE, base::Bind(&ChildControllerImpl::Init, &app_context, callback,
-                            base::Passed(&host_pipe), blocker.GetUnblocker()));
-
-  // This will block, then run whatever the controller wants.
-  blocker.Block();
-
-  app_context.Shutdown();
+  ScopedAppContext app_context;
+  callback.Run(GetShellClientRequestFromCommandLine());
 }
 
 }  // namespace shell
diff --git a/mojo/shell/runner/host/child_process_base.h b/mojo/shell/runner/host/child_process_base.h
index 49c5465..f2d23e5 100644
--- a/mojo/shell/runner/host/child_process_base.h
+++ b/mojo/shell/runner/host/child_process_base.h
@@ -6,7 +6,6 @@
 #define MOJO_SHELL_RUNNER_HOST_CHILD_PROCESS_BASE_H_
 
 #include "base/callback.h"
-#include "mojo/public/cpp/bindings/interface_request.h"
 #include "mojo/shell/public/interfaces/shell_client.mojom.h"
 
 namespace mojo {
diff --git a/mojo/shell/runner/host/child_process_host.cc b/mojo/shell/runner/host/child_process_host.cc
index 60a9947..1083128 100644
--- a/mojo/shell/runner/host/child_process_host.cc
+++ b/mojo/shell/runner/host/child_process_host.cc
@@ -22,6 +22,7 @@
 #include "mojo/public/cpp/bindings/interface_ptr_info.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/shell/native_runner_delegate.h"
+#include "mojo/shell/runner/common/client_util.h"
 #include "mojo/shell/runner/common/switches.h"
 
 #if defined(OS_LINUX) && !defined(OS_ANDROID)
@@ -47,47 +48,64 @@ ChildProcessHost::ChildProcessHost(base::TaskRunner* launch_process_runner,
       app_path_(app_path),
       start_child_process_event_(false, false),
       weak_factory_(this) {
-  node_channel_.reset(new edk::PlatformChannelPair);
-  primordial_pipe_token_ = edk::GenerateRandomToken();
-  factory_.Bind(InterfacePtrInfo<mojom::ShellClientFactory>(
-      edk::CreateParentMessagePipe(primordial_pipe_token_), 0u));
 }
 
 ChildProcessHost::~ChildProcessHost() {
-  if (!app_path_.empty())
-    CHECK(!factory_) << "Destroying ChildProcessHost before calling Join";
+  if (!app_path_.empty()) {
+    CHECK(!mojo_ipc_channel_)
+        << "Destroying ChildProcessHost before calling Join";
+  }
 }
 
-void ChildProcessHost::Start(mojom::ShellClientRequest request,
-                             const String& name,
-                             const ProcessReadyCallback& callback,
-                             const base::Closure& quit_closure) {
+mojom::ShellClientPtr ChildProcessHost::Start(
+    const String& name,
+    const ProcessReadyCallback& callback,
+    const base::Closure& quit_closure) {
   DCHECK(!child_process_.IsValid());
-  // Request is invalid in child_process_host_unittest.
-  if (request.is_pending()) {
-    factory_->CreateShellClient(std::move(request), name);
-    factory_.set_connection_error_handler(quit_closure);
+
+  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_;
   }
+
+  scoped_ptr<base::CommandLine> child_command_line(
+      new base::CommandLine(target_path));
+
+  child_command_line->AppendArguments(*parent_command_line, false);
+
+  if (target_path != app_path_)
+    child_command_line->AppendSwitchPath(switches::kChildProcess, app_path_);
+
+  if (start_sandboxed_)
+    child_command_line->AppendSwitch(switches::kEnableSandbox);
+
+  mojo_ipc_channel_.reset(new edk::PlatformChannelPair);
+  mojo_ipc_channel_->PrepareToPassClientHandleToChildProcess(
+      child_command_line.get(), &handle_passing_info_);
+
+  mojom::ShellClientPtr client =
+      PassShellClientRequestOnCommandLine(child_command_line.get());
   launch_process_runner_->PostTaskAndReply(
       FROM_HERE,
-      base::Bind(&ChildProcessHost::DoLaunch, base::Unretained(this)),
+      base::Bind(&ChildProcessHost::DoLaunch, base::Unretained(this),
+                 base::Passed(&child_command_line)),
       base::Bind(&ChildProcessHost::DidStart,
                  weak_factory_.GetWeakPtr(), callback));
+  return client;
 }
 
 void ChildProcessHost::Join() {
-  if (factory_)
+  if (mojo_ipc_channel_)
     start_child_process_event_.Wait();
-
-  factory_.reset();
-
-  // 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.
+  mojo_ipc_channel_.reset();
   if (child_process_.IsValid()) {
     int rv = -1;
     LOG_IF(ERROR, !child_process_.WaitForExit(&rv))
         << "Failed to wait for child process";
-
     child_process_.Close();
   }
 }
@@ -97,52 +115,24 @@ void ChildProcessHost::DidStart(const ProcessReadyCallback& callback) {
     callback.Run(child_process_.Pid());
   } else {
     LOG(ERROR) << "Failed to start child process";
-    factory_.reset();
+    mojo_ipc_channel_.reset();
   }
 }
 
-void ChildProcessHost::DoLaunch() {
-  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);
-  child_command_line.AppendArguments(*parent_command_line, false);
-
-  if (target_path != app_path_)
-    child_command_line.AppendSwitchPath(switches::kChildProcess, app_path_);
-
-  if (start_sandboxed_)
-    child_command_line.AppendSwitch(switches::kEnableSandbox);
-
-  if (node_channel_.get()) {
-    node_channel_->PrepareToPassClientHandleToChildProcess(
-        &child_command_line, &handle_passing_info_);
-  }
-
-  child_command_line.AppendSwitchASCII(switches::kPrimordialPipeToken,
-                                       primordial_pipe_token_);
-
+void ChildProcessHost::DoLaunch(
+    scoped_ptr<base::CommandLine> child_command_line) {
   if (delegate_) {
     delegate_->AdjustCommandLineArgumentsForTarget(target_,
-                                                   &child_command_line);
+                                                   child_command_line.get());
   }
 
   base::LaunchOptions options;
 #if defined(OS_WIN)
-  if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
-    options.handles_to_inherit = &handle_passing_info_;
-  } else {
+  options.handles_to_inherit = &handle_passing_info_;
 #if defined(OFFICIAL_BUILD)
-    CHECK(false) << "Launching mojo process with inherit_handles is insecure!";
+  CHECK(false) << "Launching mojo process with inherit_handles is insecure!";
 #endif
-    options.inherit_handles = true;
-  }
+  options.inherit_handles = true;
   options.stdin_handle = INVALID_HANDLE_VALUE;
   options.stdout_handle = GetStdHandle(STD_OUTPUT_HANDLE);
   options.stderr_handle = GetStdHandle(STD_ERROR_HANDLE);
@@ -173,27 +163,26 @@ void ChildProcessHost::DoLaunch() {
   options.fds_to_remap = &handle_passing_info_;
 #endif
   DVLOG(2) << "Launching child with command line: "
-           << child_command_line.GetCommandLineString();
+           << child_command_line->GetCommandLineString();
 #if defined(OS_LINUX) && !defined(OS_ANDROID)
   if (start_sandboxed_) {
     child_process_ =
-        sandbox::NamespaceSandbox::LaunchProcess(child_command_line, options);
+        sandbox::NamespaceSandbox::LaunchProcess(*child_command_line, options);
     if (!child_process_.IsValid()) {
       LOG(ERROR) << "Starting the process with a sandbox failed. Missing kernel"
                  << " support.";
     }
   } else
 #endif
-    child_process_ = base::LaunchProcess(child_command_line, options);
+    child_process_ = base::LaunchProcess(*child_command_line, options);
 
   if (child_process_.IsValid()) {
-    platform_channel_pair_.ChildProcessLaunched();
-    if (node_channel_.get()) {
-      node_channel_->ChildProcessLaunched();
+    if (mojo_ipc_channel_.get()) {
+      mojo_ipc_channel_->ChildProcessLaunched();
       mojo::edk::ChildProcessLaunched(
           child_process_.Handle(),
           mojo::edk::ScopedPlatformHandle(mojo::edk::PlatformHandle(
-              node_channel_->PassServerHandle().release().handle)));
+              mojo_ipc_channel_->PassServerHandle().release().handle)));
     }
   }
   start_child_process_event_.Signal();
diff --git a/mojo/shell/runner/host/child_process_host.h b/mojo/shell/runner/host/child_process_host.h
index 6c6411d..c13251a 100644
--- a/mojo/shell/runner/host/child_process_host.h
+++ b/mojo/shell/runner/host/child_process_host.h
@@ -10,9 +10,11 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/process/process.h"
 #include "base/synchronization/lock.h"
@@ -59,10 +61,9 @@ class ChildProcessHost {
 
   // |Start()|s the child process; calls |DidStart()| (on the thread on which
   // |Start()| was called) when the child has been started (or failed to start).
-  void Start(mojom::ShellClientRequest request,
-             const String& name,
-             const ProcessReadyCallback& callback,
-             const base::Closure& quit_closure);
+  mojom::ShellClientPtr Start(const String& name,
+                              const ProcessReadyCallback& callback,
+                              const base::Closure& quit_closure);
 
   // Waits for the child process to terminate.
   void Join();
@@ -71,7 +72,7 @@ class ChildProcessHost {
   void DidStart(const ProcessReadyCallback& callback);
 
  private:
-  void DoLaunch();
+  void DoLaunch(scoped_ptr<base::CommandLine> child_command_line);
 
   scoped_refptr<base::TaskRunner> launch_process_runner_;
   NativeRunnerDelegate* delegate_ = nullptr;
@@ -79,21 +80,15 @@ class ChildProcessHost {
   Identity target_;
   const base::FilePath app_path_;
   base::Process child_process_;
-  // Used for the ShellClientFactory binding.
-  edk::PlatformChannelPair platform_channel_pair_;
-  mojom::ShellClientFactoryPtr factory_;
-  edk::HandlePassingInformation handle_passing_info_;
 
-  // Used to back the NodeChannel between the parent and child node.
-  scoped_ptr<edk::PlatformChannelPair> node_channel_;
+  // Used to initialize the Mojo IPC channel between parent and child.
+  scoped_ptr<edk::PlatformChannelPair> mojo_ipc_channel_;
+  edk::HandlePassingInformation handle_passing_info_;
 
   // Since Start() calls a method on another thread, we use an event to block
   // the main thread if it tries to destruct |this| while launching the process.
   base::WaitableEvent start_child_process_event_;
 
-  // A token the child can use to connect a primordial pipe to the host.
-  std::string primordial_pipe_token_;
-
   base::WeakPtrFactory<ChildProcessHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChildProcessHost);
diff --git a/mojo/shell/runner/host/child_process_host_unittest.cc b/mojo/shell/runner/host/child_process_host_unittest.cc
index 2f57bf6..593360d 100644
--- a/mojo/shell/runner/host/child_process_host_unittest.cc
+++ b/mojo/shell/runner/host/child_process_host_unittest.cc
@@ -96,7 +96,7 @@ TEST(ChildProcessHostTest, MAYBE_StartJoin) {
                                       Identity(), base::FilePath());
   base::RunLoop run_loop;
   child_process_host.Start(
-      mojom::ShellClientRequest(), String(),
+      String(),
       base::Bind(&ProcessReadyCallbackAdapater, run_loop.QuitClosure()),
       base::Bind(&base::DoNothing));
   run_loop.Run();
diff --git a/mojo/shell/runner/host/in_process_native_runner.cc b/mojo/shell/runner/host/in_process_native_runner.cc
index c006a79..823cf5f 100644
--- a/mojo/shell/runner/host/in_process_native_runner.cc
+++ b/mojo/shell/runner/host/in_process_native_runner.cc
@@ -13,6 +13,7 @@
 #include "base/task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/platform_thread.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
 #include "mojo/shell/runner/host/native_application_support.h"
 #include "mojo/shell/runner/host/out_of_process_native_runner.h"
 #include "mojo/shell/runner/init.h"
@@ -33,17 +34,17 @@ InProcessNativeRunner::~InProcessNativeRunner() {
   }
 }
 
-void InProcessNativeRunner::Start(
+mojom::ShellClientPtr InProcessNativeRunner::Start(
     const base::FilePath& app_path,
     const Identity& target,
     bool start_sandboxed,
-    mojom::ShellClientRequest request,
     const base::Callback<void(base::ProcessId)>& pid_available_callback,
     const base::Closure& app_completed_callback) {
   app_path_ = app_path;
 
   DCHECK(!request_.is_pending());
-  request_ = std::move(request);
+  mojom::ShellClientPtr client;
+  request_ = GetProxy(&client);
 
   DCHECK(app_completed_callback_runner_.is_null());
   app_completed_callback_runner_ = base::Bind(
@@ -58,6 +59,8 @@ void InProcessNativeRunner::Start(
   thread_.reset(new base::DelegateSimpleThread(this, thread_name));
   thread_->Start();
   pid_available_callback.Run(base::kNullProcessId);
+
+  return client;
 }
 
 void InProcessNativeRunner::Run() {
diff --git a/mojo/shell/runner/host/in_process_native_runner.h b/mojo/shell/runner/host/in_process_native_runner.h
index 9f63396..675ea8d 100644
--- a/mojo/shell/runner/host/in_process_native_runner.h
+++ b/mojo/shell/runner/host/in_process_native_runner.h
@@ -30,11 +30,10 @@ class InProcessNativeRunner : public NativeRunner,
   ~InProcessNativeRunner() override;
 
   // NativeRunner:
-  void Start(
+  mojom::ShellClientPtr Start(
       const base::FilePath& app_path,
       const Identity& target,
       bool start_sandboxed,
-      InterfaceRequest<mojom::ShellClient> request,
       const base::Callback<void(base::ProcessId)>& pid_available_callback,
       const base::Closure& app_completed_callback) override;
 
diff --git a/mojo/shell/runner/host/out_of_process_native_runner.cc b/mojo/shell/runner/host/out_of_process_native_runner.cc
index 619bb1d..0bf38d5 100644
--- a/mojo/shell/runner/host/out_of_process_native_runner.cc
+++ b/mojo/shell/runner/host/out_of_process_native_runner.cc
@@ -13,6 +13,7 @@
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/task_runner.h"
+#include "mojo/shell/runner/common/client_util.h"
 #include "mojo/shell/runner/host/child_process_host.h"
 #include "mojo/shell/runner/host/in_process_native_runner.h"
 
@@ -29,11 +30,10 @@ OutOfProcessNativeRunner::~OutOfProcessNativeRunner() {
     child_process_host_->Join();
 }
 
-void OutOfProcessNativeRunner::Start(
+mojom::ShellClientPtr OutOfProcessNativeRunner::Start(
     const base::FilePath& app_path,
     const Identity& target,
     bool start_sandboxed,
-    mojom::ShellClientRequest request,
     const base::Callback<void(base::ProcessId)>& pid_available_callback,
     const base::Closure& app_completed_callback) {
   app_path_ = app_path;
@@ -43,10 +43,10 @@ void OutOfProcessNativeRunner::Start(
 
   child_process_host_.reset(new ChildProcessHost(
       launch_process_runner_, delegate_, start_sandboxed, target, app_path));
-  child_process_host_->Start(std::move(request), target.name(),
-                             pid_available_callback,
-                             base::Bind(&OutOfProcessNativeRunner::AppCompleted,
-                                        base::Unretained(this)));
+  return child_process_host_->Start(
+      target.name(), pid_available_callback,
+      base::Bind(&OutOfProcessNativeRunner::AppCompleted,
+                 base::Unretained(this)));
 }
 
 void OutOfProcessNativeRunner::AppCompleted() {
diff --git a/mojo/shell/runner/host/out_of_process_native_runner.h b/mojo/shell/runner/host/out_of_process_native_runner.h
index d56fd6c..45d7cf8 100644
--- a/mojo/shell/runner/host/out_of_process_native_runner.h
+++ b/mojo/shell/runner/host/out_of_process_native_runner.h
@@ -7,12 +7,13 @@
 
 #include <stdint.h>
 
+#include <string>
+
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "mojo/shell/native_runner.h"
-#include "mojo/shell/public/interfaces/shell_client_factory.mojom.h"
 
 namespace base {
 class TaskRunner;
@@ -33,11 +34,10 @@ class OutOfProcessNativeRunner : public NativeRunner {
   ~OutOfProcessNativeRunner() override;
 
   // NativeRunner:
-  void Start(
+  mojom::ShellClientPtr Start(
       const base::FilePath& app_path,
       const Identity& identity,
       bool start_sandboxed,
-      mojom::ShellClientRequest request,
       const base::Callback<void(base::ProcessId)>& pid_available_callback,
       const base::Closure& app_completed_callback) override;
 
diff --git a/mojo/shell/shell.cc b/mojo/shell/shell.cc
index 48370a3..beb0fb7b 100644
--- a/mojo/shell/shell.cc
+++ b/mojo/shell/shell.cc
@@ -104,8 +104,7 @@ class Shell::Instance : public mojom::Connector,
                         public InterfaceFactory<mojom::Shell>,
                         public mojom::Shell {
  public:
-  Instance(mojom::ShellClientPtr shell_client,
-           mojo::shell::Shell* shell,
+  Instance(mojo::shell::Shell* shell,
            const Identity& identity,
            const CapabilitySpec& capability_spec)
     : shell_(shell),
@@ -113,7 +112,6 @@ class Shell::Instance : public mojom::Connector,
       identity_(identity),
       capability_spec_(capability_spec),
       allow_any_application_(capability_spec.required.count("*") == 1),
-      shell_client_(std::move(shell_client)),
       pid_receiver_binding_(this),
       weak_factory_(this) {
     if (identity_.name() == kShellName ||
@@ -121,9 +119,6 @@ class Shell::Instance : public mojom::Connector,
       pid_ = base::Process::Current().Pid();
     }
     DCHECK_NE(mojom::kInvalidInstanceID, id_);
-
-    shell_client_.set_connection_error_handler(
-        base::Bind(&Instance::OnShellClientLost, base::Unretained(this)));
   }
 
   ~Instance() override {}
@@ -150,13 +145,8 @@ class Shell::Instance : public mojom::Connector,
     }
   }
 
-  void InitializeClient() {
-    shell_client_->Initialize(mojom::Identity::From(identity_), id_,
-                              base::Bind(&Instance::OnInitializeResponse,
-                                         base::Unretained(this)));
-  }
-
   void ConnectToClient(scoped_ptr<ConnectParams> params) {
+    CHECK(shell_client_.is_bound());
     params->connect_callback().Run(mojom::ConnectResult::SUCCEEDED,
                                    identity_.user_id(), id_);
     uint32_t source_id = mojom::kInvalidInstanceID;
@@ -174,28 +164,38 @@ class Shell::Instance : public mojom::Connector,
         mojom::CapabilityRequest::From(spec), params->target().name());
   }
 
+  void StartWithClient(mojom::ShellClientPtr client) {
+    CHECK(!shell_client_);
+    shell_client_ = std::move(client);
+    shell_client_.set_connection_error_handler(
+        base::Bind(&Instance::OnShellClientLost, base::Unretained(this)));
+    shell_client_->Initialize(mojom::Identity::From(identity_), id_,
+                              base::Bind(&Instance::OnInitializeResponse,
+                                         base::Unretained(this)));
+  }
+
   void StartWithClientProcessConnection(
-      mojom::ShellClientRequest request,
       mojom::ClientProcessConnectionPtr client_process_connection) {
-    factory_.Bind(mojom::ShellClientFactoryPtrInfo(
-        std::move(client_process_connection->shell_client_factory), 0u));
+    mojom::ShellClientPtr client;
+    client.Bind(mojom::ShellClientPtrInfo(
+        std::move(client_process_connection->shell_client), 0));
     pid_receiver_binding_.Bind(
         std::move(client_process_connection->pid_receiver_request));
-    factory_->CreateShellClient(std::move(request), identity_.name());
+    StartWithClient(std::move(client));
   }
 
-  void StartWithFilePath(mojom::ShellClientRequest request,
-                         const base::FilePath& path) {
+  void StartWithFilePath(const base::FilePath& path) {
+    CHECK(!shell_client_);
     scoped_ptr<NativeRunner> runner =
         shell_->native_runner_factory_->Create(path);
     bool start_sandboxed = false;
-    runner->Start(path, identity_, start_sandboxed, std::move(request),
-                  base::Bind(&Instance::PIDAvailable,
-                             weak_factory_.GetWeakPtr()),
-                  base::Bind(&mojo::shell::Shell::CleanupRunner,
-                             shell_->weak_ptr_factory_.GetWeakPtr(),
-                             runner.get()));
+    mojom::ShellClientPtr client = runner->Start(
+        path, identity_, start_sandboxed,
+        base::Bind(&Instance::PIDAvailable, weak_factory_.GetWeakPtr()),
+        base::Bind(&mojo::shell::Shell::CleanupRunner,
+                   shell_->weak_ptr_factory_.GetWeakPtr(), runner.get()));
     shell_->native_runners_.push_back(std::move(runner));
+    StartWithClient(std::move(client));
   }
 
   mojom::InstanceInfoPtr CreateInstanceInfo() const {
@@ -299,9 +299,9 @@ class Shell::Instance : public mojom::Connector,
         return false;
       }
 
-      if (!(*client_process_connection)->shell_client_factory.is_valid() ||
+      if (!(*client_process_connection)->shell_client.is_valid() ||
           !(*client_process_connection)->pid_receiver_request.is_valid()) {
-        LOG(ERROR) << "Error: must supply both shell_client_factory AND "
+        LOG(ERROR) << "Error: must supply both shell_client AND "
                    << "pid_receiver_request when sending "
                    << "client_process_connection.";
         callback.Run(mojom::ConnectResult::INVALID_ARGUMENT,
@@ -393,7 +393,6 @@ class Shell::Instance : public mojom::Connector,
   Binding<mojom::PIDReceiver> pid_receiver_binding_;
   BindingSet<mojom::Connector> connectors_;
   BindingSet<mojom::Shell> shell_bindings_;
-  mojom::ShellClientFactoryPtr factory_;
   NativeRunner* runner_ = nullptr;
   base::ProcessId pid_ = base::kNullProcessId;
   base::WeakPtrFactory<Instance> weak_factory_;
@@ -422,8 +421,9 @@ Shell::Shell(scoped_ptr<NativeRunnerFactory> native_runner_factory,
       weak_ptr_factory_(this) {
   mojom::ShellClientPtr client;
   mojom::ShellClientRequest request = GetProxy(&client);
-  CreateInstance(CreateShellIdentity(), GetPermissiveCapabilities(),
-                 std::move(client));
+  Instance* instance = CreateInstance(CreateShellIdentity(),
+                                      GetPermissiveCapabilities());
+  instance->StartWithClient(std::move(client));
   shell_connection_.reset(new ShellConnection(this, std::move(request)));
 
   if (catalog)
@@ -492,7 +492,9 @@ bool Shell::AcceptConnection(Connection* connection) {
 // Shell, private:
 
 void Shell::InitCatalog(mojom::ShellClientPtr catalog) {
-  CreateInstance(CreateCatalogIdentity(), CapabilitySpec(), std::move(catalog));
+  Instance* instance =
+      CreateInstance(CreateCatalogIdentity(), CapabilitySpec());
+  instance->StartWithClient(std::move(catalog));
 
   // TODO(beng): this doesn't work anymore.
   // Seed the catalog with manifest info for the shell & catalog.
@@ -582,10 +584,9 @@ bool Shell::ConnectToExistingInstance(scoped_ptr<ConnectParams>* params) {
 }
 
 Shell::Instance* Shell::CreateInstance(const Identity& target,
-                                       const CapabilitySpec& spec,
-                                       mojom::ShellClientPtr client) {
+                                       const CapabilitySpec& spec) {
   CHECK(target.user_id() != mojom::kInheritUserID);
-  Instance* instance = new Instance(std::move(client), this, target, spec);
+  Instance* instance = new Instance(this, target, spec);
   DCHECK(identity_to_instance_.find(target) ==
          identity_to_instance_.end());
   identity_to_instance_[target] = instance;
@@ -594,7 +595,6 @@ Shell::Instance* Shell::CreateInstance(const Identity& target,
       [this, &info](mojom::InstanceListener* listener) {
         listener->InstanceCreated(info.Clone());
       });
-  instance->InitializeClient();
   return instance;
 }
 
@@ -608,10 +608,10 @@ void Shell::AddInstanceListener(mojom::InstanceListenerPtr listener) {
   instance_listeners_.AddInterfacePtr(std::move(listener));
 }
 
-void Shell::CreateShellClient(const Identity& source,
-                              const Identity& shell_client_factory,
-                              const std::string& name,
-                              mojom::ShellClientRequest request) {
+void Shell::CreateShellClientWithFactory(const Identity& source,
+                                         const Identity& shell_client_factory,
+                                         const std::string& name,
+                                         mojom::ShellClientRequest request) {
   mojom::ShellClientFactory* factory =
       GetShellClientFactory(shell_client_factory, source);
   factory->CreateShellClient(std::move(request), name);
@@ -674,42 +674,40 @@ void Shell::OnGotResolvedName(mojom::ShellResolverPtr resolver,
 
   mojom::ClientProcessConnectionPtr client_process_connection =
       params->TakeClientProcessConnection();
-
-  mojom::ShellClientRequest request;
-  if (!client.is_bound())
-    request = GetProxy(&client);
-
-  Instance* instance = CreateInstance(target, capabilities, std::move(client));
-  instance->ConnectToClient(std::move(params));
-
-  // If a ShellClientPtr was provided, there's no more work to do: someone
-  // is already holding a corresponding ShellClientRequest.
-  if (!request.is_pending())
-    return;
-
-  if (client_process_connection.is_null() && LoadWithLoader(target, &request))
-    return;
-
-  CHECK(!file_url.is_null() && !capabilities_ptr.is_null());
-
-  if (target.name() != resolved_name) {
-    // In cases where a package alias is resolved, we have to use the instance
-    // from the original request rather than for the package itself, which will
-    // always be the same.
-    CreateShellClient(
-        source, Identity(resolved_name, target.user_id(), instance_name),
-        target.name(), std::move(request));
+  Instance* instance = CreateInstance(target, capabilities);
+
+  // Below are various paths through which a new Instance can be bound to a
+  // ShellClient proxy.
+  if (client.is_bound()) {
+    // If a ShellClientPtr was provided, there's no more work to do: someone
+    // is already holding a corresponding ShellClientRequest.
+    instance->StartWithClient(std::move(client));
+  } else if (!client_process_connection.is_null()) {
+    // Likewise if a ClientProcessConnection was given via Connect(), it
+    // provides the ShellClient proxy to use.
+    instance->StartWithClientProcessConnection(
+        std::move(client_process_connection));
   } else {
-    if (!client_process_connection.is_null()) {
-      // The client already started a process for this instance, use it.
-      instance->StartWithClientProcessConnection(
-          std::move(request), std::move(client_process_connection));
+    // Otherwise we create a new ShellClient pipe.
+    mojom::ShellClientRequest request = GetProxy(&client);
+    if (LoadWithLoader(target, &request)) {
+      instance->StartWithClient(std::move(client));
     } else {
-      // Otherwise we make our own process.
-      instance->StartWithFilePath(std::move(request),
-                                  util::UrlToFilePath(file_url.To<GURL>()));
+      CHECK(!file_url.is_null() && !capabilities_ptr.is_null());
+
+      if (target.name() != resolved_name) {
+        instance->StartWithClient(std::move(client));
+        CreateShellClientWithFactory(
+            source, Identity(resolved_name, target.user_id(), instance_name),
+            target.name(), std::move(request));
+      } else {
+        instance->StartWithFilePath(util::UrlToFilePath(file_url.To<GURL>()));
+      }
     }
   }
+
+  // Now that the instance has a ShellClient, we can connect to it.
+  instance->ConnectToClient(std::move(params));
 }
 
 bool Shell::LoadWithLoader(const Identity& target,
diff --git a/mojo/shell/shell.h b/mojo/shell/shell.h
index 03e9bfd..ca5e211 100644
--- a/mojo/shell/shell.h
+++ b/mojo/shell/shell.h
@@ -111,7 +111,7 @@ class Shell : public ShellClient {
   //
   // If |client| is not null, there must not be an instance of the target
   // application already running. The shell will create a new instance and use
-  // |client| to control.
+  // |client| to control it.
   void Connect(scoped_ptr<ConnectParams> params, mojom::ShellClientPtr client);
 
   // Returns a running instance matching |identity|.
@@ -128,22 +128,21 @@ class Shell : public ShellClient {
   bool ConnectToExistingInstance(scoped_ptr<ConnectParams>* params);
 
   Instance* CreateInstance(const Identity& target,
-                           const CapabilitySpec& spec,
-                           mojom::ShellClientPtr client);
+                           const CapabilitySpec& spec);
 
   // Called from the instance implementing mojom::Shell.
   void AddInstanceListener(mojom::InstanceListenerPtr listener);
 
-  void CreateShellClient(const Identity& source,
-                         const Identity& shell_client_factory,
-                         const std::string& name,
-                         mojom::ShellClientRequest request);
-  // Returns a running ShellClientFactory for |shell_client_factory_identity|,
-  // if there is not one running one is started for |source_identity|.
+  void CreateShellClientWithFactory(const Identity& source,
+                                    const Identity& shell_client_factory,
+                                    const std::string& name,
+                                    mojom::ShellClientRequest request);
+  // Returns a running ShellClientFactory for |shell_client_factory_identity|.
+  // If there is not one running one is started for |source_identity|.
   mojom::ShellClientFactory* GetShellClientFactory(
       const Identity& shell_client_factory_identity,
       const Identity& source_identity);
-  void OnShellClientFactoryLost(const Identity& which);;
+  void OnShellClientFactoryLost(const Identity& which);
 
   // Callback when remote Catalog resolves mojo:foo to mojo:bar.
   // |params| are the params passed to Connect().
@@ -181,7 +180,7 @@ class Shell : public ShellClient {
   IdentityToInstanceMap identity_to_instance_;
 
   IdentityToShellClientFactoryMap shell_client_factories_;
-  // Counter used to assign ids to content handlers.
+  // Counter used to assign ids to client factories.
   uint32_t shell_client_factory_id_counter_;
 
   InterfacePtrSet<mojom::InstanceListener> instance_listeners_;
diff --git a/mojo/shell/tests/connect/connect_test_driver.cc b/mojo/shell/tests/connect/connect_test_driver.cc
index 9e5a4cc..ace64df 100644
--- a/mojo/shell/tests/connect/connect_test_driver.cc
+++ b/mojo/shell/tests/connect/connect_test_driver.cc
@@ -37,6 +37,10 @@ class Driver : public mojo::ShellClient,
     connection->AddInterface<ClientProcessTest>(this);
     return true;
   }
+  void ShellConnectionLost() override {
+    // TODO: This should exit cleanly.
+    _exit(1);
+  }
 
   // mojo::InterfaceFactory<ConnectTestService>:
   void Create(mojo::Connection* connection,
diff --git a/mojo/shell/tests/shell/driver.cc b/mojo/shell/tests/shell/driver.cc
index 159920d..9d8af6b 100644
--- a/mojo/shell/tests/shell/driver.cc
+++ b/mojo/shell/tests/shell/driver.cc
@@ -29,6 +29,7 @@
 #include "mojo/shell/public/interfaces/connector.mojom.h"
 #include "mojo/shell/public/interfaces/shell.mojom.h"
 #include "mojo/shell/runner/child/test_native_main.h"
+#include "mojo/shell/runner/common/client_util.h"
 #include "mojo/shell/runner/common/switches.h"
 #include "mojo/shell/runner/init.h"
 #include "mojo/shell/tests/shell/shell_unittest.mojom.h"
@@ -71,25 +72,14 @@ class Driver : public mojo::ShellClient,
     platform_channel_pair.PrepareToPassClientHandleToChildProcess(
         &child_command_line, &handle_passing_info);
 
-    // Generate a token for the child to find and connect to a primordial pipe
-    // and pass that as well.
-    std::string primordial_pipe_token = mojo::edk::GenerateRandomToken();
-    child_command_line.AppendSwitchASCII(switches::kPrimordialPipeToken,
-                                         primordial_pipe_token);
-
-    // Allocate the pipe locally.
-    mojo::ScopedMessagePipeHandle pipe =
-        mojo::edk::CreateParentMessagePipe(primordial_pipe_token);
-
-    mojo::shell::mojom::ShellClientFactoryPtr factory;
-    factory.Bind(mojo::InterfacePtrInfo<mojo::shell::mojom::ShellClientFactory>(
-        std::move(pipe), 0u));
+    mojo::shell::mojom::ShellClientPtr client =
+        mojo::shell::PassShellClientRequestOnCommandLine(&child_command_line);
     mojo::shell::mojom::PIDReceiverPtr receiver;
 
     mojo::Identity target("exe:shell_unittest_target",
                           mojo::shell::mojom::kInheritUserID);
     mojo::Connector::ConnectParams params(target);
-    params.set_client_process_connection(std::move(factory),
+    params.set_client_process_connection(std::move(client),
                                          GetProxy(&receiver));
     scoped_ptr<mojo::Connection> connection = connector->Connect(&params);
     connection->AddConnectionCompletedClosure(
diff --git a/mojo/shell/tests/util.cc b/mojo/shell/tests/util.cc
index d57d311..8326ebe 100644
--- a/mojo/shell/tests/util.cc
+++ b/mojo/shell/tests/util.cc
@@ -66,13 +66,13 @@ scoped_ptr<Connection> LaunchAndConnectToProcess(
   mojo::ScopedMessagePipeHandle pipe =
       mojo::edk::CreateParentMessagePipe(primordial_pipe_token);
 
-  mojo::shell::mojom::ShellClientFactoryPtr factory;
-  factory.Bind(mojo::InterfacePtrInfo<mojo::shell::mojom::ShellClientFactory>(
+  mojo::shell::mojom::ShellClientPtr client;
+  client.Bind(mojo::InterfacePtrInfo<mojo::shell::mojom::ShellClient>(
       std::move(pipe), 0u));
   mojo::shell::mojom::PIDReceiverPtr receiver;
 
   mojo::Connector::ConnectParams params(target);
-  params.set_client_process_connection(std::move(factory), GetProxy(&receiver));
+  params.set_client_process_connection(std::move(client), GetProxy(&receiver));
   scoped_ptr<mojo::Connection> connection = connector->Connect(&params);
   {
     base::RunLoop loop;