C++ bindings: support using a callback as connection error handler.

mojo/public/cpp/bindings/binding.h

Index: mojo/public/cpp/bindings/binding.h
diff --git a/mojo/public/cpp/bindings/binding.h b/mojo/public/cpp/bindings/binding.h
index 8dbcbd45411c964409e53aa0c12b11ad6fbc3e4a..29708e0df25d89235a0b65466f838ed21d9adba9 100644
--- a/mojo/public/cpp/bindings/binding.h
+++ b/mojo/public/cpp/bindings/binding.h
@@ -6,6 +6,7 @@
 #define MOJO_PUBLIC_CPP_BINDINGS_BINDING_H_
 
 #include "mojo/public/c/environment/async_waiter.h"
+#include "mojo/public/cpp/bindings/callback.h"
 #include "mojo/public/cpp/bindings/error_handler.h"
 #include "mojo/public/cpp/bindings/interface_ptr.h"
 #include "mojo/public/cpp/bindings/interface_ptr_info.h"
@@ -57,7 +58,7 @@ namespace mojo {
 // types of threads callers may need to provide different waiter
 // implementations.
 template <typename Interface>
-class Binding : public ErrorHandler {
+class Binding {
  public:
   // Constructs an incomplete binding that will use the implementation |impl|.
   // The binding may be completed with a subsequent call to the |Bind| method.
@@ -100,7 +101,7 @@ class Binding : public ErrorHandler {
 
   // Tears down the binding, closing the message pipe and leaving the interface
   // implementation unbound.
-  ~Binding() override {
+  ~Binding() {
     if (internal_router_) {
       Close();
     }
@@ -120,7 +121,8 @@ class Binding : public ErrorHandler {
     internal_router_ =
         new internal::Router(handle.Pass(), filters.Pass(), waiter);
     internal_router_->set_incoming_receiver(&stub_);
-    internal_router_->set_error_handler(this);
+    internal_router_->set_connection_error_handler(
+        [this]() { connection_error_handler_.Run(); });

[qsr, 2015/06/11 09:37:53]

I'm missing something. Why do you need to do this, why not just having
set_connection_error_handler delegate to the router?

[yzshen1, 2015/06/11 16:51:45]

The current code matches the previous behavior:
* set_connection_error_handler() can be called at any time, before or after
|internal_router_| is created;
* the error handler is preserved after Close() and Unbind().

I understand that this is not consistent with what we do with InterfacePtr. But
I think it is beyond the scope of this CL to try to unify their behavior.

What do you think? Thanks!

   }
 
   // Completes a binding that was constructed with only an interface
@@ -181,14 +183,17 @@ class Binding : public ErrorHandler {
 
   // Sets an error handler that will be called if a connection error occurs on
   // the bound message pipe.
-  void set_error_handler(ErrorHandler* error_handler) {
-    error_handler_ = error_handler;
+  void set_connection_error_handler(const Closure& error_handler) {
+    connection_error_handler_ = error_handler;
   }
 
-  // Implements the |Binding|'s response to a connection error.
-  void OnConnectionError() override {
-    if (error_handler_)
-      error_handler_->OnConnectionError();
+  // Similar to the method above but uses the ErrorHandler interface instead of
+  // a callback.
+  // NOTE: Deprecated. Please use the method above.
+  // TODO(yzshen): Remove this method once all callsites are converted.
+  void set_error_handler(ErrorHandler* error_handler) {
+    set_connection_error_handler(
+        [error_handler]() { error_handler->OnConnectionError(); });
   }
 
   // Returns the interface implementation that was previously specified. Caller
@@ -213,7 +218,7 @@ class Binding : public ErrorHandler {
 
  private:
   void DestroyRouter() {
-    internal_router_->set_error_handler(nullptr);
+    internal_router_->set_connection_error_handler(Closure());
     delete internal_router_;
     internal_router_ = nullptr;
   }
@@ -221,7 +226,7 @@ class Binding : public ErrorHandler {
   internal::Router* internal_router_ = nullptr;
   typename Interface::Stub_ stub_;
   Interface* impl_;
-  ErrorHandler* error_handler_ = nullptr;
+  Closure connection_error_handler_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(Binding);
 };