Internalize ServiceConnector<>

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

Index: mojo/public/cpp/shell/lib/service_connector.h
diff --git a/mojo/public/cpp/shell/service.h b/mojo/public/cpp/shell/lib/service_connector.h
similarity index 50%
rename from mojo/public/cpp/shell/service.h
rename to mojo/public/cpp/shell/lib/service_connector.h
index c5462896775576018a52d643067d472c2430958f..4e175f59d5ab1e7694c8d428f19fb83ab0fe149f 100644
--- a/mojo/public/cpp/shell/service.h
+++ b/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();

[darin (slow to review), 2014/05/12 22:04:52]

Hmm... this requires that ServiceImpl implement OnConnectionError else there
will be a compile error. Maybe that is OK. I forced subclasses of
InterfaceImpl<..> to implement this method since it was important for memory
management. Here it is not the case since ServiceConnection<..> is managing
memory. Hmm...

+  }
+
+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 Constructor {

[darin (slow to review), 2014/05/12 22:04:52]

You might consider a more verbose name like ServiceConnectionConstructor :-/

[DaveMoore, 2014/05/14 16:50:31]

Done.

+  static ServiceConnection<ServiceImpl, Context>* New(Context* context) {
+    return new ServiceConnection<ServiceImpl, Context>(context);
+  }
+};
+
+template <typename ServiceImpl>
+struct Constructor<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 =
+        internal::Constructor<ServiceImpl, Context>::New(context_);

[darin (slow to review), 2014/05/12 22:04:52]

nit: no need for the internal:: prefix since you are in the internal namespace
already.

[DaveMoore, 2014/05/14 16:50:31]

Done.

+    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_