Removed RunUntilIdle between Stop and Close in UT, after Valgrind found a NULL addressing.

Reland of: https://codereview.chromium.org/29423003/

Added video capture capabilities retrieval and caching to VideoCaptureManager. 

The local cache of video capture names and capabilities is created in a
codepath starting in EnumerateDevices. The cache is update during 
StartCaptureForClient() and StopCaptureForClient(). 

Also added unittests (http://goo.gl/QQbpXW). 

BUG=309554, 319955
TBR=perkj, ncarter, xians1

Review URL: https://codereview.chromium.org/74703002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235728 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 325 insertions, 83 deletions

diff --git a/content/browser/renderer_host/media/video_capture_controller.cc b/content/browser/renderer_host/media/video_capture_controller.cc
index 9778a9c..1e24d1b 100644
--- a/content/browser/renderer_host/media/video_capture_controller.cc
+++ b/content/browser/renderer_host/media/video_capture_controller.cc
@@ -153,6 +153,10 @@ void VideoCaptureController::AddClient(
            << ", " << params.session_id
            << ")";
 
+  // If this is the first client added to the controller, cache the parameters.
+  if (!controller_clients_.size())
+    video_capture_format_ = params.requested_format;
+
   // Signal error in case device is already in error state.
   if (state_ == VIDEO_CAPTURE_STATE_ERROR) {
     event_handler->OnError(id);
@@ -229,6 +233,12 @@ void VideoCaptureController::ReturnBuffer(
   buffer_pool_->RelinquishConsumerHold(buffer_id, 1);
 }
 
+const media::VideoCaptureFormat&
+VideoCaptureController::GetVideoCaptureFormat() const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  return video_capture_format_;
+}
+
 scoped_refptr<media::VideoFrame>
 VideoCaptureController::VideoCaptureDeviceClient::ReserveOutputBuffer(
     const gfx::Size& size) {
diff --git a/content/browser/renderer_host/media/video_capture_controller.h b/content/browser/renderer_host/media/video_capture_controller.h
index 0e9fd78..6f013c3 100644
--- a/content/browser/renderer_host/media/video_capture_controller.h
+++ b/content/browser/renderer_host/media/video_capture_controller.h
@@ -104,6 +104,8 @@ class CONTENT_EXPORT VideoCaptureController {
                     VideoCaptureControllerEventHandler* event_handler,
                     int buffer_id);
 
+  const media::VideoCaptureFormat& GetVideoCaptureFormat() const;
+
  private:
   class VideoCaptureDeviceClient;
 
@@ -140,6 +142,8 @@ class CONTENT_EXPORT VideoCaptureController {
   // state which stops the flow of data to clients.
   VideoCaptureState state_;
 
+  media::VideoCaptureFormat video_capture_format_;
+
   base::WeakPtrFactory<VideoCaptureController> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureController);
diff --git a/content/browser/renderer_host/media/video_capture_manager.cc b/content/browser/renderer_host/media/video_capture_manager.cc
index a1fcad7..100ec44 100644
--- a/content/browser/renderer_host/media/video_capture_manager.cc
+++ b/content/browser/renderer_host/media/video_capture_manager.cc
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
+#include "base/task_runner_util.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
@@ -39,6 +40,16 @@ VideoCaptureManager::DeviceEntry::DeviceEntry(
 
 VideoCaptureManager::DeviceEntry::~DeviceEntry() {}
 
+VideoCaptureManager::DeviceInfo::DeviceInfo() {}
+
+VideoCaptureManager::DeviceInfo::DeviceInfo(
+    const media::VideoCaptureDevice::Name& name,
+    const media::VideoCaptureCapabilities& capabilities)
+    : name(name),
+      capabilities(capabilities) {}
+
+VideoCaptureManager::DeviceInfo::~DeviceInfo() {}
+
 VideoCaptureManager::VideoCaptureManager()
     : listener_(NULL),
       new_capture_session_id_(1),
@@ -67,11 +78,18 @@ void VideoCaptureManager::EnumerateDevices(MediaStreamType stream_type) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DVLOG(1) << "VideoCaptureManager::EnumerateDevices, type " << stream_type;
   DCHECK(listener_);
+
   base::PostTaskAndReplyWithResult(
-      device_loop_, FROM_HERE,
-      base::Bind(&VideoCaptureManager::GetAvailableDevicesOnDeviceThread, this,
-                 stream_type),
-      base::Bind(&VideoCaptureManager::OnDevicesEnumerated, this, stream_type));
+      device_loop_,
+      FROM_HERE,
+      base::Bind(&VideoCaptureManager::
+                      GetAvailableDevicesAndCapabilitiesOnDeviceThread,
+                 this,
+                 stream_type,
+                 devices_info_cache_),
+      base::Bind(&VideoCaptureManager::OnDeviceNamesAndCapabilitiesEnumerated,
+                 this,
+                 stream_type));
 }
 
 int VideoCaptureManager::Open(const StreamDeviceInfo& device_info) {
@@ -144,12 +162,11 @@ void VideoCaptureManager::DoStartDeviceOnDeviceThread(
       // 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::Name* found =
-          video_capture_devices_.FindById(entry->id);
+      DeviceInfo* found = FindDeviceInfoById(entry->id, devices_info_cache_);
       if (found) {
         video_capture_device.reset(use_fake_device_ ?
-            media::FakeVideoCaptureDevice::Create(*found) :
-            media::VideoCaptureDevice::Create(*found));
+            media::FakeVideoCaptureDevice::Create(found->name) :
+            media::VideoCaptureDevice::Create(found->name));
       }
       break;
     }
@@ -218,7 +235,8 @@ void VideoCaptureManager::StartCaptureForClient(
 
     device_loop_->PostTask(FROM_HERE, base::Bind(
         &VideoCaptureManager::DoStartDeviceOnDeviceThread, this,
-        entry, params_as_capability,
+        entry,
+        params_as_capability,
         base::Passed(entry->video_capture_controller->NewDeviceClient())));
   }
   // Run the callback first, as AddClient() may trigger OnFrameInfo().
@@ -252,6 +270,41 @@ void VideoCaptureManager::StopCaptureForClient(
   DestroyDeviceEntryIfNoClients(entry);
 }
 
+void VideoCaptureManager::GetDeviceCapabilities(
+    int capture_session_id,
+    media::VideoCaptureCapabilities* capabilities) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  capabilities->clear();
+
+  std::map<int, MediaStreamDevice>::iterator it =
+      sessions_.find(capture_session_id);
+  DCHECK(it != sessions_.end());
+  DVLOG(1) << "GetDeviceCapabilities for device: " << it->second.name;
+
+  DeviceInfo* device_in_use =
+      FindDeviceInfoById(it->second.id, devices_info_cache_);
+  DCHECK(device_in_use);
+  if (device_in_use) {
+    DeviceEntry* const existing_device =
+        GetDeviceEntryForMediaStreamDevice(it->second);
+    if (!existing_device) {
+      // If the device is not in use, just return all its cached capabilities.
+      *capabilities = device_in_use->capabilities;
+      return;
+    }
+    // Otherwise, get the video capture parameters in use from the controller
+    // associated to the device.
+    media::VideoCaptureFormat format =
+        existing_device->video_capture_controller->GetVideoCaptureFormat();
+    media::VideoCaptureCapability current_format(format.width,
+                                                 format.height,
+                                                 format.frame_rate,
+                                                 media::PIXEL_FORMAT_I420,
+                                                 format.frame_size_type);
+    capabilities->push_back(current_format);
+  }
+}
+
 void VideoCaptureManager::DoStopDeviceOnDeviceThread(DeviceEntry* entry) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StopDeviceTime");
   DCHECK(IsOnDeviceThread());
@@ -281,24 +334,26 @@ void VideoCaptureManager::OnClosed(MediaStreamType stream_type,
   listener_->Closed(stream_type, capture_session_id);
 }
 
-void VideoCaptureManager::OnDevicesEnumerated(
+void VideoCaptureManager::OnDeviceNamesAndCapabilitiesEnumerated(
     MediaStreamType stream_type,
-    const media::VideoCaptureDevice::Names& device_names) {
+    const DevicesInfo& new_devices_info_cache) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  if (!listener_) {
-    // Listener has been removed.
+  DVLOG(1) << "OnDeviceNameAndCapabilitiesEnumerated, #new devices: "
+           << new_devices_info_cache.size();
+  if (!listener_) {  // Listener has been removed.
     return;
   }
+  devices_info_cache_ = new_devices_info_cache;
 
-  // Transform from VCD::Name to StreamDeviceInfo.
+  // Walk the |devices_info_cache_| and transform from VCD::Name to
+  // StreamDeviceInfo for return purposes.
   StreamDeviceInfoArray devices;
-  for (media::VideoCaptureDevice::Names::const_iterator it =
-           device_names.begin(); it != device_names.end(); ++it) {
+  for (DevicesInfo::const_iterator it = devices_info_cache_.begin();
+       it != devices_info_cache_.end();
+       ++it) {
     devices.push_back(StreamDeviceInfo(
-        stream_type, it->GetNameAndModel(), it->id()));
+        stream_type, it->name.GetNameAndModel(), it->name.id()));
   }
-
   listener_->DevicesEnumerated(stream_type, devices);
 }
 
@@ -306,40 +361,65 @@ bool VideoCaptureManager::IsOnDeviceThread() const {
   return device_loop_->BelongsToCurrentThread();
 }
 
-media::VideoCaptureDevice::Names
-VideoCaptureManager::GetAvailableDevicesOnDeviceThread(
-    MediaStreamType stream_type) {
+VideoCaptureManager::DevicesInfo
+VideoCaptureManager::GetAvailableDevicesAndCapabilitiesOnDeviceThread(
+    MediaStreamType stream_type,
+    const DevicesInfo& old_device_info_cache) {
   SCOPED_UMA_HISTOGRAM_TIMER(
-      "Media.VideoCaptureManager.GetAvailableDevicesTime");
+      "Media.VideoCaptureManager."
+      "GetAvailableDevicesAndCapabilitiesOnDeviceThreadTime");
   DCHECK(IsOnDeviceThread());
-  media::VideoCaptureDevice::Names result;
-
+  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.
-      if (!use_fake_device_) {
-        media::VideoCaptureDevice::GetDeviceNames(&result);
-      } else {
-        media::FakeVideoCaptureDevice::GetDeviceNames(&result);
-      }
-
-      // TODO(nick): The correctness of device start depends on this cache being
-      // maintained, but it seems a little odd to keep a cache here. Can we
-      // eliminate it?
-      video_capture_devices_ = result;
+      if (!use_fake_device_)
+        media::VideoCaptureDevice::GetDeviceNames(&names_snapshot);
+      else
+        media::FakeVideoCaptureDevice::GetDeviceNames(&names_snapshot);
       break;
-
     case MEDIA_DESKTOP_VIDEO_CAPTURE:
       // Do nothing.
       break;
-
     default:
       NOTREACHED();
       break;
   }
-  return result;
+
+  // 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.
+  DevicesInfo new_devices_info_cache;
+  for (DevicesInfo::const_iterator it_device_info =
+           old_device_info_cache.begin();
+       it_device_info != old_device_info_cache.end();
+       ++it_device_info) {
+    media::VideoCaptureDevice::Names::iterator it;
+    for (it = names_snapshot.begin(); it != names_snapshot.end(); ++it) {
+      if (it_device_info->name.id() == it->id()) {
+        new_devices_info_cache.push_back(*it_device_info);
+        names_snapshot.erase(it);
+        break;
+      }
+    }
+  }
+
+  // Need to get the capabilities for the truly new devices in |names_snapshot|.
+  for (media::VideoCaptureDevice::Names::const_iterator it =
+           names_snapshot.begin();
+       it != names_snapshot.end();
+       ++it) {
+    media::VideoCaptureCapabilities capabilities;
+    DeviceInfo device_info(*it, media::VideoCaptureCapabilities());
+    if (!use_fake_device_) {
+      media::VideoCaptureDevice::GetDeviceSupportedFormats(
+          *it, &(device_info.capabilities));
+    } else {
+      media::FakeVideoCaptureDevice::GetDeviceSupportedFormats(
+          *it, &(device_info.capabilities));
+    }
+    new_devices_info_cache.push_back(device_info);
+  }
+  return new_devices_info_cache;
 }
 
 VideoCaptureManager::DeviceEntry*
