diff options
-rw-r--r-- | chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc | 24 | ||||
-rw-r--r-- | chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h | 10 | ||||
-rw-r--r-- | chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc | 55 | ||||
-rw-r--r-- | dbus/bus.h | 8 | ||||
-rw-r--r-- | dbus/end_to_end_async_unittest.cc | 18 | ||||
-rw-r--r-- | dbus/exported_object.cc | 40 | ||||
-rw-r--r-- | dbus/exported_object.h | 19 | ||||
-rw-r--r-- | dbus/test_service.cc | 48 | ||||
-rw-r--r-- | dbus/test_service.h | 16 |
9 files changed, 175 insertions, 63 deletions
diff --git a/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc b/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc index 921cf80..f003ea0 100644 --- a/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc +++ b/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.cc @@ -224,8 +224,9 @@ bool ProxyResolutionServiceProvider::OnOriginThread() { return base::PlatformThread::CurrentId() == origin_thread_id_; } -dbus::Response* ProxyResolutionServiceProvider::ResolveProxyHandler( - dbus::MethodCall* method_call) { +void ProxyResolutionServiceProvider::ResolveProxyHandler( + dbus::MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender) { DCHECK(OnOriginThread()); VLOG(1) << "Handing method call: " << method_call->ToString(); // The method call should contain the three string parameters. @@ -237,7 +238,8 @@ dbus::Response* ProxyResolutionServiceProvider::ResolveProxyHandler( !reader.PopString(&signal_interface) || !reader.PopString(&signal_name)) { LOG(ERROR) << "Unexpected method call: " << method_call->ToString(); - return NULL; + response_sender.Run(NULL); + return; } resolver_->ResolveProxy(source_url, @@ -245,21 +247,23 @@ dbus::Response* ProxyResolutionServiceProvider::ResolveProxyHandler( signal_name, exported_object_); - // Return an empty response for now. We'll send a signal once the - // network proxy resolution is completed. + // Send an empty response for now. We'll send a signal once the network proxy + // resolution is completed. dbus::Response* response = dbus::Response::FromMethodCall(method_call); - return response; + response_sender.Run(response); } // static -dbus::Response* ProxyResolutionServiceProvider::CallResolveProxyHandler( +void ProxyResolutionServiceProvider::CallResolveProxyHandler( base::WeakPtr<ProxyResolutionServiceProvider> provider_weak_ptr, - dbus::MethodCall* method_call) { + dbus::MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender) { if (!provider_weak_ptr) { LOG(WARNING) << "Called after the object is deleted"; - return NULL; + response_sender.Run(NULL); + return; } - return provider_weak_ptr->ResolveProxyHandler(method_call); + provider_weak_ptr->ResolveProxyHandler(method_call, response_sender); } ProxyResolutionServiceProvider* ProxyResolutionServiceProvider::Create() { diff --git a/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h b/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h index 5a26358..383c082 100644 --- a/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h +++ b/chrome/browser/chromeos/dbus/proxy_resolution_service_provider.h @@ -15,9 +15,9 @@ #include "base/synchronization/lock.h" #include "base/threading/platform_thread.h" #include "chrome/browser/chromeos/dbus/cros_dbus_service.h" +#include "dbus/exported_object.h" namespace dbus { -class ExportedObject; class MethodCall; class Response; } @@ -104,13 +104,15 @@ class ProxyResolutionServiceProvider // Callback to be invoked when ChromeOS clients send network proxy // resolution requests to the service running in chrome executable. // Called on UI thread from dbus request. - dbus::Response* ResolveProxyHandler(dbus::MethodCall* method_call); + void ResolveProxyHandler(dbus::MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender); // Calls ResolveProxyHandler() if weak_ptr is not NULL. Used to ensure a // safe shutdown. - static dbus::Response* CallResolveProxyHandler( + static void CallResolveProxyHandler( base::WeakPtr<ProxyResolutionServiceProvider> weak_ptr, - dbus::MethodCall* method_call); + dbus::MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender); // Returns true if the current thread is on the origin thread. bool OnOriginThread(); diff --git a/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc b/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc index 93b2781..bbd2a4d 100644 --- a/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc +++ b/chrome/browser/chromeos/dbus/proxy_resolution_service_provider_unittest.cc @@ -17,6 +17,7 @@ #include "base/bind.h" #include "base/logging.h" #include "base/memory/ref_counted.h" +#include "base/message_loop.h" #include "dbus/message.h" #include "dbus/mock_bus.h" #include "dbus/mock_exported_object.h" @@ -61,7 +62,8 @@ class ProxyResolutionServiceProviderTest : public testing::Test { public: ProxyResolutionServiceProviderTest() : signal_received_successfully_(false), - mock_resolver_(NULL) { + mock_resolver_(NULL), + response_received_(false) { } virtual void SetUp() { @@ -102,12 +104,12 @@ class ProxyResolutionServiceProviderTest : public testing::Test { kLibCrosServiceName, kLibCrosServicePath); // |mock_object_proxy_|'s CallMethodAndBlock() will use - // CreateResponse() to return responses. + // MockCallMethodAndBlock() to return responses. EXPECT_CALL(*mock_object_proxy_, CallMethodAndBlock(_, _)) .WillOnce(Invoke( this, - &ProxyResolutionServiceProviderTest::CreateResponse)); + &ProxyResolutionServiceProviderTest::MockCallMethodAndBlock)); // |mock_object_proxy_|'s ConnectToSignal will use // MockConnectToSignal(). EXPECT_CALL(*mock_object_proxy_, @@ -185,6 +187,9 @@ class ProxyResolutionServiceProviderTest : public testing::Test { scoped_ptr<ProxyResolutionServiceProvider> proxy_resolution_service_; dbus::ExportedObject::MethodCallCallback resolve_network_proxy_; dbus::ObjectProxy::SignalCallback on_signal_callback_; + MessageLoop message_loop_; + bool response_received_; + scoped_ptr<dbus::Response> response_; private: // Behaves as |mock_exported_object_|'s ExportMethod(). @@ -237,24 +242,40 @@ class ProxyResolutionServiceProviderTest : public testing::Test { LOG(ERROR) << "Unexpected source URL: " << source_url; } - // Creates a response for |mock_object_proxy_|. - dbus::Response* CreateResponse( + // Calls exported method and waits for a response for |mock_object_proxy_|. + dbus::Response* MockCallMethodAndBlock( dbus::MethodCall* method_call, Unused) { - if (method_call->GetInterface() == - kLibCrosServiceInterface && - method_call->GetMember() == kResolveNetworkProxy) { - // Set the serial number to non-zero, so - // dbus_message_new_method_return() won't emit a warning. - method_call->SetSerial(1); - // Run the callback captured in MockExportMethod(). This will send - // a signal, which will be received by |on_signal_callback_|. - dbus::Response* response = resolve_network_proxy_.Run(method_call); - return response; + if (method_call->GetInterface() != kLibCrosServiceInterface || + method_call->GetMember() != kResolveNetworkProxy) { + LOG(ERROR) << "Unexpected method call: " << method_call->ToString(); + return NULL; } + // Set the serial number to non-zero, so + // dbus_message_new_method_return() won't emit a warning. + method_call->SetSerial(1); + // Run the callback captured in MockExportMethod(). In addition to returning + // a response that the caller will ignore, this will send a signal, which + // will be received by |on_signal_callback_|. + resolve_network_proxy_.Run( + method_call, + base::Bind(&ProxyResolutionServiceProviderTest::OnResponse, + base::Unretained(this))); + // Wait for a response. + while (!response_received_) { + message_loop_.Run(); + } + // Return response. + return response_.release(); + } - LOG(ERROR) << "Unexpected method call: " << method_call->ToString(); - return NULL; + // Receives a response and makes it available to MockCallMethodAndBlock(). + void OnResponse(dbus::Response* response) { + response_.reset(response); + response_received_ = true; + if (message_loop_.is_running()) { + message_loop_.Quit(); + } } // Behaves as |mock_object_proxy_|'s ConnectToSignal(). @@ -100,11 +100,15 @@ class ObjectProxy; // // Exporting a method: // -// Response* Echo(dbus::MethodCall* method_call) { +// void Echo(dbus::MethodCall* method_call, +// dbus::ExportedObject::ResponseSender response_sender) { // // Do something with method_call. // Response* response = Response::FromMethodCall(method_call); // // Build response here. -// return response; +// // Can send an immediate response here to implement a synchronous service +// // or store the response_sender and send a response later to implement an +// // asynchronous service. +// response_sender.Run(response); // } // // void OnExported(const std::string& interface_name, diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc index 2f486f4..5c98fe9 100644 --- a/dbus/end_to_end_async_unittest.cc +++ b/dbus/end_to_end_async_unittest.cc @@ -223,6 +223,24 @@ TEST_F(EndToEndAsyncTest, Timeout) { ASSERT_EQ("", response_strings_[0]); } +// Tests calling a method that sends its reply asynchronously. +TEST_F(EndToEndAsyncTest, AsyncEcho) { + const char* kHello = "hello"; + + // Create the method call. + dbus::MethodCall method_call("org.chromium.TestInterface", "AsyncEcho"); + dbus::MessageWriter writer(&method_call); + writer.AppendString(kHello); + + // Call the method. + const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT; + CallMethod(&method_call, timeout_ms); + + // Check the response. + WaitForResponses(1); + EXPECT_EQ(kHello, response_strings_[0]); +} + TEST_F(EndToEndAsyncTest, NonexistentMethod) { dbus::MethodCall method_call("org.chromium.TestInterface", "Nonexistent"); diff --git a/dbus/exported_object.cc b/dbus/exported_object.cc index e4a1641..de25168 100644 --- a/dbus/exported_object.cc +++ b/dbus/exported_object.cc @@ -224,12 +224,13 @@ DBusHandlerResult ExportedObject::HandleMessage( method_call.release(), start_time)); } else { - // If the D-Bus thread is not used, just call the method directly. We - // don't need the complicated logic to wait for the method call to be - // complete. - // |response| will be deleted in OnMethodCompleted(). - Response* response = iter->second.Run(method_call.get()); - OnMethodCompleted(method_call.release(), response, start_time); + // If the D-Bus thread is not used, just call the method directly. + MethodCall* released_method_call = method_call.release(); + iter->second.Run(released_method_call, + base::Bind(&ExportedObject::SendResponse, + this, + start_time, + released_method_call)); } // It's valid to say HANDLED here, and send a method response at a later @@ -241,14 +242,27 @@ void ExportedObject::RunMethod(MethodCallCallback method_call_callback, MethodCall* method_call, base::TimeTicks start_time) { bus_->AssertOnOriginThread(); + method_call_callback.Run(method_call, + base::Bind(&ExportedObject::SendResponse, + this, + start_time, + method_call)); +} - Response* response = method_call_callback.Run(method_call); - bus_->PostTaskToDBusThread(FROM_HERE, - base::Bind(&ExportedObject::OnMethodCompleted, - this, - method_call, - response, - start_time)); +void ExportedObject::SendResponse(base::TimeTicks start_time, + MethodCall* method_call, + Response* response) { + DCHECK(method_call); + if (bus_->HasDBusThread()) { + bus_->PostTaskToDBusThread(FROM_HERE, + base::Bind(&ExportedObject::OnMethodCompleted, + this, + method_call, + response, + start_time)); + } else { + OnMethodCompleted(method_call, response, start_time); + } } void ExportedObject::OnMethodCompleted(MethodCall* method_call, diff --git a/dbus/exported_object.h b/dbus/exported_object.h index bc67bcd..7ac2f88 100644 --- a/dbus/exported_object.h +++ b/dbus/exported_object.h @@ -38,9 +38,15 @@ class ExportedObject : public base::RefCountedThreadSafe<ExportedObject> { const std::string& service_name, const std::string& object_path); + // Called to send a response from an exported method. Response* is the + // response message. Callers should pass a NULL Response* in the event + // of an error that prevents the sending of a response. + typedef base::Callback<void (Response*)> ResponseSender; + // Called when an exported method is called. MethodCall* is the request - // message. - typedef base::Callback<Response* (MethodCall*)> MethodCallCallback; + // message. ResponseSender is the callback that should be used to send a + // response. + typedef base::Callback<void (MethodCall*, ResponseSender)> MethodCallCallback; // Called when method exporting is done. // Parameters: @@ -124,7 +130,14 @@ class ExportedObject : public base::RefCountedThreadSafe<ExportedObject> { MethodCall* method_call, base::TimeTicks start_time); - // Called on completion of the method run from RunMethod(). + // Callback invoked by service provider to send a response to a method call. + // Can be called immediately from a MethodCallCallback to implement a + // synchronous service or called later to implement an asynchronous service. + void SendResponse(base::TimeTicks start_time, + MethodCall* method_call, + Response* response); + + // Called on completion of the method run from SendResponse(). // Takes ownership of |method_call| and |response|. void OnMethodCompleted(MethodCall* method_call, Response* response, diff --git a/dbus/test_service.cc b/dbus/test_service.cc index b0207b0..ea8e360 100644 --- a/dbus/test_service.cc +++ b/dbus/test_service.cc @@ -13,8 +13,8 @@ namespace dbus { -// Echo, SlowEcho, BrokenMethod. -const int TestService::kNumMethodsToExport = 3; +// Echo, SlowEcho, AsyncEcho, BrokenMethod. +const int TestService::kNumMethodsToExport = 4; TestService::Options::Options() { } @@ -148,6 +148,15 @@ void TestService::Run(MessageLoop* message_loop) { exported_object_->ExportMethod( "org.chromium.TestInterface", + "AsyncEcho", + base::Bind(&TestService::AsyncEcho, + base::Unretained(this)), + base::Bind(&TestService::OnExported, + base::Unretained(this))); + ++num_methods; + + exported_object_->ExportMethod( + "org.chromium.TestInterface", "BrokenMethod", base::Bind(&TestService::BrokenMethod, base::Unretained(this)), @@ -163,25 +172,44 @@ void TestService::Run(MessageLoop* message_loop) { message_loop->Run(); } -Response* TestService::Echo(MethodCall* method_call) { +void TestService::Echo(MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender) { MessageReader reader(method_call); std::string text_message; - if (!reader.PopString(&text_message)) - return NULL; + if (!reader.PopString(&text_message)) { + response_sender.Run(NULL); + return; + } Response* response = Response::FromMethodCall(method_call); MessageWriter writer(response); writer.AppendString(text_message); - return response; + response_sender.Run(response); } -Response* TestService::SlowEcho(MethodCall* method_call) { +void TestService::SlowEcho( + MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender) { base::PlatformThread::Sleep(TestTimeouts::tiny_timeout_ms()); - return Echo(method_call); + Echo(method_call, response_sender); +} + +void TestService::AsyncEcho( + MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender) { + // Schedule a call to Echo() to send an asynchronous response after we return. + message_loop()->PostDelayedTask(FROM_HERE, + base::Bind(&TestService::Echo, + base::Unretained(this), + method_call, + response_sender), + TestTimeouts::tiny_timeout_ms()); } -Response* TestService::BrokenMethod(MethodCall* method_call) { - return NULL; +void TestService::BrokenMethod( + MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender) { + response_sender.Run(NULL); } } // namespace dbus diff --git a/dbus/test_service.h b/dbus/test_service.h index 153613e..ea3e5ad 100644 --- a/dbus/test_service.h +++ b/dbus/test_service.h @@ -10,6 +10,7 @@ #include "base/memory/ref_counted.h" #include "base/threading/thread.h" #include "base/synchronization/waitable_event.h" +#include "dbus/exported_object.h" namespace base { class MessageLoopProxy; @@ -18,7 +19,6 @@ class MessageLoopProxy; namespace dbus { class Bus; -class ExportedObject; class MethodCall; class Response; @@ -89,14 +89,22 @@ class TestService : public base::Thread { // // Echos the text message received from the method call. - Response* Echo(MethodCall* method_call); + void Echo(MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender); // Echos the text message received from the method call, but sleeps for // TestTimeouts::tiny_timeout_ms() before returning the response. - Response* SlowEcho(MethodCall* method_call); + void SlowEcho(MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender); + + // Echos the text message received from the method call, but sends its + // response asynchronously after this callback has returned. + void AsyncEcho(MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender); // Returns NULL, instead of a valid Response. - Response* BrokenMethod(MethodCall* method_call); + void BrokenMethod(MethodCall* method_call, + dbus::ExportedObject::ResponseSender response_sender); scoped_refptr<base::MessageLoopProxy> dbus_thread_message_loop_proxy_; base::WaitableEvent on_all_methods_exported_; |