Bug 1703674 - Revoking cam or mic permission revokes both if either is capturing. r=pbz
authorJan-Ivar Bruaroey <jib@mozilla.com>
Fri, 09 Apr 2021 20:48:41 +0000
changeset 575313 abc615e5574f0d1cf7ffc544e1068023bee3003c
parent 575312 f89e1e839debb71ee6a3f49fee146a0d03c6a537
child 575314 e9057a86060fe3110bc65bd404a81096e43b3daa
push id140649
push userjbruaroey@mozilla.com
push dateFri, 09 Apr 2021 21:15:23 +0000
treeherderautoland@abc615e5574f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbz
bugs1703674
milestone89.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1703674 - Revoking cam or mic permission revokes both if either is capturing. r=pbz Differential Revision: https://phabricator.services.mozilla.com/D111174
browser/base/content/test/webrtc/browser_devices_get_user_media.js
browser/base/content/test/webrtc/head.js
browser/modules/webrtcUI.jsm
--- a/browser/base/content/test/webrtc/browser_devices_get_user_media.js
+++ b/browser/base/content/test/webrtc/browser_devices_get_user_media.js
@@ -648,17 +648,22 @@ var gTests = [
       info("deny video, request video, expect denied");
       await usePerm(undefined, false, false, true, false);
     },
   },
 
   {
     desc: "Stop Sharing removes permissions",
     run: async function checkStopSharingRemovesPermissions() {
-      async function stopAndCheckPerm(aRequestAudio, aRequestVideo) {
+      async function stopAndCheckPerm(
+        aRequestAudio,
+        aRequestVideo,
+        aStopAudio = aRequestAudio,
+        aStopVideo = aRequestVideo
+      ) {
         let uri = gBrowser.selectedBrowser.documentURI;
 
         // Initially set both permissions to 'allow'.
         PermissionTestUtils.add(uri, "microphone", Services.perms.ALLOW_ACTION);
         PermissionTestUtils.add(uri, "camera", Services.perms.ALLOW_ACTION);
         // Also set device-specific temporary allows.
         SitePermissions.setForPrincipal(
           gBrowser.contentPrincipal,
@@ -672,56 +677,66 @@ var gTests = [
           gBrowser.contentPrincipal,
           "camera^myDevice2",
           SitePermissions.ALLOW,
           SitePermissions.SCOPE_TEMPORARY,
           gBrowser.selectedBrowser,
           10000000
         );
 
-        let indicator = promiseIndicatorWindow();
-        let observerPromise1 = expectObserverCalled("getUserMedia:request");
-        let observerPromise2 = expectObserverCalled(
-          "getUserMedia:response:allow"
-        );
-        let observerPromise3 = expectObserverCalled("recording-device-events");
-        // Start sharing what's been requested.
-        let promise = promiseMessage("ok");
-        await promiseRequestDevice(aRequestAudio, aRequestVideo);
-        await promise;
-        await observerPromise1;
-        await observerPromise2;
-        await observerPromise3;
+        if (aRequestAudio || aRequestVideo) {
+          let indicator = promiseIndicatorWindow();
+          let observerPromise1 = expectObserverCalled("getUserMedia:request");
+          let observerPromise2 = expectObserverCalled(
+            "getUserMedia:response:allow"
+          );
+          let observerPromise3 = expectObserverCalled(
+            "recording-device-events"
+          );
+          // Start sharing what's been requested.
+          let promise = promiseMessage("ok");
+          await promiseRequestDevice(aRequestAudio, aRequestVideo);
+          await promise;
+          await observerPromise1;
+          await observerPromise2;
+          await observerPromise3;
 
-        await indicator;
-        await checkSharingUI(
-          { video: aRequestVideo, audio: aRequestAudio },
-          undefined,
-          undefined,
-          {
-            video: { scope: SitePermissions.SCOPE_PERSISTENT },
-            audio: { scope: SitePermissions.SCOPE_PERSISTENT },
-          }
-        );
-
-        await stopSharing(aRequestVideo ? "camera" : "microphone");
+          await indicator;
+          await checkSharingUI(
+            { video: aRequestVideo, audio: aRequestAudio },
+            undefined,
+            undefined,
+            {
+              video: { scope: SitePermissions.SCOPE_PERSISTENT },
+              audio: { scope: SitePermissions.SCOPE_PERSISTENT },
+            }
+          );
+          await stopSharing(aStopVideo ? "camera" : "microphone");
+        } else {
+          await revokePermission(aStopVideo ? "camera" : "microphone");
+        }
 
         // Check that permissions have been removed as expected.
         let audioPerm = SitePermissions.getForPrincipal(
           gBrowser.contentPrincipal,
           "microphone",
           gBrowser.selectedBrowser
         );
         let audioPermDevice = SitePermissions.getForPrincipal(
           gBrowser.contentPrincipal,
           "microphone^myDevice",
           gBrowser.selectedBrowser
         );
 
-        if (aRequestAudio) {
+        if (
+          aRequestAudio ||
+          aRequestVideo ||
+          aStopAudio ||
+          (aStopVideo && aRequestAudio)
+        ) {
           Assert.deepEqual(
             audioPerm,
             {
               state: SitePermissions.UNKNOWN,
               scope: SitePermissions.SCOPE_PERSISTENT,
             },
             "microphone permissions removed"
           );
@@ -757,17 +772,22 @@ var gTests = [
           "camera",
           gBrowser.selectedBrowser
         );
         let videoPermDevice = SitePermissions.getForPrincipal(
           gBrowser.contentPrincipal,
           "camera^myDevice2",
           gBrowser.selectedBrowser
         );
-        if (aRequestVideo) {
+        if (
+          aRequestAudio ||
+          aRequestVideo ||
+          aStopVideo ||
+          (aStopAudio && aRequestVideo)
+        ) {
           Assert.deepEqual(
             videoPerm,
             {
               state: SitePermissions.UNKNOWN,
               scope: SitePermissions.SCOPE_PERSISTENT,
             },
             "camera permissions removed"
           );
@@ -792,38 +812,47 @@ var gTests = [
             videoPermDevice,
             {
               state: SitePermissions.ALLOW,
               scope: SitePermissions.SCOPE_TEMPORARY,
             },
             "camera device-specific permissions untouched"
           );
         }
+        await checkNotSharing();
 
         // Cleanup.
         await closeStream(true);
 
         SitePermissions.removeFromPrincipal(
           gBrowser.contentPrincipal,
           "camera",
           gBrowser.selectedBrowser
         );
         SitePermissions.removeFromPrincipal(
           gBrowser.contentPrincipal,
           "microphone",
           gBrowser.selectedBrowser
         );
       }
 
-      info("request audio+video, stop sharing resets both");
+      info("request audio+video, stop sharing video resets both");
       await stopAndCheckPerm(true, true);
-      info("request audio, stop sharing resets audio only");
+      info("request audio only, stop sharing audio resets both");
       await stopAndCheckPerm(true, false);
-      info("request video, stop sharing resets video only");
+      info("request video only, stop sharing video resets both");
       await stopAndCheckPerm(false, true);
+      info("request audio only, stop sharing video resets both");
+      await stopAndCheckPerm(true, false, false, true);
+      info("request video only, stop sharing audio resets both");
+      await stopAndCheckPerm(false, true, true, false);
+      info("request neither, stop audio affects audio only");
+      await stopAndCheckPerm(false, false, true, false);
+      info("request neither, stop video affects video only");
+      await stopAndCheckPerm(false, false, false, true);
     },
   },
 
   {
     desc: "test showPermissionPanel",
     run: async function checkShowPermissionPanel() {
       if (!USING_LEGACY_INDICATOR) {
         // The indicator only links to the permission panel for the
--- a/browser/base/content/test/webrtc/head.js
+++ b/browser/base/content/test/webrtc/head.js
@@ -577,32 +577,16 @@ async function stopSharing(
   aFrameBC,
   aWindow = window
 ) {
   let promiseRecordingEvent = expectObserverCalled(
     "recording-device-events",
     1,
     aFrameBC
   );
-  aWindow.gPermissionPanel._identityPermissionBox.click();
-  let popup = aWindow.gPermissionPanel._permissionPopup;
-  // If the popup gets hidden before being shown, by stray focus/activate
-  // events, don't bother failing the test. It's enough to know that we
-  // started showing the popup.
-  let hiddenEvent = BrowserTestUtils.waitForEvent(popup, "popuphidden");
-  let shownEvent = BrowserTestUtils.waitForEvent(popup, "popupshown");
-  await Promise.race([hiddenEvent, shownEvent]);
-  let doc = aWindow.document;
-  let permissions = doc.getElementById("permission-popup-permission-list");
-  let cancelButton = permissions.querySelector(
-    ".permission-popup-permission-icon." +
-      aType +
-      "-icon ~ " +
-      ".permission-popup-permission-remove-button"
-  );
   let observerPromise1 = expectObserverCalled(
     "getUserMedia:revoke",
     1,
     aFrameBC
   );
 
   // If we are stopping screen sharing and expect to still have another stream,
   // "recording-window-ended" won't be fired.
@@ -610,28 +594,57 @@ async function stopSharing(
   if (!aShouldKeepSharing) {
     observerPromise2 = expectObserverCalled(
       "recording-window-ended",
       1,
       aFrameBC
     );
   }
 
-  cancelButton.click();
-  popup.hidePopup();
-
+  await revokePermission(aType, aShouldKeepSharing, aFrameBC, aWindow);
   await promiseRecordingEvent;
   await observerPromise1;
   await observerPromise2;
 
   if (!aShouldKeepSharing) {
     await checkNotSharing();
   }
 }
 
+async function revokePermission(
+  aType = "camera",
+  aShouldKeepSharing = false,
+  aFrameBC,
+  aWindow = window
+) {
+  aWindow.gPermissionPanel._identityPermissionBox.click();
+  let popup = aWindow.gPermissionPanel._permissionPopup;
+  // If the popup gets hidden before being shown, by stray focus/activate
+  // events, don't bother failing the test. It's enough to know that we
+  // started showing the popup.
+  let hiddenEvent = BrowserTestUtils.waitForEvent(popup, "popuphidden");
+  let shownEvent = BrowserTestUtils.waitForEvent(popup, "popupshown");
+  await Promise.race([hiddenEvent, shownEvent]);
+  let doc = aWindow.document;
+  let permissions = doc.getElementById("permission-popup-permission-list");
+  let cancelButton = permissions.querySelector(
+    ".permission-popup-permission-icon." +
+      aType +
+      "-icon ~ " +
+      ".permission-popup-permission-remove-button"
+  );
+
+  cancelButton.click();
+  popup.hidePopup();
+
+  if (!aShouldKeepSharing) {
+    await checkNotSharing();
+  }
+}
+
 function getBrowsingContextForFrame(aBrowsingContext, aFrameId) {
   if (!aFrameId) {
     return aBrowsingContext;
   }
 
   return SpecialPowers.spawn(aBrowsingContext, [aFrameId], frameId => {
     return content.document.getElementById(frameId).browsingContext;
   });
--- a/browser/modules/webrtcUI.jsm
+++ b/browser/modules/webrtcUI.jsm
@@ -595,32 +595,28 @@ var webrtcUI = {
     let browser = tab.linkedBrowser;
     let sharingState = tab._sharingState?.webRTC;
 
     // If we clear a WebRTC permission we need to remove all permissions of
     // the same type across device ids. We also need to stop active WebRTC
     // devices related to the permission.
     let perms = SitePermissions.getAllForBrowser(browser);
 
-    let sharingCameraAndMic =
-      sharingState?.camera &&
-      sharingState?.microphone &&
+    // If capturing, don't revoke one of camera/microphone without the other.
+    let sharingCameraOrMic =
+      (sharingState?.camera || sharingState?.microphone) &&
       (types.includes("camera") || types.includes("microphone"));
+
     perms
       .filter(perm => {
-        let [permId] = perm.id.split(SitePermissions.PERM_KEY_DELIMITER);
-        // It's not possible to stop sharing one of camera/microphone
-        // without the other.
-        if (
-          sharingCameraAndMic &&
-          (permId == "camera" || permId == "microphone")
-        ) {
+        let [id] = perm.id.split(SitePermissions.PERM_KEY_DELIMITER);
+        if (sharingCameraOrMic && (id == "camera" || id == "microphone")) {
           return true;
         }
-        return types.includes(permId);
+        return types.includes(id);
       })
       .forEach(perm => {
         SitePermissions.removeFromPrincipal(
           browser.contentPrincipal,
           perm.id,
           browser
         );
       });
@@ -632,20 +628,17 @@ var webrtcUI = {
     // If the device of the permission we're clearing is currently active,
     // tell the WebRTC implementation to stop sharing it.
     let { windowId } = sharingState;
 
     let windowIds = [];
     if (types.includes("screen") && sharingState.screen) {
       windowIds.push(`screen:${windowId}`);
     }
-    if (
-      (types.includes("camera") && sharingState.camera) ||
-      (types.includes("microphone") && sharingState.microphone)
-    ) {
+    if (sharingCameraOrMic) {
       windowIds.push(windowId);
     }
 
     if (!windowIds.length) {
       return;
     }
 
     let actor = sharingState.browsingContext.currentWindowGlobal.getActor(