Bug 1000253 - Remove the 'Always' option from the webrtc permissions prompt. r=jesup, r=MattN, a=lsblakk
authorFlorian Quèze <florian@queze.net>
Mon, 26 May 2014 08:52:00 -0400
changeset 193430 3a0a14c737bc4e3d460908307f8c824049d35a5c
parent 193429 5ac65440631d09f53c2434d6616977529b51ef06
child 193431 35ec38e5d96ea024a11ad200b0958c4a4475da9c
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup, MattN, lsblakk
bugs1000253
milestone30.0
Bug 1000253 - Remove the 'Always' option from the webrtc permissions prompt. r=jesup, r=MattN, a=lsblakk
browser/base/content/test/general/browser_devices_get_user_media.js
browser/modules/SitePermissions.jsm
browser/modules/webrtcUI.jsm
dom/media/MediaManager.cpp
--- a/browser/base/content/test/general/browser_devices_get_user_media.js
+++ b/browser/base/content/test/general/browser_devices_get_user_media.js
@@ -122,19 +122,18 @@ function promiseNoPopupNotification(aNam
     ok(!PopupNotifications.getNotification(aName),
        aName + " notification removed");
     deferred.resolve();
   }, "timeout waiting for popup notification " + aName + " to disappear");
 
   return deferred.promise;
 }
 