@@ -386,7 +466,8 @@ void VideoCaptureManager::DestroyDeviceEntryIfNoClients(DeviceEntry* entry) {
     entry->video_capture_controller.reset();
     device_loop_->PostTask(
         FROM_HERE,
-        base::Bind(&VideoCaptureManager::DoStopDeviceOnDeviceThread, this,
+        base::Bind(&VideoCaptureManager::DoStopDeviceOnDeviceThread,
+                   this,
                    base::Owned(entry)));
   }
 }
@@ -420,4 +501,15 @@ VideoCaptureManager::DeviceEntry* VideoCaptureManager::GetOrCreateDeviceEntry(
   return new_device;
 }
 
+VideoCaptureManager::DeviceInfo* VideoCaptureManager::FindDeviceInfoById(
+    const std::string& id,
+    DevicesInfo& device_vector) {
+  for (DevicesInfo::iterator it = device_vector.begin();
+       it != device_vector.end(); ++it) {
+    if (it->name.id() == id)
+      return &(*it);
+  }
+  return NULL;
+}
+
 }  // namespace content
diff --git a/content/browser/renderer_host/media/video_capture_manager.h b/content/browser/renderer_host/media/video_capture_manager.h
index 6f0b0ec..c1e616f 100644
--- a/content/browser/renderer_host/media/video_capture_manager.h
+++ b/content/browser/renderer_host/media/video_capture_manager.h
@@ -59,7 +59,8 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   // Called by VideoCaptureHost to locate a capture device for |capture_params|,
   // adding the Host as a client of the device's controller if successful. The
   // value of |capture_params.session_id| controls which device is selected;
