Per navigation sticky media permissions.

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 08ad71141e09351dc4cbee576a5fe5b481459875..a61d2656cc5fe671499ce45cf6d1db39ae8b254e 100644
--- a/chrome/browser/media/media_stream_devices_controller.cc
+++ b/chrome/browser/media/media_stream_devices_controller.cc
@@ -23,6 +23,7 @@
 #include "chrome/common/pref_names.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/common/media_stream_request.h"
 #include "extensions/common/constants.h"
 #include "grit/generated_resources.h"
@@ -37,19 +38,127 @@ using content::BrowserThread;
 
 namespace {
 
+// This prefix is combined with request security origins to store media access
+// permissions that the user has granted a specific page navigation instance.
+// The string value stored with the navigation instance will contain one or more
+// kMediaPermissionXxx constants that indicates the permission(s) that the user
+// has granted the page.
+const char kMediaPermissionKeyPrefix[] = "media_permissions#";
+const base::char16 kMediaPermissionAudio = static_cast<base::char16>('a');
+const base::char16 kMediaPermissionVideo = static_cast<base::char16>('v');
+
 bool HasAvailableDevicesForRequest(const content::MediaStreamRequest& request) {
-  bool has_audio_device =
-      request.audio_type == content::MEDIA_NO_SERVICE ||
-      !MediaCaptureDevicesDispatcher::GetInstance()->GetAudioCaptureDevices()
-          .empty();
-  bool has_video_device =
-      request.video_type == content::MEDIA_NO_SERVICE ||
-      !MediaCaptureDevicesDispatcher::GetInstance()->GetVideoCaptureDevices()
-          .empty();
+  const content::MediaStreamDevices* audio_devices =
+      request.audio_type == content::MEDIA_DEVICE_AUDIO_CAPTURE ?
+          &MediaCaptureDevicesDispatcher::GetInstance()
+              ->GetAudioCaptureDevices() :
+          NULL;
+
+  const content::MediaStreamDevices* video_devices =
+      request.video_type == content::MEDIA_DEVICE_VIDEO_CAPTURE ?
+          &MediaCaptureDevicesDispatcher::GetInstance()
+              ->GetVideoCaptureDevices() :
+          NULL;
+
+  // Check if we're being asked for audio and/or video and that either of those
+  // lists is empty.  If they are, we do not have devices available for the
+  // request.
+  // TODO(tommi): It's kind of strange to have this here since if we fail this
+  // test, there'll be a UI shown that indicates to the user that access to
+  // non-existing audio/video devices has been denied.  The user won't have
+  // any way to change that but there will be a UI shown which indicates that
+  // access is blocked.
+  if ((audio_devices != NULL && audio_devices->empty()) ||
+      (video_devices != NULL && video_devices->empty())) {
+    return false;
+  }
+
+  // Note: we check requested_[audio|video]_device_id before dereferencing
+  // [audio|video]_devices.  If the requested device id is non-empty, then
+  // the corresponding device list must not be NULL.
+
+  if (!request.requested_audio_device_id.empty() &&
+      !audio_devices->FindById(request.requested_audio_device_id)) {
+    return false;
+  }
+
+  if (!request.requested_video_device_id.empty() &&
+      !video_devices->FindById(request.requested_video_device_id)) {
+    return false;
+  }
+
+  return true;
+}
 
-  return has_audio_device && has_video_device;
+base::string16 GetMediaPermissionsFromNavigationEntry(
+    content::NavigationEntry* navigation_entry,
+    const content::MediaStreamRequest& request) {
+  const std::string key(kMediaPermissionKeyPrefix +
+                        request.security_origin.spec());
+
+  base::string16 permissions;
+  if (!navigation_entry->GetExtraData(key, &permissions)) {
+    DCHECK(permissions.empty());
+  }
+
+  return permissions;
+}
+
+void SetMediaPermissionsForNavigationEntry(
+    content::NavigationEntry* navigation_entry,
+    const content::MediaStreamRequest& request,
+    const base::string16& permissions) {
+  const std::string key(kMediaPermissionKeyPrefix +
+                        request.security_origin.spec());
+  permissions.empty() ?
+      navigation_entry->ClearExtraData(key) :
+      navigation_entry->SetExtraData(key, permissions);

[no longer working on chromium, 2014/05/05 08:14:36]

what happen if it sets the same entry to the navigation more than once?

[tommi (sloooow) - chröme, 2014/05/05 08:28:53]

Not sure if I understand the question, but if you think there's a bug in
SetExtraData, check out the implementation:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

[no longer working on chromium, 2014/05/05 08:43:03]

Thanks, I was just trying to understand the behavior, and wondering if the
navigation would store different permissions to the same keys. The link answers
the question with a "no" since it is a map.

[tommi (sloooow) - chröme, 2014/05/05 08:50:31]

Ah, ok, yeah.  I wasn't sure what "entry to the navigation" meant since we're
dealing with a navigation entry.

 }
 
+void SetMediaPermissionsForNavigationEntry(
+    content::NavigationEntry* navigation_entry,
+    const content::MediaStreamRequest& request,
+    bool allow_audio,
+    bool allow_video) {
+  base::string16 permissions;
+  if (allow_audio)
+    permissions += kMediaPermissionAudio;
+  if (allow_video)
+    permissions += kMediaPermissionVideo;
+  SetMediaPermissionsForNavigationEntry(navigation_entry, request, permissions);
+}
+
+bool IsRequestAllowedByNavigationEntry(
+    content::NavigationEntry* navigation_entry,
+    const content::MediaStreamRequest& request) {
+  using content::MEDIA_NO_SERVICE;
+  using content::MEDIA_DEVICE_AUDIO_CAPTURE;
+  using content::MEDIA_DEVICE_VIDEO_CAPTURE;
+
+  // If we aren't being asked for at least one of these two, fail right away.
+  if (!navigation_entry ||
+      (request.audio_type != MEDIA_DEVICE_AUDIO_CAPTURE &&
+       request.video_type != MEDIA_DEVICE_VIDEO_CAPTURE)) {
+    return false;
+  }
+
+  base::string16 permissions =
+      GetMediaPermissionsFromNavigationEntry(navigation_entry, request);
+
+  bool audio_requested_and_granted =
+      request.audio_type == MEDIA_DEVICE_AUDIO_CAPTURE &&
+      permissions.find(kMediaPermissionAudio) != base::string16::npos;
+
+  bool video_requested_and_granted =
+      request.video_type == MEDIA_DEVICE_VIDEO_CAPTURE &&
+      permissions.find(kMediaPermissionVideo) != base::string16::npos;
+
+  return
+      (audio_requested_and_granted || request.audio_type == MEDIA_NO_SERVICE) &&
+      (video_requested_and_granted || request.video_type == MEDIA_NO_SERVICE);
+}
+
+
 bool IsInKioskMode() {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode))
     return true;
