Skip to content

Commit

Permalink
[service-workers] Return value in cleanup (#13163)
Browse files Browse the repository at this point in the history
Previously, many tests un-registered service workers using the
testharness.js API `add_cleanup` without returning the promise which
tracked the operation. While this ensured the operation was started at
the completion of the test, it did not guarantee that the operation
would be completed before the test was considered "complete."

In automation scenarios where many tests are executed in rapid
succession, this enabled a race condition: the navigation could
interrupt the un-registration process. In order to account for the
possibility of registrations that persisted from previous test failures,
the tests included "setup" code to defensively un-register such workers.

The `Test#add_cleanup` method was recently extended to support
asynchronous "clean up" operations [1]. Use that API to ensure tests are
not considered "complete" until after service worker un-registration is
done.

[1] #8748
  • Loading branch information
jugglinmike authored and mkruisselbrink committed Sep 21, 2018
1 parent eb68211 commit 7897f9d
Show file tree
Hide file tree
Showing 9 changed files with 11 additions and 23 deletions.
Expand Up @@ -14,7 +14,7 @@
return service_worker_unregister_and_register(t, url, scope)
.then(function(registration) {
t.add_cleanup(function() {
registration.unregister();
return registration.unregister();
});

return wait_for_state(t, registration.installing, 'activated');
Expand Down
Expand Up @@ -40,7 +40,7 @@
t, 'resources/empty-worker.js', scope)
.then(function(registration) {
t.add_cleanup(function() {
registration.unregister();
return registration.unregister();
});

window.registrationInstance = registration;
Expand Down
Expand Up @@ -35,7 +35,7 @@
registration = r;

t.add_cleanup(function() {
r.unregister();
return r.unregister();
});

return wait_for_install(registration.installing);
Expand Down
18 changes: 3 additions & 15 deletions service-workers/service-worker/postmessage.https.html
Expand Up @@ -13,11 +13,7 @@

return service_worker_unregister_and_register(t, script, scope)
.then(r => {
// TODO: return the Promise created by `r.unregister`once
// `testharness.js` has been updated to honor thenables returned by
// cleanup functions.
// See https://github.com/web-platform-tests/wpt/pull/8748
t.add_cleanup(() => { r.unregister(); });
t.add_cleanup(() => r.unregister());
registration = r;
worker = registration.installing;

Expand Down Expand Up @@ -66,11 +62,7 @@

return service_worker_unregister_and_register(t, script, scope)
.then(r => {
// TODO: return the Promise created by `r.unregister`once
// `testharness.js` has been updated to honor thenables returned by
// cleanup functions.
// See https://github.com/web-platform-tests/wpt/pull/8748
t.add_cleanup(() => { r.unregister(); });
t.add_cleanup(() => r.unregister());

var ab = text_encoder.encode(message);
assert_equals(ab.byteLength, message.length);
Expand Down Expand Up @@ -110,11 +102,7 @@

return service_worker_unregister_and_register(t, script, scope)
.then(r => {
// TODO: return the Promise created by `r.unregister`once
// `testharness.js` has been updated to honor thenables returned by
// cleanup functions.
// See https://github.com/web-platform-tests/wpt/pull/8748
t.add_cleanup(() => { r.unregister(); });
t.add_cleanup(() => r.unregister());

var channel = new MessageChannel;
port = channel.port1;
Expand Down
Expand Up @@ -14,7 +14,7 @@
return service_worker_unregister_and_register(t, worker_url, scope)
.then(function(r) {
t.add_cleanup(function() {
r.unregister();
return r.unregister();
});
registration = r;
newest_worker = registration.installing;
Expand Down
Expand Up @@ -20,7 +20,7 @@ importScripts('/resources/testharness.js');
var cache_name = self.location.pathname + '/' + uniquifier;

test.add_cleanup(function() {
self.caches.delete(cache_name);
return self.caches.delete(cache_name);
});

return self.caches.delete(cache_name)
Expand Down
Expand Up @@ -51,7 +51,7 @@
.then(function(registration) {
sw_registration = registration;
t.add_cleanup(function() {
registration.unregister();
return registration.unregister();
});
return saw_controllerchanged;
})
Expand Down
Expand Up @@ -27,7 +27,7 @@
.then(function(registration) {
sw_registration = registration;
t.add_cleanup(function() {
registration.unregister();
return registration.unregister();
});
return wait_for_state(t, registration.installing, 'activated');
})
Expand Down
2 changes: 1 addition & 1 deletion service-workers/service-worker/synced-state.https.html
Expand Up @@ -33,7 +33,7 @@
worker = registration.installing;

t.add_cleanup(function() {
r.unregister();
return r.unregister();
});

return nextChange(worker);
Expand Down

0 comments on commit 7897f9d

Please sign in to comment.