Merge 176908 - Fix a problem where OpenDevice would open the wrong device if multiple are avaiable.

The issue comes from the fact the OpenDevice should not select the device
based on the user prefence but on the client request passed in the
requested_device_id. Only if this is not possible will it use the
default device.

BUG=166925
TEST=Manual: Plug multiple capture devices and try to select each one in flash settings.


Review URL: https://chromiumcodereview.appspot.com/11820050

TBR=pastarmovj@chromium.org
Review URL: https://codereview.chromium.org/11975042

git-svn-id: svn://svn.chromium.org/chrome/branches/1364/src@177381 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 74 insertions, 26 deletions

diff --git a/chrome/browser/media/media_internals.cc b/chrome/browser/media/media_internals.cc
index a621160..fe1ec4a 100644
--- a/chrome/browser/media/media_internals.cc
+++ b/chrome/browser/media/media_internals.cc
@@ -47,30 +47,43 @@ void GetDefaultDevicesForProfile(Profile* profile,
                                  bool video,
                                  content::MediaStreamDevices* devices) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(profile);
   DCHECK(audio || video);
 
-  MediaCaptureDevicesDispatcher* dispatcher =
-      MediaInternals::GetInstance()->GetMediaCaptureDevicesDispatcher();
   PrefService* prefs = profile->GetPrefs();
+  std::string default_device;
   if (audio) {
-    std::string default_device;
     default_device = prefs->GetString(prefs::kDefaultAudioCaptureDevice);
+    GetRequestedDevice(default_device, true, false, devices);
+  }
+
+  if (video) {
+    default_device = prefs->GetString(prefs::kDefaultVideoCaptureDevice);
+    GetRequestedDevice(default_device, false, true, devices);
+  }
+}
+
+void GetRequestedDevice(const std::string& requested_device_id,
+                        bool audio,
+                        bool video,
+                        content::MediaStreamDevices* devices) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(audio || video);
+
+  MediaCaptureDevicesDispatcher* dispatcher =
+      MediaInternals::GetInstance()->GetMediaCaptureDevicesDispatcher();
+  if (audio) {
     const content::MediaStreamDevices& audio_devices =
         dispatcher->GetAudioCaptureDevices();
     const content::MediaStreamDevice* const device =
-        FindDefaultDeviceWithId(audio_devices, default_device);
+        media::FindDefaultDeviceWithId(audio_devices, requested_device_id);
     if (device)
       devices->push_back(*device);
   }
-
   if (video) {
-    std::string default_device;
-    default_device = prefs->GetString(prefs::kDefaultVideoCaptureDevice);
     const content::MediaStreamDevices& video_devices =
-           dispatcher->GetVideoCaptureDevices();
+        dispatcher->GetVideoCaptureDevices();
     const content::MediaStreamDevice* const device =
-        FindDefaultDeviceWithId(video_devices, default_device);
+        media::FindDefaultDeviceWithId(video_devices, requested_device_id);
     if (device)
       devices->push_back(*device);
   }
diff --git a/chrome/browser/media/media_internals.h b/chrome/browser/media/media_internals.h
index 55ea413..f417f39 100644
--- a/chrome/browser/media/media_internals.h
+++ b/chrome/browser/media/media_internals.h
@@ -31,6 +31,14 @@ void GetDefaultDevicesForProfile(Profile* profile,
                                  bool video,
                                  content::MediaStreamDevices* devices);
 
+// Helper for picking the device that was requested for an OpenDevice request.
+// If the device requested is not available it will revert to using the first
+// available one instead or will return an empty list if no devices of the
+// requested kind are present.
+void GetRequestedDevice(const std::string& requested_device_id,
+                        bool audio,
+                        bool video,
+                        content::MediaStreamDevices* devices);
 }
 
 // This class stores information about currently active media.
