Mojo: Use InterfaceFactory<Interface> for service registration

mojo/public/cpp/bindings/lib/interface_impl_internal.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_BINDINGS_LIB_INTERFACE_IMPL_INTERNAL_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_LIB_INTERFACE_IMPL_INTERNAL_H_
 
 #include "mojo/public/cpp/bindings/error_handler.h"
 #include "mojo/public/cpp/bindings/interface_ptr.h"
 #include "mojo/public/cpp/bindings/lib/filter_chain.h"
 #include "mojo/public/cpp/bindings/lib/message_header_validator.h"
 #include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/system/macros.h"
 
 namespace mojo {
+

[darin (slow to review), 2014/07/15 06:10:38]

nit: no new line here is OK, right?

 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(InterfaceImplBase<Interface>* instance)
-      : router_(NULL),
-        proxy_(NULL) {
+      : router_(NULL), proxy_(NULL), delete_on_error_(false) {
     assert(instance);
     stub_.set_sink(instance);
   }
 
   virtual ~InterfaceImplState() {
     delete proxy_;
     if (router_) {
       router_->set_error_handler(NULL);
       delete router_;
     }
   }
 
   void BindProxy(
       InterfacePtr<Interface>* ptr,
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
     MessagePipe pipe;
     ptr->Bind(pipe.handle0.Pass(), waiter);
-    Bind(pipe.handle1.Pass(), waiter);
+    Bind(pipe.handle1.Pass(), waiter, true /* delete_on_error */);
   }
 
   void Bind(ScopedMessagePipeHandle handle,
-            const MojoAsyncWaiter* waiter) {
+            const MojoAsyncWaiter* waiter,
+            bool delete_on_error) {
     assert(!router_);
 
     FilterChain filters;
     filters.Append<MessageHeaderValidator>();
     filters.Append<typename Interface::RequestValidator_>();
     filters.Append<typename Interface::Client::ResponseValidator_>();
 
     router_ = new Router(handle.Pass(), filters.Pass(), waiter);
     router_->set_incoming_receiver(&stub_);
     router_->set_error_handler(this);
 
     proxy_ = new typename Client::Proxy_(router_);
+    delete_on_error_ = delete_on_error;
 
     instance()->OnConnectionEstablished();
   }
 
   bool WaitForIncomingMethodCall() {
     assert(router_);
     return router_->WaitForIncomingMessage();
   }
 
   Router* router() { return router_; }
   Client* client() { return proxy_; }
 
  private:
   InterfaceImplBase<Interface>* instance() {
     return static_cast<InterfaceImplBase<Interface>*>(stub_.sink());
   }
 
   virtual void OnConnectionError() MOJO_OVERRIDE {
+    // If the connection error behavior is not delete_on_error_ the instance
+    // might choose to delete itself in the OnConnectionError handler, which
+    // would in turn delete this.  Save the error behavior before invoking the
+    // error handler so we can correctly decide what to do.
+    bool delete_on_error = delete_on_error_;
     instance()->OnConnectionError();
+    if (delete_on_error)

[darin (slow to review), 2014/07/15 06:10:38]

It might be useful to add some debug assertions that OnConnectionError() doesn't
self-delete when it shouldn't. This could be accomplished with a pointer to bool
member variable that we set to a stack variable inside OnConnectionError before
calling out to the instance.

[jamesr, 2014/07/15 18:07:22]

I added a !NDEBUG-only boolean member that I set before this delete and assert()
that it's set if delete_on_error_ is set in the destructor of this class.  It's
caught some issues locally already, so it's definitely useful

+      delete instance();
   }
 
   Router* router_;
   typename Client::Proxy_* proxy_;
   typename Interface::Stub_ stub_;
+  bool delete_on_error_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(InterfaceImplState);
 };
 
 }  // namespace internal
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_INTERFACE_IMPL_INTERNAL_H_