VideoCaptureDeviceFactory: Change device enumeration to callback + QTKit enumerates in UI thread.

content/browser/renderer_host/media/video_capture_manager.cc

Index: content/browser/renderer_host/media/video_capture_manager.cc
diff --git a/content/browser/renderer_host/media/video_capture_manager.cc b/content/browser/renderer_host/media/video_capture_manager.cc
index 6c90c91cae3343e18a7e1ec1bca1e433b953dd56..12ef9f6c3df2c14f1d63487c4411e389dadce027 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -7,6 +7,7 @@
 #include <set>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
@@ -19,6 +20,7 @@
 #include "content/public/browser/desktop_media_id.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/media_stream_request.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/scoped_histogram_timer.h"
 #include "media/video/capture/video_capture_device.h"
 #include "media/video/capture/video_capture_device_factory.h"
@@ -122,12 +124,28 @@ void VideoCaptureManager::EnumerateDevices(MediaStreamType stream_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DVLOG(1) << "VideoCaptureManager::EnumerateDevices, type " << stream_type;
   DCHECK(listener_);
-  base::PostTaskAndReplyWithResult(
-      device_task_runner_, FROM_HERE,
-      base::Bind(&VideoCaptureManager::GetAvailableDevicesInfoOnDeviceThread,
-                 this, stream_type, devices_info_cache_),
-      base::Bind(&VideoCaptureManager::OnDevicesInfoEnumerated, this,
-                 stream_type));
+  DCHECK_EQ(stream_type, MEDIA_DEVICE_VIDEO_CAPTURE);
+
+  // TODO(mcasas): replace previous SCOPED_UMA_HISTOGRAM_TIMER(
+  // "Media.VideoCaptureManager.GetAvailableDevicesInfoOnDeviceThreadTime");
+  // with a base/timer/elapsed_timer.h ElapsedTime, constructed here and
+  // measured in OnDevicesInfoEnumerated()
+
+  base::Callback<void(media::VideoCaptureDevice::Names&)>

[perkj_chrome, 2014/05/27 12:15:08]

This magic would benefit from a comment on what we are doing.

[mcasas, 2014/05/27 15:23:40]

Done.

+      devices_enumerated_callback =
+          base::Bind(&VideoCaptureManager::ConsolidateDevicesInfoOnDeviceThread,
+                     this,
+                     media::BindToCurrentLoop(base::Bind(
+                         &VideoCaptureManager::OnDevicesInfoEnumerated,
+                         this,
+                         stream_type)),
+                     stream_type,
+                     devices_info_cache_);
+  // OK to use base::Unretained() since we own the VCDFactory.

[perkj_chrome, 2014/05/27 12:15:08]

Maybe add "and |this| is bound in devices_enumerated_callback."

[mcasas, 2014/05/27 15:23:40]

Done.

+  device_task_runner_->PostTask(FROM_HERE,
+      base::Bind(&media::VideoCaptureDeviceFactory::EnumerateDeviceNames,
+                 base::Unretained(video_capture_device_factory_.get()),
+                 devices_enumerated_callback));
 }
 
 int VideoCaptureManager::Open(const StreamDeviceInfo& device_info) {
@@ -200,9 +218,7 @@ void VideoCaptureManager::DoStartDeviceOnDeviceThread(
       DeviceInfo* found = FindDeviceInfoById(entry->id, devices_info_cache_);
       if (found) {
         video_capture_device =
-            video_capture_device_factory_->Create(
-                BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
-                found->name);
+            video_capture_device_factory_->Create(found->name);
       }
       break;
     }
@@ -434,7 +450,6 @@ void VideoCaptureManager::OnDevicesInfoEnumerated(
     MediaStreamType stream_type,
     const DeviceInfos& new_devices_info_cache) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
   if (!listener_) {
     // Listener has been removed.
     return;
@@ -456,29 +471,12 @@ bool VideoCaptureManager::IsOnDeviceThread() const {
   return device_task_runner_->BelongsToCurrentThread();
 }
 
-VideoCaptureManager::DeviceInfos
-VideoCaptureManager::GetAvailableDevicesInfoOnDeviceThread(
+void VideoCaptureManager::ConsolidateDevicesInfoOnDeviceThread(
+    base::Callback<void(const DeviceInfos&)> on_devices_enumerated_callback,
     MediaStreamType stream_type,
-    const DeviceInfos& old_device_info_cache) {
-  SCOPED_UMA_HISTOGRAM_TIMER(
-      "Media.VideoCaptureManager.GetAvailableDevicesInfoOnDeviceThreadTime");
+    const DeviceInfos& old_device_info_cache,
+    media::VideoCaptureDevice::Names& names_snapshot) {
   DCHECK(IsOnDeviceThread());
-  media::VideoCaptureDevice::Names names_snapshot;
-  switch (stream_type) {
-    case MEDIA_DEVICE_VIDEO_CAPTURE:
-      // Cache the latest enumeration of video capture devices.
-      // We'll refer to this list again in OnOpen to avoid having to
-      // enumerate the devices again.
-      video_capture_device_factory_->GetDeviceNames(&names_snapshot);
-      break;
-    case MEDIA_DESKTOP_VIDEO_CAPTURE:
-      // Do nothing.
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
-
   // Construct |new_devices_info_cache| with the cached devices that are still
   // present in the system, and remove their names from |names_snapshot|, so we
   // keep there the truly new devices.
@@ -508,7 +506,8 @@ VideoCaptureManager::GetAvailableDevicesInfoOnDeviceThread(
     ConsolidateCaptureFormats(&device_info.supported_formats);
     new_devices_info_cache.push_back(device_info);
   }
-  return new_devices_info_cache;
+
+  on_devices_enumerated_callback.Run(new_devices_info_cache);
 }
 
 VideoCaptureManager::DeviceEntry*