Bug 1703583 - automatically reject waitForEvent, waitForCondition and topicObserved promises at the end of a browser test, r=Gijs.

Differential Revision: https://phabricator.services.mozilla.com/D111115

testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm

@@ -1237,34 +1237,59 @@ var BrowserTestUtils = {
     let innerWindowId = subject.ownerGlobal?.windowGlobalChild.innerWindowId;
 
     return new Promise((resolve, reject) => {
-      subject.addEventListener(
-        eventName,
-        function listener(event) {
-          try {
-            if (checkFn && !checkFn(event)) {
-              return;
-            }
-            subject.removeEventListener(eventName, listener, capture);
-            TestUtils.executeSoon(() => {
-              ChromeUtils.addProfilerMarker(
-                "BrowserTestUtils",
-                { startTime, category: "Test", innerWindowId },
-                "waitForEvent: " + eventName
-              );
-              resolve(event);
-            });
-          } catch (ex) {
-            try {
-              subject.removeEventListener(eventName, listener, capture);
-            } catch (ex2) {
-              // Maybe the provided object does not support removeEventListener.
-            }
-            TestUtils.executeSoon(() => reject(ex));
+      let removed = false;
+      function listener(event) {
+        function cleanup() {
+          removed = true;
+          // Avoid keeping references to objects after the promise resolves.
+          subject = null;
+          checkFn = null;
+        }
+        try {
+          if (checkFn && !checkFn(event)) {
+            return;
           }
-        },
-        capture,
-        wantsUntrusted
-      );
+          subject.removeEventListener(eventName, listener, capture);
+          cleanup();
+          TestUtils.executeSoon(() => {
+            ChromeUtils.addProfilerMarker(
+              "BrowserTestUtils",
+              { startTime, category: "Test", innerWindowId },
+              "waitForEvent: " + eventName
+            );
+            resolve(event);
+          });
+        } catch (ex) {
+          try {
+            subject.removeEventListener(eventName, listener, capture);
+          } catch (ex2) {
+            // Maybe the provided object does not support removeEventListener.
+          }
+          cleanup();
+          TestUtils.executeSoon(() => reject(ex));
+        }
+      }
+
+      subject.addEventListener(eventName, listener, capture, wantsUntrusted);
+
+      TestUtils.promiseTestFinished?.then(() => {
+        if (removed) {
+          return;
+        }
+
+        subject.removeEventListener(eventName, listener, capture);
+        let text = eventName + " listener";
+        if (subject.id) {
+          text += ` on #${subject.id}`;
+        }
+        text += " not removed before the end of test";
+        reject(text);
+        ChromeUtils.addProfilerMarker(
+          "BrowserTestUtils",
+          { startTime, category: "Test", innerWindowId },
+          "waitForEvent: " + text
+        );
+      });
     });
   },
 

testing/mochitest/browser-test.js

@@ -627,6 +627,10 @@ Tester.prototype = {
         );
       }
 
+      this.resolveFinishTestPromise();
+      this.resolveFinishTestPromise = null;
+      this.TestUtils.promiseTestFinished = null;
+
       this.PromiseTestUtils.ensureDOMPromiseRejectionsProcessed();
       this.PromiseTestUtils.assertNoUncaughtRejections();
       this.PromiseTestUtils.assertNoMoreExpectedRejections();
