Misc refactoring of SessionManagerClient.

dbus/object_proxy.cc

 // Copyright (c) 2012 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.
 
 #include "dbus/object_proxy.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 118 matching lines @@
 void ObjectProxy::CallMethodWithErrorCallback(MethodCall* method_call,
                                               int timeout_ms,
                                               ResponseCallback callback,
                                               ErrorCallback error_callback) {
   bus_->AssertOnOriginThread();
 
   const base::TimeTicks start_time = base::TimeTicks::Now();
 
   if (!method_call->SetDestination(service_name_) ||
       !method_call->SetPath(object_path_)) {
-    // In case of a failure, run the error callback with NULL.
-    DBusMessage* response_message = NULL;
+    // In case of a failure, run the error callback with nullptr.
     base::Closure task = base::Bind(&ObjectProxy::RunResponseCallback,
                                     this,
                                     callback,
                                     error_callback,
                                     start_time,
-                                    response_message);
+                                    nullptr /* response_message */);
     bus_->GetOriginTaskRunner()->PostTask(FROM_HERE, task);
     return;
   }
 
   // Increment the reference count so we can safely reference the
   // underlying request message until the method call is complete. This
   // will be unref'ed in StartAsyncMethodCall().
   DBusMessage* request_message = method_call->raw_message();
   dbus_message_ref(request_message);
 
@@ skipping 83 matching lines @@
     ObjectProxy* in_object_proxy,
     ResponseCallback in_response_callback,
     ErrorCallback in_error_callback,
     base::TimeTicks in_start_time)
     : object_proxy(in_object_proxy),
       response_callback(in_response_callback),
       error_callback(in_error_callback),
       start_time(in_start_time) {
 }
 
-ObjectProxy::OnPendingCallIsCompleteData::~OnPendingCallIsCompleteData() {
-}
+ObjectProxy::OnPendingCallIsCompleteData::~OnPendingCallIsCompleteData()
+    = default;
 
 void ObjectProxy::StartAsyncMethodCall(int timeout_ms,
                                        DBusMessage* request_message,
                                        ResponseCallback response_callback,
                                        ErrorCallback error_callback,
                                        base::TimeTicks start_time) {
   bus_->AssertOnDBusThread();
 
   if (!bus_->Connect() || !bus_->SetUpAsyncOperations()) {
-    // In case of a failure, run the error callback with NULL.
-    DBusMessage* response_message = NULL;
+    // In case of a failure, run the error callback with nullptr.
     base::Closure task = base::Bind(&ObjectProxy::RunResponseCallback,
                                     this,
                                     response_callback,
                                     error_callback,
                                     start_time,
-                                    response_message);
+                                    nullptr /* response_message */);
     bus_->GetOriginTaskRunner()->PostTask(FROM_HERE, task);
 
     dbus_message_unref(request_message);
     return;
   }
 
-  DBusPendingCall* pending_call = NULL;
-
+  DBusPendingCall* pending_call = nullptr;
   bus_->SendWithReply(request_message, &pending_call, timeout_ms);
 
   // Prepare the data we'll be passing to OnPendingCallIsCompleteThunk().
   // The data will be deleted in OnPendingCallIsCompleteThunk().
   OnPendingCallIsCompleteData* data =
       new OnPendingCallIsCompleteData(this, response_callback, error_callback,
                                       start_time);
 
   // This returns false only when unable to allocate memory.
   const bool success = dbus_pending_call_set_notify(
@@ skipping 30 matching lines @@
 
 void ObjectProxy::RunResponseCallback(ResponseCallback response_callback,
                                       ErrorCallback error_callback,
                                       base::TimeTicks start_time,
                                       DBusMessage* response_message) {
   bus_->AssertOnOriginThread();
 
   bool method_call_successful = false;
   if (!response_message) {
     // The response is not received.
-    error_callback.Run(NULL);
+    error_callback.Run(nullptr);
   } else if (dbus_message_get_type(response_message) ==
              DBUS_MESSAGE_TYPE_ERROR) {
     // This will take |response_message| and release (unref) it.
     std::unique_ptr<ErrorResponse> error_response(
         ErrorResponse::FromRawMessage(response_message));
     error_callback.Run(error_response.get());
     // Delete the message  on the D-Bus thread. See below for why.
     bus_->GetDBusTaskRunner()->PostTask(
         FROM_HERE,
         base::Bind(&base::DeletePointer<ErrorResponse>,
@@ skipping 241 matching lines @@
   if (error_name == kErrorObjectUnknown)
     LOG(WARNING) << msg.str();
   else
     LOG(ERROR) << msg.str();
 }
 
 void ObjectProxy::OnCallMethodError(const std::string& interface_name,
                                     const std::string& method_name,
                                     ResponseCallback response_callback,
                                     ErrorResponse* error_response) {
+  std::string error_name;
+  std::string error_message;
   if (error_response) {
     // Error message may contain the error message as string.
+    error_name = error_response->GetErrorName();
     MessageReader reader(error_response);
-    std::string error_message;
     reader.PopString(&error_message);
-    LogMethodCallFailure(interface_name,
-                         method_name,
-                         error_response->GetErrorName(),
-                         error_message);
+  } else {
+    error_name = "(noname)";

[hashimoto, 2017/05/19 11:02:46]

How about making this more readable? (e.g. "No error response")
Usually, an error name looks like "org.freedesktop.DBus.Error.Failed" so it's
distinguishable from other errors even when you use a readable string.

[hidehiko, 2017/05/19 12:32:04]

Used "unknown error type" to be consistent with L100.

Or, we may want to pass empty string here, and handle it in LogMethodCallFailure
function? WDYT?

+    error_message = "(nomessage)";

[hashimoto, 2017/05/19 11:02:46]

error_massage is empty when error_response is non-null, but contains no string.
How about also leaving this empty to be consistent?

[hidehiko, 2017/05/19 12:32:03]

Done.

   }
-  response_callback.Run(NULL);
+  LogMethodCallFailure(
+      interface_name, method_name, error_name, error_message);
+
+  response_callback.Run(nullptr);
 }
 
 bool ObjectProxy::AddMatchRuleWithCallback(
     const std::string& match_rule,
     const std::string& absolute_signal_name,
     SignalCallback signal_callback) {
   DCHECK(!match_rule.empty());
   DCHECK(!absolute_signal_name.empty());
   bus_->AssertOnDBusThread();
 
@@ skipping 97 matching lines @@
     bool service_is_available) {
   bus_->AssertOnOriginThread();
 
   std::vector<WaitForServiceToBeAvailableCallback> callbacks;
   callbacks.swap(wait_for_service_to_be_available_callbacks_);
   for (size_t i = 0; i < callbacks.size(); ++i)
     callbacks[i].Run(service_is_available);
 }
 
 }  // namespace dbus