Begin adding support for tab mirroring via the MediaStream audio/video capturing

chrome/browser/media/media_stream_devices_controller.cc

Index: chrome/browser/media/media_stream_devices_controller.cc
diff --git a/chrome/browser/media/media_stream_devices_controller.cc b/chrome/browser/media/media_stream_devices_controller.cc
index c85af1e9423980d4d0d61632103e53d60569dfd1..29a8d161c6dc590e3a43f675111c5881885f490b 100644
--- a/chrome/browser/media/media_stream_devices_controller.cc
+++ b/chrome/browser/media/media_stream_devices_controller.cc
@@ -17,22 +17,6 @@ using content::BrowserThread;
 
 namespace {
 
-// A predicate that checks if a StreamDeviceInfo object has the same ID as the
-// device ID specified at construction.
-class DeviceIdEquals {
- public:
-  explicit DeviceIdEquals(const std::string& device_id)
-      : device_id_(device_id) {
-  }
-
-  bool operator() (const content::MediaStreamDevice& device) {
-    return device.device_id == device_id_;
-  }
-
- private:
-  std::string device_id_;
-};
-
 // A predicate that checks if a StreamDeviceInfo object has the same device
 // name as the device name specified at construction.
 class DeviceNameEquals {
@@ -49,14 +33,6 @@ class DeviceNameEquals {
   std::string device_name_;
 };
 
-// Whether |request| contains any device of given |type|.
-bool HasDevice(const content::MediaStreamRequest& request,
-               content::MediaStreamDeviceType type) {
-  content::MediaStreamDeviceMap::const_iterator device_it =
-      request.devices.find(type);
-  return device_it != request.devices.end() && !device_it->second.empty();
-}
-
 const char kAudioKey[] = "audio";
 const char kVideoKey[] = "video";
 
@@ -69,10 +45,17 @@ MediaStreamDevicesController::MediaStreamDevicesController(
     : profile_(profile),
       request_(*request),
       callback_(callback) {
-  has_audio_ =
-      HasDevice(request_, content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE);
-  has_video_ =
-      HasDevice(request_, content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE);
+  has_audio_ = false;

[tommi (sloooow) - chröme, 2012/09/10 09:17:25]

initialize these in the initializer list

[miu, 2012/09/10 21:24:38]

Done.

+  has_video_ = false;
+  for (content::MediaStreamDeviceMap::const_iterator it =
+           request_.devices.begin();
+       it != request_.devices.end(); ++it) {
+    if (content::IsAudioMediaStreamDeviceType(it->first)) {
+      has_audio_ |= !it->second.empty();
+    } else if (content::IsVideoMediaStreamDeviceType(it->first)) {
+      has_video_ |= !it->second.empty();
+    }
+  }
 }
 
 MediaStreamDevicesController::~MediaStreamDevicesController() {}
@@ -118,47 +101,70 @@ bool MediaStreamDevicesController::DismissInfoBarAndTakeActionOnSettings() {
 
 content::MediaStreamDevices
 MediaStreamDevicesController::GetAudioDevices() const {
-  if (!has_audio_)
-    return content::MediaStreamDevices();
-
-  content::MediaStreamDeviceMap::const_iterator it =
-      request_.devices.find(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE);
-  DCHECK(it != request_.devices.end());
-  return it->second;
+  content::MediaStreamDevices all_audio_devices;

[no longer working on chromium, 2012/08/31 13:38:22]

why not a early return if !has_audio_

[miu, 2012/09/01 01:32:00]

Done.

+  FindSubsetOfDevices(&content::IsAudioMediaStreamDeviceType,
+                      &all_audio_devices);
+  return all_audio_devices;
 }
 
 content::MediaStreamDevices
 MediaStreamDevicesController::GetVideoDevices() const {
-  if (!has_video_)
-    return content::MediaStreamDevices();
+  content::MediaStreamDevices all_video_devices;

[no longer working on chromium, 2012/08/31 13:38:22]

ditto

[miu, 2012/09/01 01:32:00]

Done.

+  FindSubsetOfDevices(&content::IsVideoMediaStreamDeviceType,
+                      &all_video_devices);
+  return all_video_devices;
+}
 
-  content::MediaStreamDeviceMap::const_iterator it =
-      request_.devices.find(content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE);
-  DCHECK(it != request_.devices.end());
-  return it->second;
+const std::string& MediaStreamDevicesController::GetSecurityOriginSpec() const {
+  return request_.security_origin.spec();

[tommi (sloooow) - chröme, 2012/09/10 09:17:25]

why not return const GURL&?

i.e. return request_.security_origin;

GURL is a lot richer than just the std::string and if the caller of the function
wants to treat the string as a URL, it needs to rewrap the string in a GURL.

[no longer working on chromium, 2012/09/10 11:00:17]

This function is used by the UI, and only the spec string is needed to show
which webpage is querying the devices. From what I can see, the benefit of
returning a string here is that we can deliver a string without the last "/" to
the UI. (I did not ask miu to do it since I think this should be done on another
CL)

 }
 
-const GURL& MediaStreamDevicesController::GetSecurityOrigin() const {
-  return request_.security_origin;
+bool MediaStreamDevicesController::IsSafeToAlwaysAllowAudio() const {
+  return IsSafeToAlwaysAllow(
+      &content::IsAudioMediaStreamDeviceType,
+      content::MEDIA_STREAM_DEVICE_TYPE_USER_AUDIO_CAPTURE);
+}
+
+bool MediaStreamDevicesController::IsSafeToAlwaysAllowVideo() const {
+  return IsSafeToAlwaysAllow(
+      &content::IsVideoMediaStreamDeviceType,
+      content::MEDIA_STREAM_DEVICE_TYPE_USER_VIDEO_CAPTURE);
 }
 
 void MediaStreamDevicesController::Accept(const std::string& audio_id,
                                           const std::string& video_id,
                                           bool always_allow) {
   content::MediaStreamDevices devices;
-  std::string audio_device, video_device;
-  if (has_audio_) {
-    AddDeviceWithId(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE,
-                    audio_id, &devices, &audio_device);
+  std::string audio_device_name, video_device_name;
+
+  const content::MediaStreamDevice* const audio_device =
+      FindFirstDeviceWithIdInSubset(&content::IsAudioMediaStreamDeviceType,
+                                    audio_id);
+  if (audio_device) {
+    if (audio_device->type !=
+            content::MEDIA_STREAM_DEVICE_TYPE_USER_AUDIO_CAPTURE) {
+      always_allow = false;  // override for non-user audio device type
+    }
+    devices.push_back(*audio_device);
+    audio_device_name = audio_device->name;
   }
-  if (has_video_) {
-    AddDeviceWithId(content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE,
-                    video_id, &devices, &video_device);
+
+  const content::MediaStreamDevice* const video_device =
+      FindFirstDeviceWithIdInSubset(&content::IsVideoMediaStreamDeviceType,
+                                    video_id);
+  if (video_device) {
+    if (video_device->type !=
+            content::MEDIA_STREAM_DEVICE_TYPE_USER_VIDEO_CAPTURE) {
+      always_allow = false;  // override for non-user video device type
+    }
+    devices.push_back(*video_device);
+    video_device_name = video_device->name;
   }
+
   DCHECK(!devices.empty());
 
   if (always_allow)
-    AlwaysAllowOriginAndDevices(audio_device, video_device);
+    AlwaysAllowOriginAndDevices(audio_device_name, video_device_name);
 
   callback_.Run(devices);
 }
@@ -167,24 +173,26 @@ void MediaStreamDevicesController::Deny() {
   callback_.Run(content::MediaStreamDevices());
 }
 
-void MediaStreamDevicesController::AddDeviceWithId(
-    content::MediaStreamDeviceType type,
-    const std::string& id,
-    content::MediaStreamDevices* devices,
-    std::string* device_name) {
-  DCHECK(devices);
-  content::MediaStreamDeviceMap::const_iterator device_it =
-      request_.devices.find(type);
-  if (device_it == request_.devices.end())
-    return;
+bool MediaStreamDevicesController::IsSafeToAlwaysAllow(
+    FilterByDeviceTypeFunc is_included,
+    content::MediaStreamDeviceType user_type) const {
+  DCHECK(user_type == content::MEDIA_STREAM_DEVICE_TYPE_USER_AUDIO_CAPTURE ||
+         user_type == content::MEDIA_STREAM_DEVICE_TYPE_USER_VIDEO_CAPTURE);
 
-  content::MediaStreamDevices::const_iterator it = std::find_if(
-      device_it->second.begin(), device_it->second.end(), DeviceIdEquals(id));
-  if (it == device_it->second.end())
-    return;
+  if (!request_.security_origin.SchemeIsSecure()) {
+    return false;
+  }
+
+  // If non-user devices are available for the choosing, then it's not safe.
+  for (content::MediaStreamDeviceMap::const_iterator it =
+           request_.devices.begin();
+       it != request_.devices.end(); ++it) {
+    if (it->first != user_type && is_included(it->first)) {
+      return false;
+    }
+  }
 
-  devices->push_back(*it);
-  *device_name = it->name;
+  return true;
 }
 
 bool MediaStreamDevicesController::ShouldAlwaysAllowOrigin() {
@@ -232,12 +240,12 @@ void MediaStreamDevicesController::GetAlwaysAllowedDevices(
   // devices on the lists.
   if (ShouldAlwaysAllowOrigin()) {
     if (has_audio_) {
-      *audio_id =
-          GetFirstDeviceId(content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE);
+      *audio_id = GetFirstDeviceId(
+          content::MEDIA_STREAM_DEVICE_TYPE_USER_AUDIO_CAPTURE);
     }
     if (has_video_) {
-      *video_id =
-          GetFirstDeviceId(content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE);
+      *video_id = GetFirstDeviceId(
+          content::MEDIA_STREAM_DEVICE_TYPE_USER_VIDEO_CAPTURE);
     }
     return;
   }
@@ -269,11 +277,11 @@ void MediaStreamDevicesController::GetAlwaysAllowedDevices(
 
   if (has_audio_ && !audio_name.empty()) {
     *audio_id = GetDeviceIdByName(
-        content::MEDIA_STREAM_DEVICE_TYPE_AUDIO_CAPTURE, audio_name);
+        content::MEDIA_STREAM_DEVICE_TYPE_USER_AUDIO_CAPTURE, audio_name);
   }
   if (has_video_ && !video_name.empty()) {
     *video_id = GetDeviceIdByName(
-        content::MEDIA_STREAM_DEVICE_TYPE_VIDEO_CAPTURE, video_name);
+        content::MEDIA_STREAM_DEVICE_TYPE_USER_VIDEO_CAPTURE, video_name);
   }
 }
 
@@ -303,3 +311,37 @@ std::string MediaStreamDevicesController::GetFirstDeviceId(
 
   return std::string();
 }
+
+void MediaStreamDevicesController::FindSubsetOfDevices(
+    FilterByDeviceTypeFunc is_included,
+    content::MediaStreamDevices* out) const {
+  for (content::MediaStreamDeviceMap::const_iterator it =
+           request_.devices.begin();
+       it != request_.devices.end(); ++it) {
+    if (is_included(it->first)) {
+      out->insert(out->end(), it->second.begin(), it->second.end());

[no longer working on chromium, 2012/08/31 13:38:22]

Curiously, what is the benefit of using insert here?
Isn't it less code to make the function return  content::MediaStreamDevices, and
simply return it->second here?

[miu, 2012/09/01 01:32:00]

Google C++ style discourages returning non-POD types from functions (i.e., a
pointer to an output object should be provided for delivering the results). 
Here, this guideline makes total sense: Otherwise, we'd have to make redundant
copies of the vector of MediaStreamDevices (since a temporary would have to be
created, and then the elements copied again upon returning the vector to the
caller).

http://dev.chromium.org/developers/coding-style, which refers to
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

BTW--I noticed a few other places in existing code that have style violations,
but I've elected not to fix them at this time since it would complicate the code
review process.  Clean-ups can be made in a separate code change.

+    }
+  }
+}
+
+const content::MediaStreamDevice*
+MediaStreamDevicesController::FindFirstDeviceWithIdInSubset(
+    FilterByDeviceTypeFunc is_included,
+    const std::string& device_id) const {
+  for (content::MediaStreamDeviceMap::const_iterator it =
+           request_.devices.begin();
+       it != request_.devices.end(); ++it) {
+    if (!is_included(it->first)) {
+      continue;
+    }
+    for (content::MediaStreamDevices::const_iterator device_it =
+             it->second.begin();
+         device_it != it->second.end(); ++device_it) {
+      const content::MediaStreamDevice& candidate = *device_it;
+      if (candidate.device_id == device_id) {
+        return &candidate;
+      }
+    }
+  }
+  return NULL;
+}