One feature of MojoShellContext in content is that Connect() can be called from any thread. If we want to replace this class we need this ability in the Mojo Shell client lib too. To this end I've extracted the Connect() method to its own interface which can be constructed from Shell. This interface can be cloned and passed on to other threads also.

R=sky@chromium.org
BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

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

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

11 files changed, 247 insertions, 80 deletions

diff --git a/mojo/shell/public/cpp/BUILD.gn b/mojo/shell/public/cpp/BUILD.gn
index 2ebc220..1ad6892 100644
--- a/mojo/shell/public/cpp/BUILD.gn
+++ b/mojo/shell/public/cpp/BUILD.gn
@@ -23,6 +23,7 @@ source_set("sources") {
     "application_runner.h",
     "connect.h",
     "connection.h",
+    "connector.h",
     "initialize_base_and_icu.cc",
     "interface_binder.h",
     "interface_factory.h",
@@ -31,6 +32,8 @@ source_set("sources") {
     "lib/application_runner.cc",
     "lib/connection_impl.cc",
     "lib/connection_impl.h",
+    "lib/connector_impl.cc",
+    "lib/connector_impl.h",
     "lib/interface_factory_binder.h",
     "lib/interface_registry.cc",
     "lib/shell_client.cc",
diff --git a/mojo/shell/public/cpp/connector.h b/mojo/shell/public/cpp/connector.h
new file mode 100644
index 0000000..7385e18
--- /dev/null
+++ b/mojo/shell/public/cpp/connector.h
@@ -0,0 +1,88 @@
+// 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_PUBLIC_CPP_CONNECTOR_H_
+#define MOJO_SHELL_PUBLIC_CPP_CONNECTOR_H_
+
+#include "mojo/shell/public/cpp/connection.h"
+#include "mojo/shell/public/interfaces/shell.mojom.h"
+#include "url/gurl.h"
+
+namespace mojo {
+
+shell::mojom::CapabilityFilterPtr CreatePermissiveCapabilityFilter();
+
+// An interface that encapsulates the Mojo Shell's broker interface by which
+// connections between applications are established. Once Connect() is called,
+// this class is bound to the thread the call was made on and it cannot be
+// passed to another thread without calling Clone().
+// An instance of this class is created internally by ShellConnection for use
+// on the thread ShellConnection is instantiated on, and this interface is
+// wrapped by the Shell interface.
+// To use this interface on other threads, call Shell::CloneConnector() and
+// pass the result to another thread. To pass to subsequent threads, call
+// Clone() on instances of this object.
+// While instances of this object are owned by the caller, the underlying
+// connection with the shell is bound to the lifetime of the instance that
+// created it, i.e. when the application is terminated the Connector pipe is
+// closed.
+class Connector {
+ public:
+  class ConnectParams {
+   public:
+    explicit ConnectParams(const std::string& url);
+    ~ConnectParams();
+
+    const GURL& url() { return url_; }
+    shell::mojom::CapabilityFilterPtr TakeFilter() {
+      return std::move(filter_);
+    }
+    void set_filter(shell::mojom::CapabilityFilterPtr filter) {
+      filter_ = std::move(filter);
+    }
+    void set_user_id(uint32_t user_id) { user_id_ = user_id; }
+    uint32_t user_id() const { return user_id_; }
+
+   private:
+    GURL url_;
+    shell::mojom::CapabilityFilterPtr filter_;
+    uint32_t user_id_;
+
+    DISALLOW_COPY_AND_ASSIGN(ConnectParams);
+  };
+
+  // Requests a new connection to an application. Returns a pointer to the
+  // connection if the connection is permitted by this application's delegate,
+  // or nullptr otherwise. Caller takes ownership.
+  // Once this method is called, this object is bound to the thread on which the
+  // call took place. To pass to another thread, call Clone() and pass the
+  // result.
+  virtual scoped_ptr<Connection> Connect(const std::string& url) = 0;
+  virtual scoped_ptr<Connection> Connect(ConnectParams* params) = 0;
+
+  // Connect to application identified by |request->url| and connect to the
+  // service implementation of the interface identified by |Interface|.
+  template <typename Interface>
+  void ConnectToInterface(ConnectParams* params, InterfacePtr<Interface>* ptr) {
+    scoped_ptr<Connection> connection = Connect(params);
+    if (connection)
+      connection->GetInterface(ptr);
+  }
+  template <typename Interface>
+  void ConnectToInterface(const std::string& url,
+                          InterfacePtr<Interface>* ptr) {
+    ConnectParams params(url);
+    params.set_filter(CreatePermissiveCapabilityFilter());
+    return ConnectToInterface(&params, ptr);
+  }
+
+  // Creates a new instance of this class which may be passed to another thread.
+  // The returned object may be passed multiple times until Connect() is called,
+  // at which point this method must be called again to pass again.
+  virtual scoped_ptr<Connector> Clone() = 0;
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_SHELL_PUBLIC_CPP_CONNECTOR_H_
diff --git a/mojo/shell/public/cpp/lib/application_test_base.cc b/mojo/shell/public/cpp/lib/application_test_base.cc
index 49bb65e..da56dbb 100644
--- a/mojo/shell/public/cpp/lib/application_test_base.cc
+++ b/mojo/shell/public/cpp/lib/application_test_base.cc
@@ -20,8 +20,8 @@ namespace test {
 namespace {
 // Share the application URL with multiple application tests.
 String g_url;
-uint32_t g_id = shell::mojom::Shell::kInvalidApplicationID;
-uint32_t g_user_id = shell::mojom::Shell::kUserRoot;
+uint32_t g_id = shell::mojom::Connector::kInvalidApplicationID;
+uint32_t g_user_id = shell::mojom::Connector::kUserRoot;
 
 // ShellClient request handle passed from the shell in MojoMain, stored in
 // between SetUp()/TearDown() so we can (re-)intialize new ShellConnections.
diff --git a/mojo/shell/public/cpp/lib/connection_impl.cc b/mojo/shell/public/cpp/lib/connection_impl.cc
index 7fb65b5..4897bb3 100644
--- a/mojo/shell/public/cpp/lib/connection_impl.cc
+++ b/mojo/shell/public/cpp/lib/connection_impl.cc
@@ -40,7 +40,7 @@ ConnectionImpl::ConnectionImpl(
       weak_factory_(this) {}
 
 ConnectionImpl::ConnectionImpl()
-    : remote_id_(shell::mojom::Shell::kInvalidApplicationID),
+    : remote_id_(shell::mojom::Connector::kInvalidApplicationID),
       remote_ids_valid_(false),
       local_registry_(shell::mojom::InterfaceProviderRequest(), this),
       allow_all_interfaces_(true),
@@ -48,7 +48,7 @@ ConnectionImpl::ConnectionImpl()
 
 ConnectionImpl::~ConnectionImpl() {}
 
-shell::mojom::Shell::ConnectCallback ConnectionImpl::GetConnectCallback() {
+shell::mojom::Connector::ConnectCallback ConnectionImpl::GetConnectCallback() {
   return base::Bind(&ConnectionImpl::OnGotInstanceID,
                     weak_factory_.GetWeakPtr());
 }
diff --git a/mojo/shell/public/cpp/lib/connection_impl.h b/mojo/shell/public/cpp/lib/connection_impl.h
index 00e167f..e2c407a 100644
--- a/mojo/shell/public/cpp/lib/connection_impl.h
+++ b/mojo/shell/public/cpp/lib/connection_impl.h
@@ -36,7 +36,7 @@ class ConnectionImpl : public Connection {
                  const std::set<std::string>& allowed_interfaces);
   ~ConnectionImpl() override;
 
-  shell::mojom::Shell::ConnectCallback GetConnectCallback();
+  shell::mojom::Connector::ConnectCallback GetConnectCallback();
 
  private:
   // Connection:
diff --git a/mojo/shell/public/cpp/lib/connector_impl.cc b/mojo/shell/public/cpp/lib/connector_impl.cc
new file mode 100644
index 0000000..989382e
--- /dev/null
+++ b/mojo/shell/public/cpp/lib/connector_impl.cc
@@ -0,0 +1,74 @@
+// 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/public/cpp/lib/connector_impl.h"
+
+#include "mojo/shell/public/cpp/lib/connection_impl.h"
+
+namespace mojo {
+
+Connector::ConnectParams::ConnectParams(const std::string& url)
+    : url_(url),
+      filter_(shell::mojom::CapabilityFilter::New()),
+      user_id_(shell::mojom::Connector::kUserInherit) {
+  filter_->filter.SetToEmpty();
+}
+Connector::ConnectParams::~ConnectParams() {}
+
+ConnectorImpl::ConnectorImpl(shell::mojom::ConnectorPtrInfo unbound_state)
+    : unbound_state_(std::move(unbound_state)) {}
+ConnectorImpl::~ConnectorImpl() {}
+
+scoped_ptr<Connection> ConnectorImpl::Connect(const std::string& url) {
+  ConnectParams params(url);
+  params.set_filter(CreatePermissiveCapabilityFilter());
+  return Connect(&params);
+}
+
+scoped_ptr<Connection> ConnectorImpl::Connect(ConnectParams* params) {
+  // Bind this object to the current thread the first time it is used to
+  // connect.
+  if (!connector_.is_bound()) {
+    if (!unbound_state_.is_valid())
+      return nullptr;
+    connector_.Bind(std::move(unbound_state_));
+    thread_checker_.reset(new base::ThreadChecker);
+  }
+  DCHECK(thread_checker_->CalledOnValidThread());
+
+  DCHECK(params);
+  std::string application_url = params->url().spec();
+  // We allow all interfaces on outgoing connections since we are presumably in
+  // a position to know who we're talking to.
+  // TODO(beng): is this a valid assumption or do we need to figure some way to
+  //             filter here too?
+  std::set<std::string> allowed;
+  allowed.insert("*");
+  shell::mojom::InterfaceProviderPtr local_interfaces;
+  shell::mojom::InterfaceProviderRequest local_request =
+      GetProxy(&local_interfaces);
+  shell::mojom::InterfaceProviderPtr remote_interfaces;
+  shell::mojom::InterfaceProviderRequest remote_request =
+      GetProxy(&remote_interfaces);
+  scoped_ptr<internal::ConnectionImpl> registry(new internal::ConnectionImpl(
+      application_url, application_url,
+      shell::mojom::Connector::kInvalidApplicationID, params->user_id(),
+      std::move(remote_interfaces), std::move(local_request), allowed));
+  connector_->Connect(application_url,
+                      params->user_id(),
+                      std::move(remote_request),
+                      std::move(local_interfaces),
+                      params->TakeFilter(),
+                      registry->GetConnectCallback());
+  return std::move(registry);
+}
+
+scoped_ptr<Connector> ConnectorImpl::Clone() {
+  shell::mojom::ConnectorPtr connector;
+  connector_->Clone(GetProxy(&connector));
+  return make_scoped_ptr(
+      new ConnectorImpl(connector.PassInterface()));
+}
+
+}  // namespace mojo
diff --git a/mojo/shell/public/cpp/lib/connector_impl.h b/mojo/shell/public/cpp/lib/connector_impl.h
new file mode 100644
index 0000000..9782932
--- /dev/null
+++ b/mojo/shell/public/cpp/lib/connector_impl.h
@@ -0,0 +1,35 @@
+// 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_PUBLIC_CPP_LIB_CONNECTOR_IMPL_H_
+#define MOJO_SHELL_PUBLIC_CPP_LIB_CONNECTOR_IMPL_H_
+
+#include "base/threading/thread_checker.h"
+#include "mojo/shell/public/cpp/connector.h"
+#include "mojo/shell/public/interfaces/shell.mojom.h"
+
+namespace mojo {
+
+class ConnectorImpl : public Connector {
+ public:
+  explicit ConnectorImpl(shell::mojom::ConnectorPtrInfo unbound_state);
+  ~ConnectorImpl();
+
+ private:
+  // Connector:
+  scoped_ptr<Connection> Connect(const std::string& url) override;
+  scoped_ptr<Connection> Connect(ConnectParams* params) override;
+  scoped_ptr<Connector> Clone() override;
+
+  shell::mojom::ConnectorPtrInfo unbound_state_;
+  shell::mojom::ConnectorPtr connector_;
+
+  scoped_ptr<base::ThreadChecker> thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConnectorImpl);
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_SHELL_PUBLIC_CPP_LIB_CONNECTOR_IMPL_H_
diff --git a/mojo/shell/public/cpp/lib/shell_connection.cc b/mojo/shell/public/cpp/lib/shell_connection.cc
index 8e03fb0..1a3d398 100644
--- a/mojo/shell/public/cpp/lib/shell_connection.cc
+++ b/mojo/shell/public/cpp/lib/shell_connection.cc
@@ -9,7 +9,9 @@
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "mojo/public/cpp/bindings/interface_ptr.h"
+#include "mojo/shell/public/cpp/connector.h"
 #include "mojo/shell/public/cpp/lib/connection_impl.h"
+#include "mojo/shell/public/cpp/lib/connector_impl.h"
 #include "mojo/shell/public/cpp/shell_client.h"
 #include "mojo/shell/public/cpp/shell_connection.h"
 
@@ -84,15 +86,6 @@ class AppRefCountImpl : public AppRefCount {
   DISALLOW_COPY_AND_ASSIGN(AppRefCountImpl);
 };
 
-
-ShellConnection::ConnectParams::ConnectParams(const std::string& url)
-    : url_(url),
-      filter_(shell::mojom::CapabilityFilter::New()),
-      user_id_(shell::mojom::Shell::kUserInherit) {
-  filter_->filter.SetToEmpty();
-}
-ShellConnection::ConnectParams::~ConnectParams() {}
-
 ////////////////////////////////////////////////////////////////////////////////
 // ShellConnection, public:
 
@@ -125,39 +118,16 @@ void ShellConnection::WaitForInitialize() {
 // ShellConnection, Shell implementation:
 
 scoped_ptr<Connection> ShellConnection::Connect(const std::string& url) {
-  ConnectParams params(url);
-  params.set_filter(CreatePermissiveCapabilityFilter());
-  return Connect(&params);
+  return connector_->Connect(url);
 }
 
-scoped_ptr<Connection> ShellConnection::Connect(ConnectParams* params) {
-  if (!shell_)
-    return nullptr;
-  DCHECK(params);
-  std::string application_url = params->url().spec();
-  // We allow all interfaces on outgoing connections since we are presumably in
-  // a position to know who we're talking to.
-  // TODO(beng): is this a valid assumption or do we need to figure some way to
-  //             filter here too?
-  std::set<std::string> allowed;
-  allowed.insert("*");
-  shell::mojom::InterfaceProviderPtr local_interfaces;
-  shell::mojom::InterfaceProviderRequest local_request =
-      GetProxy(&local_interfaces);
-  shell::mojom::InterfaceProviderPtr remote_interfaces;
-  shell::mojom::InterfaceProviderRequest remote_request =
-      GetProxy(&remote_interfaces);
-  scoped_ptr<internal::ConnectionImpl> registry(new internal::ConnectionImpl(
-      application_url, application_url,
-      shell::mojom::Shell::kInvalidApplicationID, params->user_id(),
-      std::move(remote_interfaces), std::move(local_request), allowed));
-  shell_->Connect(application_url,
-                  params->user_id(),
-                  std::move(remote_request),
-                  std::move(local_interfaces),
-                  params->TakeFilter(),
-                  registry->GetConnectCallback());
-  return std::move(registry);
+scoped_ptr<Connection> ShellConnection::Connect(
+    Connector::ConnectParams* params) {
+  return connector_->Connect(params);
+}
+
+scoped_ptr<Connector> ShellConnection::CloneConnector() const {
+  return connector_->Clone();
 }
 
 void ShellConnection::Quit() {
@@ -186,6 +156,11 @@ void ShellConnection::Initialize(shell::mojom::ShellPtr shell,
                                  uint32_t user_id) {
   shell_ = std::move(shell);
   shell_.set_connection_error_handler([this]() { OnConnectionError(); });
+
+  shell::mojom::ConnectorPtr connector;
+  shell_->GetConnector(GetProxy(&connector));
+  connector_.reset(new ConnectorImpl(connector.PassInterface()));
+
   client_->Initialize(this, url, id, user_id);
 }
 
diff --git a/mojo/shell/public/cpp/shell.h b/mojo/shell/public/cpp/shell.h
index f513f40..d333acb 100644
--- a/mojo/shell/public/cpp/shell.h
+++ b/mojo/shell/public/cpp/shell.h
@@ -6,16 +6,12 @@
 #define MOJO_SHELL_PUBLIC_CPP_SHELL_H_
 
 #include "mojo/shell/public/cpp/connection.h"
+#include "mojo/shell/public/cpp/connector.h"
 #include "mojo/shell/public/interfaces/shell.mojom.h"
-#include "mojo/shell/public/interfaces/shell_client.mojom.h"
 #include "url/gurl.h"
 
 namespace mojo {
 
-shell::mojom::CapabilityFilterPtr CreatePermissiveCapabilityFilter();
-
-using ShellClientRequest = InterfaceRequest<shell::mojom::ShellClient>;
-
 // An interface implementation can keep this object as a member variable to
 // hold a reference to the ShellConnection, keeping it alive as long as the
 // bound implementation exists.
@@ -35,39 +31,17 @@ class AppRefCount {
 // ShellConnection, this is the primary interface exposed to clients.
 class Shell {
  public:
-  class ConnectParams {
-   public:
-    explicit ConnectParams(const std::string& url);
-    ~ConnectParams();
-
-    const GURL& url() { return url_; }
-    shell::mojom::CapabilityFilterPtr TakeFilter() {
-      return std::move(filter_);
-    }
-    void set_filter(shell::mojom::CapabilityFilterPtr filter) {
-      filter_ = std::move(filter);
-    }
-    void set_user_id(uint32_t user_id) { user_id_ = user_id; }
-    uint32_t user_id() const { return user_id_; }
-
-   private:
-    GURL url_;
-    shell::mojom::CapabilityFilterPtr filter_;
-    uint32_t user_id_;
-
-    DISALLOW_COPY_AND_ASSIGN(ConnectParams);
-  };
-
   // Requests a new connection to an application. Returns a pointer to the
   // connection if the connection is permitted by this application's delegate,
   // or nullptr otherwise. Caller takes ownership.
   virtual scoped_ptr<Connection> Connect(const std::string& url) = 0;
-  virtual scoped_ptr<Connection> Connect(ConnectParams* params) = 0;
+  virtual scoped_ptr<Connection> Connect(Connector::ConnectParams* params) = 0;
 
   // Connect to application identified by |request->url| and connect to the
   // service implementation of the interface identified by |Interface|.
   template <typename Interface>
-  void ConnectToInterface(ConnectParams* params, InterfacePtr<Interface>* ptr) {
+  void ConnectToInterface(Connector::ConnectParams* params,
+                          InterfacePtr<Interface>* ptr) {
     scoped_ptr<Connection> connection = Connect(params);
     if (connection)
       connection->GetInterface(ptr);
@@ -75,11 +49,15 @@ class Shell {
   template <typename Interface>
   void ConnectToInterface(const std::string& url,
                           InterfacePtr<Interface>* ptr) {
-    ConnectParams params(url);
+    Connector::ConnectParams params(url);
     params.set_filter(CreatePermissiveCapabilityFilter());
     return ConnectToInterface(&params, ptr);
   }
 
+  // Returns a clone of the ShellConnection's Connector that can be passed to
+  // other threads.
+  virtual scoped_ptr<Connector> CloneConnector() const = 0;
+
   // Initiate shutdown of this application. This may involve a round trip to the
   // Shell to ensure there are no inbound service requests.
   virtual void Quit() = 0;
diff --git a/mojo/shell/public/cpp/shell_connection.h b/mojo/shell/public/cpp/shell_connection.h
index 8132425..ab24fd0 100644
--- a/mojo/shell/public/cpp/shell_connection.h
+++ b/mojo/shell/public/cpp/shell_connection.h
@@ -22,6 +22,7 @@
 namespace mojo {
 
 class AppRefCountImpl;
+class Connector;
 
 // Encapsulates a connection to the Mojo Shell in two parts:
 // - a bound InterfacePtr to mojo::shell::mojom::Shell, the primary mechanism
@@ -84,7 +85,8 @@ class ShellConnection : public Shell, public shell::mojom::ShellClient {
 
   // Shell:
   scoped_ptr<Connection> Connect(const std::string& url) override;
-  scoped_ptr<Connection> Connect(ConnectParams* params) override;
+  scoped_ptr<Connection> Connect(Connector::ConnectParams* params) override;
+  scoped_ptr<Connector> CloneConnector() const override;
   void Quit() override;
   scoped_ptr<AppRefCount> CreateAppRefCount() override;
 
@@ -125,6 +127,7 @@ class ShellConnection : public Shell, public shell::mojom::ShellClient {
   mojo::ShellClient* client_;
   Binding<shell::mojom::ShellClient> binding_;
   shell::mojom::ShellPtr shell_;
+  scoped_ptr<Connector> connector_;
   Closure termination_closure_;
   bool quit_requested_;
   int ref_count_;
diff --git a/mojo/shell/public/interfaces/shell.mojom b/mojo/shell/public/interfaces/shell.mojom
index d7160ea2..8c7f41b 100644
--- a/mojo/shell/public/interfaces/shell.mojom
+++ b/mojo/shell/public/interfaces/shell.mojom
@@ -20,9 +20,8 @@ struct CapabilityFilter {
   map<string, array<string>> filter;
 };
 
-// An interface through which a Mojo application may communicate with the Mojo
-// system and request connections to other applications.
-interface Shell {
+// Encapsulates establishing connections with other Mojo applications.
+interface Connector {
   const uint32 kInvalidApplicationID = 0;
   const uint32 kUserRoot = 0;
   const uint32 kUserInherit = 1;
@@ -83,6 +82,18 @@ interface Shell {
           InterfaceProvider? local_interfaces,
           CapabilityFilter filter) => (uint32 application_id);
 
+  // Clones this Connector so it can be passed to another thread.
+  Clone(Connector& request);
+};
+
+// Wraps functionality exposed by the Shell to a Mojo application instance.
+interface Shell {
+  // Obtain a Connector that can be used to create connections with other
+  // applications. The connector is bound in the shell to the instance that
+  // vended this Shell interface, all connectors created and cloned frmo this
+  // one are bound to the lifetime of this instance.
+  GetConnector(Connector& connector);
+
   // When there are no more instantiated services in an application, it should
   // start its shutdown process by calling this method. Additionally, it should
   // keep track of any new service requests that come in. The shell will then