-const kActionAlways = 1;
-const kActionDeny = 2;
-const kActionNever = 3;
+const kActionDeny = 1;
+const kActionNever = 2;
 
 function activateSecondaryAction(aAction) {
   let notification = PopupNotifications.panel.firstChild;
   notification.button.focus();
   let popup = notification.menupopup;
   popup.addEventListener("popupshown", function () {
     popup.removeEventListener("popupshown", arguments.callee, false);
 
@@ -448,43 +447,43 @@ let gTests = [
     checkNotSharing();
 
     // the stream is already closed, but this will do some cleanup anyway
     yield closeStream(true);
   }
 },
 
 {
-  desc: "getUserMedia prompt: Always/Never Share",
-  run: function checkRememberCheckbox() {
+  desc: "getUserMedia prompt: Never Share",
+  run: function checkNeverShare() {
     let elt = id => document.getElementById(id);
 
     function checkPerm(aRequestAudio, aRequestVideo, aAllowAudio, aAllowVideo,
-                       aExpectedAudioPerm, aExpectedVideoPerm, aNever) {
+                       aExpectedAudioPerm, aExpectedVideoPerm) {
       yield promisePopupNotificationShown("webRTC-shareDevices", () => {
         content.wrappedJSObject.requestDevice(aRequestAudio, aRequestVideo);
       });
       expectObserverCalled("getUserMedia:request");
 
       let noAudio = aAllowAudio === undefined;
       is(elt("webRTC-selectMicrophone").hidden, noAudio,
          "microphone selector expected to be " + (noAudio ? "hidden" : "visible"));
       if (!noAudio)
-        elt("webRTC-selectMicrophone-menulist").value = (aAllowAudio || aNever) ? 0 : -1;
+        elt("webRTC-selectMicrophone-menulist").value = 0;
 
       let noVideo = aAllowVideo === undefined;
       is(elt("webRTC-selectCamera").hidden, noVideo,
          "camera selector expected to be " + (noVideo ? "hidden" : "visible"));
       if (!noVideo)
-        elt("webRTC-selectCamera-menulist").value = (aAllowVideo || aNever) ? 0 : -1;
+        elt("webRTC-selectCamera-menulist").value = 0;
 
       let expectedMessage =
         (aAllowVideo || aAllowAudio) ? "ok" : "error: PERMISSION_DENIED";
       yield promiseMessage(expectedMessage, () => {
-        activateSecondaryAction(aNever ? kActionNever : kActionAlways);
+        activateSecondaryAction(kActionNever);
       });
       let expected = [];
       if (expectedMessage == "ok") {
         expectObserverCalled("getUserMedia:response:allow");
         expectObserverCalled("recording-device-events");
         if (aAllowVideo)
           expected.push("Camera");
         if (aAllowAudio)
@@ -513,59 +512,40 @@ let gTests = [
       }
       checkDevicePermissions("microphone", aExpectedAudioPerm);
       checkDevicePermissions("camera", aExpectedVideoPerm);
 
       if (expectedMessage == "ok")
         yield closeStream();
     }
 
-    // 3 cases where the user accepts the device prompt.
-    info("audio+video, user grants, expect both perms set to allow");
-    yield checkPerm(true, true, true, true, true, true);
-    info("audio only, user grants, check audio perm set to allow, video perm not set");
-    yield checkPerm(true, false, true, undefined, true, undefined);
-    info("video only, user grants, check video perm set to allow, audio perm not set");
-    yield checkPerm(false, true, undefined, true, undefined, true);
-
-    // 3 cases where the user rejects the device request.
-    // First test these cases by setting the device to 'No Audio'/'No Video'
+    // 3 cases where the user rejects the device request by using the 'Never Share' action.
     info("audio+video, user denies, expect both perms set to deny");
     yield checkPerm(true, true, false, false, false, false);
     info("audio only, user denies, expect audio perm set to deny, video not set");
     yield checkPerm(true, false, false, undefined, false, undefined);
     info("video only, user denies, expect video perm set to deny, audio perm not set");
     yield checkPerm(false, true, undefined, false, undefined, false);
-    // Now test these 3 cases again by using the 'Never Share' action.
-    info("audio+video, user denies, expect both perms set to deny");
-    yield checkPerm(true, true, false, false, false, false, true);
-    info("audio only, user denies, expect audio perm set to deny, video not set");
-    yield checkPerm(true, false, false, undefined, false, undefined, true);
-    info("video only, user denies, expect video perm set to deny, audio perm not set");
-    yield checkPerm(false, true, undefined, false, undefined, false, true);
-
-    // 2 cases where the user allows half of what's requested.
-    info("audio+video, user denies video, grants audio, " +
-         "expect video perm set to deny, audio perm set to allow.");
-    yield checkPerm(true, true, true, false, true, false);
-    info("audio+video, user denies audio, grants video, " +
-         "expect video perm set to allow, audio perm set to deny.");
-    yield checkPerm(true, true, false, true, false, true);
 
     // reset the menuitems to have no impact on the following tests.
     elt("webRTC-selectMicrophone-menulist").value = 0;
     elt("webRTC-selectCamera-menulist").value = 0;
   }
 },
 
 {
   desc: "getUserMedia without prompt: use persistent permissions",
   run: function checkUsePersistentPermissions() {
     function usePerm(aAllowAudio, aAllowVideo, aRequestAudio, aRequestVideo,
                      aExpectStream) {
+
+      // The 'Allow' option is temporarily disabled.
+      if (aExpectStream)
+        aExpectStream = undefined;
+
       let Perms = Services.perms;
       let uri = content.document.documentURIObject;
       if (aAllowAudio !== undefined) {
         Perms.add(uri, "microphone", aAllowAudio ? Perms.ALLOW_ACTION
                                                  : Perms.DENY_ACTION);
       }
       if (aAllowVideo !== undefined) {
         Perms.add(uri, "camera", aAllowVideo ? Perms.ALLOW_ACTION
@@ -584,18 +564,17 @@ let gTests = [
         // Deny the request to cleanup...
         yield promiseMessage("error: PERMISSION_DENIED", () => {
           activateSecondaryAction(kActionDeny);
         });
         expectObserverCalled("getUserMedia:response:deny");
         expectObserverCalled("recording-window-ended");
       }
       else {
-        let allow = (aAllowVideo && aRequestVideo) || (aAllowAudio && aRequestAudio);
-        let expectedMessage = allow ? "ok" : "error: PERMISSION_DENIED";
+        let expectedMessage = aExpectStream ? "ok" : "error: PERMISSION_DENIED";
         yield promiseMessage(expectedMessage, gum);
 
         if (expectedMessage == "ok") {
           expectObserverCalled("recording-device-events");
 
           // Check what's actually shared.
           let expected = [];
           if (aAllowVideo && aRequestVideo)
@@ -630,19 +609,19 @@ let gTests = [
     yield usePerm(true, false, true, false, true);
     info("allow audio, deny video, request video, expect denied");
     yield usePerm(true, false, false, true, false);
 
     // Deny audio, allow video.
     info("deny audio, allow video, request audio+video, expect ok (video)");
     yield usePerm(false, true, true, true, true);
     info("deny audio, allow video, request audio, expect denied");
-    yield usePerm(false, true, true, false, true);
+    yield usePerm(false, true, true, false, false);
     info("deny audio, allow video, request video, expect ok (video)");
-    yield usePerm(false, true, false, true, false);
+    yield usePerm(false, true, false, true, true);
 
     // Allow audio, video not set.
     info("allow audio, request audio+video, expect prompt");
     yield usePerm(true, undefined, true, true, undefined);
     info("allow audio, request audio, expect ok (audio)");
     yield usePerm(true, undefined, true, false, true);
     info("allow audio, request video, expect prompt");
     yield usePerm(true, undefined, false, true, undefined);
@@ -669,85 +648,16 @@ let gTests = [
     info("deny video, request audio, expect prompt");
     yield usePerm(undefined, false, true, false, undefined);
     info("deny video, request video, expect denied");
     yield usePerm(undefined, false, false, true, false);
   }
 },
 
 {
-  desc: "Stop Sharing removes persistent permissions",
-  run: function checkStopSharingRemovesPersistentPermissions() {
-    function stopAndCheckPerm(aRequestAudio, aRequestVideo) {
-      let Perms = Services.perms;
-      let uri = content.document.documentURIObject;
-
-      // Initially set both permissions to 'allow'.
-      Perms.add(uri, "microphone", Perms.ALLOW_ACTION);
-      Perms.add(uri, "camera", Perms.ALLOW_ACTION);
-
-      // Start sharing what's been requested.
-      yield promiseMessage("ok", () => {
-        content.wrappedJSObject.requestDevice(aRequestAudio, aRequestVideo);
-      });
-      expectObserverCalled("recording-device-events");
-      yield checkSharingUI();
-
-      PopupNotifications.getNotification("webRTC-sharingDevices").reshow();
-      let expectedIcon = "webRTC-sharingDevices";
-      if (aRequestAudio && !aRequestVideo)
-        expectedIcon = "webRTC-sharingMicrophone";
-      is(PopupNotifications.getNotification("webRTC-sharingDevices").anchorID,
-         expectedIcon + "-notification-icon", "anchored to correct icon");
-      is(PopupNotifications.panel.firstChild.getAttribute("popupid"), expectedIcon,
-         "panel using correct icon");
-
-      // Stop sharing.
-      activateSecondaryAction(kActionDeny);
-
-      yield promiseObserverCalled("recording-device-events");
-      expectObserverCalled("getUserMedia:revoke");
-
-      yield promiseNoPopupNotification("webRTC-sharingDevices");
-
-      if (gObservedTopics["recording-device-events"] == 1) {
-        todo(false, "Got the 'recording-device-events' notification twice, likely because of bug 962719");
-        gObservedTopics["recording-device-events"] = 0;
-      }
-
-      // Check that permissions have been removed as expected.
-      let audioPerm = Perms.testExactPermission(uri, "microphone");
-      if (aRequestAudio)
-        is(audioPerm, Perms.UNKNOWN_ACTION, "microphone permissions removed");
-      else
-        is(audioPerm, Perms.ALLOW_ACTION, "microphone permissions untouched");
-
-      let videoPerm = Perms.testExactPermission(uri, "camera");
-      if (aRequestVideo)
-        is(videoPerm, Perms.UNKNOWN_ACTION, "camera permissions removed");
-      else
-        is(videoPerm, Perms.ALLOW_ACTION, "camera permissions untouched");
-
-      // Cleanup.
-      yield closeStream(true);
-
-      Perms.remove(uri.host, "camera");
-      Perms.remove(uri.host, "microphone");
-    }
-
-    info("request audio+video, stop sharing resets both");
-    yield stopAndCheckPerm(true, true);
-    info("request audio, stop sharing resets audio only");
-    yield stopAndCheckPerm(true, false);
-    info("request video, stop sharing resets video only");
-    yield stopAndCheckPerm(false, true);
-  }
-},
-
-{
   desc: "'Always Allow' ignored and not shown on http pages",
   run: function checkNoAlwaysOnHttp() {
     // Load an http page instead of the https version.
     let deferred = Promise.defer();
     let browser = gBrowser.selectedTab.linkedBrowser;
     browser.addEventListener("load", function onload() {
       browser.removeEventListener("load", onload, true);
       deferred.resolve();
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -181,18 +181,22 @@ let gPermissionObject = {
         return SitePermissions.SESSION;
 
       return SitePermissions.ALLOW;
     }
   },
 
   "desktop-notification": {},
 
-  "camera": {},
-  "microphone": {},
+  "camera": {
+    states: [ SitePermissions.UNKNOWN, SitePermissions.BLOCK ]
+  },
+  "microphone": {
+    states: [ SitePermissions.UNKNOWN, SitePermissions.BLOCK ]
+  },
 
   "popup": {
     getDefault: function () {
       return Services.prefs.getBoolPref("dom.disable_open_during_load") ?
                SitePermissions.BLOCK : SitePermissions.ALLOW;
     }
   },
 
--- a/browser/modules/webrtcUI.jsm
+++ b/browser/modules/webrtcUI.jsm
@@ -158,27 +158,16 @@ function prompt(aContentWindow, aCallID,
         if (audioDevices.length)
           perms.add(uri, "microphone", perms.DENY_ACTION);
         if (videoDevices.length)
           perms.add(uri, "camera", perms.DENY_ACTION);
       }
     }
   ];
 
-  if (aSecure) {
-    // Don't show the 'Always' action if the connection isn't secure.
-    secondaryActions.unshift({
-      label: stringBundle.getString("getUserMedia.always.label"),
-      accessKey: stringBundle.getString("getUserMedia.always.accesskey"),
-      callback: function () {
-        mainAction.callback(true);
-      }
-    });
-  }
-
   let options = {
     eventCallback: function(aTopic, aNewBrowser) {
       if (aTopic == "swapping")
         return true;
 
       let chromeDoc = this.browser.ownerDocument;
 
       if (aTopic == "shown") {
@@ -217,39 +206,31 @@ function prompt(aContentWindow, aCallID,
       listDevices(camMenupopup, videoDevices);
       listDevices(micMenupopup, audioDevices);
       if (requestType == "CameraAndMicrophone") {
         let stringBundle = chromeDoc.defaultView.gNavigatorBundle;
         addDeviceToList(camMenupopup, stringBundle.getString("getUserMedia.noVideo.label"), "-1");
         addDeviceToList(micMenupopup, stringBundle.getString("getUserMedia.noAudio.label"), "-1");
       }
 
-      this.mainAction.callback = function(aRemember) {
+      this.mainAction.callback = function() {
         let allowedDevices = Cc["@mozilla.org/supports-array;1"]
                                .createInstance(Ci.nsISupportsArray);
         let perms = Services.perms;
         if (videoDevices.length) {
           let videoDeviceIndex = chromeDoc.getElementById("webRTC-selectCamera-menulist").value;
           let allowCamera = videoDeviceIndex != "-1";
           if (allowCamera)
             allowedDevices.AppendElement(videoDevices[videoDeviceIndex]);
-          if (aRemember) {
-            perms.add(uri, "camera",
-                      allowCamera ? perms.ALLOW_ACTION : perms.DENY_ACTION);
-          }
         }
         if (audioDevices.length) {
           let audioDeviceIndex = chromeDoc.getElementById("webRTC-selectMicrophone-menulist").value;
           let allowMic = audioDeviceIndex != "-1";
           if (allowMic)
             allowedDevices.AppendElement(audioDevices[audioDeviceIndex]);
-          if (aRemember) {
-            perms.add(uri, "microphone",
-                      allowMic ? perms.ALLOW_ACTION : perms.DENY_ACTION);
-          }
         }
 
         if (allowedDevices.Count() == 0) {
           denyRequest(aCallID);
           return;
         }
 
         Services.obs.notifyObservers(allowedDevices, "getUserMedia:response:allow", aCallID);
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -1433,39 +1433,31 @@ MediaManager::GetUserMedia(JSContext* aC
       do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
 
     uint32_t audioPerm = nsIPermissionManager::UNKNOWN_ACTION;
     if (c.mAudio) {
       rv = permManager->TestExactPermissionFromPrincipal(
         aWindow->GetExtantDoc()->NodePrincipal(), "microphone", &audioPerm);
       NS_ENSURE_SUCCESS(rv, rv);
-      if (audioPerm == nsIPermissionManager::PROMPT_ACTION) {
+      if (audioPerm == nsIPermissionManager::PROMPT_ACTION ||
+          audioPerm == nsIPermissionManager::ALLOW_ACTION) {
         audioPerm = nsIPermissionManager::UNKNOWN_ACTION;
       }
-      if (audioPerm == nsIPermissionManager::ALLOW_ACTION) {
-        if (!isHTTPS) {
-          audioPerm = nsIPermissionManager::UNKNOWN_ACTION;
-        }
-      }
     }
 
     uint32_t videoPerm = nsIPermissionManager::UNKNOWN_ACTION;
     if (c.mVideo) {
       rv = permManager->TestExactPermissionFromPrincipal(
         aWindow->GetExtantDoc()->NodePrincipal(), "camera", &videoPerm);
       NS_ENSURE_SUCCESS(rv, rv);
-      if (videoPerm == nsIPermissionManager::PROMPT_ACTION) {
+      if (videoPerm == nsIPermissionManager::PROMPT_ACTION ||
+          videoPerm == nsIPermissionManager::ALLOW_ACTION) {
         videoPerm = nsIPermissionManager::UNKNOWN_ACTION;
       }
-      if (videoPerm == nsIPermissionManager::ALLOW_ACTION) {
-        if (!isHTTPS) {
-          videoPerm = nsIPermissionManager::UNKNOWN_ACTION;
-        }
-      }
     }
 
     if ((!c.mAudio || audioPerm != nsIPermissionManager::UNKNOWN_ACTION) &&
         (!c.mVideo || videoPerm != nsIPermissionManager::UNKNOWN_ACTION)) {
       // All permissions we were about to request already have a saved value.
       if (c.mAudio && audioPerm == nsIPermissionManager::DENY_ACTION) {
         c.mAudio = false;
         runnable->SetContraints(c);