@@ -1050,6 +1054,9 @@ Tester.prototype = {
     // Import the test script.
     try {
       this.lastStartTimestamp = performance.now();
+      this.TestUtils.promiseTestFinished = new Promise(resolve => {
+        this.resolveFinishTestPromise = resolve;
+      });
       this._scriptLoader.loadSubScript(this.currentTest.path, scope);
       // Run the test
       this.lastStartTime = Date.now();

testing/modules/TestUtils.jsm

@@ -21,7 +21,9 @@
 var EXPORTED_SYMBOLS = ["TestUtils"];
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
-const { setTimeout } = ChromeUtils.import("resource://gre/modules/Timer.jsm");
+const { clearTimeout, setTimeout } = ChromeUtils.import(
+  "resource://gre/modules/Timer.jsm"
+);
 
 var TestUtils = {
   executeSoon(callbackFn) {
@@ -51,19 +53,49 @@ var TestUtils = {
    * @resolves The array [subject, data] from the observed notification.
    */
   topicObserved(topic, checkFn) {
+    let startTime = Cu.now();
     return new Promise((resolve, reject) => {
-      Services.obs.addObserver(function observer(subject, topic, data) {
+      let removed = false;
+      function observer(subject, topic, data) {
         try {
           if (checkFn && !checkFn(subject, data)) {
             return;
           }
           Services.obs.removeObserver(observer, topic);
+          // checkFn could reference objects that need to be destroyed before
+          // the end of the test, so avoid keeping a reference to it after the
+          // promise resolves.
+          checkFn = null;
+          removed = true;
+          ChromeUtils.addProfilerMarker(
+            "TestUtils",
+            { startTime, category: "Test" },
+            "topicObserved: " + topic
+          );
           resolve([subject, data]);
         } catch (ex) {
           Services.obs.removeObserver(observer, topic);
+          checkFn = null;
+          removed = true;
           reject(ex);
         }
-      }, topic);
+      }
+      Services.obs.addObserver(observer, topic);
+
+      TestUtils.promiseTestFinished?.then(() => {
+        if (removed) {
+          return;
+        }
+
+        Services.obs.removeObserver(observer, topic);
+        let text = topic + " observer not removed before the end of test";
+        reject(text);
+        ChromeUtils.addProfilerMarker(
+          "TestUtils",
+          { startTime, category: "Test" },
+          "topicObserved: " + text
+        );
+      });
     });
   },
 
@@ -154,8 +186,7 @@ var TestUtils = {
    * @param msg
    *        A message used to describe the condition being waited for.
    *        This message will be used to reject the promise should the
-   *        wait fail. It is also used to add a profiler marker in the
-   *        success case.
+   *        wait fail. It is also used to add a profiler marker.
    * @param interval
    *        The time interval to poll the condition function. Defaults
    *        to 100ms.
@@ -174,9 +205,17 @@ var TestUtils = {
     let startTime = Cu.now();
     return new Promise((resolve, reject) => {
       let tries = 0;
+      let timeoutId = 0;
       async function tryOnce() {
+        timeoutId = 0;
         if (tries >= maxTries) {
           msg += ` - timed out after ${maxTries} tries.`;
+          ChromeUtils.addProfilerMarker(
+            "TestUtils",
+            { startTime, category: "Test" },
+            `waitForCondition - ${msg}`
+          );
+          condition = null;
           reject(msg);
           return;
         }
@@ -185,7 +224,13 @@ var TestUtils = {
         try {
           conditionPassed = await condition();
         } catch (e) {
+          ChromeUtils.addProfilerMarker(
+            "TestUtils",
+            { startTime, category: "Test" },
+            `waitForCondition - ${msg}`
+          );
           msg += ` - threw exception: ${e}`;
+          condition = null;
           reject(msg);
           return;
         }
@@ -196,13 +241,32 @@ var TestUtils = {
             { startTime, category: "Test" },
             `waitForCondition succeeded after ${tries} retries - ${msg}`
           );
+          // Avoid keeping a reference to the condition function after the
+          // promise resolves, as this function could itself reference objects
+          // that should be GC'ed before the end of the test.
+          condition = null;
           resolve(conditionPassed);
           return;
         }
         tries++;
-        setTimeout(tryOnce, interval);
+        timeoutId = setTimeout(tryOnce, interval);
       }
 
+      TestUtils.promiseTestFinished?.then(() => {
+        if (!timeoutId) {
+          return;
+        }
+
+        clearTimeout(timeoutId);
+        msg += " - still pending at the end of the test";
+        ChromeUtils.addProfilerMarker(
+          "TestUtils",
+          { startTime, category: "Test" },
+          `waitForCondition - ${msg}`
+        );
+        reject("waitForCondition timer - " + msg);
+      });
+
       TestUtils.executeSoon(tryOnce);
     });
   },