dbus: CHECK whether ReplyCallbackHolder lose callback

Add CHECK() along the dbus call code path where ReplyCallbackHolder
is used to find our whether response callback is lost somehow.

And move DEBUG_ALIAS_FOR_CSTR before the LOG(FATAL) crash.

Bug: 1211451
Change-Id: I7f29313892f400e8b88a37453950a9791e052998
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2967203
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893439}

dbus/object_proxy.cc

@@ -113,6 +113,10 @@ ObjectProxy::ReplyCallbackHolder::ReleaseCallback() {
   return std::move(callback_);
 }
 
+bool ObjectProxy::ReplyCallbackHolder::IsNullCallback() const {
+  return callback_.is_null();
+}
+
 ObjectProxy::ObjectProxy(Bus* bus,
                          const std::string& service_name,
                          const ObjectPath& object_path,
@@ -200,6 +204,8 @@ void ObjectProxy::CallMethodWithErrorResponse(
 
   ReplyCallbackHolder callback_holder(bus_->GetOriginTaskRunner(),
                                       std::move(callback));
+  // TODO(http://crbug/1211451): Remove after fix.
+  CHECK(!callback_holder.IsNullCallback());
 
   if (!method_call->SetDestination(service_name_) ||
       !method_call->SetPath(object_path_)) {
@@ -345,6 +351,9 @@ void ObjectProxy::StartAsyncMethodCall(int timeout_ms,
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
 
+  // TODO(http://crbug/1211451): Remove after fix.
+  CHECK(!callback_holder.IsNullCallback());
+
   if (!bus_->Connect() || !bus_->SetUpAsyncOperations()) {
     // In case of a failure, run the error callback with nullptr.
     base::OnceClosure task =
@@ -387,6 +396,9 @@ void ObjectProxy::OnPendingCallIsComplete(ReplyCallbackHolder callback_holder,
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
 
+  // TODO(http://crbug/1211451): Remove after fix.
+  CHECK(!callback_holder.IsNullCallback());
+
   DBusMessage* response_message = dbus_pending_call_steal_reply(pending_call);
 
   // Either |response| or |error_response| takes ownership of the
@@ -440,6 +452,9 @@ void ObjectProxy::RunResponseOrErrorCallback(
     base::TimeTicks start_time,
     Response* response,
     ErrorResponse* error_response) {
+  // TODO(http://crbug/1211451): Remove after fix.
+  CHECK(!callback_holder.IsNullCallback());
+
   bus_->AssertOnOriginThread();
   callback_holder.ReleaseCallback().Run(response, error_response);
 
@@ -634,13 +649,13 @@ void ObjectProxy::OnCallMethod(const std::string& interface_name,
                                ErrorResponse* error_response) {
   // Crash on null `response_callback` with details of the call.
   // TODO(http://crbug/1211451): Remove after fix.
+  DEBUG_ALIAS_FOR_CSTR(interface_name_copy, interface_name.c_str(), 64);
+  DEBUG_ALIAS_FOR_CSTR(method_name_copy, method_name.c_str(), 64);
+  DEBUG_ALIAS_FOR_CSTR(object_path_copy, object_path_.value().c_str(), 64);
   LOG_IF(FATAL, response_callback.is_null())
       << "Null response_callback"
       << ", method:" << interface_name << "." << method_name
       << ", obj=" << object_path_.value();
-  DEBUG_ALIAS_FOR_CSTR(interface_name_copy, interface_name.c_str(), 64);
-  DEBUG_ALIAS_FOR_CSTR(method_name_copy, method_name.c_str(), 64);
-  DEBUG_ALIAS_FOR_CSTR(object_path_copy, object_path_.value().c_str(), 64);
 
   if (response) {
     // Method call was successful.

dbus/object_proxy.h

@@ -246,6 +246,10 @@ class CHROME_DBUS_EXPORT ObjectProxy
     // This must be called on the origin thread.
     ResponseOrErrorCallback ReleaseCallback();
 
+    // Whether |callback_| is null.
+    // TODO(http://crbug/1211451): Remove after fix.
+    bool IsNullCallback() const;
+
    private:
     scoped_refptr<base::SequencedTaskRunner> origin_task_runner_;
     ResponseOrErrorCallback callback_;