Bug 1334842 - Fix intermittent browser_temporary_permissions.js. r=Nihanth, a=test-only
authorJohann Hofmann <jhofmann@mozilla.com>
Wed, 29 Mar 2017 16:15:56 +0200
changeset 395592 2e38543017394458b73390d6f58ea2964147165d
parent 395591 fe8f22399ef3d6c8d2e224ff48c450ac004563c4
child 395593 500c5e8ec870c4dfe84effdfe1bd92f0bba5cc7f
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersNihanth, test-only
bugs1334842
milestone54.0a2
Bug 1334842 - Fix intermittent browser_temporary_permissions.js. r=Nihanth, a=test-only This intermittent was likely occurring because we set the expiry timeout for temporary permissions to a really low value in the previous test. The failing test was only failing on slow machines, leading me to believe that the time between setting and checking was larger than the 500ms timeout defined in the previous test. Thus, the permission was reset on checking it. The expiry pref was set using pushPrefEnv, which restores prefs only after the entire test was run. To just eradicate this category of problems in the future I moved the test that manipulates the expiry into its own file. MozReview-Commit-ID: 3mc5xHY4XLn
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_temporary_permissions.js
browser/base/content/test/general/browser_temporary_permissions_expiry.js
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -47,16 +47,17 @@ support-files =
   healthreport_pingData.js
   healthreport_testRemoteCommands.html
   moz.png
   navigating_window_with_download.html
   offlineQuotaNotification.cacheManifest
   offlineQuotaNotification.html
   page_style_sample.html
   parsingTestHelpers.jsm
+  permissions.html
   pinning_headers.sjs
   ssl_error_reports.sjs
   print_postdata.sjs
   searchSuggestionEngine.sjs
   searchSuggestionEngine.xml
   searchSuggestionEngine2.xml
   subtst_contextmenu.html
   subtst_contextmenu_input.html
@@ -285,18 +286,16 @@ support-files =
   svg_image.html
 [browser_page_style_menu.js]
 [browser_page_style_menu_update.js]
 [browser_parsable_css.js]
 skip-if = debug # no point in running on both opt and debug, and will likely intermittently timeout on debug
 [browser_parsable_script.js]
 skip-if = asan || (os == 'linux' && !debug && (bits == 32)) # disabled on asan because of timeouts, and bug 1172468 for the linux 32-bit pgo issue.
 [browser_permissions.js]
-support-files =
-  permissions.html
 [browser_pinnedTabs.js]
 [browser_plainTextLinks.js]
 [browser_printpreview.js]
 [browser_private_browsing_window.js]
 [browser_private_no_prompt.js]
 [browser_purgehistory_clears_sh.js]
 [browser_PageMetaData_pushstate.js]
 [browser_refreshBlocker.js]
@@ -352,19 +351,19 @@ skip-if = (os == "mac" && !e10s) # Bug 1
 [browser_tabs_close_beforeunload.js]
 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_expiry.js]
 [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,23 +1,19 @@
 /* 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 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(ORIGIN);
   let id = "geo";
 
   yield BrowserTestUtils.withNewTab(uri.spec, function*(browser) {
     SitePermissions.set(uri, id, SitePermissions.BLOCK, SitePermissions.SCOPE_TEMPORARY, browser);
 
@@ -31,77 +27,25 @@ 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(ORIGIN);
   let id = "geo";
 
   yield BrowserTestUtils.withNewTab(SUBFRAME_PAGE, function*(browser) {
     let popupshown = BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown");
 
-    // Request a permission;
+    // Request a permission.
     yield ContentTask.spawn(browser, uri.host, function(host) {
       E10SUtils.wrapHandlingUserInput(content, true, function() {
         let frame = content.document.getElementById("frame");
         let frameDoc = frame.contentWindow.document;
 
         // Make sure that the origin of our test page is different.
         Assert.notEqual(frameDoc.location.host, host);
 
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/browser_temporary_permissions_expiry.js
@@ -0,0 +1,62 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const ORIGIN = "https://example.com";
+const PERMISSIONS_PAGE = getRootDirectory(gTestPath).replace("chrome://mochitests/content", ORIGIN) + "permissions.html";
+
+const EXPIRE_TIME_MS = 100;
+const TIMEOUT_MS = 500;
+
+// Test that temporary permissions can be re-requested after they expired
+// and that the identity block is updated accordingly.
+// TODO (bug 1349144): 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);
+  });
+});