diff --git a/chrome/browser/media/media_stream_devices_controller.cc b/chrome/browser/media/media_stream_devices_controller.cc
index 0bc6cdb..bcc08d8 100644
--- a/chrome/browser/media/media_stream_devices_controller.cc
+++ b/chrome/browser/media/media_stream_devices_controller.cc
@@ -121,15 +121,26 @@ const std::string& MediaStreamDevicesController::GetSecurityOriginSpec() const {
 
 void MediaStreamDevicesController::Accept(bool update_content_setting) {
   content::MediaStreamDevices devices;
-  // If policy has blocked access to some device we might end up with both
-  // |has_audio_| and |has_video_| being false in which case we should behave as
-  // if Deny(false) was called.
   if (has_audio_ || has_video_) {
-    // Get the default devices for the request.
-    media::GetDefaultDevicesForProfile(profile_,
-                                       has_audio_,
-                                       has_video_,
-                                       &devices);
+    switch (request_.request_type) {
+      case content::MEDIA_OPEN_DEVICE:
+        // For open device request pick the desired device or fall back to the
+        // first available of the given type.
+        media::GetRequestedDevice(request_.requested_device_id,
+                                  has_audio_,
+                                  has_video_,
+                                  &devices);
+        break;
+      case content::MEDIA_DEVICE_ACCESS:
+      case content::MEDIA_GENERATE_STREAM:
+      case content::MEDIA_ENUMERATE_DEVICES:
+        // Get the default devices for the request.
+        media::GetDefaultDevicesForProfile(profile_,
+                                           has_audio_,
+                                           has_video_,
+                                           &devices);
+        break;
+    }
 
     if (update_content_setting && IsSchemeSecure() && !devices.empty())
       SetPermission(true);
diff --git a/content/browser/renderer_host/media/media_stream_manager.cc b/content/browser/renderer_host/media/media_stream_manager.cc
index 1c2e81e..53746ac 100644
--- a/content/browser/renderer_host/media/media_stream_manager.cc
+++ b/content/browser/renderer_host/media/media_stream_manager.cc
@@ -581,7 +581,8 @@ void MediaStreamManager::PostRequestToUI(const std::string& label) {
                                 request->render_view_id,
                                 request->options,
                                 request->security_origin,
-                                request->type);
+                                request->type,
+                                request->requested_device_id);
 }
 
 void MediaStreamManager::HandleRequest(const std::string& label) {
diff --git a/content/browser/renderer_host/media/media_stream_ui_controller.cc b/content/browser/renderer_host/media/media_stream_ui_controller.cc
index 6ee8661..4980e74 100644
--- a/content/browser/renderer_host/media/media_stream_ui_controller.cc
+++ b/content/browser/renderer_host/media/media_stream_ui_controller.cc
@@ -72,8 +72,10 @@ class MediaStreamRequestForUI : public MediaStreamRequest {
                           int render_vid,
                           const GURL& origin,
                           const StreamOptions& options,
-                          MediaStreamRequestType request_type)
-      : MediaStreamRequest(render_pid, render_vid, origin, request_type,
+                          MediaStreamRequestType request_type,
+                          const std::string& requested_device_id)
+      : MediaStreamRequest(render_pid, render_vid, origin,
+                           request_type, requested_device_id,
                            options.audio_type, options.video_type),
         posted_task(false) {
     DCHECK(IsAudioMediaType(options.audio_type) ||
@@ -125,14 +127,15 @@ void MediaStreamUIController::MakeUIRequest(
     int render_process_id,
     int render_view_id,
     const StreamOptions& request_options,
-    const GURL& security_origin, MediaStreamRequestType request_type) {
+    const GURL& security_origin, MediaStreamRequestType request_type,
+    const std::string& requested_device_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   // Create a new request.
   if (!requests_.insert(
           std::make_pair(label, new MediaStreamRequestForUI(
               render_process_id, render_view_id, security_origin,
-              request_options, request_type))).second) {
+              request_options, request_type, requested_device_id))).second) {
     NOTREACHED();
   }
 
diff --git a/content/browser/renderer_host/media/media_stream_ui_controller.h b/content/browser/renderer_host/media/media_stream_ui_controller.h
index 1d0cf00..bfc4d1f 100644
--- a/content/browser/renderer_host/media/media_stream_ui_controller.h
+++ b/content/browser/renderer_host/media/media_stream_ui_controller.h
@@ -45,7 +45,8 @@ class CONTENT_EXPORT MediaStreamUIController {
                      int render_view_id,
                      const StreamOptions& stream_components,
                      const GURL& security_origin,
-                     MediaStreamRequestType request_type);
+                     MediaStreamRequestType request_type,
+                     const std::string& requested_device_id);
 
   // Called to cancel a pending UI request of capture device access when the
   // user has no action for the media stream InfoBar.
diff --git a/content/browser/renderer_host/media/media_stream_ui_controller_unittest.cc b/content/browser/renderer_host/media/media_stream_ui_controller_unittest.cc
index 9f728db..b13d8f8 100644
--- a/content/browser/renderer_host/media/media_stream_ui_controller_unittest.cc
+++ b/content/browser/renderer_host/media/media_stream_ui_controller_unittest.cc
@@ -63,7 +63,8 @@ class MediaStreamDeviceUIControllerTest
                                   dummy_render_view_id,
                                   components,
                                   security_origin,
-                                  MEDIA_GENERATE_STREAM);
+                                  MEDIA_GENERATE_STREAM,
+                                  std::string());
   }
 
   scoped_ptr<MessageLoop> message_loop_;