@@ -187,6 +296,14 @@ bool MediaStreamDevicesController::DismissInfoBarAndTakeActionOnSettings() {
     return true;
   }
 
+  // Check if the navigation entry has previously been granted access.
+  content::NavigationEntry* navigation_entry =
+      web_contents_->GetController().GetVisibleEntry();
+  if (IsRequestAllowedByNavigationEntry(navigation_entry, request_)) {

[no longer working on chromium, 2014/05/05 08:14:36]

Does this navigation entry has higher priority than the media default setting ?
For example, if the entry allows the permission, but users explicitly set the
media default setting to "Do not allow sites to access your camera and
microphone", the current code will allow the usage of the devices rather than
denying.

[tommi (sloooow) - chröme, 2014/05/05 08:28:53]

Permissions are stored with the navigation entry _after_ passing all checks, so
no.
Nope. See MediaStreamDevicesController::Deny below.

[no longer working on chromium, 2014/05/05 08:43:03]

Then shouldn't these code be moved after IsDefaultMediaAccessBlocked(),  which
is 10 lines below?

[tommi (sloooow) - chröme, 2014/05/05 08:50:31]

No, it should be here.  The permissions that are checked here will only be here
after the whole list of permissions has been checked for the first request and
only once all checks have been satisfied, do we actually store the permissions
with the navigation entry.

So, if we pass this check, it means that we have already passed
IsDefaultMediaAccessBlocked and all the other checks.

[no longer working on chromium, 2014/05/05 09:27:02]

I think I got your point.
But My question is about that the default setting is changed after the first
request.
Let me explain the use case in details:
After the users approve the first request, users go to content setting and
modify the default setting to "Do not allow sites to access your camera and
microphone", the current code will allow the usage of the device since the
navigation entry has the exception, even though the default setting is "Do not
allow".

[tommi (sloooow) - chröme, 2014/05/05 10:00:13]

Ah, I see. Yes you're right.  Thanks for catching that. Fixed.

+    Accept(false);
+    return true;
+  }
+
   // Filter any parts of the request that have been blocked by default and deny
   // it if nothing is left to accept.
   if (FilterBlockedByDefaultDevices() == 0) {
@@ -200,23 +317,6 @@ bool MediaStreamDevicesController::DismissInfoBarAndTakeActionOnSettings() {
     return true;
   }
 
-  if (request_.request_type == content::MEDIA_OPEN_DEVICE) {
-    bool no_matched_audio_device =
-        (request_.audio_type == content::MEDIA_DEVICE_AUDIO_CAPTURE &&
-         !request_.requested_audio_device_id.empty() &&
-         MediaCaptureDevicesDispatcher::GetInstance()->GetRequestedAudioDevice(
-             request_.requested_audio_device_id) == NULL);
-    bool no_matched_video_device =
-        (request_.video_type == content::MEDIA_DEVICE_VIDEO_CAPTURE &&
-         !request_.requested_video_device_id.empty() &&
-         MediaCaptureDevicesDispatcher::GetInstance()->GetRequestedVideoDevice(
-             request_.requested_video_device_id) == NULL);
-    if (no_matched_audio_device || no_matched_video_device) {
-      Deny(false, content::MEDIA_DEVICE_PERMISSION_DENIED);
-      return true;
-    }
-  }
-
   // Show the infobar.
   return false;
 }
