Should the worker be removed from the registration *before* its state is set to "redundant"?

[cdumez]

While investigating some web-platform-tests failures on WebKit, a possible difference in behavior between Firefox/Chrome and the specification.

For example, when the install step fails, the spec says:
If installFailed is true, then:

1. Run the Update Worker State algorithm passing registration’s installing worker and redundant as the arguments.
2. Run the Update Registration State algorithm passing registration, "installing" and null as the arguments.

Notice the order, we first set the worker state to redundant, then we set the registration's installing worker to null. This means that when the statechange event is fired on this worker, registration.installing is not set to null yet (according to my understanding of the spec).

However, local testing shows that in Firefox and Chrome, registration.installing already returns null when the statechange event is fired at the worker (with state being redundant).

Should the specification be updated to match the behavior of browsers? Or did I misinterpret the specification?

I believe there is a similar issue in the "Clear Registration" algorithm. There is a WPT test that seems to assume that when clearing the registration, and the waiting worker becomes redundant, then all the registration's installing/waiting/active worker should be null. This test seems to pass in Chrome/Firefox.
However, my understanding is that the spec says:

1. Set installing worker state to redundant & schedule a task to fire statechange event at worker
2. Schedule a task to update ServiceWorkerRegistration(s) installing worker to null
3. Set waiting worker state to redundant & schedule a task to fire statechange event at worker
4. Schedule a task to update ServiceWorkerRegistration(s) waiting worker to null
5. Set active worker state to redundant & schedule a task to fire statechange event at worker
6. Schedule a task to update ServiceWorkerRegistration(s) active worker to null

So when the statechange is fired at the waiting worker, based on the spec, I would not expect registration.waiting / registration.active to become null right away. Instead, I would have expected registration.waiting to return null after 1 event loop iteration, and registration.active even later.

[wanderview]

Is this the same as #1270? (I was just looking at this yesterday as well.)

[mfalken]

I think your reading of the spec is correct.

For the "Clear Registration" test, are you talking about registration-service-worker-attributes.https.html? I suspect the comment there was written for an older version of the spec:

          // According to spec, Clear Registration runs Update State which is
          // immediately followed by setting active to null, which means by the
          // time the event loop turns and the Promise for statechange is
          // resolved, this will be gone.

