Minor cleanups and improvements for the D-Bus library.

- 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

6 files changed, 35 insertions, 15 deletions

diff --git a/dbus/bus.cc b/dbus/bus.cc
index a259bc0..550f390 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc
@@ -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_) {
@@ -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
@@ -400,7 +400,7 @@ bool Bus::SetUpAsyncOperations() {
       this,
       NULL);
 
-  async_operations_are_set_up_ = true;
+  async_operations_set_up_ = true;
 
   return true;
 }
@@ -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_,
@@ -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_,
diff --git a/dbus/bus.h b/dbus/bus.h
index a11dd9b..2f8d215 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h
@@ -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:
@@ -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.
diff --git a/dbus/dbus.gyp b/dbus/dbus.gyp
index c17aecf..7aadb7e 100644
--- a/dbus/dbus.gyp
+++ b/dbus/dbus.gyp
@@ -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',
       ],
diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc
index cf22a41..33e785d 100644
--- a/dbus/end_to_end_async_unittest.cc
+++ b/dbus/end_to_end_async_unittest.cc
@@ -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);
 
@@ -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
diff --git a/dbus/end_to_end_sync_unittest.cc b/dbus/end_to_end_sync_unittest.cc
index fe0489a..87b261f 100644
--- a/dbus/end_to_end_sync_unittest.cc
+++ b/dbus/end_to_end_sync_unittest.cc
@@ -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));
@@ -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();
diff --git a/dbus/mock_unittest.cc b/dbus/mock_unittest.cc
index f0289cf..022b03d 100644
--- a/dbus/mock_unittest.cc
+++ b/dbus/mock_unittest.cc
@@ -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;
@@ -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.