Closed Bug 1536619 Opened 5 months ago Closed 4 months ago

[wpt-sync] Sync PR 15852 - service worker: Improve WPT tests for async respondWith/waitUntil.

Categories

(Core :: DOM: Service Workers, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: wptsync, Unassigned)

References

()

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 15852 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/15852
Details from upstream follow.

Matt Falkenhagen <falken@chromium.org> wrote:

service worker: Improve WPT tests for async respondWith/waitUntil.

See discussion at [1] and [2].

This makes the following changes.

Adds a test for:

self.addEventListener('fetch', e => {
Promise.resolve().then(() => {
e.respondWith(new Response('hi'));
});
});

This should not throw because respondWith() is called while the event
dispatch flag is still set.

The microtask checkpoint is in "Cleanup After Running Scripts" here:
https://html.spec.whatwg.org/multipage/webappapis.html#clean-up-after-running-script

This is called from step 16.2 here:
https://heycam.github.io/webidl/#call-a-user-objects-operation

Which in turn is called from the DOM spec's "Inner Invoke" to call event
targets:
https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke

Changes the expectation for:

addEventListener('message', event => {
Promise.resolve().then(event.waitUntil(p));
});

From throws to not throws, for the same reasoning as above.

Changes the expectation for:

addEventListener('message', event => {
waitPromise = Promise.resolve();
event.waitUntil(waitPromise);
waitPromise.then(() => {
Promise.resolve().then(() => {event.waitUntil();});
});
});

From throws to not throws. This is subtle. Because all the promises
are just Promise.resolve(), the event dispatch flag is still set
by the time the second waitUntil() is called.

To test what 3. originally intended, a new test is
added which makes waitPromise a promise that does not immediately
resolve.

Changes the expectation for:

addEventListener(‘fetch’, event => {
response = Promise.resolve(new Response('RESP'));
event.respondWith(response);
response.then(() => {
Promise.resolve().then(() => {event.waitUntil();});
})
});

Again this is because the promises used resolve immediately,
so the event dispatch flag is still set.

Similarly, a new test is added to cover the original intent.

These WPT changes appear to match the behavior of Safari and Edge while
diverging from Chrome and (partially) Firefox.

[1] https://github.com/w3c/ServiceWorker/issues/1213
[2] https://github.com/w3c/ServiceWorker/issues/1394

Bug: 942414
Change-Id: I9a4a56d71d3919ed614ff78df2bdc6cc0251dadd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1524393
Commit-Queue: Matt Falkenhagen \<falken@chromium.org>
Reviewed-by: Ben Kelly \<wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641514}

Whiteboard: [wptsync downstream] → [wptsync downstream error]
Component: web-platform-tests → DOM: Service Workers
Product: Testing → Core
Whiteboard: [wptsync downstream error] → [wptsync downstream]
Ran 2 tests and 18 subtests
OK     : 2
PASS   : 16
FAIL   : 2

New tests that have failures or other problems:
/service-workers/service-worker/extendable-event-async-waituntil.https.html
    Test calling waitUntil asynchronously inside microtask of respondWith promise (event is being dispatched).: FAIL
    Test calling waitUntil in a microtask at the end of an existing extension promise handler succeeds (event is still being dispatched): FAIL
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a941738d800e
[wpt PR 15852] - service worker: Improve WPT tests for async respondWith/waitUntil., a=testonly
https://hg.mozilla.org/integration/mozilla-inbound/rev/4943b28c0d72
[wpt PR 15852] - Update wpt metadata, a=testonly
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4b704c52ae9
[wpt PR 15852] - service worker: Improve WPT tests for async respondWith/waitUntil., a=testonly
https://hg.mozilla.org/integration/mozilla-inbound/rev/a89c195336d4
[wpt PR 15852] - Update wpt metadata, a=testonly
You need to log in before you can comment on or make changes to this bug.