Bug 1308343 - Fix intermittent failures in browser_ContentSearch.js by ensuring we keep listeners alive and listening for loads correctly. r?adw draft
authorMark Banner <standard8@mozilla.com>
Wed, 12 Jul 2017 12:01:53 +0100
changeset 607560 aeca11532490fe7ded314aa9086b5dd8bbefb4c2
parent 607503 09a4282d1172ac255038e7ccacfd772140b219e2
child 637064 139392ab06cacd0962e80f3dabd1b2a00f134d1d
push id68022
push userbmo:standard8@mozilla.com
push dateWed, 12 Jul 2017 11:02:12 +0000
reviewersadw
bugs1308343
milestone56.0a1
Bug 1308343 - Fix intermittent failures in browser_ContentSearch.js by ensuring we keep listeners alive and listening for loads correctly. r?adw MozReview-Commit-ID: CZxATBbijlx
browser/modules/test/browser/browser.ini
browser/modules/test/browser/browser_ContentSearch.js
browser/modules/test/browser/contentSearch.js
--- a/browser/modules/test/browser/browser.ini
+++ b/browser/modules/test/browser/browser.ini
@@ -6,17 +6,16 @@ support-files =
 skip-if = !e10s # Bug 1373549
 [browser_BrowserUITelemetry_defaults.js]
 skip-if = !e10s # Bug 1373549
 [browser_BrowserUITelemetry_sidebar.js]
 skip-if = !e10s # Bug 1373549
 [browser_BrowserUITelemetry_syncedtabs.js]
 skip-if = !e10s # Bug 1373549
 [browser_ContentSearch.js]
-skip-if = (os == "mac" || os == "linux") # Bug 1308343
 support-files =
   contentSearch.js
   contentSearchBadImage.xml
   contentSearchSuggestions.sjs
   contentSearchSuggestions.xml
   !/browser/components/search/test/head.js
   !/browser/components/search/test/testEngine.xml
 [browser_PermissionUI.js]
--- a/browser/modules/test/browser/browser_ContentSearch.js
+++ b/browser/modules/test/browser/browser_ContentSearch.js
@@ -340,34 +340,30 @@ function waitForNewEngine(basename, numI
       ok(false, "addEngine failed with error code " + errCode);
       addDeferred.reject();
     },
   });
 
   return Promise.all([addDeferred.promise].concat(eventPromises));
 }
 
