Make sure we don't open audio or video device if blocked by policy.

chrome/browser/media/media_stream_devices_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media/media_stream_devices_controller.h"
 
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/values.h"
 #include "chrome/browser/content_settings/content_settings_provider.h"
@@ skipping 170 matching lines @@
   // Get the default devices for the request.
   content::MediaStreamDevices devices;
   bool audio_allowed = IsDeviceAudioCaptureRequestedAndAllowed();
   bool video_allowed = IsDeviceVideoCaptureRequestedAndAllowed();
   if (audio_allowed || video_allowed) {
     switch (request_.request_type) {
       case content::MEDIA_OPEN_DEVICE: {
         const content::MediaStreamDevice* device = NULL;
         // For open device request pick the desired device or fall back to the
         // first available of the given type.
-        if (request_.audio_type == content::MEDIA_DEVICE_AUDIO_CAPTURE) {
+        if (audio_allowed &&
+            request_.audio_type == content::MEDIA_DEVICE_AUDIO_CAPTURE) {
           device = MediaCaptureDevicesDispatcher::GetInstance()->
               GetRequestedAudioDevice(request_.requested_audio_device_id);
           // TODO(wjia): Confirm this is the intended behavior.
           if (!device) {
             device = MediaCaptureDevicesDispatcher::GetInstance()->
                 GetFirstAvailableAudioDevice();
           }
-        } else if (request_.video_type == content::MEDIA_DEVICE_VIDEO_CAPTURE) {
+        } else if (video_allowed &&
+            request_.video_type == content::MEDIA_DEVICE_VIDEO_CAPTURE) {
           // Pepper API opens only one device at a time.
           device = MediaCaptureDevicesDispatcher::GetInstance()->
               GetRequestedVideoDevice(request_.requested_video_device_id);
           // TODO(wjia): Confirm this is the intended behavior.
           if (!device) {
             device = MediaCaptureDevicesDispatcher::GetInstance()->
                 GetFirstAvailableVideoDevice();
           }
         }
         if (device)
           devices.push_back(*device);
         break;
       } case content::MEDIA_GENERATE_STREAM: {

[wjia(left Chromium), 2013/08/28 16:57:35]

nit: drop the case to a new line. Somehow, this slipped through.

[Henrik Grunell, 2013/08/29 08:19:51]

Done. Also added {} to the other case blocks to be consistent.

         bool needs_audio_device = audio_allowed;
         bool needs_video_device = video_allowed;

[wjia(left Chromium), 2013/08/28 16:57:35]

Should these be the following?
  needs_*_device = *_allowed && request_.*_type ==
content::MEDIA_DEVICE_*_CAPTURE;

[Henrik Grunell, 2013/08/29 08:19:51]

For request_.request_type == content::MEDIA_GENERATE_STREAM:

If request_.*_type == content::MEDIA_DEVICE_*_CAPTURE then
content::MEDIA_DEVICE_*_CAPTURE permission will be in request_permissions_. (See
ctor.) That is checked in IsDevice*CaptureRequestedAndAllowed() so it's not
needed here.

Above it's needed because for content::MEDIA_OPEN_DEVICE case, we set the
content::MEDIA_DEVICE_*_CAPTURE permission. This is ugly and confusing, etc.
There's a todo comment for fixing this in the ctor.

Preferably, most of this class should be refactored/rewritten at some point.

 
         // Get the exact audio or video device if an id is specified.
-        if (!request_.requested_audio_device_id.empty()) {
+        if (needs_audio_device && !request_.requested_audio_device_id.empty()) {
           const content::MediaStreamDevice* audio_device =
               MediaCaptureDevicesDispatcher::GetInstance()->
                   GetRequestedAudioDevice(request_.requested_audio_device_id);
           if (audio_device) {
             devices.push_back(*audio_device);
             needs_audio_device = false;
           }
         }
-        if (!request_.requested_video_device_id.empty()) {
+        if (needs_video_device && !request_.requested_video_device_id.empty()) {
           const content::MediaStreamDevice* video_device =

[tommi (sloooow) - chröme, 2013/08/28 16:13:47]

Wouldn't it make more sense to check the allowed bools here as well as above? 

[Henrik Grunell, 2013/08/29 08:19:51]

Agree. Then I also renamed the needs_* bools to get_default_* since it reflects
better what they're for.

               MediaCaptureDevicesDispatcher::GetInstance()->
                   GetRequestedVideoDevice(request_.requested_video_device_id);
           if (video_device) {
             devices.push_back(*video_device);
             needs_video_device = false;
           }
         }
 
         // If either or both audio and video devices were requested but not
         // specified by id, get the default devices.
         if (needs_audio_device || needs_video_device) {
           MediaCaptureDevicesDispatcher::GetInstance()->
               GetDefaultDevicesForProfile(profile_,
                                           needs_audio_device,
                                           needs_video_device,
                                           &devices);
         }
         break;
       } case content::MEDIA_DEVICE_ACCESS:

[wjia(left Chromium), 2013/08/28 16:57:35]

ditto.

[Henrik Grunell, 2013/08/29 08:19:51]

Done.

         // Get the default devices for the request.
         MediaCaptureDevicesDispatcher::GetInstance()->
             GetDefaultDevicesForProfile(profile_,
                                         audio_allowed,
                                         video_allowed,
                                         &devices);
         break;
       case content::MEDIA_ENUMERATE_DEVICES:
         // Do nothing.
         NOTREACHED();
@@ skipping 247 matching lines @@
       request_permissions_.find(content::MEDIA_DEVICE_AUDIO_CAPTURE);
   return it != request_permissions_.end() && it->second == MEDIA_ALLOWED;
 }
 
 bool MediaStreamDevicesController::IsDeviceVideoCaptureRequestedAndAllowed()
     const {
   MediaStreamTypePermissionMap::const_iterator it =
       request_permissions_.find(content::MEDIA_DEVICE_VIDEO_CAPTURE);
   return it != request_permissions_.end() && it->second == MEDIA_ALLOWED;
 }