Provides a way to use bindings that doesn't require subclassing

mojo/public/cpp/bindings/interface_impl.h

Index: mojo/public/cpp/bindings/interface_impl.h
diff --git a/mojo/public/cpp/bindings/interface_impl.h b/mojo/public/cpp/bindings/interface_impl.h
index da1055de535bb71445541320ab274be536bd5fe1..0ca6f2b4f7d05c383654c8112a62b56d40c792b8 100644
--- a/mojo/public/cpp/bindings/interface_impl.h
+++ b/mojo/public/cpp/bindings/interface_impl.h
@@ -12,47 +12,149 @@
 
 namespace mojo {
 
+template <typename Interface>
+class InterfaceImpl;
+
+// Mojo provides two ways to bind an implementation of a particular interface to
+// a message pipe. The distinction lies in whether the binding logic is
+// inherited from a superclass (InterfaceImpl) or whether the binding is managed
+// separately from the class (InterfaceBinding). InterfaceImpl is generally
+// simpler to use at the cost of flexibility.
+
+// InterfaceBinding is used to bind an implementation of an interface to a pipe.
+// Once bound, client() returns the client of the Interface.
+// InterfaceBinding owns the underlying pipe. Deleting InterfaceBinding
+// closes the pipe (assuming it is open).
+// InterfaceBinding can be configured to delete the supplied interface when
+// InterfaceBinding is deleted. See set_owns_interface().
+// . InterfaceBinding can own the

[DaveMoore, 2014/11/10 23:54:23]

Nit: Extra line?

[sky, 2014/11/11 00:34:57]

Done.

+template <typename Interface>
+class InterfaceBinding : public InterfaceBindingDelegate {
+ public:
+  typedef typename Interface::Client Client;
+
+  InterfaceBinding(Interface* interface, InterfaceBindingDelegate* delegate)
+      : delegate_(delegate),
+        internal_state_(interface, this),
+        destroyed_(nullptr),
+        owns_interface_(false) {}
+
+  virtual ~InterfaceBinding() {
+    if (owns_interface_)
+      delete internal_state_.instance();
+  }
+
+  void BindToPipe(
+      ScopedMessagePipeHandle handle,
+      const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+    internal_state_.Bind(handle.Pass(), false, waiter);
+  }
+
+  void BindToRequest(
+      InterfaceRequest<Interface>* request,
+      const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+    return BindToPipe(request->PassMessagePipe(), waiter);
+  }
+
+  // Whether the supplied interface should be deleted when InterfaceBinding is
+  // destroyed.
+  void set_owns_interface(bool owns_interface) {
+    owns_interface_ = owns_interface;
+  }
+
+  Client* client() { return internal_state_.client(); }
+
+ private:
+  friend class InterfaceImpl<Interface>;
+
+  void set_delete_interface_on_error(bool delete_interface_on_error) {
+    delete_interface_on_error_ = delete_interface_on_error;
+  }
+
+  // InterfaceBindingDelegate:
+  void OnConnectionEstablished() override {
+    delegate_->OnConnectionEstablished();
+  }

[DaveMoore, 2014/11/10 23:54:23]

Nit: blank line

[sky, 2014/11/11 00:34:57]

The lack of newline is done to reinforce OnConnectionError is part of
InterfaceBindingDelegate. It's the same as would be done if this was the
declaration.

+  void OnConnectionError() override {
+    bool destroyed = false;
+    destroyed_ = &destroyed;
+    // Allow OnConnectionError() to delete 'this'.
+    delegate_->OnConnectionError();
+    if (destroyed)
+      return;
+    destroyed_ = nullptr;
+    // |delete_interface_on_error_| means we're in an InterfaceImpl and need
+    // to delete the InterfaceImpl (which triggers deleting us).
+    if (internal_state_.instance_bound_to_pipe()) {
+      if (delete_interface_on_error_)
+        delete internal_state_.instance();
+      else
+        delete this;
+    }
+  }
+
+  InterfaceBindingDelegate* delegate_;
+
+  internal::InterfaceImplState<Interface> internal_state_;
+  // If non-null the destructor sets the value to true. Used to detect deletion
+  // while calling to delegate.

[DaveMoore, 2014/11/10 23:54:23]

Where does the destructor set this?

[sky, 2014/11/11 00:34:57]

D'OH! Apparently no tests needed this logic. Added in setting of destroyed_ in
destructor.

[DaveMoore, 2014/11/11 16:04:14]

If we keep this cl you should add a test for this behavior, as it's tricky.

+  bool* destroyed_;
+
+  bool owns_interface_;
+
+  // See description in OnConnectionError().
+  bool delete_interface_on_error_;
+
+  MOJO_DISALLOW_COPY_AND_ASSIGN(InterfaceBinding);
+};
+
 // 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.
 template <typename Interface>
-class InterfaceImpl : public internal::InterfaceImplBase<Interface> {
+class InterfaceImpl : public InterfaceBindingDelegate, public Interface {
  public:
   typedef typename Interface::Client Client;
   typedef Interface ImplementedInterface;
 
-  InterfaceImpl() : internal_state_(this) {}
-  virtual ~InterfaceImpl() {}
+  InterfaceImpl() : interface_binding_(nullptr) {}
+  virtual ~InterfaceImpl() { delete interface_binding_; }
 
   // Returns a proxy to the client interface. This is null upon construction,
   // and becomes non-null after OnClientConnected. NOTE: It remains non-null
   // until this instance is deleted.
-  Client* client() { return internal_state_.client(); }
+  Client* client() { return internal_state()->client(); }
 
   // Blocks the current thread for the first incoming method call, i.e., either
   // a call to a method or a client callback method. Returns |true| if a method
   // has been called, |false| in case of error. It must only be called on a
   // bound object.
   bool WaitForIncomingMethodCall() {
-    return internal_state_.WaitForIncomingMethodCall();
+    return internal_state()->WaitForIncomingMethodCall();
   }
 
-  // Called when the client has connected to this instance.
-  virtual void OnConnectionEstablished() {}
-
   // Called when the client is no longer connected to this instance. NOTE: The
   // client() method continues to return a non-null pointer after this method
   // is called. After this method is called, any method calls made on client()
   // will be silently ignored.
-  virtual void OnConnectionError() {}
+  void OnConnectionError() override {}
 
   // DO NOT USE. Exposed only for internal use and for testing.
   internal::InterfaceImplState<Interface>* internal_state() {
-    return &internal_state_;
+    return &interface_binding_->internal_state_;
+  }
+
+  // TODO(sky): move to private and friend functions that use.
+  void CreateInterfaceBinding() {
+    MOJO_DCHECK(!interface_binding_);
+    interface_binding_ = new InterfaceBinding<Interface>(this, this);
+    interface_binding_->set_owns_interface(false);
+    interface_binding_->set_delete_interface_on_error(true);
   }
 
  private:
-  internal::InterfaceImplState<Interface> internal_state_;
+  InterfaceBinding<Interface>* interface_binding_;
+
   MOJO_DISALLOW_COPY_AND_ASSIGN(InterfaceImpl);
 };
 
@@ -69,21 +171,25 @@ class InterfaceImpl : public internal::InterfaceImplBase<Interface> {
 // method will be called.
 //
 // Before returning, the instance's OnConnectionEstablished method is called.
-template <typename Impl>
+template <typename Impl,
+          typename Interface = typename Impl::ImplementedInterface>
 Impl* BindToPipe(
     Impl* instance,
     ScopedMessagePipeHandle handle,
     const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  instance->CreateInterfaceBinding();
   instance->internal_state()->Bind(handle.Pass(), true, waiter);
   return instance;
 }
 
 // Like BindToPipe but does not delete the instance after a channel error.
-template <typename Impl>
+template <typename Impl,
+          typename Interface = typename Impl::ImplementedInterface>
 Impl* WeakBindToPipe(
     Impl* instance,
     ScopedMessagePipeHandle handle,
     const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  instance->CreateInterfaceBinding();
   instance->internal_state()->Bind(handle.Pass(), false, waiter);
   return instance;
 }
@@ -104,6 +210,7 @@ Impl* BindToProxy(
     Impl* instance,
     InterfacePtr<Interface>* ptr,
     const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  instance->CreateInterfaceBinding();
   instance->internal_state()->BindProxy(ptr, true, waiter);
   return instance;
 }
@@ -114,6 +221,7 @@ Impl* WeakBindToProxy(
     Impl* instance,
     InterfacePtr<Interface>* ptr,
     const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  instance->CreateInterfaceBinding();
   instance->internal_state()->BindProxy(ptr, false, waiter);
   return instance;
 }