Bug 1331579 - Explicitly update the identity block on re-requesting expired permissions. r=Paolo draft
authorJohann Hofmann <jhofmann@mozilla.com>
Mon, 23 Jan 2017 11:44:03 +0100
changeset 467197 58e6f5b8020c01fad8da2b29e78a0c69bb78cb63
parent 466730 a338e596b1d9f37186aaeddcfaa572ae043e578d
child 543636 f777c4ffa65f68d704059163e6da185f1a7df948
push id43122
push userbmo:jhofmann@mozilla.com
push dateFri, 27 Jan 2017 10:51:18 +0000
reviewersPaolo
bugs1331579
milestone54.0a1
Bug 1331579 - Explicitly update the identity block on re-requesting expired permissions. r=Paolo MozReview-Commit-ID: CxvHSp1NjFg
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_temporary_permissions.js
browser/modules/PermissionUI.jsm
browser/modules/SitePermissions.jsm
browser/modules/test/browser_SitePermissions_expiry.js
browser/modules/webrtcUI.jsm
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -427,16 +427,17 @@ support-files =
   close_beforeunload_opens_second_tab.html
   close_beforeunload.html
 [browser_tabs_isActive.js]
 [browser_tabs_owner.js]
 [browser_temporary_permissions.js]
 support-files =
   permissions.html
   temporary_permissions_subframe.html
+  ../webrtc/get_user_media.html
 [browser_temporary_permissions_navigation.js]
 [browser_temporary_permissions_tabs.js]
 [browser_testOpenNewRemoteTabsFromNonRemoteBrowsers.js]
 run-if = e10s
 [browser_trackingUI_1.js]
 tags = trackingprotection
 support-files =
   trackingPage.html
--- a/browser/base/content/test/general/browser_temporary_permissions.js
+++ b/browser/base/content/test/general/browser_temporary_permissions.js
@@ -1,21 +1,26 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 Cu.import("resource:///modules/SitePermissions.jsm", this);
 Cu.import("resource:///modules/E10SUtils.jsm");
 