-function addTab() {
-  return new Promise(resolve => {
-    let tab = BrowserTestUtils.addTab(gBrowser);
-    gBrowser.selectedTab = tab;
-    tab.linkedBrowser.addEventListener("load", function() {
-      let url = getRootDirectory(gTestPath) + TEST_CONTENT_SCRIPT_BASENAME;
-      gMsgMan = tab.linkedBrowser.messageManager;
-      gMsgMan.sendAsyncMessage(CONTENT_SEARCH_MSG, {
-        type: "AddToWhitelist",
-        data: ["about:blank"],
-      });
-      waitForMsg(CONTENT_SEARCH_MSG, "AddToWhitelistAck").then(() => {
-        gMsgMan.loadFrameScript(url, false);
-        resolve();
-      });
-    }, {capture: true, once: true});
-    registerCleanupFunction(() => gBrowser.removeTab(tab));
+async function addTab() {
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+  registerCleanupFunction(() => gBrowser.removeTab(tab));
+
+  let url = getRootDirectory(gTestPath) + TEST_CONTENT_SCRIPT_BASENAME;
+  gMsgMan = tab.linkedBrowser.messageManager;
+  gMsgMan.sendAsyncMessage(CONTENT_SEARCH_MSG, {
+    type: "AddToWhitelist",
+    data: ["about:blank"],
   });
+
+  await waitForMsg(CONTENT_SEARCH_MSG, "AddToWhitelistAck");
+
+  gMsgMan.loadFrameScript(url, false);
 }
 
 var currentStateObj = async function() {
   let state = {
     engines: [],
     currentEngine: await currentEngineObj(),
   };
   for (let engine of Services.search.getVisibleEngines()) {
--- a/browser/modules/test/browser/contentSearch.js
+++ b/browser/modules/test/browser/contentSearch.js
@@ -15,52 +15,67 @@ content.addEventListener(SERVICE_EVENT_T
   // up with an XrayWrapper to it here, which will screw us up when trying to
   // serialize the object in sendAsyncMessage. Waive Xrays for the benefit of
   // the test machinery.
   sendAsyncMessage(TEST_MSG, Components.utils.waiveXrays(event.detail));
 });
 
 // Forward messages from the test to the in-content service.
 addMessageListener(TEST_MSG, msg => {
-  // If the message is a search, stop the page from loading and then tell the
-  // test that it loaded.
-  if (msg.data.type == "Search") {
-    waitForLoadAndStopIt(msg.data.expectedURL, url => {
+  (async function() {
+    // If the message is a search, stop the page from loading and then tell the
+    // test that it loaded.
+    let loadPromise;
+    if (msg.data.type == "Search") {
+      loadPromise = waitForLoadAndStopIt(msg.data.expectedURL);
+    }
+
+    content.dispatchEvent(
+      new content.CustomEvent(CLIENT_EVENT_TYPE, {
+        detail: msg.data,
+      })
+    );
+
+    if (msg.data.type == "Search") {
+      let url = await loadPromise;
+
       sendAsyncMessage(TEST_MSG, {
         type: "loadStopped",
         url,
       });
-    });
-  }
-
-  content.dispatchEvent(
-    new content.CustomEvent(CLIENT_EVENT_TYPE, {
-      detail: msg.data,
-    })
-  );
+    }
+  })();
 });
 
-function waitForLoadAndStopIt(expectedURL, callback) {
-  let Ci = Components.interfaces;
-  let webProgress = content.document.docShell.QueryInterface(Ci.nsIInterfaceRequestor)
-                                             .getInterface(Ci.nsIWebProgress);
-  let listener = {
-    onStateChange(webProg, req, flags, status) {
-      if (req instanceof Ci.nsIChannel) {
-        let url = req.originalURI.spec;
-        dump("waitForLoadAndStopIt: onStateChange " + url + "\n");
-        let docStart = Ci.nsIWebProgressListener.STATE_IS_DOCUMENT |
-                       Ci.nsIWebProgressListener.STATE_START;
-        if ((flags & docStart) && webProg.isTopLevel && url == expectedURL) {
-          webProgress.removeProgressListener(listener);
-          req.cancel(Components.results.NS_ERROR_FAILURE);
-          callback(url);
+// We need to keep a reference to the listener as webProgress records a weak
+// reference, hence we need to stop garbage collection cleaning it up before the
+// test is completed.
+var webProgressListener;
+
+function waitForLoadAndStopIt(expectedURL) {
+  return new Promise(resolve => {
+    let Ci = Components.interfaces;
+    let webProgress = content.document.docShell.QueryInterface(Ci.nsIInterfaceRequestor)
+                                               .getInterface(Ci.nsIWebProgress);
+    webProgressListener = {
+      onStateChange(webProg, req, flags, status) {
+        if (req instanceof Ci.nsIChannel) {
+          let url = req.originalURI.spec;
+          dump("waitForLoadAndStopIt: onStateChange " + url + "\n");
+          let docStart = Ci.nsIWebProgressListener.STATE_IS_DOCUMENT |
+                         Ci.nsIWebProgressListener.STATE_START;
+          if ((flags & docStart) && webProg.isTopLevel && url == expectedURL) {
+            webProgress.removeProgressListener(webProgressListener);
+            webProgressListener = null;
+            req.cancel(Components.results.NS_ERROR_FAILURE);
+            resolve(url);
+          }
         }
-      }
-    },
-    QueryInterface: XPCOMUtils.generateQI([
-      Ci.nsIWebProgressListener,
-      Ci.nsISupportsWeakReference,
-    ]),
-  };
-  webProgress.addProgressListener(listener, Ci.nsIWebProgress.NOTIFY_ALL);
-  dump("waitForLoadAndStopIt: Waiting for URL to load: " + expectedURL + "\n");
+      },
+      QueryInterface: XPCOMUtils.generateQI([
+        Ci.nsIWebProgressListener,
+        Ci.nsISupportsWeakReference,
+      ]),
+    };
+    webProgress.addProgressListener(webProgressListener, Ci.nsIWebProgress.NOTIFY_ALL);
+    dump("waitForLoadAndStopIt: Waiting for URL to load: " + expectedURL + "\n");
+  });
 }