CPP bindings: DCHECK when a Callback is destructed without being invoked

mojo/public/cpp/bindings/binding.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_BINDING_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_BINDING_H_
 
 #include "mojo/public/c/environment/async_waiter.h"
 #include "mojo/public/cpp/bindings/error_handler.h"
 #include "mojo/public/cpp/bindings/interface_ptr.h"
@@ skipping 82 matching lines @@
   Binding(Interface* impl,
           InterfaceRequest<Interface> request,
           const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter())
       : Binding(impl) {
     Bind(request.PassMessagePipe(), waiter);
   }
 
   // Tears down the binding, closing the message pipe and leaving the interface
   // implementation unbound.
   ~Binding() override {
+    stub_.Close();

[rudominer, 2015/03/13 23:52:36]

I was confused here. Closing the stub here makes little sense because we are
about to destroy the stub.

     if (internal_router_) {
       Close();
     }
   }
 
   // Completes a binding that was constructed with only an interface
   // implementation.  Takes ownership of |handle| and binds it to the previously
   // specified implementation. See class comment for definition of |waiter|.
   void Bind(
       ScopedMessagePipeHandle handle,
@@ skipping 36 matching lines @@
   // Blocks the calling thread until either a call arrives on the previously
   // bound message pipe, or an error occurs.
   bool WaitForIncomingMethodCall() {
     MOJO_DCHECK(internal_router_);
     return internal_router_->WaitForIncomingMessage();
   }
 
   // Closes the message pipe that was previously bound. Put this object into a
   // state where it can be rebound to a new pipe.
   void Close() {
+    stub_.Close();
     MOJO_DCHECK(internal_router_);
     internal_router_->CloseMessagePipe();
     DestroyRouter();
   }
 
   // Unbinds the underlying pipe from this binding and returns it so it can be
   // used in another context, such as on another thread or with a different
   // implementation. Put this object into a state where it can be rebound to a
   // new pipe.
   InterfaceRequest<Interface> Unbind() {
     InterfaceRequest<Interface> request =
         MakeRequest<Interface>(internal_router_->PassMessagePipe());
     DestroyRouter();
     // TODO(vtl): The |.Pass()| below is only needed due to an MSVS bug; remove
     // it once that's fixed.
     return request.Pass();
   }
 
   // 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;
   }
 
   // Implements the |Binding|'s response to a connection error.
   void OnConnectionError() override {
+    // Note that it is important that we tell |stub_| that there was a
+    // connection error before we tell |error_handler_|. This is because
+    // in response |error_handler_| may destroy a Callback that hasn't been
+    // run and this will cause |stub_| to DCHECK unless it also knows that
+    // this happened for a legitimate reason, namely that a connection error
+    // occurred.
+    stub_.OnConnectionError();
     if (error_handler_)
       error_handler_->OnConnectionError();
   }
 
   // Returns the interface implementation that was previously specified. Caller
   // does not take ownership.
   Interface* impl() { return impl_; }
 
   // Indicates whether the binding has been completed (i.e., whether a message
   // pipe has been bound to the implementation).
@@ skipping 13 matching lines @@
   typename Interface::Stub_ stub_;
   Interface* impl_;
   ErrorHandler* error_handler_ = nullptr;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(Binding);
 };
 
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_BINDING_H_