From 6b571de0807eea51c863cf0aa18403ae6c18ba65 Mon Sep 17 00:00:00 2001 From: avakulenko Date: Tue, 16 Sep 2014 18:44:09 -0700 Subject: 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} --- dbus/mock_object_proxy.h | 11 +++++++++++ dbus/mock_unittest.cc | 37 +++++++++++++++++++++++++++++++++++-- dbus/object_proxy.cc | 18 +++++++++++------- dbus/object_proxy.h | 15 +++++++++++++-- 4 files changed, 70 insertions(+), 11 deletions(-) (limited to 'dbus') 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 CallMethodAndBlockWithErrorDetails( + MethodCall* method_call, + int timeout_ms, + ScopedDBusError* error) OVERRIDE { + return scoped_ptr( + MockCallMethodAndBlockWithErrorDetails(method_call, timeout_ms, error)); + } MOCK_METHOD2(MockCallMethodAndBlock, Response*(MethodCall* method_call, int timeout_ms)); virtual scoped_ptr 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( + 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 ObjectProxy::CallMethodAndBlock(MethodCall* method_call, - int timeout_ms) { +scoped_ptr ObjectProxy::CallMethodAndBlockWithErrorDetails( + MethodCall* method_call, int timeout_ms, ScopedDBusError* error) { bus_->AssertOnDBusThread(); if (!bus_->Connect() || @@ -77,12 +77,10 @@ scoped_ptr 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 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(); } // Record time spent for the method call. Don't include failures. @@ -105,6 +103,12 @@ scoped_ptr ObjectProxy::CallMethodAndBlock(MethodCall* method_call, return Response::FromRawMessage(response_message); } +scoped_ptr 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 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); -- cgit v1.1