Mojo: Use InterfaceFactory<Interface> for service registration

This adds an InterfaceFactory<Interface> type and allows registering a
service through its provider. Using an InterfaceFactory allows the app
to be in control of the lifetime of the implementation of the interface
and hides all of the implementation details of the interface from the
application library code (i.e. no subclassing impl classes or anything
like that).  The default binding behavior is to bind the lifetime of
the impl to the lifetime of the pipe, but the application
can also weakly bind to a service in cases where it needs to manage
the lifetime explicitly.

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

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

12 files changed, 182 insertions, 185 deletions

diff --git a/mojo/public/cpp/application/application_connection.h b/mojo/public/cpp/application/application_connection.h
index 2044af1..c0e3480 100644
--- a/mojo/public/cpp/application/application_connection.h
+++ b/mojo/public/cpp/application/application_connection.h
@@ -21,51 +21,26 @@ namespace mojo {
 // 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.
+// Then implement an InterfaceFactory<Foo> that binds instances of FooImpl to
+// InterfaceRequest<Foo>s and register that on the connection.
 //
-// class FooImpl : public InterfaceImpl<Foo> {
-//  public:
-//   explicit FooImpl(ApplicationConnnection* connection) {}
-// };
+// connection->AddService(&factory);
 //
-// or
+// Or if you have multiple factories implemented by the same type, explicitly
+// specify the interface to register the factory for:
 //
-// class BarImpl : public InterfaceImpl<Bar> {
-//  public:
-//   // contexts will remain valid for the lifetime of BarImpl.
-//   BarImpl(ApplicationConnnection* connection, BarContext* service_context)
-//          : connection_(connection), servicecontext_(context) {}
+// connection->AddService<Foo>(&my_foo_and_bar_factory_);
+// connection->AddService<Bar>(&my_foo_and_bar_factory_);
 //
-// Create an ApplicationDelegate instance and pass it to the constructor
-// of an ApplicationImpl. The delegate will be called when new connections are
-// made to other applications.
-//
-// connection->AddService<FooImpl>();
-//
-// BarContext context;
-// connection->AddService<BarImpl>(&context);
+// The InterfaceFactory must outlive the ApplicationConnection.
 class ApplicationConnection {
  public:
   virtual ~ApplicationConnection();
 
-  // Impl’s constructor will receive two arguments:
-  // Impl::Impl(Application::Context* app_context,
-  //            ServiceContext* svc_context)
-  template <typename Impl, typename ServiceContext>
-  void AddService(ServiceContext* context) {
-    AddServiceConnector(
-        new internal::ServiceConnector<Impl, ServiceContext>(Impl::Name_,
-                                                             context));
-  }
-
-  // Impl’s constructor will receive one argument:
-  // Impl::Impl(Application::Context* app_context)
-  template <typename Impl>
-  void AddService() {
+  template <typename Interface>
+  void AddService(InterfaceFactory<Interface>* factory) {
     AddServiceConnector(
-        new internal::ServiceConnector<Impl, void>(Impl::Name_, NULL));
+        new internal::InterfaceFactoryConnector<Interface>(factory));
   }
 
   // Connect to the service implementing |Interface|.
@@ -96,8 +71,8 @@ class ApplicationConnection {
   // Raw ServiceProvider interface to remote application.
   virtual ServiceProvider* GetServiceProvider() = 0;
 
-private:
- virtual void AddServiceConnector(
+ private:
+  virtual void AddServiceConnector(
       internal::ServiceConnectorBase* service_connector) = 0;
 };
 
diff --git a/mojo/public/cpp/application/interface_factory.h b/mojo/public/cpp/application/interface_factory.h
new file mode 100644
index 0000000..90abd13
--- /dev/null
+++ b/mojo/public/cpp/application/interface_factory.h
@@ -0,0 +1,30 @@
+// 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_APPLICATION_INTERFACE_FACTORY_H_
+#define MOJO_PUBLIC_CPP_APPLICATION_INTERFACE_FACTORY_H_
+
+#include "mojo/public/cpp/bindings/interface_impl.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+
+namespace mojo {
+
+class ApplicationConnection;
+template <typename Interface> class InterfaceRequest;
+
+// Implement this class to provide implementations of a given interface and
+// bind them to incoming requests. The implementation of this class is
+// responsible for managing the lifetime of the implementations of the
+// interface.
+template <typename Interface>
+class InterfaceFactory {
+ public:
+  virtual ~InterfaceFactory() {}
+  virtual void Create(ApplicationConnection* connection,
+                      InterfaceRequest<Interface> request) = 0;
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_APPLICATION_INTERFACE_FACTORY_H_
diff --git a/mojo/public/cpp/application/interface_factory_with_context.h b/mojo/public/cpp/application/interface_factory_with_context.h
new file mode 100644
index 0000000..8ea7215
--- /dev/null
+++ b/mojo/public/cpp/application/interface_factory_with_context.h
@@ -0,0 +1,34 @@
+// 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_APPLICATION_INTERFACE_FACTORY_WITH_CONTEXT_H_
+#define MOJO_PUBLIC_CPP_APPLICATION_INTERFACE_FACTORY_WITH_CONTEXT_H_
+
+#include "mojo/public/cpp/application/interface_factory.h"
+
+namespace mojo {
+
+// Use this class to allocate and bind instances of Impl constructed with a
+// context parameter to interface requests. The lifetime of the constructed
+// Impls is bound to the pipe.
+template <typename Impl,
+          typename Context,
+          typename Interface = typename Impl::ImplementedInterface>
+class InterfaceFactoryWithContext : public InterfaceFactory<Interface> {
+ public:
+  explicit InterfaceFactoryWithContext(Context* context) : context_(context) {}
+  virtual ~InterfaceFactoryWithContext() {}
+
+  virtual void Create(ApplicationConnection* connection,
+                      InterfaceRequest<Interface> request) MOJO_OVERRIDE {
+    BindToRequest(new Impl(context_), &request);
+  }
+
+ private:
+  Context* context_;
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_APPLICATION_INTERFACE_FACTORY_WITH_CONTEXT_H_
diff --git a/mojo/public/cpp/application/lib/mojo_main_chromium.cc b/mojo/public/cpp/application/lib/mojo_main_chromium.cc
index 269b7a2..d8c5e3f 100644
--- a/mojo/public/cpp/application/lib/mojo_main_chromium.cc
+++ b/mojo/public/cpp/application/lib/mojo_main_chromium.cc
@@ -14,12 +14,16 @@ extern "C" APPLICATION_EXPORT MojoResult CDECL MojoMain(
 #if !defined(COMPONENT_BUILD)
   base::AtExitManager at_exit;
 #endif
-  base::MessageLoop loop;
-  scoped_ptr<mojo::ApplicationDelegate> delegate(
-      mojo::ApplicationDelegate::Create());
-  mojo::ApplicationImpl app(delegate.get());
-  app.BindShell(shell_handle);
-  loop.Run();
+  scoped_ptr<mojo::ApplicationDelegate> delegate;
+  {
+    // We have to shut down the MessageLoop before destroying the
+    // ApplicationDelegate.
+    base::MessageLoop loop;
+    delegate.reset(mojo::ApplicationDelegate::Create());
+    mojo::ApplicationImpl app(delegate.get());
+    app.BindShell(shell_handle);
+    loop.Run();
+  }
 
   return MOJO_RESULT_OK;
 }
diff --git a/mojo/public/cpp/application/lib/mojo_main_standalone.cc b/mojo/public/cpp/application/lib/mojo_main_standalone.cc
index 0ba078b..6071b6b 100644
--- a/mojo/public/cpp/application/lib/mojo_main_standalone.cc
+++ b/mojo/public/cpp/application/lib/mojo_main_standalone.cc
@@ -10,12 +10,17 @@
 extern "C" APPLICATION_EXPORT MojoResult CDECL MojoMain(
     MojoHandle shell_handle) {
   mojo::Environment env;
-  mojo::RunLoop loop;
-  mojo::ApplicationDelegate* delegate = mojo::ApplicationDelegate::Create();
+  mojo::ApplicationDelegate* delegate = NULL;
   {
-    mojo::ApplicationImpl app(delegate);
-    app.BindShell(shell_handle);
-    loop.Run();
+    // We have to shut down the RunLoop before destroying the
+    // ApplicationDelegate.
+    mojo::RunLoop loop;
+    delegate = mojo::ApplicationDelegate::Create();
+    {
+      mojo::ApplicationImpl app(delegate);
+      app.BindShell(shell_handle);
+      loop.Run();
+    }
   }
   delete delegate;
 
diff --git a/mojo/public/cpp/application/lib/service_connector.h b/mojo/public/cpp/application/lib/service_connector.h
index d2de48b..aeb7de1 100644
--- a/mojo/public/cpp/application/lib/service_connector.h
+++ b/mojo/public/cpp/application/lib/service_connector.h
@@ -5,10 +5,8 @@
 #ifndef MOJO_PUBLIC_CPP_APPLICATION_LIB_SERVICE_CONNECTOR_H_
 #define MOJO_PUBLIC_CPP_APPLICATION_LIB_SERVICE_CONNECTOR_H_
 
-#include <assert.h>
-
-#include <vector>
-
+#include "mojo/public/cpp/application/interface_factory.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
 #include "mojo/public/interfaces/service_provider/service_provider.mojom.h"
 
 namespace mojo {
@@ -16,59 +14,6 @@ class ApplicationConnection;
 
 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:
-  explicit ServiceConnection(ApplicationConnection* connection)
-      : ServiceImpl(connection) {}
-  ServiceConnection(ApplicationConnection* connection,
-                    Context* context) : ServiceImpl(connection, 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_;
-
-  MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceConnection);
-};
-
-template <typename ServiceImpl, typename Context>
-struct ServiceConstructor {
-  static ServiceConnection<ServiceImpl, Context>* New(
-      ApplicationConnection* connection,
-      Context* context) {
-    return new ServiceConnection<ServiceImpl, Context>(
-        connection, context);
-  }
-};
-
-template <typename ServiceImpl>
-struct ServiceConstructor<ServiceImpl, void> {
- public:
-  static ServiceConnection<ServiceImpl, void>* New(
-      ApplicationConnection* connection,
-      void* context) {
-    return new ServiceConnection<ServiceImpl, void>(connection);
-  }
-};
-
 class ServiceConnectorBase {
  public:
   ServiceConnectorBase(const std::string& name);
@@ -86,53 +31,22 @@ class ServiceConnectorBase {
   MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceConnectorBase);
 };
 
-template <class ServiceImpl, typename Context=void>
-class ServiceConnector : public internal::ServiceConnectorBase {
+template <typename Interface>
+class InterfaceFactoryConnector : public ServiceConnectorBase {
  public:
-  ServiceConnector(const std::string& name, Context* context = NULL)
-      : ServiceConnectorBase(name), context_(context) {}
-
-  virtual ~ServiceConnector() {
-    ConnectionList doomed;
-    doomed.swap(connections_);
-    for (typename ConnectionList::iterator it = doomed.begin();
-         it != doomed.end(); ++it) {
-      delete *it;
-    }
-    assert(connections_.empty());  // No one should have added more!
-  }
+  explicit InterfaceFactoryConnector(InterfaceFactory<Interface>* factory)
+      : ServiceConnectorBase(Interface::Name_), factory_(factory) {}
+  virtual ~InterfaceFactoryConnector() {}
 
   virtual void ConnectToService(const std::string& name,
-                                ScopedMessagePipeHandle handle) MOJO_OVERRIDE {
-    ServiceConnection<ServiceImpl, Context>* impl =
-        ServiceConstructor<ServiceImpl, Context>::New(application_connection_,
-                                                      context_);
-    impl->set_service_connector(this);
-    BindToPipe(impl, handle.Pass());
-
-    connections_.push_back(impl);
-  }
-
-  void RemoveConnection(ServiceImpl* impl) {
-    // Called from ~ServiceImpl, in response to a connection error.
-    for (typename ConnectionList::iterator it = connections_.begin();
-         it != connections_.end(); ++it) {
-      if (*it == impl) {
-        delete impl;
-        connections_.erase(it);
-        return;
-      }
-    }
+                                ScopedMessagePipeHandle client_handle) {
+    factory_->Create(application_connection_,
+                     MakeRequest<Interface>(client_handle.Pass()));
   }
 
-  Context* context() const { return context_; }
-
  private:
-  typedef std::vector<ServiceImpl*> ConnectionList;
-  ConnectionList connections_;
-  Context* context_;
-
-  MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceConnector);
+  InterfaceFactory<Interface>* factory_;
+  MOJO_DISALLOW_COPY_AND_ASSIGN(InterfaceFactoryConnector);
 };
 
 }  // namespace internal
diff --git a/mojo/public/cpp/bindings/interface_impl.h b/mojo/public/cpp/bindings/interface_impl.h
index 4a78cb4..da1055d 100644
--- a/mojo/public/cpp/bindings/interface_impl.h
+++ b/mojo/public/cpp/bindings/interface_impl.h
@@ -19,6 +19,7 @@ template <typename Interface>
 class InterfaceImpl : public internal::InterfaceImplBase<Interface> {
  public:
   typedef typename Interface::Client Client;
+  typedef Interface ImplementedInterface;
 
   InterfaceImpl() : internal_state_(this) {}
   virtual ~InterfaceImpl() {}
@@ -57,12 +58,15 @@ class InterfaceImpl : public internal::InterfaceImplBase<Interface> {
 
 // Takes an instance of an InterfaceImpl<..> subclass and binds it to the given
 // MessagePipe. The instance is returned for convenience in member initializer
-// lists, etc. If the pipe is closed, the instance's OnConnectionError method
-// will be called.
+// lists, etc.
+//
+// If the pipe is closed, the instance's OnConnectionError method will be called
+// and then the instance will be deleted.
 //
 // The instance is also bound to the current thread. Its methods will only be
-// called on the current thread, and if the current thread exits, then it will
-// also be deleted, and along with it, its end point of the pipe will be closed.
+// called on the current thread, and if the current thread exits, then the end
+// point of the pipe will be closed and the error handler's OnConnectionError
+// method will be called.
 //
 // Before returning, the instance's OnConnectionEstablished method is called.
 template <typename Impl>
@@ -70,14 +74,25 @@ Impl* BindToPipe(
     Impl* instance,
     ScopedMessagePipeHandle handle,
     const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
-  instance->internal_state()->Bind(handle.Pass(), waiter);
+  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>
+Impl* WeakBindToPipe(
+    Impl* instance,
+    ScopedMessagePipeHandle handle,
+    const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  instance->internal_state()->Bind(handle.Pass(), false, waiter);
   return instance;
 }
 
 // Takes an instance of an InterfaceImpl<..> subclass and binds it to the given
 // InterfacePtr<..>. The instance is returned for convenience in member
 // initializer lists, etc. If the pipe is closed, the instance's
-// OnConnectionError method will be called.
+// OnConnectionError method will be called and then the instance will be
+// deleted.
 //
 // The instance is also bound to the current thread. Its methods will only be
 // called on the current thread, and if the current thread exits, then it will
@@ -89,14 +104,25 @@ Impl* BindToProxy(
     Impl* instance,
     InterfacePtr<Interface>* ptr,
     const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
-  instance->internal_state()->BindProxy(ptr, waiter);
+  instance->internal_state()->BindProxy(ptr, true, waiter);
+  return instance;
+}
+
+// Like BindToProxy but does not delete the instance after a channel error.
+template <typename Impl, typename Interface>
+Impl* WeakBindToProxy(
+    Impl* instance,
+    InterfacePtr<Interface>* ptr,
+    const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  instance->internal_state()->BindProxy(ptr, false, waiter);
   return instance;
 }
 
 // Takes an instance of an InterfaceImpl<..> subclass and binds it to the given
 // InterfaceRequest<..>. The instance is returned for convenience in member
 // initializer lists, etc. If the pipe is closed, the instance's
-// OnConnectionError method will be called.
+// OnConnectionError method will be called and then the instance will be
+// deleted.
 //
 // The instance is also bound to the current thread. Its methods will only be
 // called on the current thread, and if the current thread exits, then it will
@@ -112,6 +138,15 @@ Impl* BindToRequest(
   return BindToPipe(instance, request->PassMessagePipe(), waiter);
 }
 
+// Like BindToRequest but does not delete the instance after a channel error.
+template <typename Impl, typename Interface>
+Impl* WeakBindToRequest(
+    Impl* instance,
+    InterfaceRequest<Interface>* request,
+    const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  return WeakBindToPipe(instance, request->PassMessagePipe(), waiter);
+}
+
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_INTERFACE_IMPL_H_
diff --git a/mojo/public/cpp/bindings/lib/interface_impl_internal.h b/mojo/public/cpp/bindings/lib/interface_impl_internal.h
index d94d12d..79f59d8 100644
--- a/mojo/public/cpp/bindings/lib/interface_impl_internal.h
+++ b/mojo/public/cpp/bindings/lib/interface_impl_internal.h
@@ -31,12 +31,21 @@ class InterfaceImplState : public ErrorHandler {
 
   explicit InterfaceImplState(InterfaceImplBase<Interface>* instance)
       : router_(NULL),
-        proxy_(NULL) {
+        proxy_(NULL),
+        instance_bound_to_pipe_(false)
+#ifndef NDEBUG
+        ,
+        deleting_instance_due_to_error_(false)
+#endif
+  {
     MOJO_DCHECK(instance);
     stub_.set_sink(instance);
   }
 
   virtual ~InterfaceImplState() {
+#ifndef NDEBUG
+    MOJO_DCHECK(!instance_bound_to_pipe_ || deleting_instance_due_to_error_);
+#endif
     delete proxy_;
     if (router_) {
       router_->set_error_handler(NULL);
@@ -46,13 +55,15 @@ class InterfaceImplState : public ErrorHandler {
 
   void BindProxy(
       InterfacePtr<Interface>* ptr,
+      bool instance_bound_to_pipe,
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
     MessagePipe pipe;
     ptr->Bind(pipe.handle0.Pass(), waiter);
-    Bind(pipe.handle1.Pass(), waiter);
+    Bind(pipe.handle1.Pass(), instance_bound_to_pipe, waiter);
   }
 
   void Bind(ScopedMessagePipeHandle handle,
+            bool instance_bound_to_pipe,
             const MojoAsyncWaiter* waiter) {
     MOJO_DCHECK(!router_);
 
@@ -67,6 +78,8 @@ class InterfaceImplState : public ErrorHandler {
 
     proxy_ = new typename Client::Proxy_(router_);
 
+    instance_bound_to_pipe_ = instance_bound_to_pipe;
+
     instance()->OnConnectionEstablished();
   }
 
@@ -84,12 +97,27 @@ class InterfaceImplState : public ErrorHandler {
   }
 
   virtual void OnConnectionError() MOJO_OVERRIDE {
+    // If the the instance is not bound to the pipe, 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 bound = instance_bound_to_pipe_;
     instance()->OnConnectionError();
+    if (!bound)
+      return;
+#ifndef NDEBUG
+    deleting_instance_due_to_error_ = true;
+#endif
+    delete instance();
   }
 
   Router* router_;
   typename Client::Proxy_* proxy_;
   typename Interface::Stub_ stub_;
+  bool instance_bound_to_pipe_;
+#ifndef NDEBUG
+  bool deleting_instance_due_to_error_;
+#endif
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(InterfaceImplState);
 };
diff --git a/mojo/public/cpp/bindings/tests/handle_passing_unittest.cc b/mojo/public/cpp/bindings/tests/handle_passing_unittest.cc
index d2e287f..696e613 100644
--- a/mojo/public/cpp/bindings/tests/handle_passing_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/handle_passing_unittest.cc
@@ -29,10 +29,6 @@ class StringRecorder {
 class ImportedInterfaceImpl
     : public InterfaceImpl<imported::ImportedInterface> {
  public:
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    delete this;
-  }
-
   virtual void DoSomething() MOJO_OVERRIDE {
     do_something_count_++;
   }
@@ -46,10 +42,6 @@ int ImportedInterfaceImpl::do_something_count_ = 0;
 
 class SampleNamedObjectImpl : public InterfaceImpl<sample::NamedObject> {
  public:
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    delete this;
-  }
-
   virtual void SetName(const mojo::String& name) MOJO_OVERRIDE {
     name_ = name;
   }
@@ -65,10 +57,6 @@ class SampleNamedObjectImpl : public InterfaceImpl<sample::NamedObject> {
 
 class SampleFactoryImpl : public InterfaceImpl<sample::Factory> {
  public:
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    delete this;
-  }
-
   virtual void DoStuff(sample::RequestPtr request,
                        ScopedMessagePipeHandle pipe) MOJO_OVERRIDE {
     std::string text1;
diff --git a/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc b/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc
index 533d990..e3e9f1e 100644
--- a/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc
@@ -41,10 +41,6 @@ class MathCalculatorImpl : public InterfaceImpl<math::Calculator> {
     got_connection_ = true;
   }
 
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    delete this;
-  }
-
   virtual void Clear() MOJO_OVERRIDE {
     client()->Output(total_);
   }
@@ -164,10 +160,6 @@ class ReentrantServiceImpl : public InterfaceImpl<sample::Service> {
     got_connection_ = true;
   }
 
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    delete this;
-  }
-
   bool got_connection() const {
     return got_connection_;
   }
diff --git a/mojo/public/cpp/bindings/tests/request_response_unittest.cc b/mojo/public/cpp/bindings/tests/request_response_unittest.cc
index 338d968..a082a09 100644
--- a/mojo/public/cpp/bindings/tests/request_response_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/request_response_unittest.cc
@@ -15,10 +15,6 @@ namespace {
 
 class ProviderImpl : public InterfaceImpl<sample::Provider> {
  public:
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    delete this;
-  }
-
   virtual void EchoString(
       const String& a,
       const Callback<void(String)>& callback) MOJO_OVERRIDE {
diff --git a/mojo/public/cpp/bindings/tests/validation_unittest.cc b/mojo/public/cpp/bindings/tests/validation_unittest.cc
index 9269212..6463a9e 100644
--- a/mojo/public/cpp/bindings/tests/validation_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/validation_unittest.cc
@@ -275,10 +275,6 @@ class IntegrationTestInterface1Impl
 
   virtual void Method0(BasicStructPtr param0) MOJO_OVERRIDE {
   }
-
-  virtual void OnConnectionError() MOJO_OVERRIDE {
-    delete this;
-  }
 };
 
 TEST_F(ValidationTest, InputParser) {