summaryrefslogtreecommitdiffstats
path: root/dbus
diff options
context:
space:
mode:
authorharuki@chromium.org <haruki@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-11-14 11:02:59 +0000
committerharuki@chromium.org <haruki@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-11-14 11:02:59 +0000
commit6d36c0cadfc825db5336a04c1c0778ebe2652a92 (patch)
tree451eecc45061e3bc58c02deb20032a37d49c733f /dbus
parent8a2e513493bd5b0d2eb8e068975efc2e91fb5250 (diff)
downloadchromium_src-6d36c0cadfc825db5336a04c1c0778ebe2652a92.zip
chromium_src-6d36c0cadfc825db5336a04c1c0778ebe2652a92.tar.gz
chromium_src-6d36c0cadfc825db5336a04c1c0778ebe2652a92.tar.bz2
Make SignalSenderVerificationTest more robust
Add more assertions and a callback to check the result of RequestOwnership. Original test can hang when test_service2_ tries to acquire the ownership before D-Bus recognizes test_service_'s disconnection. In that situation, test_service2_ cannot own the name and cannot send a message. BUG=158689 TEST=unittests Review URL: https://chromiumcodereview.appspot.com/11358111 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167649 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'dbus')
-rw-r--r--dbus/object_proxy.cc23
-rw-r--r--dbus/object_proxy.h10
-rw-r--r--dbus/signal_sender_verification_unittest.cc105
-rw-r--r--dbus/test_service.cc29
-rw-r--r--dbus/test_service.h19
5 files changed, 153 insertions, 33 deletions
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index c8f00d7..5e9bf5b 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -408,6 +408,12 @@ void ObjectProxy::OnConnected(OnConnectedCallback on_connected_callback,
on_connected_callback.Run(interface_name, signal_name, success);
}
+void ObjectProxy::SetNameOwnerChangedCallback(SignalCallback callback) {
+ bus_->AssertOnOriginThread();
+
+ name_owner_changed_callback_ = callback;
+}
+
DBusHandlerResult ObjectProxy::HandleMessage(
DBusConnection* connection,
DBusMessage* raw_message) {
@@ -430,7 +436,7 @@ DBusHandlerResult ObjectProxy::HandleMessage(
if (path.value() == kDbusSystemObjectPath &&
signal->GetMember() == "NameOwnerChanged") {
// Handle NameOwnerChanged separately
- return HandleNameOwnerChanged(signal.get());
+ return HandleNameOwnerChanged(signal.Pass());
}
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
@@ -619,7 +625,8 @@ void ObjectProxy::UpdateNameOwnerAndBlock() {
service_name_owner_.clear();
}
-DBusHandlerResult ObjectProxy::HandleNameOwnerChanged(Signal* signal) {
+DBusHandlerResult ObjectProxy::HandleNameOwnerChanged(
+ scoped_ptr<Signal> signal) {
DCHECK(signal);
bus_->AssertOnDBusThread();
@@ -627,13 +634,23 @@ DBusHandlerResult ObjectProxy::HandleNameOwnerChanged(Signal* signal) {
if (signal->GetMember() == "NameOwnerChanged" &&
signal->GetInterface() == "org.freedesktop.DBus" &&
signal->GetSender() == "org.freedesktop.DBus") {
- MessageReader reader(signal);
+ MessageReader reader(signal.get());
std::string name, old_owner, new_owner;
if (reader.PopString(&name) &&
reader.PopString(&old_owner) &&
reader.PopString(&new_owner) &&
name == service_name_) {
service_name_owner_ = new_owner;
+ if (!name_owner_changed_callback_.is_null()) {
+ const base::TimeTicks start_time = base::TimeTicks::Now();
+ Signal* released_signal = signal.release();
+ bus_->PostTaskToOriginThread(FROM_HERE,
+ base::Bind(&ObjectProxy::RunMethod,
+ this,
+ start_time,
+ name_owner_changed_callback_,
+ released_signal));
+ }
return DBUS_HANDLER_RESULT_HANDLED;
}
}
diff --git a/dbus/object_proxy.h b/dbus/object_proxy.h
index bec0894..801c577 100644
--- a/dbus/object_proxy.h
+++ b/dbus/object_proxy.h
@@ -142,6 +142,11 @@ class CHROME_DBUS_EXPORT ObjectProxy
SignalCallback signal_callback,
OnConnectedCallback on_connected_callback);
+ // Sets a callback for "NameOwnerChanged" signal. The callback is called on
+ // the origin thread when D-Bus system sends "NameOwnerChanged" for the name
+ // represented by |service_name_|.
+ virtual void SetNameOwnerChangedCallback(SignalCallback callback);
+
// Detaches from the remote object. The Bus object will take care of
// detaching so you don't have to do this manually.
//
@@ -254,7 +259,7 @@ class CHROME_DBUS_EXPORT ObjectProxy
void UpdateNameOwnerAndBlock();
// Handles NameOwnerChanged signal from D-Bus's special message bus.
- DBusHandlerResult HandleNameOwnerChanged(dbus::Signal* signal);
+ DBusHandlerResult HandleNameOwnerChanged(scoped_ptr<dbus::Signal> signal);
scoped_refptr<Bus> bus_;
std::string service_name_;
@@ -268,6 +273,9 @@ class CHROME_DBUS_EXPORT ObjectProxy
typedef std::map<std::string, SignalCallback> MethodTable;
MethodTable method_table_;
+ // The callback called when NameOwnerChanged signal is received.
+ SignalCallback name_owner_changed_callback_;
+
std::set<std::string> match_rules_;
const bool ignore_service_unknown_errors_;
diff --git a/dbus/signal_sender_verification_unittest.cc b/dbus/signal_sender_verification_unittest.cc
index 91db93f..6e59ea4 100644
--- a/dbus/signal_sender_verification_unittest.cc
+++ b/dbus/signal_sender_verification_unittest.cc
@@ -20,7 +20,9 @@
// The test for sender verification in ObjectProxy.
class SignalSenderVerificationTest : public testing::Test {
public:
- SignalSenderVerificationTest() {
+ SignalSenderVerificationTest()
+ : on_name_owner_changed_called_(false),
+ on_ownership_called_(false) {
}
virtual void SetUp() {
@@ -35,21 +37,6 @@ class SignalSenderVerificationTest : public testing::Test {
thread_options.message_loop_type = MessageLoop::TYPE_IO;
ASSERT_TRUE(dbus_thread_->StartWithOptions(thread_options));
- // Start the test service, using the D-Bus thread.
- dbus::TestService::Options options;
- options.dbus_thread_message_loop_proxy = dbus_thread_->message_loop_proxy();
- test_service_.reset(new dbus::TestService(options));
- ASSERT_TRUE(test_service_->StartService());
- ASSERT_TRUE(test_service_->WaitUntilServiceIsStarted());
- ASSERT_TRUE(test_service_->HasDBusThread());
-
- // Same setup for the second TestService. This service should not have the
- // ownership of the name at this point.
- test_service2_.reset(new dbus::TestService(options));
- ASSERT_TRUE(test_service2_->StartService());
- ASSERT_TRUE(test_service2_->WaitUntilServiceIsStarted());
- ASSERT_TRUE(test_service2_->HasDBusThread());
-
// Create the client, using the D-Bus thread.
dbus::Bus::Options bus_options;
bus_options.bus_type = dbus::Bus::SESSION;
@@ -62,6 +49,10 @@ class SignalSenderVerificationTest : public testing::Test {
dbus::ObjectPath("/org/chromium/TestObject"));
ASSERT_TRUE(bus_->HasDBusThread());
+ object_proxy_->SetNameOwnerChangedCallback(
+ base::Bind(&SignalSenderVerificationTest::OnNameOwnerChanged,
+ base::Unretained(this)));
+
// Connect to the "Test" signal of "org.chromium.TestInterface" from
// the remote object.
object_proxy_->ConnectToSignal(
@@ -73,6 +64,29 @@ class SignalSenderVerificationTest : public testing::Test {
base::Unretained(this)));
// Wait until the object proxy is connected to the signal.
message_loop_.Run();
+
+ // Start the test service, using the D-Bus thread.
+ dbus::TestService::Options options;
+ options.dbus_thread_message_loop_proxy = dbus_thread_->message_loop_proxy();
+ test_service_.reset(new dbus::TestService(options));
+ ASSERT_TRUE(test_service_->StartService());
+ ASSERT_TRUE(test_service_->WaitUntilServiceIsStarted());
+ ASSERT_TRUE(test_service_->HasDBusThread());
+ ASSERT_TRUE(test_service_->has_ownership());
+
+ // Same setup for the second TestService. This service should not have the
+ // ownership of the name at this point.
+ test_service2_.reset(new dbus::TestService(options));
+ ASSERT_TRUE(test_service2_->StartService());
+ ASSERT_TRUE(test_service2_->WaitUntilServiceIsStarted());
+ ASSERT_TRUE(test_service2_->HasDBusThread());
+ ASSERT_FALSE(test_service2_->has_ownership());
+
+ // The name should be owned and known at this point.
+ if (!on_name_owner_changed_called_)
+ message_loop_.Run();
+ ASSERT_FALSE(latest_name_owner_.empty());
+
}
virtual void TearDown() {
@@ -91,6 +105,31 @@ class SignalSenderVerificationTest : public testing::Test {
test_service2_->Stop();
}
+ void OnOwnership(bool expected, bool success) {
+ ASSERT_EQ(expected, success);
+ // PostTask to quit the MessageLoop as this is called from D-Bus thread.
+ message_loop_.PostTask(
+ FROM_HERE,
+ base::Bind(&SignalSenderVerificationTest::OnOwnershipInternal,
+ base::Unretained(this)));
+ }
+
+ void OnOwnershipInternal() {
+ on_ownership_called_ = true;
+ message_loop_.Quit();
+ }
+
+ void OnNameOwnerChanged(dbus::Signal* signal) {
+ dbus::MessageReader reader(signal);
+ std::string name, old_owner, new_owner;
+ ASSERT_TRUE(reader.PopString(&name));
+ ASSERT_TRUE(reader.PopString(&old_owner));
+ ASSERT_TRUE(reader.PopString(&new_owner));
+ latest_name_owner_ = new_owner;
+ on_name_owner_changed_called_ = true;
+ message_loop_.Quit();
+ }
+
protected:
// Called when the "Test" signal is received, in the main thread.
@@ -123,6 +162,13 @@ class SignalSenderVerificationTest : public testing::Test {
scoped_ptr<dbus::TestService> test_service2_;
// Text message from "Test" signal.
std::string test_signal_string_;
+
+ // The known latest name owner of TestService. Updated in OnNameOwnerChanged.
+ std::string latest_name_owner_;
+
+ // Boolean flags to record callback calls.
+ bool on_name_owner_changed_called_;
+ bool on_ownership_called_;
};
TEST_F(SignalSenderVerificationTest, TestSignalAccepted) {
@@ -157,7 +203,7 @@ TEST_F(SignalSenderVerificationTest, TestSignalRejected) {
EXPECT_EQ(samples1->TotalCount() + 1, samples2->TotalCount());
}
-TEST_F(SignalSenderVerificationTest, DISABLED_TestOwnerChanged) {
+TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
const char kMessage[] = "hello, world";
// Send the test signal from the exported object.
@@ -167,9 +213,30 @@ TEST_F(SignalSenderVerificationTest, DISABLED_TestOwnerChanged) {
WaitForTestSignal();
ASSERT_EQ(kMessage, test_signal_string_);
- // Release and aquire the name ownership.
+ // Release and acquire the name ownership.
+ // latest_name_owner_ should be non empty as |test_service_| owns the name.
+ ASSERT_FALSE(latest_name_owner_.empty());
test_service_->ShutdownAndBlock();
- test_service2_->RequestOwnership();
+ // OnNameOwnerChanged will PostTask to quit the message loop.
+ message_loop_.Run();
+ // latest_name_owner_ should be empty as the owner is gone.
+ ASSERT_TRUE(latest_name_owner_.empty());
+
+ // Reset the flag as NameOwnerChanged is already received in setup.
+ on_name_owner_changed_called_ = false;
+ test_service2_->RequestOwnership(
+ base::Bind(&SignalSenderVerificationTest::OnOwnership,
+ base::Unretained(this), true));
+ // Both of OnNameOwnerChanged() and OnOwnership() should quit the MessageLoop,
+ // but there's no expected order of those 2 event.
+ message_loop_.Run();
+ if (!on_name_owner_changed_called_ || !on_ownership_called_)
+ message_loop_.Run();
+ ASSERT_TRUE(on_name_owner_changed_called_);
+ ASSERT_TRUE(on_ownership_called_);
+
+ // latest_name_owner_ becomes non empty as the new owner appears.
+ ASSERT_FALSE(latest_name_owner_.empty());
// Now the second service owns the name.
const char kNewMessage[] = "hello, new world";
diff --git a/dbus/test_service.cc b/dbus/test_service.cc
index 87c6e01..0d99d57 100644
--- a/dbus/test_service.cc
+++ b/dbus/test_service.cc
@@ -13,6 +13,13 @@
#include "dbus/object_path.h"
#include "dbus/property.h"
+namespace {
+
+void EmptyCallback(bool /* success */) {
+}
+
+} // namespace
+
namespace dbus {
// Echo, SlowEcho, AsyncEcho, BrokenMethod, GetAll, Get, Set.
@@ -95,7 +102,8 @@ void TestService::SendTestSignalFromRootInternal(const std::string& message) {
bus_->RequestOwnership("org.chromium.TestService",
base::Bind(&TestService::OnOwnership,
- base::Unretained(this)));
+ base::Unretained(this),
+ base::Bind(&EmptyCallback)));
// Use "/" just like dbus-send does.
ExportedObject* root_object =
@@ -103,22 +111,28 @@ void TestService::SendTestSignalFromRootInternal(const std::string& message) {
root_object->SendSignal(&signal);
}
-void TestService::RequestOwnership() {
+void TestService::RequestOwnership(base::Callback<void(bool)> callback) {
message_loop()->PostTask(
FROM_HERE,
base::Bind(&TestService::RequestOwnershipInternal,
- base::Unretained(this)));
+ base::Unretained(this),
+ callback));
}
-void TestService::RequestOwnershipInternal() {
+void TestService::RequestOwnershipInternal(
+ base::Callback<void(bool)> callback) {
bus_->RequestOwnership("org.chromium.TestService",
base::Bind(&TestService::OnOwnership,
- base::Unretained(this)));
+ base::Unretained(this),
+ callback));
}
-void TestService::OnOwnership(const std::string& service_name,
+void TestService::OnOwnership(base::Callback<void(bool)> callback,
+ const std::string& service_name,
bool success) {
+ has_ownership_ = success;
LOG_IF(ERROR, !success) << "Failed to own: " << service_name;
+ callback.Run(success);
}
void TestService::OnExported(const std::string& interface_name,
@@ -146,7 +160,8 @@ void TestService::Run(MessageLoop* message_loop) {
bus_->RequestOwnership("org.chromium.TestService",
base::Bind(&TestService::OnOwnership,
- base::Unretained(this)));
+ base::Unretained(this),
+ base::Bind(&EmptyCallback)));
exported_object_ = bus_->GetExportedObject(
dbus::ObjectPath("/org/chromium/TestObject"));
diff --git a/dbus/test_service.h b/dbus/test_service.h
index 17f27ca..30f8216 100644
--- a/dbus/test_service.h
+++ b/dbus/test_service.h
@@ -66,7 +66,12 @@ class TestService : public base::Thread {
void SendTestSignalFromRoot(const std::string& message);
// Request the ownership of a well-known name "TestService".
- void RequestOwnership();
+ // |callback| will be called with the result when an ownership request is
+ // completed.
+ void RequestOwnership(base::Callback<void(bool)> callback);
+
+ // Returns whether this instance has the name ownership or not.
+ bool has_ownership() const { return has_ownership_; }
private:
// Helper function for SendTestSignal().
@@ -79,7 +84,12 @@ class TestService : public base::Thread {
void ShutdownAndBlockInternal();
// Called when an ownership request is completed.
- void OnOwnership(const std::string& service_name,
+ // |callback| is the callback to be called with the result. |service_name| is
+ // the requested well-known bus name. |callback| and |service_name| are bound
+ // when the service requests the ownership. |success| is the result of the
+ // completed request, and is propagated to |callback|.
+ void OnOwnership(base::Callback<void(bool)> callback,
+ const std::string& service_name,
bool success);
// Called when a method is exported.
@@ -131,13 +141,16 @@ class TestService : public base::Thread {
void SendPropertyChangedSignalInternal(const std::string& name);
// Helper function for RequestOwnership().
- void RequestOwnershipInternal();
+ void RequestOwnershipInternal(base::Callback<void(bool)> callback);
scoped_refptr<base::MessageLoopProxy> dbus_thread_message_loop_proxy_;
base::WaitableEvent on_all_methods_exported_;
// The number of methods actually exported.
int num_exported_methods_;
+ // True iff this instance has successfully acquired the name ownership.
+ bool has_ownership_;
+
scoped_refptr<Bus> bus_;
ExportedObject* exported_object_;
};