(And yea I think can dupe this to #1270 or vice versa)

[wanderview]

I think we should reverse the order of updating in the spec. The registration should be updated before we fire the statechange event, IMO.

[mfalken]

Yes, I tend to think that when the worker is "redundant" it should be dead and gone from the registration. I don't know if there's a particular reason the spec ended up with this other order. It seems like the intent was "redundant" means the worker is "going away" based on the non-normative note here: https://w3c.github.io/ServiceWorker/#ref-for-dom-serviceworkerstate-redundant%E2%91%A0

[wanderview]

The spec originally didn't have any task queuing to update the DOM objects. I think the order just got reversed on this one call site when that was added.

[cdumez]

Well there are at least 2 call sites, Clear and Install (when it fails).

[mfalken]

I'm OK with changing the spec then, especially since it sounds like this order wasn't intended.

[asakusuma]

I've seen what looks like the same root issue, where registration.active.state === 'redundant'. I've been able to reproduce this issue intermittently in headless chrome service worker tests.

[jakearchibald]

As a general rule, we should ensure that the following properties are updated:

• installing/waiting/active on registration objects.
• state on service worker objects.
• controller on navigator.serviceWorker.

…before notifying the developer that any of those things have changed. Our current notifications are:

• updatefound event on registration objects.
• statechange event on service worker objects.
• navigator.serviceWorker.ready.
• controllerchange event on navigator.serviceWorker.
• install, activate event on the service worker global.

I'll review the spec for this, and ideally come up with a system that enforces the above.

[jakearchibald]

Some research on what the spec says vs what browsers do:

One worker, going through the steps

When I say "reg: null, null, worker" I mean, on the registration, installing was null, waiting was null, active was a worker:

	on register resolve()	updatefound after resolve	on "installed"	on "activating"	on "activated"
Spec	reg: worker, null, null	Yes. reg: worker, null, null	reg: null, worker, null	reg: null, null, worker	reg: null, null, worker

Chrome, Firefox and Safari all behave the same as the spec.

An new active worker

This is where things are different. I'm going to compare:

• Current spec
• Central algorithm for updating service worker state and registrations #1416 - this is my ideal, which tries to make all related object changes before firing events.
• Update order #1436 - a much smaller change that updates registration objects before firing
• Browsers.

In this test, worker1 is active, worker2 is waiting. worker2 becomes active, worker1 becomes redundant.

	Event order	when worker1 "redundant"	when worker2 "activating"
Spec	redundant, activating	worker2 "installed" reg: null, worker2, worker1	worker1 "redundant" reg: null, null, worker2
#1416	redundant, activating	worker2 "activating" reg: null, null, worker2	worker1 "redundant" reg: null, null, worker2
#1436	redundant, activating	worker2 "installed" reg: null, null, worker2	worker1 "redundant" reg: null, null, worker2
Chrome	redundant, activating	worker2 "installed" reg: null, worker2, worker1	worker1 "redundant" reg: null, null, worker2
Safari	redundant, activating	worker2 "installed" reg: null, worker2, worker1	worker1 "redundant" reg: null, null, worker2
Firefox	activating, redundant	worker2 "activating" reg: null, null, worker2	worker1 "redundant" reg: null, null, worker2

Chrome and Safari match the current spec, but that means a redundant worker is seen within the registration.

Firefox gets the order wrong.

A new waiting worker

In this test, worker1 is active, worker2 is waiting, worker3 is installing. Then, worker3 becomes waiting, worker 2 becomes redundant.

The story is the same as above.

Clear registration

In this test, worker1 is active, worker2 is waiting. The registration is unregistered.

	Redundancy order	when worker2 "redundant"	when worker1 "redundant"
Spec	worker2, worker1	worker1 "activated" worker2 "redundant" reg: null, worker2, worker1	worker1 "redundant" worker2 "redundant" reg: null, null, worker1
#1416	worker2, worker1	worker1 "redundant" worker2 "redundant" reg: null, null, null	worker1 "redundant" worker2 "redundant" reg: null, null, null
#1436	worker2, worker1	worker1 "activated" worker2 "redundant" reg: null, null, null	worker1 "redundant" worker2 "redundant" reg: null, null, null
Chrome	worker2, worker1	worker1 "activated" worker2 "redundant" reg: null, null, null	worker1 "redundant" worker2 "redundant" reg: null, null, null
Safari	worker2, worker1	worker1 "activated" worker2 "redundant" reg: null, null, null	worker1 "redundant" worker2 "redundant" reg: null, null, null
Firefox	worker2, worker1	worker1 "redundant" worker2 "redundant" reg: null, null, null	worker1 "redundant" worker2 "redundant" reg: null, null, null

Here the browsers break from the spec (I think they're following a test) and update the registration before firing any statechange events. Firefox goes further and updates both workers before firing any events.

I think our options are:

• Spec what Chrome and Safari are doing.
  • In some cases the registration will be updated before the statechange events fire, sometimes they won't.
  • Within a transaction, 'statechange' happens on the youngest worker first.
  • Within a transaction, worker.state is updated just before that worker's 'statechange' (meaning other workers will be showing old values)
• Update order #1436
  • For a transaction, update the registration before firing any 'statechange' events.
  • Within a transaction, 'statechange' happens on the youngest worker first.
  • Within a transaction, worker.state is updated just before that worker's 'statechange' (meaning other workers will be showing old values).
• Central algorithm for updating service worker state and registrations #1416
  • For a transaction, update the registration before firing any 'statechange' events.
  • For a transaction, update all worker.state properties before firing any 'statechange' events.
  • Within a transaction, 'statechange' happens on the youngest worker first.