Move the device enumerate/open/close work to device thread from IO thread

content/browser/renderer_host/media/audio_input_device_manager_unittest.cc

Index: content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
diff --git a/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc b/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
index fe57150d6747de15616b50c29d8ad9b4a1fd0b82..38917d4f5c47851f45a860a8b16c362a5e4bfe0a 100644
--- a/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
+++ b/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
@@ -8,6 +8,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
+#include "content/browser/browser_main_loop.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager_event_handler.h"
@@ -15,6 +16,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using content::BrowserMainLoop;
 using content::BrowserThread;
 using content::BrowserThreadImpl;
 using media_stream::AudioInputDeviceManager;
@@ -105,28 +107,60 @@ class AudioInputDeviceManagerTest : public testing::Test {
     message_loop_.reset(new MessageLoop(MessageLoop::TYPE_IO));
     io_thread_.reset(new BrowserThreadImpl(BrowserThread::IO,
                                            message_loop_.get()));
-    audio_manager_.reset(media::AudioManager::Create());
 
-    manager_ = new AudioInputDeviceManager(audio_manager_.get());
+    device_thread_.reset(new base::Thread("AudioInputDeviceManagerTestThread"));

[scherkus (not reviewing), 2012/06/27 00:52:31]

given that you can inject message loops I'm curious whether you could (or want
to) use message_loop_ here and avoid the syncing between threads

[no longer working on chromium, 2012/06/27 14:07:16]

True, I did not do this simply because I was not sure if it was a good way to do
the testing.

Thanks for pointing out, done now. 

+    CHECK(device_thread_->Start());
+
+    manager_ = new AudioInputDeviceManager(
+        device_thread_->message_loop_proxy());
     audio_input_listener_.reset(new MockAudioInputDeviceManagerListener());
     manager_->Register(audio_input_listener_.get());
 
+    audio_manager_.reset(media::AudioManager::Create());
+    manager_->SetAudioManagerForTesting(audio_manager_.get());

[scherkus (not reviewing), 2012/06/27 00:52:31]

can't we recreate a BrowserMainLoop for each test + use its audio_manager()?
what are we gaining by recreating our own manager?

[no longer working on chromium, 2012/06/27 14:07:16]

Good idea, done.

+
     // Gets the enumerated device list from the AudioInputDeviceManager.
     manager_->EnumerateDevices();
     EXPECT_CALL(*audio_input_listener_, DevicesEnumerated(_))
         .Times(1);
 
-    // Waits for the callback.
-    message_loop_->RunAllPending();
+    // Sync up the threads to make sure we get the list.
+    SyncWithAudioInputDeviceManagerThread();
   }
 
   virtual void TearDown() {
     manager_->Unregister();
     io_thread_.reset();
+    device_thread_->Stop();
+  }
+
+  // Called on the device thread.
+  static void PostQuitMessageLoop(MessageLoop* message_loop) {
+    message_loop->PostTask(FROM_HERE, MessageLoop::QuitClosure());
+  }
+
+  // Called on the main thread.
+  static void PostQuitOnAudioInputDeviceManagerThread(
+      MessageLoop* message_loop, MessageLoop* device_message_loop) {
+    device_message_loop->PostTask(
+        FROM_HERE, base::Bind(&PostQuitMessageLoop, message_loop));
+  }
+
+  // SyncWithAudioInputDeviceManagerThread() waits until all pending tasks on
+  // the audio_input_device_manager thread are executed while also processing
+  // pending task in message_loop_ on the current thread.
+  void SyncWithAudioInputDeviceManagerThread() {
+    message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&PostQuitOnAudioInputDeviceManagerThread,
+                   message_loop_.get(),
+                   device_thread_->message_loop()));
+    message_loop_->Run();
   }
 
   scoped_ptr<MessageLoop> message_loop_;
   scoped_ptr<BrowserThreadImpl> io_thread_;
+  scoped_ptr<base::Thread> device_thread_;
   scoped_refptr<AudioInputDeviceManager> manager_;
   scoped_ptr<MockAudioInputDeviceManagerListener> audio_input_listener_;
   scoped_ptr<media::AudioManager> audio_manager_;
@@ -161,8 +195,7 @@ TEST_F(AudioInputDeviceManagerTest, OpenAndCloseDevice) {
                        session_id))
         .Times(1);
 
