Mojo: Internalize ServiceConnector<>

Authored by davemoore@chromium.org, see https://codereview.chromium.org/275363002/

R=viettrungluu@chromium.org, darin@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270867 0039d316-1c4b-4281-b951-d872f2087c98

mojo/dbus/dbus_external_service.h

@@ -74,7 +74,7 @@ class DBusExternalService : public DBusExternalServiceBase {
     }
     virtual void Activate(ScopedMessagePipeHandle shell_handle) OVERRIDE {
       app_.reset(new Application(shell_handle.Pass()));
-      app_->AddServiceConnector(new ServiceConnector<ServiceImpl>());
+      app_->AddService<ServiceImpl>();
     }
    private:
     DBusExternalService* service_;

mojo/examples/launcher/launcher.cc

@@ -188,20 +188,18 @@ class LauncherController : public views::TextfieldController {
   DISALLOW_COPY_AND_ASSIGN(LauncherController);
 };
 
-class LauncherImpl : public ServiceConnection<Launcher, LauncherImpl>,
+class LauncherImpl : public InterfaceImpl<Launcher>,
                      public URLReceiver {
  public:
-  LauncherImpl()
-      : launcher_controller_(this),
+  explicit LauncherImpl(Application* app)
+      : app_(app),
+        launcher_controller_(this),
         pending_show_(false) {
-  }
-
-  void Initialize() {
     screen_.reset(ScreenMojo::Create());
     gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE, screen_.get());
 
     NativeViewportPtr viewport;
-    ConnectTo(shell(), "mojo:mojo_native_viewport_service", &viewport);
+    app_->ConnectTo("mojo:mojo_native_viewport_service", &viewport);
 
     window_tree_host_.reset(new WindowTreeHostMojo(
         viewport.Pass(), gfx::Rect(50, 50, 450, 60),
@@ -250,6 +248,7 @@ class LauncherImpl : public ServiceConnection<Launcher, LauncherImpl>,
     }
   }
 
+  Application* app_;
   scoped_ptr<ScreenMojo> screen_;
   scoped_ptr<LauncherWindowTreeClient> window_tree_client_;
   scoped_ptr<aura::client::FocusClient> focus_client_;
@@ -287,8 +286,7 @@ extern "C" LAUNCHER_EXPORT MojoResult CDECL MojoMain(
   aura::Env::CreateInstance(true);
 
   mojo::Application app(shell_handle);
-  app.AddServiceConnector(
-      new mojo::ServiceConnector<mojo::examples::LauncherImpl>());
+  app.AddService<mojo::examples::LauncherImpl>(&app);
 
   loop.Run();
   return MOJO_RESULT_OK;

mojo/mojo_public.gypi

@@ -368,9 +368,10 @@
       'type': 'static_library',
       'sources': [
         'public/cpp/shell/application.h',
-        'public/cpp/shell/service.h',
+        'public/cpp/shell/connect.h',
         'public/cpp/shell/lib/application.cc',
-        'public/cpp/shell/lib/service.cc',
+        'public/cpp/shell/lib/service_connector.cc',
+        'public/cpp/shell/lib/service_connector.h',
       ],
       'dependencies': [
         'mojo_shell_bindings',

mojo/public/cpp/bindings/error_handler.h

@@ -14,15 +14,6 @@ class ErrorHandler {
   virtual void OnConnectionError() = 0;
 };
 
-// Used when you'd like to extend a base class with the same method signature
-// as ErrorHandler.
-template <typename Base>
-class WithErrorHandler : public Base {
- public:
-  virtual ~WithErrorHandler() {}
-  virtual void OnConnectionError() = 0;
-};
-
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_ERROR_HANDLER_H_

mojo/public/cpp/bindings/interface_impl.h

@@ -13,18 +13,17 @@ namespace mojo {
 // InterfaceImpl<..> is designed to be the base class of an interface
 // implementation. It may be bound to a pipe or a proxy, see BindToPipe and
 // BindToProxy.
-//
-// NOTE: A base class of WithErrorHandler<Interface> is used to avoid multiple
-// inheritance. This base class inserts the signature of ErrorHandler into the
-// inheritance chain.
 template <typename Interface>
-class InterfaceImpl : public WithErrorHandler<Interface> {
+class InterfaceImpl : public internal::InterfaceImplBase<Interface> {
  public:
   typedef typename Interface::Client Client;
 
   InterfaceImpl() : internal_state_(this) {}
   virtual ~InterfaceImpl() {}
 
+  // Subclasses can override this to handle post connection initialization.
+  virtual void OnConnectionEstablished() {}
+
   // Subclasses must handle connection errors.
   virtual void OnConnectionError() = 0;
 

mojo/public/cpp/bindings/lib/interface_impl_internal.h

@@ -14,12 +14,20 @@
 namespace mojo {
 namespace internal {
 
+template <typename Interface>
+class InterfaceImplBase : public Interface {
+ public:
+  virtual ~InterfaceImplBase() {}
+  virtual void OnConnectionEstablished() = 0;
+  virtual void OnConnectionError() = 0;
+};
+
 template <typename Interface>
 class InterfaceImplState : public ErrorHandler {
  public:
   typedef typename Interface::Client Client;
 
-  explicit InterfaceImplState(WithErrorHandler<Interface>* instance)
+  explicit InterfaceImplState(InterfaceImplBase<Interface>* instance)
       : router_(NULL),
         client_(NULL),
         proxy_(NULL) {
@@ -58,7 +66,8 @@ class InterfaceImplState : public ErrorHandler {
 
     proxy_ = new typename Client::Proxy_(router_);
 
-    stub_.sink()->SetClient(proxy_);
+    instance()->SetClient(proxy_);
+    instance()->OnConnectionEstablished();
   }
 
   Router* router() { return router_; }
@@ -67,9 +76,12 @@ class InterfaceImplState : public ErrorHandler {
   Client* client() { return client_; }
 
  private:
+  InterfaceImplBase<Interface>* instance() {
+    return static_cast<InterfaceImplBase<Interface>*>(stub_.sink());
+  }
+
   virtual void OnConnectionError() MOJO_OVERRIDE {
-    static_cast<WithErrorHandler<Interface>*>(stub_.sink())->
-        OnConnectionError();
+    instance()->OnConnectionError();
   }
 
   Router* router_;

mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc

@@ -32,7 +32,13 @@ class MathCalculatorImpl : public InterfaceImpl<math::Calculator> {
  public:
   virtual ~MathCalculatorImpl() {}
 
-  MathCalculatorImpl() : total_(0.0) {
+  MathCalculatorImpl()
+      : total_(0.0),
+        got_connection_(false) {
+  }
+
+  virtual void OnConnectionEstablished() MOJO_OVERRIDE {
+    got_connection_ = true;
   }
 
   virtual void OnConnectionError() MOJO_OVERRIDE {
@@ -53,8 +59,13 @@ class MathCalculatorImpl : public InterfaceImpl<math::Calculator> {
     client()->Output(total_);
   }
 
- private:
+  bool got_connection() const {
+    return got_connection_;
+  }
+
+private:
   double total_;
+  bool got_connection_;
 };
 
 class MathCalculatorUIImpl : public math::CalculatorUI {
@@ -116,7 +127,8 @@ class InterfacePtrTest : public testing::Test {
 
 TEST_F(InterfacePtrTest, EndToEnd) {
   math::CalculatorPtr calc;
-  BindToProxy(new MathCalculatorImpl(), &calc);
+  MathCalculatorImpl* impl = BindToProxy(new MathCalculatorImpl(), &calc);
+  EXPECT_TRUE(impl->got_connection());
 
   // Suppose this is instantiated in a process that has pipe1_.
   MathCalculatorUIImpl calculator_ui(calc.Pass());

mojo/public/cpp/shell/application.h

@@ -7,24 +7,61 @@
 
 #include <vector>
 
-#include "mojo/public/cpp/shell/service.h"
+#include "mojo/public/cpp/shell/connect.h"
+#include "mojo/public/cpp/shell/lib/service_connector.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/public/interfaces/shell/shell.mojom.h"
 
 namespace mojo {
 
+// Utility class for creating ShellClients that vend service instances.
+// To use define a class that implements your specific server api, e.g. FooImpl
+// to implement a service named Foo.
+// That class must subclass an InterfaceImpl specialization.
+//
+// If there is context that is to be shared amongst all instances, define a
+// constructor with that class as its only argument, otherwise define an empty
+// constructor.
+//
+// class FooImpl : public InterfaceImpl<Foo> {
+//  public:
+//   FooImpl() {}
+// };
+//
+// or
+//
+// class BarImpl : public InterfaceImpl<Bar> {
+//  public:
+//   // context will remain valid for the lifetime of BarImpl.
+//   BarImpl(BarContext* context) : context_(context) {}
+//  private:
+//   BarContext* context;
+// };
+//
+// Create an Application instance that collects any service implementations.
+//
+// Application app(shell_handle);
+// app.AddService<FooImpl>();
+//
+// BarContext context;
+// app.AddService<BarImpl>(&context);
+//
+//
 class Application : public internal::ServiceConnectorBase::Owner {
  public:
   explicit Application(ScopedMessagePipeHandle shell_handle);
   explicit Application(MojoHandle shell_handle);
   virtual ~Application();
 
-  // internal::ServiceConnectorBase::Owner methods.
-  // Takes ownership of |service_connector|.
-  virtual void AddServiceConnector(
-      internal::ServiceConnectorBase* service_connector) MOJO_OVERRIDE;
-  virtual void RemoveServiceConnector(
-      internal::ServiceConnectorBase* service_connector) MOJO_OVERRIDE;
+  template <typename Impl, typename Context>
+  void AddService(Context* context) {
+    AddServiceConnector(new internal::ServiceConnector<Impl, Context>(context));
+  }
+
+  template <typename Impl>
+  void AddService() {
+    AddServiceConnector(new internal::ServiceConnector<Impl, void>(NULL));
+  }
 
   template <typename Interface>
   void ConnectTo(const std::string& url, InterfacePtr<Interface>* ptr) {
@@ -33,11 +70,20 @@ class Application : public internal::ServiceConnectorBase::Owner {
 
  protected:
   // ShellClient methods.
+  // Override this to dispatch to correct service when there's more than one.
+  // TODO(davemoore): Augment this with name registration.
   virtual void AcceptConnection(const mojo::String& url,
                                 ScopedMessagePipeHandle client_handle)
       MOJO_OVERRIDE;
 
  private:
+  // internal::ServiceConnectorBase::Owner methods.
+  // Takes ownership of |service_connector|.
+  virtual void AddServiceConnector(
+      internal::ServiceConnectorBase* service_connector) MOJO_OVERRIDE;
+  virtual void RemoveServiceConnector(
+      internal::ServiceConnectorBase* service_connector) MOJO_OVERRIDE;
+
   typedef std::vector<internal::ServiceConnectorBase*> ServiceConnectorList;
   ServiceConnectorList service_connectors_;
 };

mojo/public/cpp/shell/connect.h

+// 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.
+
+#ifndef MOJO_PUBLIC_CPP_SHELL_CONNECT_H_
+#define MOJO_PUBLIC_CPP_SHELL_CONNECT_H_
+
+#include "mojo/public/cpp/bindings/allocation_scope.h"
+#include "mojo/public/interfaces/shell/shell.mojom.h"
+
+namespace mojo {
+
+template <typename Interface>
+inline void ConnectTo(Shell* shell, const std::string& url,
+                      InterfacePtr<Interface>* ptr) {
+  MessagePipe pipe;
+  ptr->Bind(pipe.handle0.Pass());
+
+  AllocationScope scope;
+  shell->Connect(url, pipe.handle1.Pass());
+}
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_SHELL_CONNECT_H_

mojo/public/cpp/shell/lib/service.cc → mojo/public/cpp/shell/lib/service_connector.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "mojo/public/cpp/shell/service.h"
+#include "mojo/public/cpp/shell/lib/service_connector.h"
 
 namespace mojo {
 namespace internal {

mojo/public/cpp/shell/service.h → mojo/public/cpp/shell/lib/service_connector.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef MOJO_PUBLIC_SHELL_SERVICE_H_
-#define MOJO_PUBLIC_SHELL_SERVICE_H_
+#ifndef MOJO_PUBLIC_CPP_SHELL_LIB_SERVICE_CONNECTOR_H_
+#define MOJO_PUBLIC_CPP_SHELL_LIB_SERVICE_CONNECTOR_H_
 
 #include <assert.h>
 
@@ -12,54 +12,53 @@
 #include "mojo/public/cpp/bindings/allocation_scope.h"
 #include "mojo/public/interfaces/shell/shell.mojom.h"
 
-// Utility classes for creating ShellClients that vend service instances.
-// To use define a class that implements your specific server api, e.g. FooImpl
-// to implement a service named Foo. That class must define an empty constructor
-// and the Initialize() method.
-// class FooImpl : public Foo {
-//  public:
-//   FooImpl();
-//   void Initialize();
-//  private:
-//   ServiceConnector<FooImpl>* service_connector_;
-// };
-//
-//
-// To simplify further FooImpl can use the ServiceConnection<> template.
-// class FooImpl : public ServiceConnection<Foo, FooImpl> {
-//  public:
-//   FooImpl();
-//   ...
-//   <Foo implementation>
-// };
-//
-// Instances of FooImpl will be created by a specialized ServiceConnector
-//
-// ServiceConnector<FooImpl>
-//
-// Optionally the classes can be specializeed with a shared context
-// class ServiceConnector<FooImpl, MyContext>
-// and
-// class FooImpl : public ServiceConnection<Foo, FooImpl, MyContext>
-//
-// foo_connector = new ServiceConnector<FooImpl, MyContext>(my_context);
-// instances of FooImpl can call context() and retrieve the value of my_context.
-//
-// Lastly create an Application instance that collects all the
-// ServiceConnectors.
-//
-// Application app(shell_handle);
-// app.AddServiceConnector(new ServiceConnector<FooImpl>);
-//
-//
-// Specialization of ServiceConnector.
-// ServiceImpl: Implementation of Service interface.
-// Context: Optional type of shared context.v
-//
-//
 namespace mojo {
-
 namespace internal {
+
+template <class ServiceImpl, typename Context>
+class ServiceConnector;
+
+// Specialization of ServiceConnection.
+// ServiceImpl: Subclass of InterfaceImpl<...>.
+// Context: Type of shared context.
+template <class ServiceImpl, typename Context>
+class ServiceConnection : public ServiceImpl {
+ public:
+  ServiceConnection() : ServiceImpl() {}
+  ServiceConnection(Context* context) : ServiceImpl(context) {}
+
+  virtual void OnConnectionError() MOJO_OVERRIDE {
+    service_connector_->RemoveConnection(static_cast<ServiceImpl*>(this));
+    ServiceImpl::OnConnectionError();
+  }
+
+private:
+  friend class ServiceConnector<ServiceImpl, Context>;
+
+  // Called shortly after this class is instantiated.
+  void set_service_connector(
+      ServiceConnector<ServiceImpl, Context>* connector) {
+    service_connector_ = connector;
+  }
+
+  ServiceConnector<ServiceImpl, Context>* service_connector_;
+};
+
+template <typename ServiceImpl, typename Context>
+struct ServiceConstructor {
+  static ServiceConnection<ServiceImpl, Context>* New(Context* context) {
+    return new ServiceConnection<ServiceImpl, Context>(context);
+  }
+};
+
+template <typename ServiceImpl>
+struct ServiceConstructor<ServiceImpl, void> {
+ public:
+  static ServiceConnection<ServiceImpl, void>* New(void* context) {
+    return new ServiceConnection<ServiceImpl, void>();
+  }
+};
+
 class ServiceConnectorBase {
  public:
   class Owner : public ShellClient {
@@ -88,7 +87,6 @@ class ServiceConnectorBase {
  protected:
   Owner* owner_;
 };
-}  // namespace internal
 
 template <class ServiceImpl, typename Context=void>
 class ServiceConnector : public internal::ServiceConnectorBase {
@@ -107,12 +105,12 @@ class ServiceConnector : public internal::ServiceConnectorBase {
 
   virtual void AcceptConnection(const std::string& url,
                                 ScopedMessagePipeHandle handle) MOJO_OVERRIDE {
-    ServiceImpl* impl = BindToPipe(new ServiceImpl(), handle.Pass());
-    impl->set_connector(this);
+    ServiceConnection<ServiceImpl, Context>* impl =
+        ServiceConstructor<ServiceImpl, Context>::New(context_);
+    impl->set_service_connector(this);
+    BindToPipe(impl, handle.Pass());
 
     connections_.push_back(impl);
-
-    impl->Initialize();
   }
 
   void RemoveConnection(ServiceImpl* impl) {
@@ -137,58 +135,7 @@ class ServiceConnector : public internal::ServiceConnectorBase {
   Context* context_;
 };
 
-// Specialization of ServiceConnection.
-// ServiceInterface: Service interface.
-// ServiceImpl: Subclass of ServiceConnection<...>.
-// Context: Optional type of shared context.
-template <class ServiceInterface, class ServiceImpl, typename Context=void>
-class ServiceConnection : public InterfaceImpl<ServiceInterface> {
- protected:
-  // NOTE: shell() and context() are not available at construction time.
-  // Initialize() will be called once those are available.
-  ServiceConnection() : service_connector_(NULL) {}
-
-  virtual ~ServiceConnection() {}
-
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    service_connector_->RemoveConnection(static_cast<ServiceImpl*>(this));
-  }
-
-  // Shadow this method in ServiceImpl to perform one-time initialization.
-  // At the time this is called, shell() and context() will be available.
-  // NOTE: No need to call the base class Initialize from your subclass. It
-  // will always be a no-op.
-  void Initialize() {}
-
-  Shell* shell() {
-    return service_connector_->shell();
-  }
-
-  Context* context() const {
-    return service_connector_->context();
-  }
-
- private:
-  friend class ServiceConnector<ServiceImpl, Context>;
-
-  // Called shortly after this class is instantiated.
-  void set_connector(ServiceConnector<ServiceImpl, Context>* connector) {
-    service_connector_ = connector;
-  }
-
-  ServiceConnector<ServiceImpl, Context>* service_connector_;
-};
-
-template <typename Interface>
-inline void ConnectTo(Shell* shell, const std::string& url,
-                      InterfacePtr<Interface>* ptr) {
-  MessagePipe pipe;
-  ptr->Bind(pipe.handle0.Pass());
-
-  AllocationScope scope;
-  shell->Connect(url, pipe.handle1.Pass());
-}
-
+}  // namespace internal
 }  // namespace mojo
 
-#endif  // MOJO_PUBLIC_SHELL_SERVICE_H_
+#endif  // MOJO_PUBLIC_CPP_SHELL_LIB_SERVICE_CONNECTOR_H_

mojo/service_manager/service_manager_unittest.cc

@@ -24,24 +24,28 @@ struct TestContext {
   int num_loader_deletes;
 };
 
-class TestServiceImpl :
-    public ServiceConnection<TestService, TestServiceImpl, TestContext> {
+class TestServiceImpl : public InterfaceImpl<TestService> {
  public:
+  explicit TestServiceImpl(TestContext* context) : context_(context) {
+    ++context_->num_impls;
+  }
+
   virtual ~TestServiceImpl() {
-    if (context())
-      --context()->num_impls;
+    --context_->num_impls;
   }
 
-  void Initialize() {
-    if (context())
-      ++context()->num_impls;
+  // InterfaceImpl<TestService> implementation.
+  virtual void OnConnectionError() OVERRIDE {
   }
 
   // TestService implementation:
   virtual void Test(const mojo::String& test_string) OVERRIDE {
-    context()->last_test_string = test_string.To<std::string>();
+    context_->last_test_string = test_string.To<std::string>();
     client()->AckTest();
   }
+
+ private:
+  TestContext* context_;
 };
 
 class TestClientImpl : public TestClient {
@@ -98,8 +102,7 @@ class TestServiceLoader : public ServiceLoader {
                            ScopedMessagePipeHandle shell_handle) OVERRIDE {
     ++num_loads_;
     test_app_.reset(new Application(shell_handle.Pass()));
-    test_app_->AddServiceConnector(
-        new ServiceConnector<TestServiceImpl, TestContext>(context_));
+    test_app_->AddService<TestServiceImpl>(context_);
   }
 
   virtual void OnServiceError(ServiceManager* manager,

mojo/services/dbus_echo/dbus_echo_service.cc

@@ -16,12 +16,13 @@
 #include "mojo/services/dbus_echo/echo.mojom.h"
 
 namespace {
-class EchoServiceImpl
-    : public mojo::ServiceConnection<mojo::EchoService, EchoServiceImpl> {
+class EchoServiceImpl : public mojo::InterfaceImpl<mojo::EchoService> {
  public:
   EchoServiceImpl() {}
   virtual ~EchoServiceImpl() {}
 
+  virtual void OnConnectionError() OVERRIDE {}
+
  protected:
   virtual void Echo(
       const mojo::String& in_to_echo,

mojo/services/native_viewport/native_viewport_service.cc

@@ -28,13 +28,12 @@ bool IsRateLimitedEventType(ui::Event* event) {
 }
 
 class NativeViewportImpl
-    : public ServiceConnection<mojo::NativeViewport,
-                               NativeViewportImpl,
-                               shell::Context>,
+    : public InterfaceImpl<mojo::NativeViewport>,
       public NativeViewportDelegate {
  public:
-  NativeViewportImpl()
-      : widget_(gfx::kNullAcceleratedWidget),
+  NativeViewportImpl(shell::Context* context)
+      : context_(context),
+        widget_(gfx::kNullAcceleratedWidget),
         waiting_for_event_ack_(false) {}
   virtual ~NativeViewportImpl() {
     // Destroy the NativeViewport early on as it may call us back during
@@ -42,9 +41,11 @@ class NativeViewportImpl
     native_viewport_.reset();
   }
 
+  virtual void OnConnectionError() OVERRIDE {}
+
   virtual void Create(const Rect& bounds) OVERRIDE {
     native_viewport_ =
-        services::NativeViewport::Create(context(), this);
+        services::NativeViewport::Create(context_, this);
     native_viewport_->Init(bounds);
     client()->OnCreated();
     OnBoundsChanged(bounds);
@@ -174,6 +175,7 @@ class NativeViewportImpl
   }
 
  private:
+  shell::Context* context_;
   gfx::AcceleratedWidget widget_;
   scoped_ptr<services::NativeViewport> native_viewport_;
   ScopedMessagePipeHandle command_buffer_handle_;
@@ -189,9 +191,7 @@ MOJO_NATIVE_VIEWPORT_EXPORT mojo::Application*
     CreateNativeViewportService(mojo::shell::Context* context,
                                 mojo::ScopedMessagePipeHandle shell_handle) {
   mojo::Application* app = new mojo::Application(shell_handle.Pass());
-  app->AddServiceConnector(
-      new mojo::ServiceConnector<mojo::services::NativeViewportImpl,
-                                 mojo::shell::Context>(context));
+  app->AddService<mojo::services::NativeViewportImpl>(context);
   return app;
 }
 

mojo/services/public/cpp/view_manager/lib/view_manager_synchronizer.cc

@@ -6,7 +6,7 @@
 
 #include "base/bind.h"
 #include "base/run_loop.h"
-#include "mojo/public/cpp/shell/service.h"
+#include "mojo/public/cpp/shell/connect.h"
 #include "mojo/public/interfaces/shell/shell.mojom.h"
 #include "mojo/services/public/cpp/view_manager/lib/view_manager_private.h"
 #include "mojo/services/public/cpp/view_manager/lib/view_private.h"

mojo/services/view_manager/main.cc

@@ -26,10 +26,8 @@ extern "C" VIEW_MANAGER_EXPORT MojoResult CDECL MojoMain(
   base::MessageLoop loop;
   mojo::Application app(shell_handle);
   mojo::view_manager::service::RootNodeManager root_node_manager(app.shell());
-  app.AddServiceConnector(
-      new mojo::ServiceConnector<
-          mojo::view_manager::service::ViewManagerConnection,
-          mojo::view_manager::service::RootNodeManager>(&root_node_manager));
+  app.AddService<mojo::view_manager::service::ViewManagerConnection>(
+      &root_node_manager);
   loop.Run();
 
   return MOJO_RESULT_OK;

mojo/services/view_manager/root_view_manager.cc

@@ -7,7 +7,7 @@
 #include "base/auto_reset.h"
 #include "mojo/aura/screen_mojo.h"
 #include "mojo/aura/window_tree_host_mojo.h"
-#include "mojo/public/cpp/shell/service.h"
+#include "mojo/public/cpp/shell/connect.h"
 #include "mojo/public/interfaces/shell/shell.mojom.h"
 #include "mojo/services/view_manager/root_node_manager.h"
 #include "ui/aura/client/default_capture_client.h"

mojo/services/view_manager/view_manager_connection.cc

@@ -59,7 +59,9 @@ void NodeToINode(Node* node,
 
 }  // namespace
 
-ViewManagerConnection::ViewManagerConnection() : id_(0) {
+ViewManagerConnection::ViewManagerConnection(RootNodeManager* root_node_manager)
+    : root_node_manager_(root_node_manager),
+      id_(0) {
 }
 
 ViewManagerConnection::~ViewManagerConnection() {
@@ -78,22 +80,25 @@ ViewManagerConnection::~ViewManagerConnection() {
   }
 
   STLDeleteContainerPairSecondPointers(node_map_.begin(), node_map_.end());
-  context()->RemoveConnection(this);
+  root_node_manager_->RemoveConnection(this);
 }
 
-void ViewManagerConnection::Initialize() {
-  DCHECK_EQ(0, id_);  // Should only get Initialize() once.
-  id_ = context()->GetAndAdvanceNextConnectionId();
-  context()->AddConnection(this);
-  client()->OnConnectionEstablished(id_, context()->next_server_change_id());
+void ViewManagerConnection::OnConnectionEstablished() {
+  DCHECK_EQ(0, id_);  // Should only get SetClient() once.
+  id_ = root_node_manager_->GetAndAdvanceNextConnectionId();
+  root_node_manager_->AddConnection(this);
+  client()->OnConnectionEstablished(
+      id_, root_node_manager_->next_server_change_id());
 }
 
+void ViewManagerConnection::OnConnectionError() {}
+
 Node* ViewManagerConnection::GetNode(const NodeId& id) {
   if (id_ == id.connection_id) {
     NodeMap::iterator i = node_map_.find(id.node_id);
     return i == node_map_.end() ? NULL : i->second;
   }
-  return context()->GetNode(id);
+  return root_node_manager_->GetNode(id);
 }
 
 View* ViewManagerConnection::GetView(const ViewId& id) {
@@ -101,7 +106,7 @@ View* ViewManagerConnection::GetView(const ViewId& id) {
     ViewMap::const_iterator i = view_map_.find(id.view_id);
     return i == view_map_.end() ? NULL : i->second;
   }
-  return context()->GetView(id);
+  return root_node_manager_->GetView(id);
 }
 
 void ViewManagerConnection::NotifyNodeHierarchyChanged(
@@ -141,7 +146,8 @@ bool ViewManagerConnection::DeleteNodeImpl(ViewManagerConnection* source,
   if (!node)
     return false;
   RootNodeManager::ScopedChange change(
-      source, context(), RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID);
+      source, root_node_manager_,
+      RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID);
   if (node->GetParent())
     node->GetParent()->Remove(node);
   std::vector<Node*> children(node->GetChildren());
@@ -151,7 +157,7 @@ bool ViewManagerConnection::DeleteNodeImpl(ViewManagerConnection* source,
   node_map_.erase(node_id.node_id);
   delete node;
   node = NULL;
-  context()->NotifyNodeDeleted(node_id);
+  root_node_manager_->NotifyNodeDeleted(node_id);
   return true;
 }
 
@@ -162,7 +168,7 @@ bool ViewManagerConnection::DeleteViewImpl(ViewManagerConnection* source,
   if (!view)
     return false;
   RootNodeManager::ScopedChange change(
-      source, context(),
+      source, root_node_manager_,
       RootNodeManager::CHANGE_TYPE_DONT_ADVANCE_SERVER_CHANGE_ID);
   if (view->node())
     view->node()->SetView(NULL);
@@ -171,7 +177,7 @@ bool ViewManagerConnection::DeleteViewImpl(ViewManagerConnection* source,
   // valid.
   const ViewId view_id_copy(view_id);
   delete view;
-  context()->NotifyViewDeleted(view_id_copy);
+  root_node_manager_->NotifyViewDeleted(view_id_copy);
   return true;
 }
 
@@ -184,7 +190,7 @@ bool ViewManagerConnection::SetViewImpl(const NodeId& node_id,
   if (!view && view_id != ViewId())
     return false;
   RootNodeManager::ScopedChange change(
-      this, context(),
+      this, root_node_manager_,
       RootNodeManager::CHANGE_TYPE_DONT_ADVANCE_SERVER_CHANGE_ID);
   node->SetView(view);
   return true;
@@ -207,7 +213,7 @@ void ViewManagerConnection::DeleteNode(
     TransportNodeId transport_node_id,
     const Callback<void(bool)>& callback) {
   const NodeId node_id(NodeIdFromTransportId(transport_node_id));
-  ViewManagerConnection* connection = context()->GetConnection(
+  ViewManagerConnection* connection = root_node_manager_->GetConnection(
       node_id.connection_id);
   callback.Run(connection &&
                connection->DeleteNodeImpl(this, node_id));
@@ -219,14 +225,14 @@ void ViewManagerConnection::AddNode(
     TransportChangeId server_change_id,
     const Callback<void(bool)>& callback) {
   bool success = false;
-  if (server_change_id == context()->next_server_change_id()) {
+  if (server_change_id == root_node_manager_->next_server_change_id()) {
     Node* parent = GetNode(NodeIdFromTransportId(parent_id));
     Node* child = GetNode(NodeIdFromTransportId(child_id));
     if (parent && child && child->GetParent() != parent &&
         !child->window()->Contains(parent->window())) {
       success = true;
       RootNodeManager::ScopedChange change(
-          this, context(),
+          this, root_node_manager_,
           RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID);
       parent->Add(child);
     }
@@ -239,12 +245,12 @@ void ViewManagerConnection::RemoveNodeFromParent(
     TransportChangeId server_change_id,
     const Callback<void(bool)>& callback) {
   bool success = false;
-  if (server_change_id == context()->next_server_change_id()) {
+  if (server_change_id == root_node_manager_->next_server_change_id()) {
     Node* node = GetNode(NodeIdFromTransportId(node_id));
     if (node && node->GetParent()) {
       success = true;
       RootNodeManager::ScopedChange change(
-          this, context(),
+          this, root_node_manager_,
           RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID);
       node->GetParent()->Remove(node);
     }
@@ -281,7 +287,7 @@ void ViewManagerConnection::DeleteView(
     TransportViewId transport_view_id,
     const Callback<void(bool)>& callback) {
   const ViewId view_id(ViewIdFromTransportId(transport_view_id));
-  ViewManagerConnection* connection = context()->GetConnection(
+  ViewManagerConnection* connection = root_node_manager_->GetConnection(
       view_id.connection_id);
   callback.Run(connection && connection->DeleteViewImpl(this, view_id));
 }
@@ -316,13 +322,13 @@ void ViewManagerConnection::SetViewContents(
 void ViewManagerConnection::OnNodeHierarchyChanged(const NodeId& node,
                                                    const NodeId& new_parent,
                                                    const NodeId& old_parent) {
-  context()->NotifyNodeHierarchyChanged(node, new_parent, old_parent);
+  root_node_manager_->NotifyNodeHierarchyChanged(node, new_parent, old_parent);
 }
 
 void ViewManagerConnection::OnNodeViewReplaced(const NodeId& node,
                                                const ViewId& new_view_id,
                                                const ViewId& old_view_id) {
-  context()->NotifyNodeViewReplaced(node, new_view_id, old_view_id);
+  root_node_manager_->NotifyNodeViewReplaced(node, new_view_id, old_view_id);
 }
 
 }  // namespace service

mojo/services/view_manager/view_manager_connection.h

@@ -9,7 +9,6 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "mojo/public/cpp/shell/service.h"
 #include "mojo/services/public/interfaces/view_manager/view_manager.mojom.h"
 #include "mojo/services/view_manager/ids.h"
 #include "mojo/services/view_manager/node_delegate.h"
@@ -32,17 +31,16 @@ class View;
 
 // Manages a connection from the client.
 class MOJO_VIEW_MANAGER_EXPORT ViewManagerConnection
-    : public ServiceConnection<IViewManager, ViewManagerConnection,
-                               RootNodeManager>,
+    : public InterfaceImpl<IViewManager>,
       public NodeDelegate {
  public:
-  ViewManagerConnection();
+  ViewManagerConnection(RootNodeManager* root_node_manager);
   virtual ~ViewManagerConnection();
 
-  TransportConnectionId id() const { return id_; }
+  virtual void OnConnectionEstablished() MOJO_OVERRIDE;
+  virtual void OnConnectionError() MOJO_OVERRIDE;
 
-  // Invoked when connection is established.
-  void Initialize();
+  TransportConnectionId id() const { return id_; }
 
   // Returns the Node with the specified id.
   Node* GetNode(const NodeId& id);
@@ -111,9 +109,10 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerConnection
   virtual void OnNodeViewReplaced(const NodeId& node,
                                   const ViewId& new_view_id,
                                   const ViewId& old_view_id) OVERRIDE;
+  RootNodeManager* root_node_manager_;
 
   // Id of this connection as assigned by RootNodeManager. Assigned in
-  // Initialize().
+  // OnConnectionEstablished().
   TransportConnectionId id_;
 
   NodeMap node_map_;

mojo/services/view_manager/view_manager_connection_unittest.cc

@@ -12,7 +12,7 @@
 #include "base/strings/stringprintf.h"
 #include "mojo/public/cpp/bindings/allocation_scope.h"
 #include "mojo/public/cpp/environment/environment.h"
-#include "mojo/public/cpp/shell/service.h"
+#include "mojo/public/cpp/shell/connect.h"
 #include "mojo/services/public/cpp/view_manager/util.h"
 #include "mojo/services/public/cpp/view_manager/view_manager_types.h"
 #include "mojo/services/public/interfaces/view_manager/view_manager.mojom.h"

mojo/shell/view_manager_loader.cc

@@ -25,10 +25,8 @@ void ViewManagerLoader::LoadService(ServiceManager* manager,
     root_node_manager_.reset(
         new view_manager::service::RootNodeManager(app->shell()));
   }
-  app->AddServiceConnector(
-      new ServiceConnector<view_manager::service::ViewManagerConnection,
-                           view_manager::service::RootNodeManager>(
-                               root_node_manager_.get()));
+  app->AddService<view_manager::service::ViewManagerConnection>(
+      root_node_manager_.get());
   apps_.push_back(app.release());
 }