From 8b3feba5083f5de0d97453e2cf2d082b8d282708 Mon Sep 17 00:00:00 2001
From: Andreu Botella <abb@randomunok.com>
Date: Tue, 10 Aug 2021 16:44:02 +0200
Subject: [PATCH] Change `Tests.prototype.all_done` to require either tests or
 pending remotes

`Tests.prototype.all_done` used to treat the presence of at least one
test as a necessary condition for all tests to be done. This would be
false in cases where an exception is thrown inside the `setup` function,
but the code path that runs in that case doesn't use the `all_done()`
function.

However, if an exception is thrown inside the `setup` function in a
worker test, `tests.all_done()` is called in that code path, which
results in the completion message from the worker being ignored. This
change fixes this by changing the condition to require the presence of
either tests or remote contexts.

For service worker tests, however, that is not enough, since service
worker registration is asynchronous. This change lets the
`fetch_tests_from_worker` function take a promise or an async function
that returns the worker, and it guarantees that the test won't be
considered complete until the promise is resolved. The HTML boilerplate
for service worker tests is also changed in turn.

Closes #29777.
---
 resources/testharness.js                      | 30 ++++++++++++++-----
 tools/serve/serve.py                          | 12 ++++----
 .../docroot/foo.any.serviceworker.html        |  6 ++--
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/resources/testharness.js b/resources/testharness.js
index f85b19fd9bd90c..a88129c8e0df70 100644
--- a/resources/testharness.js
+++ b/resources/testharness.js
@@ -2942,7 +2942,8 @@
     };
 
     Tests.prototype.all_done = function() {
-        return this.tests.length > 0 && test_environment.all_loaded &&
+        return (this.tests.length > 0 || this.pending_remotes.length > 0) &&
+                test_environment.all_loaded &&
                 (this.num_pending === 0 || this.is_aborted) && !this.wait_for_finish &&
                 !this.processing_callbacks &&
                 !this.pending_remotes.some(function(w) { return w.running; });
@@ -3168,18 +3169,33 @@
         );
     };
 
-    Tests.prototype.fetch_tests_from_worker = function(worker) {
+    Tests.prototype.fetch_tests_from_worker = function(promiseOrWorker) {
         if (this.phase >= this.phases.COMPLETE) {
             return;
         }
 
-        var remoteContext = this.create_remote_worker(worker);
-        this.pending_remotes.push(remoteContext);
-        return remoteContext.done;
+        if (typeof promiseOrWorker === "function") {
+            promiseOrWorker = promiseOrWorker();
+        }
+
+        if (self.Promise && promiseOrWorker instanceof self.Promise) {
+            const dummy_remote = { running: true };
+            this.pending_remotes.push(dummy_remote);
+            return promiseOrWorker.then((worker) => {
+                dummy_remote.running = false;
+                var remoteContext = this.create_remote_worker(worker);
+                this.pending_remotes.push(remoteContext);
+                return remoteContext.done;
+            });
+        } else {
+            var remoteContext = this.create_remote_worker(promiseOrWorker);
+            this.pending_remotes.push(remoteContext);
+            return remoteContext.done;
+        }
     };
 
-    function fetch_tests_from_worker(port) {
-        return tests.fetch_tests_from_worker(port);
+    function fetch_tests_from_worker(promiseOrPort) {
+        return tests.fetch_tests_from_worker(promiseOrPort);
     }
     expose(fetch_tests_from_worker, 'fetch_tests_from_worker');
 
diff --git a/tools/serve/serve.py b/tools/serve/serve.py
index e7154f2f604e21..7914230240143a 100644
--- a/tools/serve/serve.py
+++ b/tools/serve/serve.py
@@ -311,13 +311,13 @@ class ServiceWorkersHandler(HtmlWrapperHandler):
 <script src="/resources/testharnessreport.js"></script>
 <div id=log></div>
 <script>
-(async function() {
+fetch_tests_from_worker(async function() {
   const scope = 'does/not/exist';
   let reg = await navigator.serviceWorker.getRegistration(scope);
   if (reg) await reg.unregister();
   reg = await navigator.serviceWorker.register("%(path)s%(query)s", {scope});
-  fetch_tests_from_worker(reg.installing);
-})();
+  return reg.installing;
+});
 </script>
 """
 
@@ -333,7 +333,7 @@ class ServiceWorkerModulesHandler(HtmlWrapperHandler):
 <script src="/resources/testharnessreport.js"></script>
 <div id=log></div>
 <script>
-(async function() {
+fetch_tests_from_worker(async function() {
   const scope = 'does/not/exist';
   let reg = await navigator.serviceWorker.getRegistration(scope);
   if (reg) await reg.unregister();
@@ -341,8 +341,8 @@ class ServiceWorkerModulesHandler(HtmlWrapperHandler):
     "%(path)s%(query)s",
     { scope, type: 'module' },
   );
-  fetch_tests_from_worker(reg.installing);
-})();
+  return reg.installing;
+});
 </script>
 """
 
diff --git a/tools/wptserve/tests/functional/docroot/foo.any.serviceworker.html b/tools/wptserve/tests/functional/docroot/foo.any.serviceworker.html
index 8dcb11a37687ff..e6531ced9c5e98 100644
--- a/tools/wptserve/tests/functional/docroot/foo.any.serviceworker.html
+++ b/tools/wptserve/tests/functional/docroot/foo.any.serviceworker.html
@@ -5,11 +5,11 @@
 <script src="/resources/testharnessreport.js"></script>
 <div id=log></div>
 <script>
-(async function() {
+fetch_tests_from_worker(async function() {
   const scope = 'does/not/exist';
   let reg = await navigator.serviceWorker.getRegistration(scope);
   if (reg) await reg.unregister();
   reg = await navigator.serviceWorker.register("/foo.any.worker.js", {scope});
-  fetch_tests_from_worker(reg.installing);
-})();
+  return reg.installing;
+});
 </script>