-  // this value should be a session id previously returned by Open().
+  // this value should be a session id previously returned by Open(). The device
+  // capabilities are reduced to |capture_params|.
   //
   // If the device is not already started (i.e., no other client is currently
   // capturing from this device), this call will cause a VideoCaptureController
@@ -82,10 +83,28 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
                             VideoCaptureControllerID client_id,
                             VideoCaptureControllerEventHandler* client_handler);
 
+  // Retrieves the available capture capabilities for a particular device. The
+  // capabilities are cached during device(s) enumeration.
+  void GetDeviceCapabilities(int capture_session_id,
+                             media::VideoCaptureCapabilities* capabilities);
+
  private:
   virtual ~VideoCaptureManager();
   struct DeviceEntry;
 
+  // This data structure is a convenient wrap of a devices' name and associated
+  // video capture capabilities, and a flag that indicates if in use.
+  struct DeviceInfo {
+    DeviceInfo();
+    DeviceInfo(const media::VideoCaptureDevice::Name& name,
+               const media::VideoCaptureCapabilities& capabilities);
+    ~DeviceInfo();
+
+    media::VideoCaptureDevice::Name name;
+    media::VideoCaptureCapabilities capabilities;
+  };
+  typedef std::vector<DeviceInfo> DevicesInfo;
+
   // Check to see if |entry| has no clients left on its controller. If so,
   // remove it from the list of devices, and delete it asynchronously. |entry|
   // may be freed by this function.