diff --git a/content/public/common/media_stream_request.cc b/content/public/common/media_stream_request.cc
index 016807c..8d2f667 100644
--- a/content/public/common/media_stream_request.cc
+++ b/content/public/common/media_stream_request.cc
@@ -4,6 +4,8 @@
 
 #include "content/public/common/media_stream_request.h"
 
+#include "base/logging.h"
+
 namespace content {
 
 bool IsAudioMediaType(MediaStreamType type) {
@@ -34,12 +36,14 @@ MediaStreamRequest::MediaStreamRequest(
     int render_view_id,
     const GURL& security_origin,
     MediaStreamRequestType request_type,
+    const std::string& requested_device_id,
     MediaStreamType audio_type,
     MediaStreamType video_type)
     : render_process_id(render_process_id),
       render_view_id(render_view_id),
       security_origin(security_origin),
       request_type(request_type),
+      requested_device_id(requested_device_id),
       audio_type(audio_type),
       video_type(video_type) {
 }
diff --git a/content/public/common/media_stream_request.h b/content/public/common/media_stream_request.h
index e50d4e9..d29a809 100644
--- a/content/public/common/media_stream_request.h
+++ b/content/public/common/media_stream_request.h
@@ -76,6 +76,7 @@ struct CONTENT_EXPORT MediaStreamRequest {
       int render_view_id,
       const GURL& security_origin,
       MediaStreamRequestType request_type,
+      const std::string& requested_device_id,
       MediaStreamType audio_type,
       MediaStreamType video_type);
 
@@ -91,11 +92,16 @@ struct CONTENT_EXPORT MediaStreamRequest {
   GURL security_origin;
 
   // Stores the type of request that was made to the media controller. Right now
-  // this is only used to destinguish between WebRTC and Pepper requests, as the
+  // this is only used to distinguish between WebRTC and Pepper requests, as the
   // latter should not be subject to user approval but only to policy check.
   // Pepper requests are signified by the |MEDIA_OPEN_DEVICE| value.
   MediaStreamRequestType request_type;
 
+  // Stores the requested device id. Used only if the |request_type| filed is
+  // set to |MEDIA_OPEN_DEVICE| to indicate which device the request is for as
+  // in that case the decision is not left to the user but to the media client.
+  std::string requested_device_id;
+
   // Flag to indicate if the request contains audio.
   MediaStreamType audio_type;