Re: [w3c/ServiceWorker] async waitUntil microtask issue (#1213)

I am looking at Chrome’s implementation in this area again.

I don’t understand why 1 and 2 would be different in https://github.com/w3c/ServiceWorker/issues/1213#issuecomment-359122656. I think those would both throw. Upon waitPromise fulfilling, the spec queues a microtask to decrement the promise count, and I’d expect that microtask to run before the waitUntil() in both cases.

I’m looking at the current WPT expectations at it looks aligned with this discussion except for one case. Here are the interesting ones I looked at:

**Test: [waitUntil in a different microtask without an existing extension throws](https://github.com/web-platform-tests/wpt/blob/8ab29ab9f6ccec92deb35fff33fceb0862916852/service-workers/service-worker/resources/extendable-event-async-waituntil.js#L21)**

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

- Correct expectation: Does not throw (current expectation is wrong)
- Browsers: Throws: Chrome; Does not throw: Firefox, Edge, Safari

Rationale (from https://github.com/w3c/ServiceWorker/issues/1394#issuecomment-472918644): The event dispatch flag is still set while in the microtask. 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


**Test: [waitUntil at the end of the microtask turn throws](https://github.com/web-platform-tests/wpt/blob/a16738c524a65946b0ff7df6be37ae076ff5e860/service-workers/service-worker/resources/extendable-event-async-waituntil.js#L33)**

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

- Correct expectation: Throws.
- Browsers: Throws: Chrome, Firefox; Does not throw: Edge, Safari

Rationale: Upon fulfillment of p, the spec queues a microtask to decrement the pending promise count  at step 5 here:
https://w3c.github.io/ServiceWorker/#wait-until-method


This microtask runs before waitUntil() is called, so the promise count reaches zero, and the event is not active.

**Test: [waitUntil asynchronously inside microtask of respondWith promise.](https://github.com/web-platform-tests/wpt/blob/8ab29ab9f6ccec92deb35fff33fceb0862916852/service-workers/service-worker/resources/extendable-event-async-waituntil.js#L66)**

```
addEventListener(‘fetch’, event => {
  event.respondWith(p);
  p.then(() => {
    Promise.resolve().then(() => { event.waitUntil(q); });
  });     
```
- Correct expectation: Throws
- Browsers: Throws: Chrome, Firefox; Does not throw: Edge, Safari

Rationale: Upon fulfillment of p, the spec queues a microtask to decrement the pending promise count at step 5 here:
https://w3c.github.io/ServiceWorker/#fetch-event-respondwith


This microtask runs before waitUntil() is called, so the promise count reaches zero, and the event is not active.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/issues/1213#issuecomment-473149567

Received on Friday, 15 March 2019 03:53:38 UTC