-    // Waits for the callback.
-    message_loop_->RunAllPending();
+    SyncWithAudioInputDeviceManagerThread();
   }
 }
 
@@ -192,8 +225,7 @@ TEST_F(AudioInputDeviceManagerTest, OpenMultipleDevices) {
                        session_id[index]))
         .Times(1);
 
-    // Waits for the callback.
-    message_loop_->RunAllPending();
+    SyncWithAudioInputDeviceManagerThread();
   }
 
   // Checks if the session_ids are unique.
@@ -211,8 +243,7 @@ TEST_F(AudioInputDeviceManagerTest, OpenMultipleDevices) {
                        session_id[i]))
         .Times(1);
 
-    // Waits for the callback.
-    message_loop_->RunAllPending();
+    SyncWithAudioInputDeviceManagerThread();
   }
 }
 
@@ -233,8 +264,7 @@ TEST_F(AudioInputDeviceManagerTest, OpenNotExistingDevice) {
                      session_id))
       .Times(1);
 
-  // Waits for the callback.
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 }
 
 // Opens default device twice.
@@ -273,8 +303,7 @@ TEST_F(AudioInputDeviceManagerTest, OpenDeviceTwice) {
                      second_session_id))
       .Times(1);
 
-  // Waits for the callback.
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 }
 
 // Starts and closes the sessions after opening the devices.
@@ -307,7 +336,7 @@ TEST_F(AudioInputDeviceManagerTest, StartAndStopSession) {
                 Opened(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                        session_id[index]))
         .Times(1);
-    message_loop_->RunAllPending();
+    SyncWithAudioInputDeviceManagerThread();
 
     manager_->Start(session_id[index], audio_input_event_handler.get());
     EXPECT_CALL(*audio_input_event_handler,
@@ -321,7 +350,7 @@ TEST_F(AudioInputDeviceManagerTest, StartAndStopSession) {
                 Closed(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                        session_id[index]))
         .Times(1);
-    message_loop_->RunAllPending();
+    SyncWithAudioInputDeviceManagerThread();
   }
 }
 
@@ -354,7 +383,7 @@ TEST_F(AudioInputDeviceManagerTest, CloseWithoutStopSession) {
                 Opened(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                        session_id[index]))
         .Times(1);
-    message_loop_->RunAllPending();
+    SyncWithAudioInputDeviceManagerThread();
 
     manager_->Start(session_id[index], audio_input_event_handler.get());
     EXPECT_CALL(*audio_input_event_handler,
@@ -372,7 +401,7 @@ TEST_F(AudioInputDeviceManagerTest, CloseWithoutStopSession) {
                 Closed(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                        session_id[index]))
         .Times(1);
-    message_loop_->RunAllPending();
+    SyncWithAudioInputDeviceManagerThread();
   }
 }
 
@@ -407,7 +436,7 @@ TEST_F(AudioInputDeviceManagerTest, StartDeviceTwice) {
               Opened(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                      second_session_id))
       .Times(1);
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 
   // Calls Start()/Stop()/Close() for the default device twice.
   manager_->Start(first_session_id, first_event_handler.get());
@@ -434,7 +463,7 @@ TEST_F(AudioInputDeviceManagerTest, StartDeviceTwice) {
               Closed(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                      second_session_id))
       .Times(1);
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 }
 
 // Starts an invalid session.
@@ -456,7 +485,7 @@ TEST_F(AudioInputDeviceManagerTest, StartInvalidSession) {
               Opened(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                      session_id))
       .Times(1);
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 
   // Starts a non-opened device.
   // This should fail and trigger error code 'kDeviceNotAvailable'.
@@ -473,7 +502,7 @@ TEST_F(AudioInputDeviceManagerTest, StartInvalidSession) {
               Closed(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                      session_id))
       .Times(1);
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 }
 
 // Starts a session twice, the first time should succeed, while the second
@@ -496,7 +525,7 @@ TEST_F(AudioInputDeviceManagerTest, StartSessionTwice) {
               Opened(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                      session_id))
       .Times(1);
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 
   // Starts the session, it should succeed.
   manager_->Start(session_id, audio_input_event_handler.get());
@@ -519,7 +548,7 @@ TEST_F(AudioInputDeviceManagerTest, StartSessionTwice) {
               Closed(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
                      session_id))
       .Times(1);
-  message_loop_->RunAllPending();
+  SyncWithAudioInputDeviceManagerThread();
 }
 
 }  // namespace media_stream