Adding error signalling from device to VideocaptureManager to relay up to MediaStream and WebKit.

content/browser/renderer_host/media/video_capture_manager.cc

Index: content/browser/renderer_host/media/video_capture_manager.cc
===================================================================
--- content/browser/renderer_host/media/video_capture_manager.cc	(revision 89207)
+++ content/browser/renderer_host/media/video_capture_manager.cc	(working copy)
@@ -36,11 +36,9 @@
   vc_device_thread_.Stop();
 }
 
-bool VideoCaptureManager::Register(MediaStreamProviderListener* listener) {
+void VideoCaptureManager::Register(MediaStreamProviderListener* listener) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  DCHECK(!listener_);

[John Knottenbelt, 2011/06/16 15:20:43]

Why is this check is no longer appropriate?
If it is still appropriate, would it also be appropriate to add a DCHECK in
VideoCaptureManager::Unregister?

[mflodman1, 2011/06/20 19:48:03]

Added DCHECK in both places to track possible user errors.

   listener_ = listener;
-  return true;
 }
 
 void VideoCaptureManager::Unregister() {
@@ -58,13 +56,12 @@
                         &VideoCaptureManager::OnEnumerateDevices));
 }
 
-MediaCaptureSessionId VideoCaptureManager::Open(
-    const MediaCaptureDeviceInfo& device) {
+int VideoCaptureManager::Open(const StreamDeviceInfo& device) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(listener_);
 
   // Generate a new id for this device
-  MediaCaptureSessionId video_capture_session_id = new_capture_session_id_++;
+  int video_capture_session_id = new_capture_session_id_++;
 
   vc_device_thread_.message_loop()->PostTask(
       FROM_HERE,
@@ -76,7 +73,7 @@
   return video_capture_session_id;
 }
 
-void VideoCaptureManager::Close(MediaCaptureSessionId capture_session_id) {
+void VideoCaptureManager::Close(int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(listener_);
 
@@ -112,6 +109,11 @@
                         stopped_task));
 }
 
+void VideoCaptureManager::Error(
+    const media::VideoCaptureSessionId capture_session_id) {
+  PostOnError(capture_session_id, kDeviceNotAvailable);
+}
+
 void VideoCaptureManager::UseFakeDevice() {
   use_fake_device_ = true;
 }
@@ -127,12 +129,12 @@
       new media::VideoCaptureDevice::Names());
   GetAvailableDevices(device_names.get());
 
-  MediaCaptureDevices devices;
+  StreamDeviceInfoArray devices;
   for (media::VideoCaptureDevice::Names::iterator it =
            device_names.get()->begin(); it != device_names.get()->end(); ++it) {
     bool opened = DeviceOpened(*it);
-    devices.push_back(MediaCaptureDeviceInfo(kVideoCapture, it->device_name,
-                                             it->unique_id, opened));
+    devices.push_back(StreamDeviceInfo(kVideoCapture, it->device_name,
+                                       it->unique_id, opened));
   }
 
   PostOnDevicesEnumerated(devices);
@@ -142,8 +144,8 @@
   device_names.get()->clear();

[Leandro Graciá Gil, 2011/06/16 17:39:53]

Independently of the comment about the type, if you use scoped_ptr objects you
should be able to use the overloaded -> operator without calling .get().

[mflodman1, 2011/06/20 19:48:03]

Removed.

 }
 
-void VideoCaptureManager::OnOpen(MediaCaptureSessionId capture_session_id,
-                                 const MediaCaptureDeviceInfo device) {
+void VideoCaptureManager::OnOpen(int capture_session_id,
+                                 const StreamDeviceInfo device) {
   DCHECK(IsOnCaptureDeviceThread());
   DCHECK(devices_.find(capture_session_id) == devices_.end());
 
@@ -175,8 +177,7 @@
   PostOnOpened(capture_session_id);
 }
 
-void VideoCaptureManager::OnClose(
-    MediaCaptureSessionId capture_session_id) {
+void VideoCaptureManager::OnClose(int capture_session_id) {
   DCHECK(IsOnCaptureDeviceThread());
 
   VideoCaptureDevices::iterator it = devices_.find(capture_session_id);
@@ -204,9 +205,9 @@
         new media::VideoCaptureDevice::Names());
     GetAvailableDevices(device_names.get());

[John Knottenbelt, 2011/06/16 15:20:43]

Is it possible for device_names to be empty after this call?

[mflodman1, 2011/06/20 19:48:03]

Good point. Rewritten based on feedback above and calling error function if the
device list is empty.

 
-    MediaCaptureDeviceInfo device(kVideoCapture,
-                                  device_names.get()->front().device_name,
-                                  device_names.get()->front().unique_id, false);
+    StreamDeviceInfo device(kVideoCapture,
+                            device_names.get()->front().device_name,
+                            device_names.get()->front().unique_id, false);
 
     // Call OnOpen to open using the first device in the list
     OnOpen(capture_params.session_id, device);
@@ -253,8 +254,7 @@
   }
 }
 
