Mojo C++ bindings: add more CHECKs to investigate crashes.

BUG=725605,740044,741047

Change-Id: Ibeee3338455826284044df7695681af319547bad
Reviewed-on: https://chromium-review.googlesource.com/572380Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486938}

mojo/public/cpp/bindings/BUILD.gn

@@ -68,6 +68,7 @@ component("bindings") {
     "lib/control_message_handler.h",
     "lib/control_message_proxy.cc",
     "lib/control_message_proxy.h",
+    "lib/debug_util.h",
     "lib/deque.h",
     "lib/filter_chain.cc",
     "lib/fixed_buffer.cc",

mojo/public/cpp/bindings/interface_endpoint_client.h

@@ -26,6 +26,7 @@
 #include "mojo/public/cpp/bindings/filter_chain.h"
 #include "mojo/public/cpp/bindings/lib/control_message_handler.h"
 #include "mojo/public/cpp/bindings/lib/control_message_proxy.h"
+#include "mojo/public/cpp/bindings/lib/debug_util.h"
 #include "mojo/public/cpp/bindings/message.h"
 #include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
 
@@ -38,7 +39,8 @@ class InterfaceEndpointController;
 // endpoint, either the implementation side or the client side.
 // It should only be accessed and destructed on the creating thread.
 class MOJO_CPP_BINDINGS_EXPORT InterfaceEndpointClient
-    : NON_EXPORTED_BASE(public MessageReceiverWithResponder) {
+    : NON_EXPORTED_BASE(public MessageReceiverWithResponder),
+      NON_EXPORTED_BASE(private internal::LifeTimeTrackerForDebugging) {
  public:
   // |receiver| is okay to be null. If it is not null, it must outlive this
   // object.

mojo/public/cpp/bindings/lib/debug_util.h

+// Copyright 2017 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_DEBUG_UTIL_H_
+#define MOJO_PUBLIC_CPP_BINDINGS_LIB_DEBUG_UTIL_H_
+
+namespace mojo {
+namespace internal {
+
+// TODO(crbug.com/741047): Remove this after the bug is fixed.
+class LifeTimeTrackerForDebugging {
+ public:
+  LifeTimeTrackerForDebugging() {}
+
+  ~LifeTimeTrackerForDebugging() { state_ = DESTROYED; }
+
+  void CheckObjectIsValid() {
+    CHECK(this);
+
+    // This adress has been used to construct a LifeTimeTrackerForDebugging.
+    CHECK(state_ == ALIVE || state_ == DESTROYED);
+
+    // If the previous check succeeds but this fails, it is likely to be
+    // use-after-free problem.
+    CHECK_EQ(ALIVE, state_);
+  }
+
+ private:
+  enum MagicValue : uint64_t {
+    ALIVE = 0x1029384756afbecd,
+    DESTROYED = 0xdcebfa6574839201
+  };
+
+  uint64_t state_ = ALIVE;
+};
+
+}  // namespace internal
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_DEBUG_UTIL_H_

mojo/public/cpp/bindings/lib/interface_endpoint_client.cc

@@ -167,11 +167,14 @@ InterfaceEndpointClient::InterfaceEndpointClient(
 InterfaceEndpointClient::~InterfaceEndpointClient() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  // TODO(crbug.com/741047): Remove this.
+  // TODO(crbug.com/741047): Remove these checks.
+  CheckObjectIsValid();
   CHECK(task_runner_->RunsTasksInCurrentSequence());
 
-  if (controller_)
+  if (controller_) {
+    CHECK(handle_.group_controller());
     handle_.group_controller()->DetachEndpointClient(handle_);
+  }
 }
 
 AssociatedGroup* InterfaceEndpointClient::associated_group() {

mojo/public/cpp/bindings/lib/multiplex_router.cc

@@ -506,12 +506,16 @@ InterfaceEndpointController* MultiplexRouter::AttachEndpointClient(
 
 void MultiplexRouter::DetachEndpointClient(
     const ScopedInterfaceEndpointHandle& handle) {
+  // TODO(crbug.com/741047): Remove this check.
+  CheckObjectIsValid();
+
   const InterfaceId id = handle.id();
 
   DCHECK(IsValidInterfaceId(id));
 
   MayAutoLock locker(&lock_);
-  DCHECK(base::ContainsKey(endpoints_, id));
+  // TODO(crbug.com/741047): change this to DCEHCK.
+  CHECK(base::ContainsKey(endpoints_, id));
 
   InterfaceEndpoint* endpoint = endpoints_[id].get();
   endpoint->DetachClient();
@@ -668,6 +672,9 @@ bool MultiplexRouter::OnPeerAssociatedEndpointClosed(
 void MultiplexRouter::OnPipeConnectionError() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  // TODO(crbug.com/741047): Remove this check.
+  CheckObjectIsValid();
+
   scoped_refptr<MultiplexRouter> protector(this);
   MayAutoLock locker(&lock_);
 

mojo/public/cpp/bindings/lib/multiplex_router.h

@@ -25,6 +25,7 @@
 #include "mojo/public/cpp/bindings/connector.h"
 #include "mojo/public/cpp/bindings/filter_chain.h"
 #include "mojo/public/cpp/bindings/interface_id.h"
+#include "mojo/public/cpp/bindings/lib/debug_util.h"
 #include "mojo/public/cpp/bindings/lib/deque.h"
 #include "mojo/public/cpp/bindings/message_header_validator.h"
 #include "mojo/public/cpp/bindings/pipe_control_message_handler.h"
@@ -55,7 +56,8 @@ namespace internal {
 class MOJO_CPP_BINDINGS_EXPORT MultiplexRouter
     : NON_EXPORTED_BASE(public MessageReceiver),
       public AssociatedGroupController,
-      NON_EXPORTED_BASE(public PipeControlMessageHandlerDelegate) {
+      NON_EXPORTED_BASE(public PipeControlMessageHandlerDelegate),
+      NON_EXPORTED_BASE(private LifeTimeTrackerForDebugging) {
  public:
   enum Config {
     // There is only the master interface running on this router. Please note