Bug 1263378 - Intermittent leaks from browser_bug461710.js. r=standard8 draft
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 21 Jun 2017 13:30:13 +0200
changeset 599195 5cdb8a0de4cdc5afd923ab596f9c01ebf344bc9f
parent 599167 61edb6b490719686c8e9f8e3bf4bdd401aa1c2b5
child 634703 17e3cc169c8b04f251589193f2e124853b233160
push id65447
push usermak77@bonardo.net
push dateThu, 22 Jun 2017 20:33:57 +0000
reviewersstandard8
bugs1263378, 461710
milestone56.0a1
Bug 1263378 - Intermittent leaks from browser_bug461710.js. r=standard8 MozReview-Commit-ID: FQQkGaiE0xZ
browser/base/content/test/tabs/browser_navigatePinnedTab.js
browser/base/content/test/urlbar/browser_bug304198.js
browser/base/content/test/urlbar/browser_pasteAndGo.js
browser/components/places/tests/browser/browser_bookmarklet_windowOpen.js
testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
toolkit/components/places/tests/browser/browser_bug399606.js
toolkit/components/places/tests/browser/browser_bug461710.js
uriloader/exthandler/tests/mochitest/browser_web_protocol_handlers.js
--- a/browser/base/content/test/tabs/browser_navigatePinnedTab.js
+++ b/browser/base/content/test/tabs/browser_navigatePinnedTab.js
@@ -26,17 +26,17 @@ add_task(async function() {
   await BrowserTestUtils.browserLoaded(browser);
 
   is(appTab.linkedBrowser.currentURI.spec, TEST_LINK_CHANGED,
      "New page loaded in the app tab");
   is(gBrowser.tabs.length, initialTabsNo, "No additional tabs were opened");
 
   // Now check that opening a link that does create a new tab works,
   // and also that it nulls out the opener.
-  let pageLoadPromise = BrowserTestUtils.browserLoaded(appTab.linkedBrowser, "http://example.com/");
+  let pageLoadPromise = BrowserTestUtils.browserLoaded(appTab.linkedBrowser, false, "http://example.com/");
   await BrowserTestUtils.loadURI(appTab.linkedBrowser, "http://example.com/");
   info("Started loading example.com");
   await pageLoadPromise;
   info("Loaded example.com");
   let newTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, "http://example.org/");
   await ContentTask.spawn(browser, null, async function() {
     let link = content.document.createElement("a");
     link.href = "http://example.org/";
--- a/browser/base/content/test/urlbar/browser_bug304198.js
+++ b/browser/base/content/test/urlbar/browser_bug304198.js
@@ -6,19 +6,19 @@ add_task(async function() {
   let charsToDelete, deletedURLTab, fullURLTab, partialURLTab, testPartialURL, testURL;
 
   charsToDelete = 5;
   deletedURLTab = BrowserTestUtils.addTab(gBrowser);
   fullURLTab = BrowserTestUtils.addTab(gBrowser);
   partialURLTab = BrowserTestUtils.addTab(gBrowser);
   testURL = "http://example.org/browser/browser/base/content/test/urlbar/dummy_page.html";
 
-  let loaded1 = BrowserTestUtils.browserLoaded(deletedURLTab.linkedBrowser, testURL);
-  let loaded2 = BrowserTestUtils.browserLoaded(fullURLTab.linkedBrowser, testURL);
-  let loaded3 = BrowserTestUtils.browserLoaded(partialURLTab.linkedBrowser, testURL);
+  let loaded1 = BrowserTestUtils.browserLoaded(deletedURLTab.linkedBrowser, false, testURL);
+  let loaded2 = BrowserTestUtils.browserLoaded(fullURLTab.linkedBrowser, false, testURL);
+  let loaded3 = BrowserTestUtils.browserLoaded(partialURLTab.linkedBrowser, false, testURL);
   deletedURLTab.linkedBrowser.loadURI(testURL);
   fullURLTab.linkedBrowser.loadURI(testURL);
   partialURLTab.linkedBrowser.loadURI(testURL);
   await Promise.all([loaded1, loaded2, loaded3]);
 
   testURL = gURLBar.trimValue(testURL);
   testPartialURL = testURL.substr(0, (testURL.length - charsToDelete));
 
--- a/browser/base/content/test/urlbar/browser_pasteAndGo.js
+++ b/browser/base/content/test/urlbar/browser_pasteAndGo.js
@@ -22,17 +22,17 @@ add_task(async function() {
         "anonid", "textbox-input-box");
       let cxmenu = document.getAnonymousElementByAttribute(textBox,
         "anonid", "input-box-contextmenu");
       let cxmenuPromise = BrowserTestUtils.waitForEvent(cxmenu, "popupshown");
       EventUtils.synthesizeMouseAtCenter(gURLBar, {type: "contextmenu", button: 2});
       await cxmenuPromise;
       let menuitem = document.getAnonymousElementByAttribute(textBox,
         "anonid", "paste-and-go");
-      let browserLoadedPromise = BrowserTestUtils.browserLoaded(browser, url.replace(/\n/g, ""));
+      let browserLoadedPromise = BrowserTestUtils.browserLoaded(browser, false, url.replace(/\n/g, ""));
       EventUtils.synthesizeMouseAtCenter(menuitem, {});
       // Using toSource in order to get the newlines escaped:
       info("Paste and go, loading " + url.toSource());
       await browserLoadedPromise;
       ok(true, "Successfully loaded " + url);
     });
   }
 });
--- a/browser/components/places/tests/browser/browser_bookmarklet_windowOpen.js
+++ b/browser/components/places/tests/browser/browser_bookmarklet_windowOpen.js
@@ -46,15 +46,15 @@ add_task(async function openKeywordBookm
   EventUtils.synthesizeKey("VK_RETURN", {});
 
   info("Waiting for tab being created");
   let {target: tab} = await tabCreatedPromise;
   info("Got tab");
   let browser = tab.linkedBrowser;
   if (!browser.currentURI || browser.currentURI.spec != TEST_URL) {
     info("Waiting for browser load");
-    await BrowserTestUtils.browserLoaded(browser);
+    await BrowserTestUtils.browserLoaded(browser, false, TEST_URL);
   }
   is(browser.currentURI && browser.currentURI.spec, TEST_URL, "Tab with expected URL loaded.");
   info("Waiting to remove tab");
   await Promise.all([ BrowserTestUtils.removeTab(tab),
                       BrowserTestUtils.removeTab(moztab) ]);
 });
--- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
+++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ -256,16 +256,21 @@ this.BrowserTestUtils = {
    *        If a function, takes a URL and returns true if that's the load we're
    *        interested in. If a string, gives the URL of the load we're interested
    *        in. If not present, the first load resolves the promise.
    *
    * @return {Promise}
    * @resolves When a load event is triggered for the browser.
    */
   browserLoaded(browser, includeSubFrames=false, wantLoad=null) {
+    // Passing a url as second argument is a common mistake we should prevent.
+    if (includeSubFrames && typeof includeSubFrames != "boolean") {
+      throw("The second argument to browserLoaded should be a boolean.");
+    }
+
     // If browser belongs to tabbrowser-tab, ensure it has been
     // inserted into the document.
     let tabbrowser = browser.ownerGlobal.gBrowser;
     if (tabbrowser && tabbrowser.getTabForBrowser) {
       tabbrowser._insertBrowser(tabbrowser.getTabForBrowser(browser));
     }
 
     function isWanted(url) {
--- a/toolkit/components/places/tests/browser/browser_bug399606.js
+++ b/toolkit/components/places/tests/browser/browser_bug399606.js
@@ -1,76 +1,55 @@
 /* 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/. */
 
-gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
+add_task(async function() {
+  registerCleanupFunction(PlacesTestUtils.clearHistory);
 
-function test() {
-  waitForExplicitFinish();
-
-  var URIs = [
+  const URIS = [
     "http://example.com/tests/toolkit/components/places/tests/browser/399606-window.location.href.html",
     "http://example.com/tests/toolkit/components/places/tests/browser/399606-history.go-0.html",
     "http://example.com/tests/toolkit/components/places/tests/browser/399606-location.replace.html",
     "http://example.com/tests/toolkit/components/places/tests/browser/399606-location.reload.html",
     "http://example.com/tests/toolkit/components/places/tests/browser/399606-httprefresh.html",
     "http://example.com/tests/toolkit/components/places/tests/browser/399606-window.location.html",
   ];
-  var hs = Cc["@mozilla.org/browser/nav-history-service;1"].
-           getService(Ci.nsINavHistoryService);
 
   // Create and add history observer.
-  var historyObserver = {
-    visitCount: [],
+  let historyObserver = {
+    count: 0,
+    expectedURI: null,
+    onVisit(aURI) {
+      info("Received onVisit: " + aURI.spec);
+      if (aURI.equals(this.expectedURI)) {
+        this.count++;
+      }
+    },
     onBeginUpdateBatch() {},
     onEndUpdateBatch() {},
-    onVisit(aURI, aVisitID, aTime, aSessionID, aReferringID,
-                      aTransitionType) {
-      info("Received onVisit: " + aURI.spec);
-      if (aURI.spec in this.visitCount)
-        this.visitCount[aURI.spec]++;
-      else
-        this.visitCount[aURI.spec] = 1;
-    },
     onTitleChanged() {},
     onDeleteURI() {},
     onClearHistory() {},
     onPageChanged() {},
     onDeleteVisits() {},
     QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver])
   };
-  hs.addObserver(historyObserver);
 
-  function confirm_results() {
-    gBrowser.removeCurrentTab();
-    hs.removeObserver(historyObserver, false);
-    for (let aURI in historyObserver.visitCount) {
-      is(historyObserver.visitCount[aURI], 1,
-         "onVisit has been received right number of times for " + aURI);
-    }
-    PlacesTestUtils.clearHistory().then(finish);
+  async function promiseLoadedThreeTimes(uri) {
+    historyObserver.count = 0;
+    historyObserver.expectedURI = Services.io.newURI(uri);
+    let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+    PlacesUtils.history.addObserver(historyObserver);
+    gBrowser.loadURI(uri);
+    await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false, uri);
+    await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false, uri);
+    await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false, uri);
+    PlacesUtils.history.removeObserver(historyObserver);
+    await BrowserTestUtils.removeTab(tab);
   }
 
-  var loadCount = 0;
-  function handleLoad(aEvent) {
-    loadCount++;
-    info("new load count is " + loadCount);
-
-    if (loadCount == 3) {
-      gBrowser.removeEventListener("DOMContentLoaded", handleLoad, true);
-      gBrowser.loadURI("about:blank");
-      executeSoon(check_next_uri);
-    }
+  for (let uri of URIS) {
+    await promiseLoadedThreeTimes(uri);
+    is(historyObserver.count, 1,
+      "onVisit has been received right number of times for " + uri);
   }
-
-  function check_next_uri() {
-    if (URIs.length) {
-      let uri = URIs.shift();
-      loadCount = 0;
-      gBrowser.addEventListener("DOMContentLoaded", handleLoad, true);
-      gBrowser.loadURI(uri);
-    } else {
-      confirm_results();
-    }
-  }
-  executeSoon(check_next_uri);
-}
+});
--- a/toolkit/components/places/tests/browser/browser_bug461710.js
+++ b/toolkit/components/places/tests/browser/browser_bug461710.js
@@ -1,82 +1,78 @@
 const kRed = "rgb(255, 0, 0)";
 const kBlue = "rgb(0, 0, 255)";
 
 const prefix = "http://example.com/tests/toolkit/components/places/tests/browser/461710_";
 
 add_task(async function() {
+  registerCleanupFunction(PlacesTestUtils.clearHistory);
   let contentPage = prefix + "iframe.html";
-  let normalWindow = await BrowserTestUtils.openNewBrowserWindow();
 
-  let browser = normalWindow.gBrowser.selectedBrowser;
-  BrowserTestUtils.loadURI(browser, contentPage);
-  await BrowserTestUtils.browserLoaded(browser, contentPage);
+  let normalWindow = await BrowserTestUtils.openNewBrowserWindow();
+  let normalBrowser = normalWindow.gBrowser.selectedBrowser;
+  await BrowserTestUtils.loadURI(normalBrowser, contentPage);
+  await BrowserTestUtils.browserLoaded(normalBrowser, false, contentPage);
 
   let privateWindow = await BrowserTestUtils.openNewBrowserWindow({private: true});
-
-  browser = privateWindow.gBrowser.selectedBrowser;
-  BrowserTestUtils.loadURI(browser, contentPage);
-  await BrowserTestUtils.browserLoaded(browser, contentPage);
+  let privateBrowser = privateWindow.gBrowser.selectedBrowser;
+  BrowserTestUtils.loadURI(privateBrowser, contentPage);
+  await BrowserTestUtils.browserLoaded(privateBrowser, false, contentPage);
 
   let tests = [{
-    win: normalWindow,
+    private: false,
     topic: "uri-visit-saved",
     subtest: "visited_page.html"
   }, {
-    win: normalWindow,
+    private: false,
     topic: "visited-status-resolution",
     subtest: "link_page.html",
     color: kRed,
     message: "Visited link coloring should work outside of private mode"
   }, {
-    win: privateWindow,
+    private: true,
     topic: "visited-status-resolution",
     subtest: "link_page-2.html",
     color: kBlue,
     message: "Visited link coloring should not work inside of private mode"
   }, {
-    win: normalWindow,
+    private: false,
     topic: "visited-status-resolution",
     subtest: "link_page-3.html",
     color: kRed,
     message: "Visited link coloring should work outside of private mode"
   }];
 
-  let visited_page_url = prefix + tests[0].subtest;
+  let uri = Services.io.newURI(prefix + tests[0].subtest);
   for (let test of tests) {
-    let promise = new Promise(resolve => {
-      let uri = NetUtil.newURI(visited_page_url);
-      Services.obs.addObserver(function observe(aSubject) {
-        if (uri.equals(aSubject.QueryInterface(Ci.nsIURI))) {
-          Services.obs.removeObserver(observe, test.topic);
-          resolve();
-        }
-      }, test.topic);
-    });
-    ContentTask.spawn(test.win.gBrowser.selectedBrowser, prefix + test.subtest, async function(aSrc) {
+    let promise = TestUtils.topicObserved(test.topic,
+      subject => uri.equals(subject.QueryInterface(Ci.nsIURI)));
+    let browser = test.private ? privateBrowser : normalBrowser;
+    await ContentTask.spawn(browser, prefix + test.subtest, async function(aSrc) {
       content.document.getElementById("iframe").src = aSrc;
     });
     await promise;
 
     if (test.color) {
       // In e10s waiting for visited-status-resolution is not enough to ensure links
       // have been updated, because it only tells us that messages to update links
       // have been dispatched. We must still wait for the actual links to update.
       await BrowserTestUtils.waitForCondition(async function() {
-        let color = await ContentTask.spawn(test.win.gBrowser.selectedBrowser, null, async function() {
+        let color = await ContentTask.spawn(browser, null, async function() {
           let iframe = content.document.getElementById("iframe");
           let elem = iframe.contentDocument.getElementById("link");
           return content.QueryInterface(Ci.nsIInterfaceRequestor)
                         .getInterface(Ci.nsIDOMWindowUtils)
                         .getVisitedDependentComputedStyle(elem, "", "color");
         });
         return (color == test.color);
       }, test.message);
       // The harness will consider the test as failed overall if there were no
       // passes or failures, so record it as a pass.
       ok(true, test.message);
     }
   }
 
-  await BrowserTestUtils.closeWindow(normalWindow);
+  let promisePBExit = TestUtils.topicObserved("last-pb-context-exited");
   await BrowserTestUtils.closeWindow(privateWindow);
+  await promisePBExit;
+  await BrowserTestUtils.closeWindow(normalWindow);
 });
--- a/uriloader/exthandler/tests/mochitest/browser_web_protocol_handlers.js
+++ b/uriloader/exthandler/tests/mochitest/browser_web_protocol_handlers.js
@@ -1,16 +1,16 @@
 let testURL = "http://example.com/browser/" +
   "uriloader/exthandler/tests/mochitest/protocolHandler.html";
 
 add_task(async function() {
   // Load a page registering a protocol handler.
   let browser = gBrowser.selectedBrowser;
   browser.loadURI(testURL);
-  await BrowserTestUtils.browserLoaded(browser, testURL);
+  await BrowserTestUtils.browserLoaded(browser, false, testURL);
 
   // Register the protocol handler by clicking the notificationbar button.
   let notificationValue = "Protocol Registration: testprotocol";
   let getNotification = () =>
     gBrowser.getNotificationBox().getNotificationWithValue(notificationValue);
   await BrowserTestUtils.waitForCondition(getNotification);
   let notification = getNotification();
   let button =