Added video capture capabilities retrieval and caching to VideoCaptureManager

content/browser/renderer_host/media/video_capture_manager.h

Index: content/browser/renderer_host/media/video_capture_manager.h
diff --git a/content/browser/renderer_host/media/video_capture_manager.h b/content/browser/renderer_host/media/video_capture_manager.h
index 6f0b0ec4702dedd2e9e2f43caecee942542ec5e8..dc8b0ef2263ab9b7b78e4ec26b31d879d0cb34a0 100644
--- a/content/browser/renderer_host/media/video_capture_manager.h
+++ b/content/browser/renderer_host/media/video_capture_manager.h
@@ -82,6 +82,12 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
                             VideoCaptureControllerID client_id,
                             VideoCaptureControllerEventHandler* client_handler);
 
+  // Retrieve the available capture capabilities for a particular device. The
+  // capabilities are cached during GetAvailableDevicesOnDeviceThread() and
+  // updated on DoStartDeviceOnDeviceThread(). The call is asynchronous and the
+  // reply comes via OnDeviceCapabilitiesEnumerated().

[tommi (sloooow) - chröme, 2013/10/25 14:15:34]

nit: nice comment in general but I don't think you need to explain the private
implementation in detail.  It would be good to mention on what thread the caller
should expect a callback and when.  The callback (as far as the caller is
concerned) is actually DeviceCapabilitiesEnumerated on the listener interface
and not OnDeviceCapabilitiesEnumerated which is a part of the private
implementation.

[mcasas, 2013/10/28 12:50:08]

Boosted prosaic skills :) + corrected callback name.

+  void EnumerateDeviceCapabilities(const StreamDeviceInfo& device_info);
+
  private:
   virtual ~VideoCaptureManager();
   struct DeviceEntry;
@@ -96,6 +102,9 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   void OnClosed(MediaStreamType type, int capture_session_id);
   void OnDevicesEnumerated(MediaStreamType stream_type,
                            const media::VideoCaptureDevice::Names& names);
+  void OnDeviceCapabilitiesEnumerated(
+      const StreamDeviceInfo& device_info,
+      const media::VideoCaptureCapabilities& capabilities);
 
   // Find a DeviceEntry by its device ID and type, if it is already opened.
   DeviceEntry* GetDeviceEntryForMediaStreamDevice(
@@ -115,6 +124,10 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   media::VideoCaptureDevice::Names GetAvailableDevicesOnDeviceThread(
       MediaStreamType stream_type);
 
+  // Queries and returns the video capture capabilities of a device.
+  media::VideoCaptureCapabilities GetDeviceCapabilitiesOnDeviceThread(
+      const StreamDeviceInfo& device_info) const;
+
   // Create and Start a new VideoCaptureDevice, storing the result in
   // |entry->video_capture_device|. Ownership of |client| passes to
   // the device.
@@ -177,6 +190,13 @@ class CONTENT_EXPORT VideoCaptureManager : public MediaStreamProvider {
   // device thread.
   media::VideoCaptureDevice::Names video_capture_devices_;
 
+  // Local cache of video capture capabilities, indexed by unique id. It is
+  // created in GetAvailableDevicesOnDeviceThread(), updated in
+  // DoStartDeviceOnDeviceThread() and retrieved for a particular device id in
+  // EnumerateDeviceCapabilities(), read only.

[tommi (sloooow) - chröme, 2013/10/25 14:15:34]

is this last part true?  It looks like EnumerateDeviceCapabilities doesn't touch
this variable and it really shouldn't since it does not run on the device
thread.

[mcasas, 2013/10/28 12:50:08]

Correct, this comment has not been updated after I moved from ignoring-threads
implementation to now-let's-respect-threads implementation. It should read
"retrieved _via_..." since that's what clients of the class will see.

+  std::map<std::string, media::VideoCaptureCapabilities>
+      video_capture_capabilities_;

[tommi (sloooow) - chröme, 2013/10/25 14:15:34]

Instead of an extra map, can we change video_capture_devices_ to be a list of
VideoCaptureDevice::Name + VideoCaptureCapabilities?  (note singular in both
cases).
As is, we have to be careful about keeping these two containers in sync.
Something like:

struct VideoCaptureDevice {
  media::VideoCaptureDevice::Name name;
  media::VideoCaptureCapabilities capabilities;
};

std::list<VideoDevice> video_capture_devices_;

(std::vector might also be fine)

[mcasas, 2013/10/28 12:50:08]

Done. It turned out to be a class - first due to Chromium style (complex
constructor due to non-POD members), and bc it needed a two param constructor
for the vector. Had to add a method outside this class called
FindDeviceInfoById(), which might have rendered useless Name::FindById().

+
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureManager);
 };