-void VideoCaptureManager::OnOpened(
-    MediaCaptureSessionId capture_session_id) {
+void VideoCaptureManager::OnOpened(int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (listener_ == NULL) {
     // Listener has been removed
@@ -263,8 +263,7 @@
   listener_->Opened(kVideoCapture, capture_session_id);
 }
 
-void VideoCaptureManager::OnClosed(
-    MediaCaptureSessionId capture_session_id) {
+void VideoCaptureManager::OnClosed(int capture_session_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (listener_ == NULL) {
     // Listener has been removed
@@ -274,7 +273,7 @@
 }
 
 void VideoCaptureManager::OnDevicesEnumerated(
-    const MediaCaptureDevices& devices) {
+    const StreamDeviceInfoArray& devices) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (listener_ == NULL) {

[Leandro Graciá Gil, 2011/06/16 17:39:53]

I think style says we should use if (!listener_) in these situations, but I'm
not completely sure. Can you check it?

[mflodman1, 2011/06/20 19:48:03]

Done.

     // Listener has been removed
@@ -283,7 +282,7 @@
   listener_->DevicesEnumerated(kVideoCapture, devices);
 }
 
-void VideoCaptureManager::OnError(MediaCaptureSessionId capture_session_id,
+void VideoCaptureManager::OnError(int capture_session_id,
                                   MediaStreamProviderError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (listener_ == NULL) {
@@ -293,8 +292,7 @@
   listener_->Error(kVideoCapture, capture_session_id, error);
 }
 
-void VideoCaptureManager::PostOnOpened(
-    MediaCaptureSessionId capture_session_id) {
+void VideoCaptureManager::PostOnOpened(int capture_session_id) {
   DCHECK(IsOnCaptureDeviceThread());
   BrowserThread::PostTask(BrowserThread::IO,
                           FROM_HERE,
@@ -303,8 +301,7 @@
                                             capture_session_id));
 }
 
-void VideoCaptureManager::PostOnClosed(
-    MediaCaptureSessionId capture_session_id) {
+void VideoCaptureManager::PostOnClosed(int capture_session_id) {
   DCHECK(IsOnCaptureDeviceThread());
   BrowserThread::PostTask(BrowserThread::IO,
                             FROM_HERE,
@@ -313,9 +310,9 @@
                                               capture_session_id));
 }
 
-void VideoCaptureManager::PostOnDevicesEnumerated(MediaCaptureDevices devices) {
+void VideoCaptureManager::PostOnDevicesEnumerated(
+    StreamDeviceInfoArray devices) {

[Leandro Graciá Gil, 2011/06/16 17:39:53]

Looks like this could be a const reference to me. You are passing an STL vector
from the same thread when calling this.

[mflodman1, 2011/06/20 19:48:03]

Done.

   DCHECK(IsOnCaptureDeviceThread());
-
   BrowserThread::PostTask(BrowserThread::IO,
                           FROM_HERE,
                           NewRunnableMethod(
@@ -324,7 +321,7 @@
                               devices));
 }
 
-void VideoCaptureManager::PostOnError(MediaCaptureSessionId capture_session_id,
+void VideoCaptureManager::PostOnError(int capture_session_id,
                                       MediaStreamProviderError error) {
   // Don't check thread here, can be called from both IO thread and device
   // thread.
@@ -366,8 +363,7 @@
   return false;
 }
 
-bool VideoCaptureManager::DeviceOpened(
-    const MediaCaptureDeviceInfo& device_info) {
+bool VideoCaptureManager::DeviceOpened(const StreamDeviceInfo& device_info) {
   DCHECK(IsOnCaptureDeviceThread());
 
   for (VideoCaptureDevices::iterator it = devices_.begin();
@@ -381,3 +377,4 @@
 }
 
 }  // namespace media
+

[scherkus (not reviewing), 2011/06/17 03:03:52]

nit: get rid of blank line

[mflodman1, 2011/06/20 19:48:03]

Done.