Skip to content

Commit

Permalink
Minor cleanups and improvements for the D-Bus library.
Browse files Browse the repository at this point in the history
- Add mock_export_object.{cc,h} to dbus.gyp, which were missing.
- Add a comment about shutdown of Bus in bus.h.
- Update mock_unittest.cc to call ShutdownAndBlock().
- Replace DCHECKs with LOG(ERROR)s followed by early exit.
- Add virtual to SetUp() and TearDown() in tests.
- Renamed a member variable to make it clearer.

BUG=chromium:90036
TEST=dbus_unittests


Review URL: http://codereview.chromium.org/7745044

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98560 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
satorux@chromium.org committed Aug 27, 2011
1 parent e2dcc6b commit ea78b1e
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 15 deletions.
23 changes: 14 additions & 9 deletions dbus/bus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ Bus::Bus(const Options& options)
origin_loop_(MessageLoop::current()),
origin_thread_id_(base::PlatformThread::CurrentId()),
dbus_thread_id_(base::kInvalidThreadId),
async_operations_are_set_up_(false),
async_operations_set_up_(false),
num_pending_watches_(0),
num_pending_timeouts_(0) {
if (dbus_thread_) {
Expand Down Expand Up @@ -370,7 +370,7 @@ bool Bus::SetUpAsyncOperations() {
DCHECK(connection_);
AssertOnDBusThread();

if (async_operations_are_set_up_)
if (async_operations_set_up_)
return true;

// Process all the incoming data if any, so that OnDispatchStatus() will
Expand Down Expand Up @@ -400,7 +400,7 @@ bool Bus::SetUpAsyncOperations() {
this,
NULL);

async_operations_are_set_up_ = true;
async_operations_set_up_ = true;

return true;
}
Expand Down Expand Up @@ -500,9 +500,11 @@ bool Bus::TryRegisterObjectPath(const std::string& object_path,
DCHECK(connection_);
AssertOnDBusThread();

DCHECK(registered_object_paths_.find(object_path) ==
registered_object_paths_.end())
<< "Object path already registered: " << object_path;
if (registered_object_paths_.find(object_path) !=
registered_object_paths_.end()) {
LOG(ERROR) << "Object path already registered: " << object_path;
return false;
}

const bool success = dbus_connection_try_register_object_path(
connection_,
Expand All @@ -519,9 +521,12 @@ void Bus::UnregisterObjectPath(const std::string& object_path) {
DCHECK(connection_);
AssertOnDBusThread();

DCHECK(registered_object_paths_.find(object_path) !=
registered_object_paths_.end())
<< "Requested to unregister an unknown object path: " << object_path;
if (registered_object_paths_.find(object_path) ==
registered_object_paths_.end()) {
LOG(ERROR) << "Requested to unregister an unknown object path: "
<< object_path;
return;
}

const bool success = dbus_connection_unregister_object_path(
connection_,
Expand Down
8 changes: 7 additions & 1 deletion dbus/bus.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ class ObjectProxy;
// call). To err on the safe side, we consider all libdbus functions that
// deal with the connection to dbus-damoen to be blocking.
//
// SHUTDOWN
//
// The Bus object must be shut down manually by Shutdown() or
// ShutdownAndBlock(). We require the manual shutdown as we should not
// issue blocking calls in the destructor.
//
// EXAMPLE USAGE:
//
// Synchronous method call:
Expand Down Expand Up @@ -448,7 +454,7 @@ class Bus : public base::RefCountedThreadSafe<Bus> {
scoped_refptr<dbus::ExportedObject> > ExportedObjectTable;
ExportedObjectTable exported_object_table_;

bool async_operations_are_set_up_;
bool async_operations_set_up_;

// Counters to make sure that OnAddWatch()/OnRemoveWatch() and
// OnAddTimeout()/OnRemoveTimeou() are balanced.
Expand Down
2 changes: 2 additions & 0 deletions dbus/dbus.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
'sources': [
'mock_bus.cc',
'mock_bus.h',
'mock_exported_object.cc',
'mock_exported_object.h',
'mock_object_proxy.cc',
'mock_object_proxy.h',
],
Expand Down
4 changes: 2 additions & 2 deletions dbus/end_to_end_async_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class EndToEndAsyncTest : public testing::Test {
EndToEndAsyncTest() {
}

void SetUp() {
virtual void SetUp() {
// Make the main thread not to allow IO.
base::ThreadRestrictions::SetIOAllowed(false);

Expand Down Expand Up @@ -65,7 +65,7 @@ class EndToEndAsyncTest : public testing::Test {
message_loop_.Run();
}

void TearDown() {
virtual void TearDown() {
bus_->Shutdown(base::Bind(&EndToEndAsyncTest::OnShutdown,
base::Unretained(this)));
// Wait until the bus is shutdown. OnShutdown() will be called in
Expand Down
4 changes: 2 additions & 2 deletions dbus/end_to_end_sync_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class EndToEndSyncTest : public testing::Test {
EndToEndSyncTest() {
}

void SetUp() {
virtual void SetUp() {
// Start the test service;
dbus::TestService::Options options;
test_service_.reset(new dbus::TestService(options));
Expand All @@ -36,7 +36,7 @@ class EndToEndSyncTest : public testing::Test {
ASSERT_FALSE(client_bus_->HasDBusThread());
}

void TearDown() {
virtual void TearDown() {
test_service_->Shutdown();
ASSERT_TRUE(test_service_->WaitUntilServiceIsShutdown());
test_service_->Stop();
Expand Down
9 changes: 8 additions & 1 deletion dbus/mock_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class MockTest : public testing::Test {
MockTest() {
}

void SetUp() {
virtual void SetUp() {
// Create a mock bus.
dbus::Bus::Options options;
options.bus_type = dbus::Bus::SYSTEM;
Expand Down Expand Up @@ -52,6 +52,13 @@ class MockTest : public testing::Test {
EXPECT_CALL(*mock_bus_, GetObjectProxy("org.chromium.TestService",
"/org/chromium/TestObject"))
.WillOnce(Return(mock_proxy_.get()));

// ShutdownAndBlock() will be called in TearDown().
EXPECT_CALL(*mock_bus_, ShutdownAndBlock()).WillOnce(Return());
}

virtual void TearDown() {
mock_bus_->ShutdownAndBlock();
}

// Called when the response is received.
Expand Down

0 comments on commit ea78b1e

Please sign in to comment.