Plug some of the leaks in dbus_unittests.

Also, remove the deprecated base::MessageLoop::Run() calls
everywhere in dbus unittests.

BUG=365669
R=armansito@chromium.org

Review URL: https://codereview.chromium.org/523623003

Cr-Commit-Position: refs/heads/master@{#293113}

dbus/end_to_end_async_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/stl_util.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/thread.h"
@@ -77,7 +78,8 @@ class EndToEndAsyncTest : public testing::Test {
         base::Bind(&EndToEndAsyncTest::OnConnected,
                    base::Unretained(this)));
     // Wait until the object proxy is connected to the signal.
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
 
     // Connect to the "Test2" signal of "org.chromium.TestInterface" from
     // the remote object. There was a bug where we were emitting error
@@ -92,7 +94,8 @@ class EndToEndAsyncTest : public testing::Test {
         base::Bind(&EndToEndAsyncTest::OnConnected,
                    base::Unretained(this)));
     // Wait until the object proxy is connected to the signal.
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
 
     // Create a second object proxy for the root object.
     root_object_proxy_ = bus_->GetObjectProxy("org.chromium.TestService",
@@ -109,7 +112,8 @@ class EndToEndAsyncTest : public testing::Test {
         base::Bind(&EndToEndAsyncTest::OnConnected,
                    base::Unretained(this)));
     // Wait until the root object proxy is connected to the signal.
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
   }
 
   virtual void TearDown() {
@@ -172,7 +176,8 @@ class EndToEndAsyncTest : public testing::Test {
   // Wait for the give number of responses.
   void WaitForResponses(size_t num_responses) {
     while (response_strings_.size() < num_responses) {
-      message_loop_.Run();
+      run_loop_.reset(new base::RunLoop);
+      run_loop_->Run();
     }
   }
 
@@ -188,13 +193,14 @@ class EndToEndAsyncTest : public testing::Test {
     } else {
       response_strings_.push_back(std::string());
     }
-    message_loop_.Quit();
+    run_loop_->Quit();
   };
 
   // Wait for the given number of errors.
   void WaitForErrors(size_t num_errors) {
     while (error_names_.size() < num_errors) {
-      message_loop_.Run();
+      run_loop_.reset(new base::RunLoop);
+      run_loop_->Run();
     }
   }
 
@@ -208,7 +214,7 @@ class EndToEndAsyncTest : public testing::Test {
     } else {
       error_names_.push_back(std::string());
     }
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when the "Test" signal is received, in the main thread.
@@ -216,7 +222,7 @@ class EndToEndAsyncTest : public testing::Test {
   void OnTestSignal(Signal* signal) {
     MessageReader reader(signal);
     ASSERT_TRUE(reader.PopString(&test_signal_string_));
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when the "Test" signal is received, in the main thread, by
@@ -225,13 +231,13 @@ class EndToEndAsyncTest : public testing::Test {
   void OnRootTestSignal(Signal* signal) {
     MessageReader reader(signal);
     ASSERT_TRUE(reader.PopString(&root_test_signal_string_));
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when the "Test2" signal is received, in the main thread.
   void OnTest2Signal(Signal* signal) {
     MessageReader reader(signal);
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when connected to the signal.
@@ -239,22 +245,24 @@ class EndToEndAsyncTest : public testing::Test {
                    const std::string& signal_name,
                    bool success) {
     ASSERT_TRUE(success);
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when the connection with dbus-daemon is disconnected.
   void OnDisconnected() {
-    message_loop_.Quit();
+    run_loop_->Quit();
     ++on_disconnected_call_count_;
   }
 
   // Wait for the hey signal to be received.
   void WaitForTestSignal() {
     // OnTestSignal() will quit the message loop.
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
   }
 
   base::MessageLoop message_loop_;
+  scoped_ptr<base::RunLoop> run_loop_;
   std::vector<std::string> response_strings_;
   std::vector<std::string> error_names_;
   scoped_ptr<base::Thread> dbus_thread_;
@@ -537,10 +545,11 @@ TEST_F(EndToEndAsyncTest, EmptyResponseCallback) {
                             timeout_ms,
                             ObjectProxy::EmptyResponseCallback());
   // Post a delayed task to quit the message loop.
+  run_loop_.reset(new base::RunLoop);
   message_loop_.PostDelayedTask(FROM_HERE,
-                                base::MessageLoop::QuitClosure(),
+                                run_loop_->QuitClosure(),
                                 TestTimeouts::tiny_timeout());
-  message_loop_.Run();
+  run_loop_->Run();
   // We cannot tell if the empty callback is called, but at least we can
   // check if the test does not crash.
 }
@@ -584,7 +593,8 @@ TEST_F(EndToEndAsyncTest, DisconnectedSignal) {
                                       base::Bind(&Bus::ClosePrivateConnection,
                                                  base::Unretained(bus_.get())));
   // OnDisconnected callback quits message loop.
-  message_loop_.Run();
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
   EXPECT_EQ(1, on_disconnected_call_count_);
 }
 
@@ -608,7 +618,8 @@ class SignalMultipleHandlerTest : public EndToEndAsyncTest {
         base::Bind(&SignalMultipleHandlerTest::OnAdditionalConnected,
                    base::Unretained(this)));
     // Wait until the object proxy is connected to the signal.
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
   }
 
  protected:
@@ -617,7 +628,7 @@ class SignalMultipleHandlerTest : public EndToEndAsyncTest {
   void OnAdditionalTestSignal(Signal* signal) {
     MessageReader reader(signal);
     ASSERT_TRUE(reader.PopString(&additional_test_signal_string_));
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when connected to the signal.
@@ -625,7 +636,7 @@ class SignalMultipleHandlerTest : public EndToEndAsyncTest {
                              const std::string& signal_name,
                              bool success) {
     ASSERT_TRUE(success);
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Text message from "Test" signal delivered to additional handler.

dbus/mock_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "dbus/message.h"
 #include "dbus/mock_bus.h"
 #include "dbus/mock_exported_object.h"
@@ -72,12 +73,13 @@ class MockTest : public testing::Test {
       MessageReader reader(response);
       ASSERT_TRUE(reader.PopString(&response_string_));
     }
-    message_loop_.Quit();
+    run_loop_->Quit();
   };
 
  protected:
   std::string response_string_;
   base::MessageLoop message_loop_;
+  scoped_ptr<base::RunLoop> run_loop_;
   scoped_refptr<MockBus> mock_bus_;
   scoped_refptr<MockObjectProxy> mock_proxy_;
 
@@ -169,12 +171,13 @@ TEST_F(MockTest, CallMethod) {
   writer.AppendString(kHello);
 
   // Call the method.
+  run_loop_.reset(new base::RunLoop);
   proxy->CallMethod(&method_call,
                     ObjectProxy::TIMEOUT_USE_DEFAULT,
                     base::Bind(&MockTest::OnResponse,
                                base::Unretained(this)));
   // Run the message loop to let OnResponse be called.
-  message_loop_.Run();
+  run_loop_->Run();
 
   EXPECT_EQ(kHello, response_string_);
 }

dbus/object_manager_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "dbus/bus.h"
@@ -105,11 +106,13 @@ class ObjectManagerTest
     // Stopping a thread is considered an IO operation, so do this after
     // allowing IO.
     test_service_->Stop();
+
+    base::RunLoop().RunUntilIdle();
   }
 
   void MethodCallback(Response* response) {
     method_callback_called_ = true;
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
  protected:
@@ -117,21 +120,21 @@ class ObjectManagerTest
   virtual void ObjectAdded(const ObjectPath& object_path,
                            const std::string& interface_name) OVERRIDE {
     added_objects_.push_back(std::make_pair(object_path, interface_name));
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when an object is removed.
   virtual void ObjectRemoved(const ObjectPath& object_path,
                              const std::string& interface_name) OVERRIDE {
     removed_objects_.push_back(std::make_pair(object_path, interface_name));
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when a property value is updated.
   void OnPropertyChanged(const ObjectPath& object_path,
                          const std::string& name) {
     updated_properties_.push_back(name);
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   static const size_t kExpectedObjects = 1;
@@ -139,8 +142,10 @@ class ObjectManagerTest
 
   void WaitForObject() {
     while (added_objects_.size() < kExpectedObjects ||
-           updated_properties_.size() < kExpectedProperties)
-      message_loop_.Run();
+           updated_properties_.size() < kExpectedProperties) {
+      run_loop_.reset(new base::RunLoop);
+      run_loop_->Run();
+    }
     for (size_t i = 0; i < kExpectedObjects; ++i)
       added_objects_.erase(added_objects_.begin());
     for (size_t i = 0; i < kExpectedProperties; ++i)
@@ -148,14 +153,17 @@ class ObjectManagerTest
   }
 
   void WaitForRemoveObject() {
-    while (removed_objects_.size() < kExpectedObjects)
-      message_loop_.Run();
+    while (removed_objects_.size() < kExpectedObjects) {
+      run_loop_.reset(new base::RunLoop);
+      run_loop_->Run();
+    }
     for (size_t i = 0; i < kExpectedObjects; ++i)
       removed_objects_.erase(removed_objects_.begin());
   }
 
   void WaitForMethodCallback() {
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
     method_callback_called_ = false;
   }
 
@@ -177,6 +185,7 @@ class ObjectManagerTest
   }
 
   base::MessageLoop message_loop_;
+  scoped_ptr<base::RunLoop> run_loop_;
   scoped_ptr<base::Thread> dbus_thread_;
   scoped_refptr<Bus> bus_;
   ObjectManager* object_manager_;

dbus/property_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "dbus/bus.h"
@@ -105,20 +106,22 @@ class PropertyTest : public testing::Test {
   // waited for.
   void PropertyCallback(const std::string& id, bool success) {
     last_callback_ = id;
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
  protected:
   // Called when a property value is updated.
   void OnPropertyChanged(const std::string& name) {
     updated_properties_.push_back(name);
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Waits for the given number of updates.
   void WaitForUpdates(size_t num_updates) {
-    while (updated_properties_.size() < num_updates)
-      message_loop_.Run();
+    while (updated_properties_.size() < num_updates) {
+      run_loop_.reset(new base::RunLoop);
+      run_loop_->Run();
+    }
     for (size_t i = 0; i < num_updates; ++i)
       updated_properties_.erase(updated_properties_.begin());
   }
@@ -136,11 +139,13 @@ class PropertyTest : public testing::Test {
   // other; you can set this to whatever you wish.
   void WaitForCallback(const std::string& id) {
     while (last_callback_ != id) {
-      message_loop_.Run();
+      run_loop_.reset(new base::RunLoop);
+      run_loop_->Run();
     }
   }
 
   base::MessageLoop message_loop_;
+  scoped_ptr<base::RunLoop> run_loop_;
   scoped_ptr<base::Thread> dbus_thread_;
   scoped_refptr<Bus> bus_;
   ObjectProxy* object_proxy_;

dbus/signal_sender_verification_unittest.cc

@@ -8,6 +8,7 @@
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
+#include "base/run_loop.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_restrictions.h"
@@ -65,7 +66,8 @@ class SignalSenderVerificationTest : public testing::Test {
         base::Bind(&SignalSenderVerificationTest::OnConnected,
                    base::Unretained(this)));
     // Wait until the object proxy is connected to the signal.
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
 
     // Start the test service, using the D-Bus thread.
     TestService::Options options;
@@ -85,8 +87,10 @@ class SignalSenderVerificationTest : public testing::Test {
     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();
+    if (!on_name_owner_changed_called_) {
+      run_loop_.reset(new base::RunLoop);
+      run_loop_->Run();
+    }
     ASSERT_FALSE(latest_name_owner_.empty());
   }
 
@@ -117,7 +121,7 @@ class SignalSenderVerificationTest : public testing::Test {
 
   void OnOwnershipInternal() {
     on_ownership_called_ = true;
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   void OnNameOwnerChanged(bool* called_flag,
@@ -125,7 +129,7 @@ class SignalSenderVerificationTest : public testing::Test {
                           const std::string& new_owner) {
     latest_name_owner_ = new_owner;
     *called_flag = true;
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when the "Test" signal is received, in the main thread.
@@ -133,7 +137,7 @@ class SignalSenderVerificationTest : public testing::Test {
   void OnTestSignal(Signal* signal) {
     MessageReader reader(signal);
     ASSERT_TRUE(reader.PopString(&test_signal_string_));
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
   // Called when connected to the signal.
@@ -141,14 +145,15 @@ class SignalSenderVerificationTest : public testing::Test {
                    const std::string& signal_name,
                    bool success) {
     ASSERT_TRUE(success);
-    message_loop_.Quit();
+    run_loop_->Quit();
   }
 
  protected:
   // Wait for the hey signal to be received.
   void WaitForTestSignal() {
     // OnTestSignal() will quit the message loop.
-    message_loop_.Run();
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
   }
 
   // Stopping a thread is considered an IO operation, so we need to fiddle with
@@ -160,6 +165,7 @@ class SignalSenderVerificationTest : public testing::Test {
   }
 
   base::MessageLoop message_loop_;
+  scoped_ptr<base::RunLoop> run_loop_;
   scoped_ptr<base::Thread> dbus_thread_;
   scoped_refptr<Bus> bus_;
   ObjectProxy* object_proxy_;
@@ -224,7 +230,8 @@ TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
   ASSERT_FALSE(latest_name_owner_.empty());
   test_service_->ShutdownAndBlock();
   // OnNameOwnerChanged will PostTask to quit the message loop.
-  message_loop_.Run();
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
   // latest_name_owner_ should be empty as the owner is gone.
   ASSERT_TRUE(latest_name_owner_.empty());
 
@@ -236,9 +243,12 @@ TEST_F(SignalSenderVerificationTest, TestOwnerChanged) {
                  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();
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
+  if (!on_name_owner_changed_called_ || !on_ownership_called_) {
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
+  }
   ASSERT_TRUE(on_name_owner_changed_called_);
   ASSERT_TRUE(on_ownership_called_);
 
@@ -259,7 +269,8 @@ TEST_F(SignalSenderVerificationTest, TestOwnerStealing) {
   ASSERT_FALSE(latest_name_owner_.empty());
   test_service_->ShutdownAndBlock();
   // OnNameOwnerChanged will PostTask to quit the message loop.
-  message_loop_.Run();
+  run_loop_.reset(new base::RunLoop);
+  run_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.
@@ -276,7 +287,8 @@ TEST_F(SignalSenderVerificationTest, TestOwnerStealing) {
   ASSERT_TRUE(stealable_test_service.has_ownership());
 
   // OnNameOwnerChanged will PostTask to quit the message loop.
-  message_loop_.Run();
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
 
   // Send a signal to check that the service is correctly owned.
   const char kMessage[] = "hello, world";
@@ -295,9 +307,12 @@ TEST_F(SignalSenderVerificationTest, TestOwnerStealing) {
                  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();
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
+  if (!on_name_owner_changed_called_ || !on_ownership_called_) {
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
+  }
   ASSERT_TRUE(on_name_owner_changed_called_);
   ASSERT_TRUE(on_ownership_called_);
 
@@ -335,7 +350,8 @@ TEST_F(SignalSenderVerificationTest, DISABLED_TestMultipleObjects) {
       base::Bind(&SignalSenderVerificationTest::OnConnected,
                  base::Unretained(this)));
   // Wait until the object proxy is connected to the signal.
-  message_loop_.Run();
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
 
   // Send the test signal from the exported object.
   test_service_->SendTestSignal(kMessage);
@@ -349,7 +365,8 @@ TEST_F(SignalSenderVerificationTest, DISABLED_TestMultipleObjects) {
   ASSERT_FALSE(latest_name_owner_.empty());
   test_service_->ShutdownAndBlock();
   // OnNameOwnerChanged will PostTask to quit the message loop.
-  message_loop_.Run();
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
   // latest_name_owner_ should be empty as the owner is gone.
   ASSERT_TRUE(latest_name_owner_.empty());
 
@@ -362,8 +379,10 @@ TEST_F(SignalSenderVerificationTest, DISABLED_TestMultipleObjects) {
   // Both of OnNameOwnerChanged() and OnOwnership() should quit the MessageLoop,
   // but there's no expected order of those 2 event.
   while (!on_name_owner_changed_called_ || !second_name_owner_changed_called ||
-         !on_ownership_called_)
-    message_loop_.Run();
+         !on_ownership_called_) {
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
+  }
   ASSERT_TRUE(on_name_owner_changed_called_);
   ASSERT_TRUE(second_name_owner_changed_called);
   ASSERT_TRUE(on_ownership_called_);