Refactor the VideoCaptureDevice::Name struct.

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 919684d310d6b6407e7d1176a0814c205577476e..397b1cd87adce5d189bc46ba650e12d961c90d27 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -142,7 +142,7 @@ void VideoCaptureManager::OnEnumerateDevices(MediaStreamType stream_type) {
            device_names.begin(); it != device_names.end(); ++it) {
     bool opened = DeviceOpened(*it);
     devices->push_back(StreamDeviceInfo(
-        stream_type, it->device_name, it->unique_id, opened));
+        stream_type, it->name(), it->id(), opened));
   }
 
   PostOnDevicesEnumerated(stream_type, devices.Pass());
@@ -167,9 +167,25 @@ void VideoCaptureManager::OnOpen(int capture_session_id,
   }
 
   // Open the device.
+
+  // FindById will return NULL on failure.
   media::VideoCaptureDevice::Name vc_device_name;
-  vc_device_name.device_name = device.device.name;
-  vc_device_name.unique_id = device.device.id;
+  if (device.device.type == MEDIA_DEVICE_VIDEO_CAPTURE) {
+    // We look up the device id from the renderer in our local enumeration
+    // since the renderer does not have all the information that might be
+    // held in the browser-side VideoCaptureDevice::Name structure.
+    media::VideoCaptureDevice::Names device_names;
+    GetAvailableDevices(device.device.type, &device_names);
+    media::VideoCaptureDevice::Name* found =
+        device_names.FindById(device.device.id);
+    if (found)
+      vc_device_name = *found;
+  }

[wjia(left Chromium), 2013/06/26 18:16:44]

What's the reason to add this logic (of enumeration) here? If the device is
detached, it will error out when trying to open the device.

It seems even though the device can't be found, we will use
Name(device.device.name, device.device.id) anyway as below.

[tommi (sloooow) - chröme, 2013/06/27 14:46:02]

Yes, currently that's going to be the case.  The reason this is like it is
though is to use the Name instance as it comes from the enumeration, where it
might have more information than just name and id.  On Windows we have an actual
need today to also store the API from which the enumeration comes.

Instead of doing this enumeration again here though, we can cache the video
capture device names in OnEnumerateDevices and refer to that list here.

Also, I've noticed that the 'use_fake_device_' flag should only apply to
MEDIA_DEVICE_VIDEO_CAPTURE, so I'm going to move that check to that switch case.

The vc_device_name variable should also only be used for
MEDIA_DEVICE_VIDEO_CAPTURE, so I'll reduce the scope of it to the same case
label.

+
+  if (vc_device_name.id().empty()) {
+    vc_device_name = media::VideoCaptureDevice::Name(
+        device.device.name, device.device.id);
+  }
 
   if (use_fake_device_) {
     video_capture_device =
@@ -183,7 +199,7 @@ void VideoCaptureManager::OnOpen(int capture_session_id,
       }
       case MEDIA_TAB_VIDEO_CAPTURE: {
         video_capture_device = WebContentsVideoCaptureDevice::Create(
-            vc_device_name.unique_id);
+            vc_device_name.id());
         break;
       }
       case MEDIA_SCREEN_VIDEO_CAPTURE: {
@@ -202,6 +218,7 @@ void VideoCaptureManager::OnOpen(int capture_session_id,
       }
     }
   }
+
   if (!video_capture_device) {
     PostOnError(capture_session_id, kDeviceNotAvailable);
     return;
@@ -431,8 +448,7 @@ bool VideoCaptureManager::DeviceOpened(
 
   for (VideoCaptureDevices::iterator it = devices_.begin();
        it != devices_.end(); ++it) {
-    if (device_name.unique_id ==
-            it->second.capture_device->device_name().unique_id) {
+    if (device_name.id() == it->second.capture_device->device_name().id()) {
       // We've found the device!
       return true;
     }
@@ -447,7 +463,7 @@ media::VideoCaptureDevice* VideoCaptureManager::GetOpenedDevice(
   for (VideoCaptureDevices::iterator it = devices_.begin();
        it != devices_.end(); it++) {
     if (device_info.device.id ==
-            it->second.capture_device->device_name().unique_id) {
+            it->second.capture_device->device_name().id()) {
       return it->second.capture_device;
     }
   }
@@ -555,8 +571,9 @@ media::VideoCaptureDevice* VideoCaptureManager::GetDeviceInternal(
       return NULL;
     }
     StreamDeviceInfo device(MEDIA_DEVICE_VIDEO_CAPTURE,
-                            device_names.front().device_name,
-                            device_names.front().unique_id, false);
+                            device_names.front().name(),
+                            device_names.front().id(),
+                            false);
 
     // Call OnOpen to open using the first device in the list.
     OnOpen(capture_session_id, device);