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

content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc

Index: content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
diff --git a/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc b/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
index 0539bb84d2080a9eb0c725b04d81a8f0fa266147..8c6d1b0a09e626051966667a8d79c34da6b0f397 100644
--- a/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
+++ b/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/message_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/media_stream_dispatcher_host.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/browser/renderer_host/media/mock_media_observer.h"
@@ -39,11 +40,11 @@ namespace media_stream {
 class MockMediaStreamDispatcherHost : public MediaStreamDispatcherHost,
                                       public content::TestContentBrowserClient {
  public:
-  MockMediaStreamDispatcherHost(content::ResourceContext* resource_context,
-                                MessageLoop* message_loop,
-                                media::AudioManager* audio_manager)
-      : MediaStreamDispatcherHost(resource_context, kProcessId, audio_manager),
-        message_loop_(message_loop) {}
+  MockMediaStreamDispatcherHost(MessageLoop* message_loop,
+                                MediaStreamManager* manager)
+      : MediaStreamDispatcherHost(kProcessId),
+        message_loop_(message_loop),
+        manager_(manager) {}
 
   // A list of mock methods.
   MOCK_METHOD4(OnStreamGenerated,
@@ -80,7 +81,7 @@ class MockMediaStreamDispatcherHost : public MediaStreamDispatcherHost,
   // This method is used to dispatch IPC messages to the renderer. We intercept
   // these messages here and dispatch to our mock methods to verify the
   // conversation between this object and the renderer.
-  virtual bool Send(IPC::Message* message) {
+  virtual bool Send(IPC::Message* message) OVERRIDE {
     CHECK(message);
 
     // In this method we dispatch the messages to the according handlers as if
@@ -102,6 +103,11 @@ class MockMediaStreamDispatcherHost : public MediaStreamDispatcherHost,
     return true;
   }
 
+  // Use our own MediaStreamManager.
+  virtual MediaStreamManager* GetManager() OVERRIDE {
+    return manager_;
+  }
+
   // These handler methods do minimal things and delegate to the mock methods.
   void OnStreamGenerated(
       const IPC::Message& msg,
@@ -141,38 +147,43 @@ class MockMediaStreamDispatcherHost : public MediaStreamDispatcherHost,
   }
 
   MessageLoop* message_loop_;
+  MediaStreamManager* manager_;
 };
 
 class MediaStreamDispatcherHostTest : public testing::Test {
  public:
   MediaStreamDispatcherHostTest() : old_client_(NULL),
                                     old_browser_client_(NULL) {}
+  virtual ~MediaStreamDispatcherHostTest() {}
 
   void WaitForResult() {
     message_loop_->Run();
   }
 
  protected:
-  virtual void SetUp() {
-    message_loop_.reset(new MessageLoop(MessageLoop::TYPE_IO));
-    // ResourceContext must be created on UI thread.
-    ui_thread_.reset(new BrowserThreadImpl(BrowserThread::UI,
-                                           message_loop_.get()));
+  virtual void SetUp() OVERRIDE {
     // MediaStreamManager must be created and called on IO thread.
+    message_loop_.reset(new MessageLoop(MessageLoop::TYPE_IO));
     io_thread_.reset(new BrowserThreadImpl(BrowserThread::IO,
                                            message_loop_.get()));
 
-    audio_manager_.reset(media::AudioManager::Create());
-
     // Create our own media observer.
     media_observer_.reset(new MockMediaObserver());
 
+    // Create our own MediaStreamManager.
+    audio_manager_.reset(media::AudioManager::Create());
+    scoped_refptr<media_stream::AudioInputDeviceManager>
+        audio_input_device_manager(
+            new media_stream::AudioInputDeviceManager(audio_manager_.get()));
+    scoped_refptr<media_stream::VideoCaptureManager> video_capture_manager(
+        new media_stream::VideoCaptureManager());
+    media_stream_manager_.reset(new media_stream::MediaStreamManager(
+        audio_input_device_manager, video_capture_manager));
     // Make sure we use fake devices to avoid long delays.
-    MediaStreamManager::GetForResourceContext(
-        &resource_context_, audio_manager_.get())->UseFakeDevice();
+    media_stream_manager_->UseFakeDevice();

[tommi (sloooow) - chröme, 2012/07/04 13:46:48]

I think this is a good example of how great this change is.  Now, since we
create the media stream manager in the test and inject it, we can (assuming we
don't really need this in production) get rid of all the "UseFakeDevice" hack
that's in the production code.  We could instead provide a special
implementation of the media stream manager that always uses the fake device when
the tests are run and not mess up the production code for it.
I don't think we should do that in this cl, just pointing out the possibilities
now. Very nice.

[no longer working on chromium, 2012/07/04 14:31:39]

True, thanks for all the good ideas. I will add a TODO to remind us of removing
the UseFakeDevice().

 
-    host_ = new MockMediaStreamDispatcherHost(
-        &resource_context_, message_loop_.get(), audio_manager_.get());
+    host_ = new MockMediaStreamDispatcherHost(message_loop_.get(),
+                                              media_stream_manager_.get());
 
     // Use the fake content client and browser.
     old_client_ = content::GetContentClient();
@@ -182,9 +193,8 @@ class MediaStreamDispatcherHostTest : public testing::Test {
     content_client_->set_browser_for_testing(host_);
   }
 
-  virtual void TearDown() {
-    // Needed to make sure the manager finishes all tasks on its own thread.
-    SyncWithVideoCaptureManagerThread();
+  virtual void TearDown() OVERRIDE {
+    message_loop_->RunAllPending();
 
     // Recover the old browser client and content client.
     content::GetContentClient()->set_browser_for_testing(old_browser_client_);
@@ -192,41 +202,11 @@ class MediaStreamDispatcherHostTest : public testing::Test {
     content_client_.reset();
   }
 
-  // Called on the VideoCaptureManager thread.
-  static void PostQuitMessageLoop(MessageLoop* message_loop) {
-    message_loop->PostTask(FROM_HERE, MessageLoop::QuitClosure());
-  }
-
-  // Called on the main thread.
-  static void PostQuitOnVideoCaptureManagerThread(
-      MessageLoop* message_loop,
-      media_stream::MediaStreamManager* media_stream_manager) {
-    media_stream_manager->video_capture_manager()->GetMessageLoop()->
-        PostTask(FROM_HERE,
-                 base::Bind(&PostQuitMessageLoop, message_loop));
-  }
-
-  // SyncWithVideoCaptureManagerThread() waits until all pending tasks on the
-  // video_capture_manager thread are executed while also processing pending
-  // task in message_loop_ on the current thread. It is used to synchronize
-  // with the video capture manager thread when we are stopping a video
-  // capture device.
-  void SyncWithVideoCaptureManagerThread() {
-    message_loop_->PostTask(
-        FROM_HERE,
-        base::Bind(&PostQuitOnVideoCaptureManagerThread,
-                   message_loop_.get(),
-                   MediaStreamManager::GetForResourceContext(
-                       &resource_context_, audio_manager_.get())));
-    message_loop_->Run();
-  }
-
   scoped_refptr<MockMediaStreamDispatcherHost> host_;
   scoped_ptr<MessageLoop> message_loop_;
-  scoped_ptr<BrowserThreadImpl> ui_thread_;
   scoped_ptr<BrowserThreadImpl> io_thread_;
   scoped_ptr<media::AudioManager> audio_manager_;
-  content::MockResourceContext resource_context_;
+  scoped_ptr<MediaStreamManager> media_stream_manager_;
   content::ContentClient* old_client_;
   content::ContentBrowserClient* old_browser_client_;
   scoped_ptr<content::ContentClient> content_client_;
@@ -346,9 +326,7 @@ TEST_F(MediaStreamDispatcherHostTest, FailDevice) {
 
   EXPECT_CALL(*host_, OnVideoDeviceFailed(kRenderId, 0));
   int session_id = host_->video_devices_[0].session_id;
-  MediaStreamManager::GetForResourceContext(
-      &resource_context_, audio_manager_.get())->
-          video_capture_manager()->Error(session_id);
+  media_stream_manager_->video_capture_manager()->Error(session_id);
   WaitForResult();
   EXPECT_EQ(host_->video_devices_.size(), 0u);
   EXPECT_EQ(host_->NumberOfStreams(), 1u);