Get rid of Error out param from SendWithReplyAndBlock.

To make it clearer what is input/output, and also let callers access
errors only for error cases.

BUG=1459945
TEST=Ran tryjob

Change-Id: I9ecb129fbf6c7ac142604e29585b7bc8a61dfca0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4666670
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1166444}

diff --git a/dbus/bus.cc b/dbus/bus.cc
index 31d7f04..2f24e01 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc

@@ -148,6 +148,11 @@
   base::WeakPtrFactory<Timeout> weak_ptr_factory_{this};
 };
 
+// Converts DBusError into dbus::Error.
+Error ToError(const internal::ScopedDBusError& error) {
+  return error.is_set() ? Error(error.name(), error.message()) : Error();
+}
+
 }  // namespace
 
 Bus::Options::Options()
@@ -630,11 +635,10 @@
   return true;
 }
 
-DBusMessage* Bus::SendWithReplyAndBlock(DBusMessage* request,
-                                        int timeout_ms,
-                                        Error* error) {
+base::expected<std::unique_ptr<Response>, Error> Bus::SendWithReplyAndBlock(
+    DBusMessage* request,
+    int timeout_ms) {
   DCHECK(connection_);
-  DCHECK(error);
   AssertOnDBusThread();
 
   base::ElapsedTimer elapsed;
@@ -644,9 +648,6 @@
   internal::ScopedDBusError dbus_error;
   DBusMessage* reply = dbus_connection_send_with_reply_and_block(
       connection_, request, timeout_ms, dbus_error.get());
-  if (dbus_error.is_set()) {
-    *error = Error(dbus_error.name(), dbus_error.message());
-  }
   constexpr base::TimeDelta kLongCall = base::Seconds(1);
   LOG_IF(WARNING, elapsed.Elapsed() >= kLongCall)
       << "Bus::SendWithReplyAndBlock took "
@@ -656,7 +657,11 @@
       << ", interface=" << dbus_message_get_interface(request)
       << ", member=" << dbus_message_get_member(request);
 
-  return reply;
+  if (!reply) {
+    return base::unexpected(ToError(dbus_error));
+  }
+
+  return base::ok(Response::FromRawMessage(reply));
 }
 
 void Bus::SendWithReply(DBusMessage* request,
@@ -921,22 +926,17 @@
     return "";
   }
 
-  Error error;
-  DBusMessage* response_message =
-      SendWithReplyAndBlock(get_name_owner_call.raw_message(),
-                            ObjectProxy::TIMEOUT_USE_DEFAULT, &error);
-  if (!response_message) {
+  auto result = SendWithReplyAndBlock(get_name_owner_call.raw_message(),
+                                      ObjectProxy::TIMEOUT_USE_DEFAULT);
+  if (!result.has_value()) {
     if (options == REPORT_ERRORS) {
-      LOG(ERROR) << "Failed to get name owner. Got " << error.name() << ": "
-                 << error.message();
+      LOG(ERROR) << "Failed to get name owner. Got " << result.error().name()
+                 << ": " << result.error().message();
     }
     return "";
   }
 
-  std::unique_ptr<Response> response(
-      Response::FromRawMessage(response_message));
-  MessageReader reader(response.get());
-
+  MessageReader reader(result->get());
   std::string service_owner;
   if (!reader.PopString(&service_owner))
     service_owner.clear();

diff --git a/dbus/bus.h b/dbus/bus.h
index 015b662..539a02f 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h

@@ -9,6 +9,7 @@
 #include <stdint.h>
 
 #include <map>
+#include <memory>
 #include <set>
 #include <string>
 #include <utility>
@@ -19,7 +20,9 @@
 #include "base/memory/ref_counted.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
+#include "base/types/expected.h"
 #include "dbus/dbus_export.h"
+#include "dbus/error.h"
 #include "dbus/object_path.h"
 
 namespace base {
@@ -28,10 +31,10 @@
 
 namespace dbus {
 
-class Error;
 class ExportedObject;
 class ObjectManager;
 class ObjectProxy;
+class Response;
 
 // Bus is used to establish a connection with D-Bus, create object
 // proxies, and export objects.
@@ -449,9 +452,8 @@
   // TODO(crbug.com/1459945): Use base::expected<void, Error> to return error.
   //
   // BLOCKING CALL.
-  virtual DBusMessage* SendWithReplyAndBlock(DBusMessage* request,
-                                             int timeout_ms,
-                                             Error* error);
+  virtual base::expected<std::unique_ptr<Response>, Error>
+  SendWithReplyAndBlock(DBusMessage* request, int timeout_ms);
 
   // Requests to send a message to the bus. The reply is handled with
   // |pending_call| at a later time.

diff --git a/dbus/mock_bus.h b/dbus/mock_bus.h
index 0f20393..55d088a 100644
--- a/dbus/mock_bus.h
+++ b/dbus/mock_bus.h

@@ -9,6 +9,7 @@
 
 #include "base/task/sequenced_task_runner.h"
 #include "dbus/bus.h"
+#include "dbus/message.h"
 #include "dbus/object_path.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -42,10 +43,10 @@
                                               ServiceOwnershipOptions options));
   MOCK_METHOD1(ReleaseOwnership, bool(const std::string& service_name));
   MOCK_METHOD0(SetUpAsyncOperations, bool());
-  MOCK_METHOD3(SendWithReplyAndBlock,
-               DBusMessage*(DBusMessage* request,
-                            int timeout_ms,
-                            Error* error));
+  MOCK_METHOD2(
+      SendWithReplyAndBlock,
+      base::expected<std::unique_ptr<Response>, Error>(DBusMessage* request,
+                                                       int timeout_ms));
   MOCK_METHOD3(SendWithReply, void(DBusMessage* request,
                                    DBusPendingCall** pending_call,
                                    int timeout_ms));

diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index 93183dd..9296cb2 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc

@@ -139,22 +139,19 @@
     return nullptr;
   }
 
-  DBusMessage* request_message = method_call->raw_message();
-
   // Send the message synchronously.
-  DBusMessage* response_message =
-      bus_->SendWithReplyAndBlock(request_message, timeout_ms, error);
-
+  auto result =
+      bus_->SendWithReplyAndBlock(method_call->raw_message(), timeout_ms);
   statistics::AddBlockingSentMethodCall(
       service_name_, method_call->GetInterface(), method_call->GetMember());
-
-  if (!response_message) {
+  if (!result.has_value()) {
     LogMethodCallFailure(method_call->GetInterface(), method_call->GetMember(),
-                         error->name(), error->message());
+                         result.error().name(), result.error().message());
+    *error = std::move(result.error());
     return nullptr;
   }
 
-  return Response::FromRawMessage(response_message);
+  return std::move(result.value());
 }
 
 std::unique_ptr<Response> ObjectProxy::CallMethodAndBlock(

diff --git a/dbus/scoped_dbus_error.h b/dbus/scoped_dbus_error.h
index 8b04182..258bf606 100644
--- a/dbus/scoped_dbus_error.h
+++ b/dbus/scoped_dbus_error.h

@@ -19,8 +19,8 @@
 
   DBusError* get() { return &error_; }
   bool is_set() const;
-  const char* name() { return error_.name; }
-  const char* message() { return error_.message; }
+  const char* name() const { return error_.name; }
+  const char* message() const { return error_.message; }
 
  private:
   DBusError error_;