@@ -94,8 +113,9 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   // Helpers to report an event to our Listener.
   void OnOpened(MediaStreamType type, int capture_session_id);
   void OnClosed(MediaStreamType type, int capture_session_id);
-  void OnDevicesEnumerated(MediaStreamType stream_type,
-                           const media::VideoCaptureDevice::Names& names);
+  void OnDeviceNamesAndCapabilitiesEnumerated(
+      MediaStreamType stream_type,
+      const DevicesInfo& new_devices_info_cache);
 
   // Find a DeviceEntry by its device ID and type, if it is already opened.
   DeviceEntry* GetDeviceEntryForMediaStreamDevice(
@@ -111,9 +131,13 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
 
   bool IsOnDeviceThread() const;
 
-  // Queries and returns the available device IDs.
-  media::VideoCaptureDevice::Names GetAvailableDevicesOnDeviceThread(
-      MediaStreamType stream_type);
+  // Queries the Names of the devices in the system; the capabilities of the new
+  // devices are also queried, and consolidated with the copy of the local
+  // device info cache passed. The consolidated list of devices+capabilities is
+  // returned.
+  DevicesInfo GetAvailableDevicesAndCapabilitiesOnDeviceThread(
+      MediaStreamType stream_type,
+      const DevicesInfo& old_device_info_cache);
 
   // Create and Start a new VideoCaptureDevice, storing the result in
   // |entry->video_capture_device|. Ownership of |client| passes to
@@ -127,6 +151,9 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   // |entry->video_capture_device|.
   void DoStopDeviceOnDeviceThread(DeviceEntry* entry);
 
+  DeviceInfo* FindDeviceInfoById(const std::string& id,
+                                 DevicesInfo& device_vector);
+
   // The message loop of media stream device thread, where VCD's live.
   scoped_refptr<base::MessageLoopProxy> device_loop_;
 
@@ -167,16 +194,19 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   typedef std::set<DeviceEntry*> DeviceEntries;
   DeviceEntries devices_;
 
+  // Local cache of the enumerated video capture devices' names and capture
+  // capabilities. A snapshot of the current devices and their capabilities is
+  // composed in GetAvailableDevicesAndCapabilitiesOnDeviceThread() --coming
+  // from EnumerateDevices()--, and this snapshot is used to update this list in
+  // OnDeviceNamesAndCapabilitiesEnumerated(). GetDeviceCapabilities() will
+  // use this list if the device is not started, otherwise it will retrieve the
+  // active device capture format from the VideoCaptureController associated.
+  DevicesInfo devices_info_cache_;
+
   // Set to true if using fake video capture devices for testing, false by
   // default. This is only used for the MEDIA_DEVICE_VIDEO_CAPTURE device type.
   bool use_fake_device_;
 
-  // We cache the enumerated video capture devices in
-  // GetAvailableDevicesOnDeviceThread() and then later look up the requested ID
-  // when a device is created in DoStartDeviceOnDeviceThread(). Used only on the
-  // device thread.
-  media::VideoCaptureDevice::Names video_capture_devices_;
-
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureManager);
 };
 