@@ -305,6 +405,15 @@ void MediaStreamDevicesController::Accept(bool update_content_setting) {
                                           get_default_video_device,
                                           &devices);
         }
+
+        // Tag this navigation entry with the granted permissions.
+        // This avoids repeated prompts for requests accessed via http.
+        content::NavigationEntry* navigation_entry =
+            web_contents_->GetController().GetVisibleEntry();
+        if (navigation_entry) {
+          SetMediaPermissionsForNavigationEntry(
+              navigation_entry, request_, audio_allowed, video_allowed);
+        }
         break;
       }
       case content::MEDIA_DEVICE_ACCESS: {
@@ -354,6 +463,14 @@ void MediaStreamDevicesController::Deny(
   DLOG(WARNING) << "MediaStreamDevicesController::Deny: " << result;
   NotifyUIRequestDenied();
 
+  // Clear previously allowed permissions from the navigation entry if any.
+  content::NavigationEntry* navigation_entry =

[no longer working on chromium, 2014/05/05 08:14:36]

Deny(false) can be triggered by clicking "X" button, is it intentional to clear
the allowed permission?

[tommi (sloooow) - chröme, 2014/05/05 08:28:53]

Yes, intentional.

[no longer working on chromium, 2014/05/05 08:43:03]

Good.

+      web_contents_->GetController().GetVisibleEntry();
+  if (navigation_entry) {
+    SetMediaPermissionsForNavigationEntry(
+        navigation_entry, request_, false, false);
+  }
+
   if (update_content_setting) {
     CHECK_EQ(content::MEDIA_DEVICE_PERMISSION_DENIED, result);
     SetPermission(false);
@@ -591,7 +708,7 @@ void MediaStreamDevicesController::SetPermission(bool allowed) const {
       CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
   if (request_permissions_.find(content::MEDIA_DEVICE_AUDIO_CAPTURE) !=
       request_permissions_.end()) {
-      profile_->GetHostContentSettingsMap()->SetContentSetting(
+    profile_->GetHostContentSettingsMap()->SetContentSetting(
         primary_pattern,
         ContentSettingsPattern::Wildcard(),
         CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,