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

content/browser/renderer_host/media/media_stream_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/media_stream_manager.h"
 
 #include <list>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 #include "content/browser/renderer_host/media/media_stream_device_settings.h"
 #include "content/browser/renderer_host/media/media_stream_requester.h"
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 #include "content/browser/resource_context_impl.h"
 #include "content/common/media/media_stream_options.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/media_observer.h"
 #include "googleurl/src/gurl.h"
-#include "media/audio/audio_manager.h"
 
 using content::BrowserThread;
 
 static const char* kMediaStreamManagerKeyName = "content_media_stream_manager";
 
 namespace media_stream {
 
 // Creates a random label used to identify requests.
 static std::string RandomLabel() {
   // An earlier PeerConnection spec,
@@ skipping 75 matching lines @@
   std::vector<RequestState> state;
   RequestType type;
   int render_process_id;
   int render_view_id;
   GURL security_origin;
   std::string requested_device_id;
   StreamDeviceInfoArray audio_devices;
   StreamDeviceInfoArray video_devices;
 };
 
+// Static function to delete the device thread on FILE thread.
+void ShutdownOnFileThread(base::Thread* thread) {
+  DCHECK(thread);

[jam, 2012/06/26 22:32:01]

nit: unnecessary, if it's NULL the next line will crash which is just as useful
of a signal

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

Done.

+  thread->Stop();
+  delete thread;
+}
+
 // static
 MediaStreamManager* MediaStreamManager::GetForResourceContext(
-    content::ResourceContext* resource_context,
-    media::AudioManager* audio_manager) {
+    content::ResourceContext* resource_context) {
   MediaStreamManager* rv = static_cast<MediaStreamManager*>(
       resource_context->GetUserData(kMediaStreamManagerKeyName));
   if (!rv) {
-    rv = new MediaStreamManager(audio_manager);
+    rv = new MediaStreamManager();
     resource_context->SetUserData(kMediaStreamManagerKeyName, rv);
   }
   return rv;
 }
 
-MediaStreamManager::MediaStreamManager(media::AudioManager* audio_manager)
+MediaStreamManager::MediaStreamManager()
     : ALLOW_THIS_IN_INITIALIZER_LIST(
           device_settings_(new MediaStreamDeviceSettings(this))),
-      enumeration_in_progress_(content::NUM_MEDIA_STREAM_DEVICE_TYPES, false),
-      audio_manager_(audio_manager) {
+      enumeration_in_progress_(content::NUM_MEDIA_STREAM_DEVICE_TYPES, false) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 }
 
 MediaStreamManager::~MediaStreamManager() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (video_capture_manager_.get())
     video_capture_manager_->Unregister();
   if (audio_input_device_manager_.get())
     audio_input_device_manager_->Unregister();
+
+  if (device_thread_.get()) {
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,

[jam, 2012/06/26 22:32:01]

nit: instead of the FILE thread, I would use the blocking pool thread (i.e.
using BrowserThread::PostBlockingPoolTask). Since if this thread can have a long
running task, you don't want to block the FILE thread for that long as well.

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

Great idea. Thanks for pointing out.
Do you know if the PostBlockingPoolTask is running on one single thread?
I am thinking removing the device thread and using an existing thread instead,
can it be a good idea to handle all the tasks via
BrowserThread::PostBlockingPoolTask? One restriction as I know is that the video
open/start/stop/close need to run on a single thread.

[jam, 2012/06/27 16:13:14]

it uses multiple threads. you can pass in a token with the PostTask call to make
all tasks with the same token run on the same thread. see
http://src.chromium.org/viewvc/chrome/trunk/src/base/threading/sequenced_work...
for more information

+                            base::Bind(&ShutdownOnFileThread,
+                                       device_thread_.release()));
+  }
 }
 
 VideoCaptureManager* MediaStreamManager::video_capture_manager() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  EnsureDeviceThread();
   if (!video_capture_manager_.get()) {
-    video_capture_manager_ = new VideoCaptureManager();
+    video_capture_manager_ = new VideoCaptureManager(GetMessageLoop());
     video_capture_manager_->Register(this);
   }
   return video_capture_manager_.get();
 }
 
 AudioInputDeviceManager* MediaStreamManager::audio_input_device_manager() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  EnsureDeviceThread();
   if (!audio_input_device_manager_.get()) {
-    audio_input_device_manager_ =  new AudioInputDeviceManager(audio_manager_);
+    audio_input_device_manager_ =  new AudioInputDeviceManager(
+        GetMessageLoop());
     audio_input_device_manager_->Register(this);
   }
   return audio_input_device_manager_.get();
 }
 
 void MediaStreamManager::GenerateStream(MediaStreamRequester* requester,
                                         int render_process_id,
                                         int render_view_id,
                                         const StreamOptions& options,
                                         const GURL& security_origin,
@@ skipping 189 matching lines @@
         base::Bind(&MediaStreamDeviceSettings::RequestCaptureDeviceUsage,
                    base::Unretained(device_settings_.get()),
                    request_label, new_request->render_process_id,
                    new_request->render_view_id, new_request->options,
                    new_request->security_origin));
   }
 
   (*label) = request_label;
 }
 
+void MediaStreamManager::EnsureDeviceThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (device_thread_.get())
+    return;
+
+  device_thread_.reset(new base::Thread("MediaStreamDeviceThread"));
+  CHECK(device_thread_->Start());
+}
+
 void MediaStreamManager::Opened(MediaStreamType stream_type,
                                 int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   // Find the request containing this device and mark it as used.
   DeviceRequest* request = NULL;
   StreamDeviceInfoArray* devices = NULL;
   std::string label;
   for (DeviceRequests::iterator request_it = requests_.begin();
        request_it != requests_.end() && request == NULL; ++request_it) {
@@ skipping 226 matching lines @@
     return;
   }
 }
 
 void MediaStreamManager::UseFakeDevice() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   video_capture_manager()->UseFakeDevice();
   device_settings_->UseFakeUI();
 }
 
+scoped_refptr<base::MessageLoopProxy> MediaStreamManager::GetMessageLoop() {
+  return device_thread_.get() ? device_thread_->message_loop_proxy() : NULL;

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

I don't think you need this method as in both call sites you have
EnsureDeviceThread() so I'd inline device_thread_->message_loop_proxy()

if there isn't a device thread you'll crash on the NULL deref immediately as
opposed to crashing sometime later because you passed in a NULL pointer into
objects 

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

It is mainly for
media_stream_dispatch_host_unittest.cc::SyncWithVideoCaptureManagerThread (looks
like I need to change the name though)

+}
+
 void MediaStreamManager::NotifyObserverDevicesOpened(DeviceRequest* request) {
   content::MediaObserver* media_observer =
       content::GetContentClient()->browser()->GetMediaObserver();
   content::MediaStreamDevices opened_devices;
   DevicesFromRequest(request, &opened_devices);
   DCHECK(!opened_devices.empty());
   media_observer->OnCaptureDevicesOpened(request->render_process_id,
                                          request->render_view_id,
                                          opened_devices);
 }
@@ skipping 62 matching lines @@
   if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE) {
     return video_capture_manager();
   } else if (stream_type == content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE) {
     return audio_input_device_manager();
   }
   NOTREACHED();
   return NULL;
 }
 
 }  // namespace media_stream