diff --git a/content/browser/renderer_host/media/video_capture_manager_unittest.cc b/content/browser/renderer_host/media/video_capture_manager_unittest.cc
index a986e0b..437b35b 100644
--- a/content/browser/renderer_host/media/video_capture_manager_unittest.cc
+++ b/content/browser/renderer_host/media/video_capture_manager_unittest.cc
@@ -16,6 +16,7 @@
 #include "content/browser/renderer_host/media/video_capture_controller_event_handler.h"
 #include "content/browser/renderer_host/media/video_capture_manager.h"
 #include "content/common/media/media_stream_options.h"
+#include "media/video/capture/fake_video_capture_device.h"
 #include "media/video/capture/video_capture_device.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -167,7 +168,8 @@ TEST_F(VideoCaptureManagerTest, OpenTwice) {
 
   InSequence s;
   EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
-      .Times(1).WillOnce(SaveArg<1>(&devices));
+      .Times(1)
+      .WillOnce(SaveArg<1>(&devices));
   EXPECT_CALL(*listener_, Opened(MEDIA_DEVICE_VIDEO_CAPTURE, _)).Times(2);
   EXPECT_CALL(*listener_, Closed(MEDIA_DEVICE_VIDEO_CAPTURE, _)).Times(2);
 
@@ -191,6 +193,100 @@ TEST_F(VideoCaptureManagerTest, OpenTwice) {
   vcm_->Unregister();
 }
 
