Bug 1477273 - Allow autoplay-media permission to be temporarily allowed. r=johannh
authorDale Harvey <dale@arandomurl.com>
Fri, 03 Aug 2018 16:00:55 +0100
changeset 486999 906fa624f03ba319ba1286058c13b2ead72678b4
parent 486998 42d6f6784fc21d04838241d35d3ceb82a1ecc935
child 487000 7df7135ad04c688b90bb8e6e995dde508b7f41ad
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1477273
milestone63.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 1477273 - Allow autoplay-media permission to be temporarily allowed. r=johannh MozReview-Commit-ID: JlnH2f1KW3U
browser/modules/PermissionUI.jsm
browser/modules/SitePermissions.jsm
browser/modules/test/browser/browser_SitePermissions.js
--- a/browser/modules/PermissionUI.jsm
+++ b/browser/modules/PermissionUI.jsm
@@ -332,18 +332,20 @@ var PermissionPromptPrototype = {
               // Only remember permission for session if in PB mode.
               if (PrivateBrowsingUtils.isBrowserPrivate(this.browser)) {
                 scope = SitePermissions.SCOPE_SESSION;
               }
               SitePermissions.set(this.principal.URI,
                                   this.permissionKey,
                                   promptAction.action,
                                   scope);
-            } else if (promptAction.action == SitePermissions.BLOCK) {
-              // Temporarily store BLOCK permissions only.
+            } else if (promptAction.action == SitePermissions.BLOCK ||
+                       SitePermissions.permitTemporaryAllow(this.permissionKey)) {
+              // Temporarily store BLOCK permissions only unless permission object
+              // sets permitTemporaryAllow: true
               // SitePermissions does not consider subframes when storing temporary
               // permissions on a tab, thus storing ALLOW could be exploited.
               SitePermissions.set(this.principal.URI,
                                   this.permissionKey,
                                   promptAction.action,
                                   SitePermissions.SCOPE_TEMPORARY,
                                   this.browser);
             }
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -6,26 +6,26 @@ var EXPORTED_SYMBOLS = [ "SitePermission
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 var gStringBundle =
   Services.strings.createBundle("chrome://browser/locale/sitePermissions.properties");
 
 /**
- * A helper module to manage temporarily blocked permissions.
+ * A helper module to manage temporary permissions.
  *
  * Permissions are keyed by browser, so methods take a Browser
  * element to identify the corresponding permission set.
  *
  * This uses a WeakMap to key browsers, so that entries are
  * automatically cleared once the browser stops existing
  * (once there are no other references to the browser object);
  */
-const TemporaryBlockedPermissions = {
+const TemporaryPermissions = {
   // This is a three level deep map with the following structure:
   //
   // Browser => {
   //   <prePath>: {
   //     <permissionID>: {Number} <timeStamp>
   //   }
   // }
   //
@@ -33,42 +33,42 @@ const TemporaryBlockedPermissions = {
   // value is an object with URI prePaths as keys. The keys of that object
   // are ids that identify permissions that were set for the specific URI.
   // The final value is an object containing the timestamp of when the permission
   // was set (in order to invalidate after a certain amount of time has passed).
   _stateByBrowser: new WeakMap(),
 
   // Private helper method that bundles some shared behavior for
   // get() and getAll(), e.g. deleting permissions when they have expired.
-  _get(entry, prePath, id, timeStamp) {
-    if (timeStamp == null) {
+  _get(entry, prePath, id, permission) {
+    if (permission == null || permission.timeStamp == null) {
       delete entry[prePath][id];
       return null;
     }
-    if (timeStamp + SitePermissions.temporaryPermissionExpireTime < Date.now()) {
+    if (permission.timeStamp + SitePermissions.temporaryPermissionExpireTime < Date.now()) {
       delete entry[prePath][id];
       return null;
     }
-    return {id, state: SitePermissions.BLOCK, scope: SitePermissions.SCOPE_TEMPORARY};
+    return {id, state: permission.state, scope: SitePermissions.SCOPE_TEMPORARY};
   },
 
   // Sets a new permission for the specified browser.
-  set(browser, id) {
+  set(browser, id, state) {
     if (!browser) {
       return;
     }
     if (!this._stateByBrowser.has(browser)) {
       this._stateByBrowser.set(browser, {});
     }
     let entry = this._stateByBrowser.get(browser);
     let prePath = browser.currentURI.prePath;
     if (!entry[prePath]) {
       entry[prePath] = {};
     }
-    entry[prePath][id] = Date.now();
+    entry[prePath][id] = {timeStamp: Date.now(), state};
   },
 
   // Removes a permission with the specified id for the specified browser.
   remove(browser, id) {
     if (!browser) {
       return;
     }
     let entry = this._stateByBrowser.get(browser);
@@ -287,17 +287,17 @@ var SitePermissions = {
    *         - state: a constant representing the current permission state
    *           (e.g. SitePermissions.ALLOW)
    *         - scope: a constant representing how long the permission will
    *           be kept.
    */
   getAllForBrowser(browser) {
     let permissions = {};
 
-    for (let permission of TemporaryBlockedPermissions.getAll(browser)) {
+    for (let permission of TemporaryPermissions.getAll(browser)) {
       permission.scope = this.SCOPE_TEMPORARY;
       permissions[permission.id] = permission;
     }
 
     for (let permission of GloballyBlockedPermissions.getAll(browser)) {
       permissions[permission.id] = permission;
     }
 
@@ -412,16 +412,33 @@ var SitePermissions = {
   showGloballyBlocked(permissionID) {
     if (permissionID in gPermissionObject &&
         gPermissionObject[permissionID].showGloballyBlocked)
       return gPermissionObject[permissionID].showGloballyBlocked;
 
     return false;
   },
 
+  /*
+   * Return whether SitePermissions is permitted to store a TEMPORARY ALLOW
+   * state for a particular permission.
+   *
+   * @param {string} permissionID
+   *        The ID to get the state for.
+   *
+   * @return boolean Whether storing TEMPORARY ALLOW is permitted.
+   */
+  permitTemporaryAllow(permissionID) {
+    if (permissionID in gPermissionObject &&
+        gPermissionObject[permissionID].permitTemporaryAllow)
+      return gPermissionObject[permissionID].permitTemporaryAllow;
+
+    return false;
+  },
+
   /**
    * Returns the state and scope of a particular permission for a given URI.
    *
    * This method will NOT dispatch a "PermissionStateChange" event on the specified
    * browser if a temporary permission was removed because it has expired.
    *
    * @param {nsIURI} uri
    *        The URI to check.
@@ -456,17 +473,17 @@ var SitePermissions = {
           result.scope = this.SCOPE_POLICY;
         }
       }
     }
 
     if (result.state == defaultState) {
       // If there's no persistent permission saved, check if we have something
       // set temporarily.
-      let value = TemporaryBlockedPermissions.get(browser, permissionID);
+      let value = TemporaryPermissions.get(browser, permissionID);
 
       if (value) {
         result.state = value.state;
         result.scope = this.SCOPE_TEMPORARY;
       }
     }
 
     return result;
@@ -515,27 +532,27 @@ var SitePermissions = {
     // Save temporary permissions.
     if (scope == this.SCOPE_TEMPORARY) {
       // We do not support setting temp ALLOW for security reasons.
       // In its current state, this permission could be exploited by subframes
       // on the same page. This is because for BLOCK we ignore the request
       // URI and only consider the current browser URI, to avoid notification spamming.
       //
       // If you ever consider removing this line, you likely want to implement
-      // a more fine-grained TemporaryBlockedPermissions that temporarily blocks for the
+      // a more fine-grained TemporaryPermissions that temporarily blocks for the
       // entire browser, but temporarily allows only for specific frames.
-      if (state != this.BLOCK) {
+      if (state != this.BLOCK && !this.permitTemporaryAllow(permissionID)) {
         throw "'Block' is the only permission we can save temporarily on a browser";
       }
 
       if (!browser) {
         throw "TEMPORARY scoped permissions require a browser object";
       }
 
-      TemporaryBlockedPermissions.set(browser, permissionID);
+      TemporaryPermissions.set(browser, permissionID, state);
 
       browser.dispatchEvent(new browser.ownerGlobal
                                        .CustomEvent("PermissionStateChange"));
     } else if (this.isSupportedURI(uri)) {
       let perms_scope = Services.perms.EXPIRE_NEVER;
       if (scope == this.SCOPE_SESSION) {
         perms_scope = Services.perms.EXPIRE_SESSION;
       } else if (scope == this.SCOPE_POLICY) {
@@ -557,47 +574,47 @@ var SitePermissions = {
    *        The id of the permission.
    * @param {Browser} browser (optional)
    *        The browser object to remove temporary permissions on.
    */
   remove(uri, permissionID, browser) {
     if (this.isSupportedURI(uri))
       Services.perms.remove(uri, permissionID);
 
-    // TemporaryBlockedPermissions.get() deletes expired permissions automatically,
-    if (TemporaryBlockedPermissions.get(browser, permissionID)) {
+    // TemporaryPermissions.get() deletes expired permissions automatically,
+    if (TemporaryPermissions.get(browser, permissionID)) {
       // If it exists but has not expired, remove it explicitly.
-      TemporaryBlockedPermissions.remove(browser, permissionID);
+      TemporaryPermissions.remove(browser, permissionID);
       // Send a PermissionStateChange event only if the permission hasn't expired.
       browser.dispatchEvent(new browser.ownerGlobal
                                        .CustomEvent("PermissionStateChange"));
     }
   },
 
   /**
    * Clears all permissions that were temporarily saved.
    *
    * @param {Browser} browser
    *        The browser object to clear.
    */
   clearTemporaryPermissions(browser) {
-    TemporaryBlockedPermissions.clear(browser);
+    TemporaryPermissions.clear(browser);
   },
 
   /**
    * Copy all permissions that were temporarily saved on one
    * browser object to a new browser.
    *
    * @param {Browser} browser
    *        The browser object to copy from.
    * @param {Browser} newBrowser
    *        The browser object to copy to.
    */
   copyTemporaryPermissions(browser, newBrowser) {
-    TemporaryBlockedPermissions.copy(browser, newBrowser);
+    TemporaryPermissions.copy(browser, newBrowser);
   },
 
   /**
    * Returns the localized label for the permission with the given ID, to be
    * used in a UI for managing permissions.
    *
    * @param {string} permissionID
    *        The permission to get the label for.
@@ -697,16 +714,17 @@ var gPermissionObject = {
    *    Defaults to ALLOW, BLOCK and the default state (see getDefault).
    *    The PROMPT_HIDE state is deliberately excluded from "plugin:flash" since we
    *    don't want to expose a "Hide Prompt" button to the user through pageinfo.
    */
 
   "autoplay-media": {
     exactHostMatch: true,
     showGloballyBlocked: true,
+    permitTemporaryAllow: true,
     getDefault() {
       let state = Services.prefs.getIntPref("media.autoplay.default",
                                             Ci.nsIAutoplay.PROMPT);
       if (state == Ci.nsIAutoplay.ALLOWED) {
         return SitePermissions.ALLOW;
       } if (state == Ci.nsIAutoplay.BLOCKED) {
         return SitePermissions.BLOCK;
       }
--- a/browser/modules/test/browser/browser_SitePermissions.js
+++ b/browser/modules/test/browser/browser_SitePermissions.js
@@ -14,16 +14,39 @@ add_task(async function testTempAllowThr
 
   await BrowserTestUtils.withNewTab(uri.spec, function(browser) {
     Assert.throws(function() {
       SitePermissions.set(uri, id, SitePermissions.ALLOW, SitePermissions.SCOPE_TEMPORARY, browser);
     }, /'Block' is the only permission we can save temporarily on a browser/);
   });
 });
 
+// Tests that we can set TEMPORARY ALLOW permissions for autoplay-media
+add_task(async function testTempAutoplayAllowed() {
+  let uri = Services.io.newURI("https://example.com");
+  let permId = "autoplay-media";
+
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, uri.spec);
+
+  SitePermissions.set(uri, permId, SitePermissions.ALLOW,
+                      SitePermissions.SCOPE_TEMPORARY, tab.linkedBrowser);
+
+  let permissions = SitePermissions.getAllPermissionDetailsForBrowser(tab.linkedBrowser);
+
+  let autoplay = permissions.find(({id}) => id === "autoplay-media");
+  Assert.deepEqual(autoplay, {
+    id: "autoplay-media",
+    label: "Automatically Play Media with Sound",
+    state: SitePermissions.ALLOW,
+    scope: SitePermissions.SCOPE_TEMPORARY,
+  });
+
+  BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});
+
 // This tests the SitePermissions.getAllPermissionDetailsForBrowser function.
 add_task(async function testGetAllPermissionDetailsForBrowser() {
   let uri = Services.io.newURI("https://example.com");
 
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, uri.spec);
 
   Services.prefs.setIntPref("permissions.default.shortcuts", 2);