Add ObjectProxy::CallMethodAndBlockWithErrorDetails methods

Currently, ObjectProxy::CallMethodAndBlock() returns NULL on
failure but the D-Bus error details are not passed to the caller.

Added a method that would return the D-Bus error information
to the caller and switched ObjectProxy::CallMethodAndBlock()
to use the new method instead.

BUG=chromium:414838
TEST=Build Chrome
TEST=ninja -C out/Debug dbus_unittests && out/Debug/dbus_unittests

Review URL: https://codereview.chromium.org/563763004

Cr-Commit-Position: refs/heads/master@{#295212}

4 files changed, 70 insertions, 11 deletions

diff --git a/dbus/mock_object_proxy.h b/dbus/mock_object_proxy.h
index fcad860..b5900ad 100644
--- a/dbus/mock_object_proxy.h
+++ b/dbus/mock_object_proxy.h
@@ -25,6 +25,17 @@ class MockObjectProxy : public ObjectProxy {
   // uncopyable. This is a workaround which defines |MockCallMethodAndBlock| as
   // a mock method and makes |CallMethodAndBlock| call the mocked method.
   // Use |MockCallMethodAndBlock| for setting/testing expectations.
+  MOCK_METHOD3(MockCallMethodAndBlockWithErrorDetails,
+               Response*(MethodCall* method_call,
+                         int timeout_ms,
+                         ScopedDBusError* error));
+  virtual scoped_ptr<Response> CallMethodAndBlockWithErrorDetails(
+      MethodCall* method_call,
+      int timeout_ms,
+      ScopedDBusError* error) OVERRIDE {
+    return scoped_ptr<Response>(
+        MockCallMethodAndBlockWithErrorDetails(method_call, timeout_ms, error));
+  }
   MOCK_METHOD2(MockCallMethodAndBlock, Response*(MethodCall* method_call,
                                                  int timeout_ms));
   virtual scoped_ptr<Response> CallMethodAndBlock(MethodCall* method_call,
diff --git a/dbus/mock_unittest.cc b/dbus/mock_unittest.cc
index c1e0f9b..bfdc5a30 100644
--- a/dbus/mock_unittest.cc
+++ b/dbus/mock_unittest.cc
@@ -13,6 +13,7 @@
 #include "dbus/mock_exported_object.h"
 #include "dbus/mock_object_proxy.h"
 #include "dbus/object_path.h"
+#include "dbus/scoped_dbus_error.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -44,6 +45,10 @@ class MockTest : public testing::Test {
     // CreateMockProxyResponse() to return responses.
     EXPECT_CALL(*mock_proxy_.get(), MockCallMethodAndBlock(_, _))
         .WillRepeatedly(Invoke(this, &MockTest::CreateMockProxyResponse));
+    EXPECT_CALL(*mock_proxy_.get(),
+                MockCallMethodAndBlockWithErrorDetails(_, _, _))
+        .WillRepeatedly(
+            Invoke(this, &MockTest::CreateMockProxyResponseWithErrorDetails));
 
     // Set an expectation so mock_proxy's CallMethod() will use
     // HandleMockProxyResponseWithMessageLoop() to return responses.
@@ -104,6 +109,12 @@ class MockTest : public testing::Test {
     return NULL;
   }
 
+  Response* CreateMockProxyResponseWithErrorDetails(
+      MethodCall* method_call, int timeout_ms, ScopedDBusError* error) {
+    dbus_set_error(error->get(), DBUS_ERROR_NOT_SUPPORTED, "Not implemented");
+    return NULL;
+  }
+
   // Creates a response and runs the given response callback in the
   // message loop with the response. Used to implement for |mock_proxy_|.
   void HandleMockProxyResponseWithMessageLoop(
@@ -127,7 +138,7 @@ class MockTest : public testing::Test {
   }
 };
 
-// This test demonstrates how to mock a synchronos method call using the
+// This test demonstrates how to mock a synchronous method call using the
 // mock classes.
 TEST_F(MockTest, CallMethodAndBlock) {
   const char kHello[] = "Hello";
@@ -155,7 +166,29 @@ TEST_F(MockTest, CallMethodAndBlock) {
   EXPECT_EQ(kHello, text_message);
 }
 
-// This test demonstrates how to mock an asynchronos method call using the
+TEST_F(MockTest, CallMethodAndBlockWithErrorDetails) {
+  // Get an object proxy from the mock bus.
+  ObjectProxy* proxy = mock_bus_->GetObjectProxy(
+      "org.chromium.TestService",
+      ObjectPath("/org/chromium/TestObject"));
+
+  // Create a method call.
+  MethodCall method_call("org.chromium.TestInterface", "Echo");
+
+  ScopedDBusError error;
+  // Call the method.
+  scoped_ptr<Response> response(
+      proxy->CallMethodAndBlockWithErrorDetails(
+          &method_call, ObjectProxy::TIMEOUT_USE_DEFAULT, &error));
+
+  // Check the response.
+  ASSERT_FALSE(response.get());
+  ASSERT_TRUE(error.is_set());
+  EXPECT_STREQ(DBUS_ERROR_NOT_SUPPORTED, error.name());
+  EXPECT_STREQ("Not implemented", error.message());
+}
+
+// This test demonstrates how to mock an asynchronous method call using the
 // mock classes.
 TEST_F(MockTest, CallMethod) {
   const char kHello[] = "hello";
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index 016425d..59584f2 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -66,8 +66,8 @@ ObjectProxy::~ObjectProxy() {
 // Originally we tried to make |method_call| a const reference, but we
 // gave up as dbus_connection_send_with_reply_and_block() takes a
 // non-const pointer of DBusMessage as the second parameter.
-scoped_ptr<Response> ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
-                                                     int timeout_ms) {
+scoped_ptr<Response> ObjectProxy::CallMethodAndBlockWithErrorDetails(
+    MethodCall* method_call, int timeout_ms, ScopedDBusError* error) {
   bus_->AssertOnDBusThread();
 
   if (!bus_->Connect() ||
@@ -77,12 +77,10 @@ scoped_ptr<Response> ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
 
   DBusMessage* request_message = method_call->raw_message();
 
-  ScopedDBusError error;
-
   // Send the message synchronously.
   const base::TimeTicks start_time = base::TimeTicks::Now();
   DBusMessage* response_message =
-      bus_->SendWithReplyAndBlock(request_message, timeout_ms, error.get());
+      bus_->SendWithReplyAndBlock(request_message, timeout_ms, error->get());
   // Record if the method call is successful, or not. 1 if successful.
   UMA_HISTOGRAM_ENUMERATION("DBus.SyncMethodCallSuccess",
                             response_message ? 1 : 0,
@@ -94,8 +92,8 @@ scoped_ptr<Response> ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
   if (!response_message) {
     LogMethodCallFailure(method_call->GetInterface(),
                          method_call->GetMember(),
-                         error.is_set() ? error.name() : "unknown error type",
-                         error.is_set() ? error.message() : "");
+                         error->is_set() ? error->name() : "unknown error type",
+                         error->is_set() ? error->message() : "");
     return scoped_ptr<Response>();
   }
   // Record time spent for the method call. Don't include failures.
@@ -105,6 +103,12 @@ scoped_ptr<Response> ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
   return Response::FromRawMessage(response_message);
 }
 
+scoped_ptr<Response> ObjectProxy::CallMethodAndBlock(MethodCall* method_call,
+                                                     int timeout_ms) {
+  ScopedDBusError error;
+  return CallMethodAndBlockWithErrorDetails(method_call, timeout_ms, &error);
+}
+
 void ObjectProxy::CallMethod(MethodCall* method_call,
                              int timeout_ms,
                              ResponseCallback callback) {
diff --git a/dbus/object_proxy.h b/dbus/object_proxy.h
index 2d4ea68..1400328 100644
--- a/dbus/object_proxy.h
+++ b/dbus/object_proxy.h
@@ -25,13 +25,14 @@ class Bus;
 class ErrorResponse;
 class MethodCall;
 class Response;
+class ScopedDBusError;
 class Signal;
 
 // ObjectProxy is used to communicate with remote objects, mainly for
 // calling methods of these objects.
 //
 // ObjectProxy is a ref counted object, to ensure that |this| of the
-// object is is alive when callbacks referencing |this| are called; the
+// object is alive when callbacks referencing |this| are called; the
 // bus always holds at least one of those references so object proxies
 // always last as long as the bus that created them.
 class CHROME_DBUS_EXPORT ObjectProxy
@@ -91,6 +92,16 @@ class CHROME_DBUS_EXPORT ObjectProxy
       OnConnectedCallback;
 
   // Calls the method of the remote object and blocks until the response
+  // is returned. Returns NULL on error with the error details specified
+  // in the |error| object.
+  //
+  // BLOCKING CALL.
+  virtual scoped_ptr<Response> CallMethodAndBlockWithErrorDetails(
+      MethodCall* method_call,
+      int timeout_ms,
+      ScopedDBusError* error);
+
+  // Calls the method of the remote object and blocks until the response
   // is returned. Returns NULL on error.
   //
   // BLOCKING CALL.
@@ -305,7 +316,7 @@ class CHROME_DBUS_EXPORT ObjectProxy
 
   const bool ignore_service_unknown_errors_;
 
-  // Known name owner of the well-known bus name represnted by |service_name_|.
+  // Known name owner of the well-known bus name represented by |service_name_|.
   std::string service_name_owner_;
 
   DISALLOW_COPY_AND_ASSIGN(ObjectProxy);