+// Connect and disconnect devices.
+TEST_F(VideoCaptureManagerTest, ConnectAndDisconnectDevices) {
+  StreamDeviceInfoArray devices;
+  int number_of_devices_keep =
+      media::FakeVideoCaptureDevice::NumberOfFakeDevices();
+
+  InSequence s;
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1)
+      .WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+  ASSERT_EQ(devices.size(), 2u);
+
+  // Simulate we remove 1 fake device.
+  media::FakeVideoCaptureDevice::SetNumberOfFakeDevices(1);
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1)
+      .WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+  ASSERT_EQ(devices.size(), 1u);
+
+  // Simulate we add 2 fake devices.
+  media::FakeVideoCaptureDevice::SetNumberOfFakeDevices(3);
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1)
+      .WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+  ASSERT_EQ(devices.size(), 3u);
+
+  vcm_->Unregister();
+  media::FakeVideoCaptureDevice::SetNumberOfFakeDevices(number_of_devices_keep);
+}
+
+// Enumerate devices and open the first, then check the capabilities. Then start
+// the opened device. The capability list should be reduced to just one format,
+// and this should be the one used when configuring-starting the device. Finally
+// stop the device and check that the capabilities have been restored.
+TEST_F(VideoCaptureManagerTest, ManipulateDeviceAndCheckCapabilities) {
+  StreamDeviceInfoArray devices;
+
+  InSequence s;
+  EXPECT_CALL(*listener_, DevicesEnumerated(MEDIA_DEVICE_VIDEO_CAPTURE, _))
+      .Times(1)
+      .WillOnce(SaveArg<1>(&devices));
+  vcm_->EnumerateDevices(MEDIA_DEVICE_VIDEO_CAPTURE);
+  message_loop_->RunUntilIdle();
+
+  EXPECT_CALL(*listener_, Opened(MEDIA_DEVICE_VIDEO_CAPTURE, _)).Times(1);
+  int video_session_id = vcm_->Open(devices.front());
+  message_loop_->RunUntilIdle();
+
+  // When the device has been opened, we should see all the devices'
+  // capabilities.
+  media::VideoCaptureCapabilities device_capabilities;
+  vcm_->GetDeviceCapabilities(video_session_id, &device_capabilities);
+  ASSERT_EQ(devices.size(), 2u);
+  ASSERT_GT(device_capabilities.size(), 1u);
+  EXPECT_GT(device_capabilities[0].width, 1);
+  EXPECT_GT(device_capabilities[0].height, 1);
+  EXPECT_GT(device_capabilities[0].frame_rate, 1);
+  EXPECT_GT(device_capabilities[1].width, 1);
+  EXPECT_GT(device_capabilities[1].height, 1);
+  EXPECT_GT(device_capabilities[1].frame_rate, 1);
+
+  VideoCaptureControllerID client_id = StartClient(video_session_id, true);
+  message_loop_->RunUntilIdle();
+  // After StartClient(), the device's capabilities should be reduced to one.
+  vcm_->GetDeviceCapabilities(video_session_id, &device_capabilities);
+  ASSERT_EQ(device_capabilities.size(), 1u);
+  EXPECT_GT(device_capabilities[0].width, 1);
+  EXPECT_GT(device_capabilities[0].height, 1);
+  EXPECT_GT(device_capabilities[0].frame_rate, 1);
+
+  EXPECT_CALL(*listener_, Closed(MEDIA_DEVICE_VIDEO_CAPTURE, _)).Times(1);
+  StopClient(client_id);
+  // After StopClient(), the device's list of capabilities should be restored
+  // to the original one.
+  vcm_->GetDeviceCapabilities(video_session_id, &device_capabilities);
+  ASSERT_GT(device_capabilities.size(), 1u);
+  EXPECT_GT(device_capabilities[0].width, 1);
+  EXPECT_GT(device_capabilities[0].height, 1);
+  EXPECT_GT(device_capabilities[0].frame_rate, 1);
+  EXPECT_GT(device_capabilities[1].width, 1);
+  EXPECT_GT(device_capabilities[1].height, 1);
+  EXPECT_GT(device_capabilities[1].frame_rate, 1);
+
+  vcm_->Close(video_session_id);
+  message_loop_->RunUntilIdle();
+  vcm_->Unregister();
+}
+
 // Open two different devices.
 TEST_F(VideoCaptureManagerTest, OpenTwo) {
   StreamDeviceInfoArray devices;
diff --git a/media/video/capture/fake_video_capture_device.cc b/media/video/capture/fake_video_capture_device.cc
index c36670c..90778a2 100644
--- a/media/video/capture/fake_video_capture_device.cc
+++ b/media/video/capture/fake_video_capture_device.cc
@@ -19,15 +19,23 @@ namespace media {
 static const int kFakeCaptureTimeoutMs = 50;
 static const int kFakeCaptureBeepCycle = 20;  // Visual beep every 1s.
 static const int kFakeCaptureCapabilityChangePeriod = 30;
-enum { kNumberOfFakeDevices = 2 };
 
+int FakeVideoCaptureDevice::number_of_fake_devices_ = 2;
 bool FakeVideoCaptureDevice::fail_next_create_ = false;
 
+void FakeVideoCaptureDevice::SetNumberOfFakeDevices(int num) {
+  number_of_fake_devices_ = num;
+}
+
+int FakeVideoCaptureDevice::NumberOfFakeDevices(void) {
+  return number_of_fake_devices_;
+}
+
 void FakeVideoCaptureDevice::GetDeviceNames(Names* const device_names) {
   // Empty the name list.
   device_names->erase(device_names->begin(), device_names->end());
 
-  for (int n = 0; n < kNumberOfFakeDevices; n++) {
+  for (int n = 0; n < number_of_fake_devices_; n++) {
     Name name(base::StringPrintf("fake_device_%d", n),
               base::StringPrintf("/dev/video%d", n));
     device_names->push_back(name);
@@ -37,12 +45,19 @@ void FakeVideoCaptureDevice::GetDeviceNames(Names* const device_names) {
 void FakeVideoCaptureDevice::GetDeviceSupportedFormats(
     const Name& device,
     VideoCaptureCapabilities* formats) {
-  VideoCaptureCapability capture_format;
-  capture_format.color = media::PIXEL_FORMAT_I420;
-  capture_format.width = 640;
-  capture_format.height = 480;
-  capture_format.frame_rate = 1000 / kFakeCaptureTimeoutMs;
-  formats->push_back(capture_format);
+  formats->clear();
+  VideoCaptureCapability capture_format_640x480;
+  capture_format_640x480.color = media::PIXEL_FORMAT_I420;
+  capture_format_640x480.width = 640;
+  capture_format_640x480.height = 480;
+  capture_format_640x480.frame_rate = 1000 / kFakeCaptureTimeoutMs;
+  formats->push_back(capture_format_640x480);
+  VideoCaptureCapability capture_format_320x240;
+  capture_format_320x240.color = media::PIXEL_FORMAT_I420;
+  capture_format_320x240.width = 320;
+  capture_format_320x240.height = 240;
+  capture_format_320x240.frame_rate = 1000 / kFakeCaptureTimeoutMs;
+  formats->push_back(capture_format_320x240);
 }
 
 VideoCaptureDevice* FakeVideoCaptureDevice::Create(const Name& device_name) {
@@ -50,7 +65,7 @@ VideoCaptureDevice* FakeVideoCaptureDevice::Create(const Name& device_name) {
     fail_next_create_ = false;
     return NULL;
   }
-  for (int n = 0; n < kNumberOfFakeDevices; ++n) {
+  for (int n = 0; n < number_of_fake_devices_; ++n) {
     std::string possible_id = base::StringPrintf("/dev/video%d", n);
     if (device_name.id().compare(possible_id) == 0) {
       return new FakeVideoCaptureDevice();
diff --git a/media/video/capture/fake_video_capture_device.h b/media/video/capture/fake_video_capture_device.h
index 174ba06a..c69fbdf 100644
--- a/media/video/capture/fake_video_capture_device.h
+++ b/media/video/capture/fake_video_capture_device.h
@@ -23,6 +23,8 @@ class MEDIA_EXPORT FakeVideoCaptureDevice : public VideoCaptureDevice {
   // Used for testing. This will make sure the next call to Create will
   // return NULL;
   static void SetFailNextCreate();
+  static void SetNumberOfFakeDevices(int num);
+  static int NumberOfFakeDevices();
 
   static void GetDeviceNames(Names* device_names);
   static void GetDeviceSupportedFormats(const Name& device,
@@ -63,6 +65,7 @@ class MEDIA_EXPORT FakeVideoCaptureDevice : public VideoCaptureDevice {
   std::vector<VideoCaptureCapability> capabilities_roster_;
   int capabilities_roster_index_;
 
+  static int number_of_fake_devices_;
   static bool fail_next_create_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeVideoCaptureDevice);
diff --git a/media/video/capture/mac/video_capture_device_mac.mm b/media/video/capture/mac/video_capture_device_mac.mm
index b353b12..60b193e 100644
--- a/media/video/capture/mac/video_capture_device_mac.mm
+++ b/media/video/capture/mac/video_capture_device_mac.mm
@@ -208,10 +208,17 @@ bool VideoCaptureDeviceMac::Init() {
   DCHECK_EQ(loop_proxy_, base::MessageLoopProxy::current());
   DCHECK_EQ(state_, kNotInitialized);
 
+  // TODO(mcasas): The following check might not be necessary; if the device has
+  // disappeared after enumeration and before coming here, opening would just
+  // fail but not necessarily produce a crash.
   Names device_names;
   GetDeviceNames(&device_names);
-  Name* found = device_names.FindById(device_name_.id());
-  if (!found)
+  Names::iterator it = device_names.begin();
+  for (; it != device_names.end(); ++it) {
+    if (it->id() == device_name_.id())
+      break;
+  }
+  if (it == device_names.end())
     return false;
 
   capture_device_ =
diff --git a/media/video/capture/video_capture_device.cc b/media/video/capture/video_capture_device.cc
index cd1f535..c370d09 100644
--- a/media/video/capture/video_capture_device.cc
+++ b/media/video/capture/video_capture_device.cc
@@ -17,15 +17,6 @@ const std::string VideoCaptureDevice::Name::GetNameAndModel() const {
   return device_name_ + suffix;
 }
 
-VideoCaptureDevice::Name*
-VideoCaptureDevice::Names::FindById(const std::string& id) {
-  for (iterator it = begin(); it != end(); ++it) {
-    if (it->id() == id)
-      return &(*it);
-  }
-  return NULL;
-}
-
 VideoCaptureDevice::~VideoCaptureDevice() {}
 
 }  // namespace media
diff --git a/media/video/capture/video_capture_device.h b/media/video/capture/video_capture_device.h
index 824cdb4..38600c5 100644
--- a/media/video/capture/video_capture_device.h
+++ b/media/video/capture/video_capture_device.h
@@ -18,6 +18,7 @@
 #include "base/logging.h"
 #include "base/time/time.h"
 #include "media/base/media_export.h"
+#include "media/base/video_frame.h"
 #include "media/video/capture/video_capture_types.h"
 
 namespace media {
@@ -109,14 +110,7 @@ class MEDIA_EXPORT VideoCaptureDevice {
   };
 
   // Manages a list of Name entries.
-  class MEDIA_EXPORT Names
-      : public NON_EXPORTED_BASE(std::list<Name>) {
-   public:
-    // Returns NULL if no entry was found by that ID.
-    Name* FindById(const std::string& id);
-
-    // Allow generated copy constructor and assignment.
-  };
+  typedef std::list<Name> Names;
 
   class MEDIA_EXPORT Client {
    public: