Bug 1379447 - fix intermittent failure in browser_alltabslistener caused by tab hitting about:blank when waiting for page stops, r?dao draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 19 Jul 2017 23:17:01 +0100
changeset 615877 243bd38cd094042b301625c554a3d3af82e1b1ef
parent 615779 e8400551c2e39f24c75a009ebed496c7acd7bf47
child 639317 b647174f966b03fc87ddeebc93301bf9e7c739bb
push id70520
push usergijskruitbosch@gmail.com
push dateWed, 26 Jul 2017 14:31:54 +0000
reviewersdao
bugs1379447
milestone56.0a1
Bug 1379447 - fix intermittent failure in browser_alltabslistener caused by tab hitting about:blank when waiting for page stops, r?dao MozReview-Commit-ID: Sn4uj4jMDc
browser/base/content/test/general/browser_alltabslistener.js
browser/base/content/test/general/browser_bug477014.js
browser/base/content/test/general/browser_e10s_switchbrowser.js
browser/base/content/test/general/browser_restore_isAppTab.js
browser/base/content/test/general/head.js
browser/base/content/test/urlbar/browser_urlbar_stop_pending.js
testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
--- a/browser/base/content/test/general/browser_alltabslistener.js
+++ b/browser/base/content/test/general/browser_alltabslistener.js
@@ -91,18 +91,18 @@ function test() {
   gForegroundTab = BrowserTestUtils.addTab(gBrowser);
   gBackgroundBrowser = gBrowser.getBrowserForTab(gBackgroundTab);
   gForegroundBrowser = gBrowser.getBrowserForTab(gForegroundTab);
   gBrowser.selectedTab = gForegroundTab;
 
   // We must wait until a page has completed loading before
   // starting tests or we get notifications from that
   let promises = [
-    waitForDocLoadComplete(gBackgroundBrowser),
-    waitForDocLoadComplete(gForegroundBrowser)
+    BrowserTestUtils.browserStopped(gBackgroundBrowser, kBasePage),
+    BrowserTestUtils.browserStopped(gForegroundBrowser, kBasePage)
   ];
   gBackgroundBrowser.loadURI(kBasePage);
   gForegroundBrowser.loadURI(kBasePage);
   Promise.all(promises).then(startTest1);
 }
 
 function runTest(browser, url, next) {
   gFrontNotificationsPos = 0;
--- a/browser/base/content/test/general/browser_bug477014.js
+++ b/browser/base/content/test/general/browser_bug477014.js
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // That's a gecko!
 const iconURLSpec = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABGdBTUEAAK/INwWK6QAAABl0RVh0U29mdHdhcmUAQWRvYmUgSW1hZ2VSZWFkeXHJZTwAAAHWSURBVHjaYvz//z8DJQAggJiQOe/fv2fv7Oz8rays/N+VkfG/iYnJfyD/1+rVq7ffu3dPFpsBAAHEAHIBCJ85c8bN2Nj4vwsDw/8zQLwKiO8CcRoQu0DxqlWrdsHUwzBAAIGJmTNnPgYa9j8UqhFElwPxf2MIDeIrKSn9FwSJoRkAEEAM0DD4DzMAyPi/G+QKY4hh5WAXGf8PDQ0FGwJ22d27CjADAAIIrLmjo+MXA9R2kAHvGBA2wwx6B8W7od6CeQcggKCmCEL8bgwxYCbUIGTDVkHDBia+CuotgACCueD3TDQN75D4xmAvCoK9ARMHBzAw0AECiBHkAlC0Mdy7x9ABNA3obAZXIAa6iKEcGlMVQHwWyjYuL2d4v2cPg8vZswx7gHyAAAK7AOif7SAbOqCmn4Ha3AHFsIDtgPq/vLz8P4MSkJ2W9h8ggBjevXvHDo4FQUQg/kdypqCg4H8lUIACnQ/SOBMYI8bAsAJFPcj1AAEEjwVQqLpAbXmH5BJjqI0gi9DTAAgDBBCcAVLkgmQ7yKCZxpCQxqUZhAECCJ4XgMl493ug21ZD+aDAXH0WLM4A9MZPXJkJIIAwTAR5pQMalaCABQUULttBGCCAGCnNzgABBgAMJ5THwGvJLAAAAABJRU5ErkJggg==";
 var testPage = "data:text/plain,test bug 477014";
 
 add_task(async function() {
   let tabToDetach = BrowserTestUtils.addTab(gBrowser, testPage);
-  await waitForDocLoadComplete(tabToDetach.linkedBrowser);
+  await BrowserTestUtils.browserStopped(tabToDetach.linkedBrowser);
 
   gBrowser.setIcon(tabToDetach, iconURLSpec,
                    Services.scriptSecurityManager.getSystemPrincipal());
   tabToDetach.setAttribute("busy", "true");
 
   // detach and set the listener on the new window
   let newWindow = gBrowser.replaceTabWithWindow(tabToDetach);
   await promiseWaitForEvent(tabToDetach.linkedBrowser, "SwapDocShells");
--- a/browser/base/content/test/general/browser_e10s_switchbrowser.js
+++ b/browser/base/content/test/general/browser_e10s_switchbrowser.js
@@ -61,30 +61,30 @@ function clear_history() {
 // Waits for a load and updates the known history
 var waitForLoad = async function(uri) {
   info("Loading " + uri);
   // Longwinded but this ensures we don't just shortcut to LoadInNewProcess
   gBrowser.selectedBrowser.webNavigation.loadURI(uri, Ci.nsIWebNavigation.LOAD_FLAGS_NONE,
                                                  null, null, null,
                                                  Services.scriptSecurityManager.getSystemPrincipal());
 
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   gExpectedHistory.index++;
   gExpectedHistory.entries.push({
     uri: gBrowser.currentURI.spec,
     title: gBrowser.contentTitle
   });
 };
 
 // Waits for a load and updates the known history
 var waitForLoadWithFlags = async function(uri, flags = Ci.nsIWebNavigation.LOAD_FLAGS_NONE) {
   info("Loading " + uri + " flags = " + flags);
   gBrowser.selectedBrowser.loadURIWithFlags(uri, flags, null, null, null);
 
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   if (!(flags & Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_HISTORY)) {
 
     if (flags & Ci.nsIWebNavigation.LOAD_FLAGS_REPLACE_HISTORY) {
       gExpectedHistory.entries.pop();
     } else {
       gExpectedHistory.index++;
     }
 
@@ -93,24 +93,24 @@ var waitForLoadWithFlags = async functio
       title: gBrowser.contentTitle
     });
   }
 };
 
 var back = async function() {
   info("Going back");
   gBrowser.goBack();
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   gExpectedHistory.index--;
 };
 
 var forward = async function() {
   info("Going forward");
   gBrowser.goForward();
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   gExpectedHistory.index++;
 };
 
 // Tests that navigating from a page that should be in the remote process and
 // a page that should be in the main process works and retains history
 add_task(async function test_navigation() {
   let expectedRemote = gMultiProcessBrowser;
 
@@ -201,28 +201,28 @@ add_task(async function test_synchronous
 
   info("2");
   // Load another page
   info("Loading about:robots");
   await BrowserTestUtils.loadURI(gBrowser.selectedBrowser, "about:robots");
   is(gBrowser.selectedBrowser.isRemoteBrowser, false, "Remote attribute should be correct");
   is(gBrowser.selectedBrowser.permanentKey, permanentKey, "browser.permanentKey is still the same");
 
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   is(gBrowser.selectedBrowser.isRemoteBrowser, false, "Remote attribute should be correct");
   is(gBrowser.selectedBrowser.permanentKey, permanentKey, "browser.permanentKey is still the same");
 
   info("3");
   // Load the remote page again
   info("Loading http://example.org/" + DUMMY_PATH);
   await BrowserTestUtils.loadURI(gBrowser.selectedBrowser, "http://example.org/" + DUMMY_PATH);
   is(gBrowser.selectedBrowser.isRemoteBrowser, expectedRemote, "Remote attribute should be correct");
   is(gBrowser.selectedBrowser.permanentKey, permanentKey, "browser.permanentKey is still the same");
 
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   is(gBrowser.selectedBrowser.isRemoteBrowser, expectedRemote, "Remote attribute should be correct");
   is(gBrowser.selectedBrowser.permanentKey, permanentKey, "browser.permanentKey is still the same");
 
   info("4");
   gBrowser.removeCurrentTab();
   clear_history();
 });
 
--- a/browser/base/content/test/general/browser_restore_isAppTab.js
+++ b/browser/base/content/test/general/browser_restore_isAppTab.js
@@ -48,56 +48,56 @@ var restart = async function(browser) {
 
   await promiseTabLoaded(tab);
 };
 
 add_task(async function navigate() {
   let tab = BrowserTestUtils.addTab(gBrowser, "about:robots");
   let browser = tab.linkedBrowser;
   gBrowser.selectedTab = tab;
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   loadFrameScript(browser);
   let isAppTab = await isBrowserAppTab(browser);
   ok(!isAppTab, "Docshell shouldn't think it is an app tab");
 
   gBrowser.pinTab(tab);
   isAppTab = await isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   gBrowser.loadURI(DUMMY);
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   loadFrameScript(browser);
   isAppTab = await isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   gBrowser.unpinTab(tab);
   isAppTab = await isBrowserAppTab(browser);
   ok(!isAppTab, "Docshell shouldn't think it is an app tab");
 
   gBrowser.pinTab(tab);
   isAppTab = await isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   gBrowser.loadURI("about:robots");
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   loadFrameScript(browser);
   isAppTab = await isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
   gBrowser.removeCurrentTab();
 });
 
 add_task(async function crash() {
   if (!gMultiProcessBrowser || !("nsICrashReporter" in Ci))
     return;
 
   let tab = BrowserTestUtils.addTab(gBrowser, DUMMY);
   let browser = tab.linkedBrowser;
   gBrowser.selectedTab = tab;
-  await waitForDocLoadComplete();
+  await BrowserTestUtils.browserStopped(gBrowser);
   loadFrameScript(browser);
   let isAppTab = await isBrowserAppTab(browser);
   ok(!isAppTab, "Docshell shouldn't think it is an app tab");
 
   gBrowser.pinTab(tab);
   isAppTab = await isBrowserAppTab(browser);
   ok(isAppTab, "Docshell should think it is an app tab");
 
--- a/browser/base/content/test/general/head.js
+++ b/browser/base/content/test/general/head.js
@@ -354,55 +354,16 @@ function promiseHistoryClearedState(aURI
            "history visit " + uri.spec + " should " + niceStr + " exist");
         callbackDone();
       });
     });
 
   });
 }
 
-/**
- * Waits for the next load to complete in any browser or the given browser.
- * If a <tabbrowser> is given it waits for a load in any of its browsers.
- *
- * @return promise
- */
-function waitForDocLoadComplete(aBrowser = gBrowser) {
-  return new Promise(resolve => {
-    let listener = {
-      onStateChange(webProgress, req, flags, status) {
-        let docStop = Ci.nsIWebProgressListener.STATE_IS_NETWORK |
-                      Ci.nsIWebProgressListener.STATE_STOP;
-        info("Saw state " + flags.toString(16) + " and status " + status.toString(16));
-
-        // When a load needs to be retargetted to a new process it is cancelled
-        // with NS_BINDING_ABORTED so ignore that case
-        if ((flags & docStop) == docStop && status != Cr.NS_BINDING_ABORTED) {
-          aBrowser.removeProgressListener(this);
-          waitForDocLoadComplete.listeners.delete(this);
-
-          let chan = req.QueryInterface(Ci.nsIChannel);
-          info("Browser loaded " + chan.originalURI.spec);
-          resolve();
-        }
-      },
-      QueryInterface: XPCOMUtils.generateQI([Ci.nsIWebProgressListener,
-                                             Ci.nsISupportsWeakReference])
-    };
-    aBrowser.addProgressListener(listener);
-    waitForDocLoadComplete.listeners.add(listener);
-    info("Waiting for browser load");
-  });
-}
-
-// Keep a set of progress listeners for waitForDocLoadComplete() to make sure
-// they're not GC'ed before we saw the page load.
-waitForDocLoadComplete.listeners = new Set();
-registerCleanupFunction(() => waitForDocLoadComplete.listeners.clear());
-
 var FullZoomHelper = {
 
   selectTabAndWaitForLocationChange: function selectTabAndWaitForLocationChange(tab) {
     if (!tab)
       throw new Error("tab must be given.");
     if (gBrowser.selectedTab == tab)
       return Promise.resolve();
 
--- a/browser/base/content/test/urlbar/browser_urlbar_stop_pending.js
+++ b/browser/base/content/test/urlbar/browser_urlbar_stop_pending.js
@@ -69,17 +69,17 @@ add_task(async function() {
   info("added link");
   let newTabPromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer, "TabOpen");
   // Middle click the link:
   await BrowserTestUtils.synthesizeMouseAtCenter("#clickme", { button: 1 }, tab.linkedBrowser);
   // get new tab, switch to it
   let newTab = (await newTabPromise).target;
   await BrowserTestUtils.switchTab(gBrowser, newTab);
   is(gURLBar.value, SLOW_HOST, "Should have slow page in URL bar");
-  let browserStoppedPromise = BrowserTestUtils.browserStopped(newTab.linkedBrowser);
+  let browserStoppedPromise = BrowserTestUtils.browserStopped(newTab.linkedBrowser, null, true);
   BrowserStop();
   await browserStoppedPromise;
 
   is(gURLBar.value, SLOW_HOST, "Should still have slow page in URL bar after stop");
   await BrowserTestUtils.removeTab(newTab);
   await BrowserTestUtils.removeTab(tab);
 });
 /**
@@ -116,23 +116,23 @@ add_task(async function() {
   info("added link");
   let newTabPromise = BrowserTestUtils.waitForEvent(gBrowser.tabContainer, "TabOpen");
   // Middle click the link:
   await BrowserTestUtils.synthesizeMouseAtCenter("#clickme", { button: 1 }, tab.linkedBrowser);
   // get new tab, switch to it
   let newTab = (await newTabPromise).target;
   await BrowserTestUtils.switchTab(gBrowser, newTab);
   is(gURLBar.value, SLOW_HOST1, "Should have slow page in URL bar");
-  let browserStoppedPromise = BrowserTestUtils.browserStopped(newTab.linkedBrowser);
+  let browserStoppedPromise = BrowserTestUtils.browserStopped(newTab.linkedBrowser, null, true);
   gURLBar.value = SLOW_HOST2;
   gURLBar.handleCommand();
   await browserStoppedPromise;
 
   is(gURLBar.value, SLOW_HOST2, "Should have second slow page in URL bar");
-  browserStoppedPromise = BrowserTestUtils.browserStopped(newTab.linkedBrowser);
+  browserStoppedPromise = BrowserTestUtils.browserStopped(newTab.linkedBrowser, null, true);
   BrowserStop();
   await browserStoppedPromise;
 
   is(gURLBar.value, SLOW_HOST2, "Should still have second slow page in URL bar after stop");
   await BrowserTestUtils.removeTab(newTab);
   await BrowserTestUtils.removeTab(tab);
 });
 
--- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
+++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ -313,49 +313,65 @@ this.BrowserTestUtils = {
     let mm = win.messageManager;
     return this.waitForMessage(mm, "browser-test-utils:loadEvent", (msg) => {
       let selectedBrowser = win.gBrowser.selectedBrowser;
       return msg.target == selectedBrowser &&
              (aboutBlank || selectedBrowser.currentURI.spec != "about:blank")
     });
   },
 
+  _webProgressListeners: new Set(),
+
   /**
    * Waits for the web progress listener associated with this tab to fire a
    * STATE_STOP for the toplevel document.
    *
    * @param {xul:browser} browser
    *        A xul:browser.
+   * @param {String} expectedURI (optional)
+   *        A specific URL to check the channel load against
+   * @param {Boolean} checkAborts (optional, defaults to false)
+   *        Whether NS_BINDING_ABORTED stops 'count' as 'real' stops
+   *        (e.g. caused by the stop button or equivalent APIs)
    *
    * @return {Promise}
    * @resolves When STATE_STOP reaches the tab's progress listener
    */
-  browserStopped(browser) {
+  browserStopped(browser, expectedURI, checkAborts=false) {
     return new Promise(resolve => {
+      const kDocStopFlags = Ci.nsIWebProgressListener.STATE_IS_NETWORK |
+                            Ci.nsIWebProgressListener.STATE_STOP;
       let wpl = {
         onStateChange(aWebProgress, aRequest, aStateFlags, aStatus) {
-          if (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
+          dump("Saw state " + aStateFlags.toString(16) + " and status " + aStatus.toString(16) + "\n");
+          if (aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK &&
+              aStateFlags & Ci.nsIWebProgressListener.STATE_STOP &&
+              (checkAborts || aStatus != Cr.NS_BINDING_ABORTED) &&
               aWebProgress.isTopLevel) {
-            browser.webProgress.removeProgressListener(filter);
-            filter.removeProgressListener(wpl);
-            resolve();
+            let chan = aRequest.QueryInterface(Ci.nsIChannel);
+            dump("Browser loaded " + chan.originalURI.spec + "\n");
+            if (!expectedURI || chan.originalURI.spec == expectedURI) {
+              browser.removeProgressListener(wpl);
+              BrowserTestUtils._webProgressListeners.delete(wpl);
+              resolve();
+            }
           };
         },
         onSecurityChange() {},
         onStatusChange() {},
         onLocationChange() {},
         QueryInterface: XPCOMUtils.generateQI([
           Ci.nsIWebProgressListener,
           Ci.nsIWebProgressListener2,
+          Ci.nsISupportsWeakReference,
         ]),
       };
-      const filter = Cc["@mozilla.org/appshell/component/browser-status-filter;1"]
-                       .createInstance(Ci.nsIWebProgress);
-      filter.addProgressListener(wpl, Ci.nsIWebProgress.NOTIFY_ALL);
-      browser.webProgress.addProgressListener(filter, Ci.nsIWebProgress.NOTIFY_ALL);
+      browser.addProgressListener(wpl);
+      this._webProgressListeners.add(wpl);
+      dump("Waiting for browser load" + (expectedURI ? (" of " + expectedURI) : "") + "\n");
     });
   },
 
   /**
    * Waits for the next tab to open and load a given URL.
    *
    * The method doesn't wait for the tab contents to load.
    *