-const SUBFRAME_PAGE = "https://example.com/browser/browser/base/content/test/general/temporary_permissions_subframe.html";
+const ORIGIN = "https://example.com";
+const PERMISSIONS_PAGE = getRootDirectory(gTestPath).replace("chrome://mochitests/content", ORIGIN) + "permissions.html"
+const SUBFRAME_PAGE = getRootDirectory(gTestPath).replace("chrome://mochitests/content", ORIGIN) + "temporary_permissions_subframe.html"
+
+const EXPIRE_TIME_MS = 100;
+const TIMEOUT_MS = 500;
 
 // Test that setting temp permissions triggers a change in the identity block.
 add_task(function* testTempPermissionChangeEvents() {
-  let uri = NetUtil.newURI("https://example.com");
+  let uri = NetUtil.newURI(ORIGIN);
   let id = "geo";
 
   yield BrowserTestUtils.withNewTab(uri.spec, function*(browser) {
     SitePermissions.set(uri, id, SitePermissions.BLOCK, SitePermissions.SCOPE_TEMPORARY, browser);
 
     Assert.deepEqual(SitePermissions.get(uri, id, browser), {
       state: SitePermissions.BLOCK,
       scope: SitePermissions.SCOPE_TEMPORARY,
@@ -26,19 +31,71 @@ add_task(function* testTempPermissionCha
     Assert.notEqual(geoIcon.boxObject.width, 0, "geo anchor should be visible");
 
     SitePermissions.remove(uri, id, browser);
 
     Assert.equal(geoIcon.boxObject.width, 0, "geo anchor should not be visible");
   });
 });
 
+// Test that temporary permissions can be re-requested after they expired
+// and that the identity block is updated accordingly.
+// TODO (blocked by bug 1334421): Write a check for webrtc permissions,
+// because they use a different code path.
+add_task(function* testTempPermissionRequestAfterExpiry() {
+  yield SpecialPowers.pushPrefEnv({set: [
+        ["privacy.temporary_permission_expire_time_ms", EXPIRE_TIME_MS],
+  ]});
+
+  let uri = NetUtil.newURI(ORIGIN);
+  let id = "geo";
+
+  yield BrowserTestUtils.withNewTab(PERMISSIONS_PAGE, function*(browser) {
+    let blockedIcon = gIdentityHandler._identityBox
+      .querySelector(`.blocked-permission-icon[data-permission-id='${id}']`);
+
+    SitePermissions.set(uri, id, SitePermissions.BLOCK, SitePermissions.SCOPE_TEMPORARY, browser);
+
+    Assert.deepEqual(SitePermissions.get(uri, id, browser), {
+      state: SitePermissions.BLOCK,
+      scope: SitePermissions.SCOPE_TEMPORARY,
+    });
+
+    ok(blockedIcon.hasAttribute("showing"), "blocked permission icon is shown");
+
+    yield new Promise((c) => setTimeout(c, TIMEOUT_MS));
+
+    Assert.deepEqual(SitePermissions.get(uri, id, browser), {
+      state: SitePermissions.UNKNOWN,
+      scope: SitePermissions.SCOPE_PERSISTENT,
+    });
+
+    let popupshown = BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown");
+
+    // Request a permission;
+    yield BrowserTestUtils.synthesizeMouseAtCenter(`#${id}`, {}, browser);
+
+    yield popupshown;
+
+    ok(!blockedIcon.hasAttribute("showing"), "blocked permission icon is not shown");
+
+    let popuphidden = BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popuphidden");
+
+    let notification = PopupNotifications.panel.firstChild;
+    EventUtils.synthesizeMouseAtCenter(notification.secondaryButton, {});
+
+    yield popuphidden;
+
+    SitePermissions.remove(uri, id, browser);
+  });
+});
+
 // Test that temp blocked permissions requested by subframes (with a different URI) affect the whole page.
 add_task(function* testTempPermissionSubframes() {
-  let uri = NetUtil.newURI("https://example.com");
+  let uri = NetUtil.newURI(ORIGIN);
   let id = "geo";
 
   yield BrowserTestUtils.withNewTab(SUBFRAME_PAGE, function*(browser) {
     let popupshown = BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown");
 
     // Request a permission;
     yield ContentTask.spawn(browser, uri.host, function(host) {
       E10SUtils.wrapHandlingUserInput(content, true, function() {
--- a/browser/modules/PermissionUI.jsm
+++ b/browser/modules/PermissionUI.jsm
@@ -274,16 +274,21 @@ this.PermissionPromptPrototype = {
         this.cancel();
         return;
       }
 
       if (state == SitePermissions.ALLOW) {
         this.allow();
         return;
       }
+
+      // Tell the browser to refresh the identity block display in case there
+      // are expired permission states.
+      this.browser.dispatchEvent(new this.browser.ownerGlobal
+                                         .CustomEvent("PermissionStateChange"));
     }
 
     // Transform the PermissionPrompt actions into PopupNotification actions.
     let popupNotificationActions = [];
     for (let promptAction of this.promptActions) {
       let action = {
         label: promptAction.label,
         accessKey: promptAction.accessKey,
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@@ -124,16 +124,23 @@ const TemporaryBlockedPermissions = {
   copy(browser, newBrowser) {
     let entry = this._stateByBrowser.get(browser);
     if (entry) {
       this._stateByBrowser.set(newBrowser, entry);
     }
   },
 };
 
+/**
+ * A module to manage permanent and temporary permissions
+ * by URI and browser.
+ *
+ * Some methods have the side effect of dispatching a "PermissionStateChange"
+ * event on changes to temporary permissions, as mentioned in the respective docs.
+ */
 this.SitePermissions = {
   // Permission states.
   UNKNOWN: Services.perms.UNKNOWN_ACTION,
   ALLOW: Services.perms.ALLOW_ACTION,
   BLOCK: Services.perms.DENY_ACTION,
   ALLOW_COOKIES_FOR_SESSION: Components.interfaces.nsICookiePermission.ACCESS_SESSION,
 
   // Permission scopes.
@@ -291,16 +298,19 @@ this.SitePermissions = {
       return gPermissionObject[permissionID].getDefault();
 
     return this.UNKNOWN;
   },
 
   /**
    * 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.
    * @param {String} permissionID
    *        The id of the permission.
    * @param {Browser} browser (optional)
    *        The browser object to check for temporary permissions.
    *
    * @return {Object} an object with the keys:
@@ -339,16 +349,18 @@ this.SitePermissions = {
       }
     }
 
     return result;
   },
 
   /**
    * Sets the state of a particular permission for a given URI or browser.
+   * This method will dispatch a "PermissionStateChange" event on the specified
+   * browser if a temporary permission was set
    *
    * @param {nsIURI} uri
    *        The URI to set the permission for.
    *        Note that this will be ignored if the scope is set to SCOPE_TEMPORARY
    * @param {String} permissionID
    *        The id of the permission.
    * @param {SitePermissions state} state
    *        The state of the permission.
@@ -397,16 +409,18 @@ this.SitePermissions = {
       }
 
       Services.perms.add(uri, permissionID, state, perms_scope);
     }
   },
 
   /**
    * Removes the saved state of a particular permission for a given URI and/or browser.
+   * This method will dispatch a "PermissionStateChange" event on the specified
+   * browser if a temporary permission was removed.
    *
    * @param {nsIURI} uri
    *        The URI to remove the permission for.
    * @param {String} permissionID
    *        The id of the permission.
    * @param {Browser} browser (optional)
    *        The browser object to remove temporary permissions on.
    */
--- a/browser/modules/test/browser_SitePermissions_expiry.js
+++ b/browser/modules/test/browser_SitePermissions_expiry.js
@@ -1,33 +1,36 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 Cu.import("resource:///modules/SitePermissions.jsm", this);
 
+const EXPIRE_TIME_MS = 100;
+const TIMEOUT_MS = 500;
+
 // This tests the time delay to expire temporary permission entries.
 add_task(function* testTemporaryPermissionExpiry() {
   SpecialPowers.pushPrefEnv({set: [
-        ["privacy.temporary_permission_expire_time_ms", 100],
+        ["privacy.temporary_permission_expire_time_ms", EXPIRE_TIME_MS],
   ]});
 
   let uri = Services.io.newURI("https://example.com")
   let id = "camera";
 
   yield BrowserTestUtils.withNewTab(uri.spec, function*(browser) {
     SitePermissions.set(uri, id, SitePermissions.BLOCK, SitePermissions.SCOPE_TEMPORARY, browser);
 
     Assert.deepEqual(SitePermissions.get(uri, id, browser), {
       state: SitePermissions.BLOCK,
       scope: SitePermissions.SCOPE_TEMPORARY,
     });
 
-    yield new Promise((c) => setTimeout(c, 500));
+    yield new Promise((c) => setTimeout(c, TIMEOUT_MS));
 
     Assert.deepEqual(SitePermissions.get(uri, id, browser), {
       state: SitePermissions.UNKNOWN,
       scope: SitePermissions.SCOPE_PERSISTENT,
     });
   });
 });
--- a/browser/modules/webrtcUI.jsm
+++ b/browser/modules/webrtcUI.jsm
@@ -368,16 +368,21 @@ function prompt(aBrowser, aRequest) {
   if ((audioDevices.length && SitePermissions
         .get(null, "microphone", aBrowser).state == SitePermissions.BLOCK) ||
       (videoDevices.length && SitePermissions
         .get(null, sharingScreen ? "screen" : "camera", aBrowser).state == SitePermissions.BLOCK)) {
     denyRequest(aBrowser, aRequest);
     return;
   }
 
+  // Tell the browser to refresh the identity block display in case there
+  // are expired permission states.
+  aBrowser.dispatchEvent(new aBrowser.ownerGlobal
+                                     .CustomEvent("PermissionStateChange"));
+
   let uri = Services.io.newURI(aRequest.documentURI);
   let host = getHost(uri);
   let chromeDoc = aBrowser.ownerDocument;
   let stringBundle = chromeDoc.defaultView.gNavigatorBundle;
 
   // Mind the order, because for simplicity we're iterating over the list using
   // "includes()". This allows the rotation of string identifiers. We list the
   // full identifiers here so they can be cross-referenced more easily.