Modify Binding<>, StrongBinding<>, and AssociatedBinding<> to enforce that an error handler callback is only set after binding to a message pipe.

Also, reset the callback when the binding unbound from the message pipe.
This replicates the behaviour of InterfacePtr<>.

BUG=569694

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

Cr-Commit-Position: refs/heads/master@{#365749}

4 files changed, 28 insertions, 6 deletions

diff --git a/mojo/public/cpp/bindings/associated_binding.h b/mojo/public/cpp/bindings/associated_binding.h
index dd40291..bf6d2f9 100644
--- a/mojo/public/cpp/bindings/associated_binding.h
+++ b/mojo/public/cpp/bindings/associated_binding.h
@@ -92,6 +92,7 @@ class AssociatedBinding {
   void Close() {
     DCHECK(endpoint_client_);
     endpoint_client_.reset();
+    connection_error_handler_.reset();
   }
 
   // Unbinds and returns the associated interface request so it can be
@@ -105,12 +106,18 @@ class AssociatedBinding {
         &request, endpoint_client_->PassHandle());
 
     endpoint_client_.reset();
+    connection_error_handler_.reset();
 
     return request.Pass();
   }
 
   // Sets an error handler that will be called if a connection error occurs.
+  //
+  // This method may only be called after this AssociatedBinding has been bound
+  // to a message pipe. The error handler will be reset when this
+  // AssociatedBinding is unbound or closed.
   void set_connection_error_handler(const Closure& error_handler) {
+    DCHECK(is_bound());
     connection_error_handler_ = error_handler;
   }
 
diff --git a/mojo/public/cpp/bindings/binding.h b/mojo/public/cpp/bindings/binding.h
index a15536c..c0a8be5 100644
--- a/mojo/public/cpp/bindings/binding.h
+++ b/mojo/public/cpp/bindings/binding.h
@@ -195,7 +195,12 @@ class Binding {
 
   // Sets an error handler that will be called if a connection error occurs on
   // the bound message pipe.
+  //
+  // This method may only be called after this Binding has been bound to a
+  // message pipe. The error handler will be reset when this Binding is unbound
+  // or closed.
   void set_connection_error_handler(const Closure& error_handler) {
+    DCHECK(is_bound());
     internal_state_.set_connection_error_handler(error_handler);
   }
 
diff --git a/mojo/public/cpp/bindings/lib/binding_state.h b/mojo/public/cpp/bindings/lib/binding_state.h
index dea3a6b..496dcf6 100644
--- a/mojo/public/cpp/bindings/lib/binding_state.h
+++ b/mojo/public/cpp/bindings/lib/binding_state.h
@@ -91,6 +91,7 @@ class BindingState<Interface, false> {
   }
 
   void set_connection_error_handler(const Closure& error_handler) {
+    DCHECK(is_bound());
     connection_error_handler_ = error_handler;
   }
 
@@ -115,6 +116,7 @@ class BindingState<Interface, false> {
     router_->set_connection_error_handler(Closure());
     delete router_;
     router_ = nullptr;
+    connection_error_handler_.reset();
   }
 
   internal::Router* router_ = nullptr;
@@ -179,6 +181,7 @@ class BindingState<Interface, true> {
     endpoint_client_.reset();
     router_->CloseMessagePipe();
     router_ = nullptr;
+    connection_error_handler_.reset();
   }
 
   InterfaceRequest<GenericInterface> Unbind() {
@@ -186,10 +189,12 @@ class BindingState<Interface, true> {
     InterfaceRequest<GenericInterface> request =
         MakeRequest<GenericInterface>(router_->PassMessagePipe());
     router_ = nullptr;
+    connection_error_handler_.reset();
     return request.Pass();
   }
 
   void set_connection_error_handler(const Closure& error_handler) {
+    DCHECK(is_bound());
     connection_error_handler_ = error_handler;
   }
 
diff --git a/mojo/public/cpp/bindings/strong_binding.h b/mojo/public/cpp/bindings/strong_binding.h
index 860260e..863bca1 100644
--- a/mojo/public/cpp/bindings/strong_binding.h
+++ b/mojo/public/cpp/bindings/strong_binding.h
@@ -48,16 +48,14 @@ class StrongBinding {
   MOJO_MOVE_ONLY_TYPE(StrongBinding)
 
  public:
-  explicit StrongBinding(Interface* impl) : binding_(impl) {
-    binding_.set_connection_error_handler([this]() { OnConnectionError(); });
-  }
+  explicit StrongBinding(Interface* impl) : binding_(impl) {}
 
   StrongBinding(
       Interface* impl,
       ScopedMessagePipeHandle handle,
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter())
       : StrongBinding(impl) {
-    binding_.Bind(handle.Pass(), waiter);
+    Bind(handle.Pass(), waiter);
   }
 
   StrongBinding(
@@ -65,7 +63,7 @@ class StrongBinding {
       InterfacePtr<Interface>* ptr,
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter())
       : StrongBinding(impl) {
-    binding_.Bind(ptr, waiter);
+    Bind(ptr, waiter);
   }
 
   StrongBinding(
@@ -73,7 +71,7 @@ class StrongBinding {
       InterfaceRequest<Interface> request,
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter())
       : StrongBinding(impl) {
-    binding_.Bind(request.Pass(), waiter);
+    Bind(request.Pass(), waiter);
   }
 
   ~StrongBinding() {}
@@ -83,6 +81,7 @@ class StrongBinding {
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
     assert(!binding_.is_bound());
     binding_.Bind(handle.Pass(), waiter);
+    binding_.set_connection_error_handler([this]() { OnConnectionError(); });
   }
 
   void Bind(
@@ -90,6 +89,7 @@ class StrongBinding {
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
     assert(!binding_.is_bound());
     binding_.Bind(ptr, waiter);
+    binding_.set_connection_error_handler([this]() { OnConnectionError(); });
   }
 
   void Bind(
@@ -97,6 +97,7 @@ class StrongBinding {
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
     assert(!binding_.is_bound());
     binding_.Bind(request.Pass(), waiter);
+    binding_.set_connection_error_handler([this]() { OnConnectionError(); });
   }
 
   bool WaitForIncomingMethodCall() {
@@ -104,7 +105,11 @@ class StrongBinding {
   }
 
   // Note: The error handler must not delete the interface implementation.
+  //
+  // This method may only be called after this StrongBinding has been bound to a
+  // message pipe.
   void set_connection_error_handler(const Closure& error_handler) {
+    assert(binding_.is_bound());
     connection_error_handler_ = error_handler;
   }