ready promise is kind of weird with overlapping scopes

[wanderview]

Consider this situation:

// Window foo.com/bar/baz/index.html
await navigator.serviceWorker.register('sw.js', { scope: '/bar' });

navigator.serviceWorker.ready.then(swr => {
  console.log(swr.scope);
});

await navigator.serviceWorker.register('sw.js', { scope: '/bar/baz' });

navigator.serviceWorker.ready.then(swr => {
  console.log(swr.scope);
});

The first ready promise gets the '/bar' scope registration and waits for it to activate. This is step 4.3 of here:

https://w3c.github.io/ServiceWorker/#navigator-service-worker-ready

Assuming that the '/bar' worker does not activate immediately, we might get the '/bar/baz' worker registration started.

What should the second .ready promise access do? The promise is not settled yet, but the previous access is in a parallel bit of algorithm waiting for '/bar' to activate. Should the second access block on that somehow or start waiting on '/bar/baz' instead?

Kind of a corner case, but its a bit weird.

[wanderview]

Also, it feels wrong to me that the spec kind of makes the result here depend on when the ready promise is accessed. It feels like the spec should automatically resolve the ready promises of any matching clients whether the promise has been accessed or not. That would provide the most predictable behavior.

[wanderview]

Maybe waiting to do any logic until the first ready promise access is intended to help avoid weirdness like bug #1279, but it doesn't seem particularly effective and leads to more complications.

[mfalken]

If I understand correctly, both accesses have to resolve to the same result, as they return the same promise (similar to my comment on #1279)

[wanderview]

If I understand correctly, both accesses have to resolve to the same result, as they return the same promise (similar to my comment on #1279)

Sure, but which result? There are two possibilities AFAICT:

1. The second .ready access waits for the first access's parallel steps to complete. So the first access determines the result.
2. The second .ready access starts a second set of parallel steps and its a race to see which one completes first.

I think (1) is objectively better, but the spec kind of says (2) right now. I think these steps need to set some kind of flag on the context object indicating "ready promise is being processed" and then make step 1 early abort if that is set:

https://w3c.github.io/ServiceWorker/#navigator-service-worker-ready

Making step 1 wait for settled creates the potential for two sets of parallel steps that race each other.

[wanderview]

Also, it feels wrong to me that the spec kind of makes the result here depend on when the ready promise is accessed. It feels like the spec should automatically resolve the ready promises of any matching clients whether the promise has been accessed or not. That would provide the most predictable behavior.

Oh, I also thought about this a bit more. I do actually like waiting for the first access to start this process. I don't really want to send ready promise updates out to a bunch of clients that aren't even looking at them.

Edit: Also, this change wouldn't get rid of the issue where "when you check determines what you get". It just moves the "when you checked" time from .ready access to when your client is created.

[mfalken]

Ah, I see what you mean. Chrome does (1). Once we start the "ready" process, any subsequent access just gets added as a result callback to the process. After the ready result is known, any access gets back that result immediately.

It does look like the spec needs a kind of "ready promise is being processed" flag to describe that.

[jungkees]

It feels like the spec should automatically resolve the ready promises of any matching clients whether the promise has been accessed or not.

I happen to have been working on #1277 from a day before this issue was filed.

It seems (1) still doesn't solve the multiple registrations case. Would it be impossible at all to make .ready attribute getter return a new promise when it detects a more specific-scoped registration?

[wanderview]

I think its discouraged to change a property that returns a promise. It would be more conventional if it was a function.

The other part of the spec that I don't think browsers implement is the bit about locking in the registration and waiting for it to activate in step 3 here:

https://w3c.github.io/ServiceWorker/#navigator-service-worker-ready

The implication of the current spec text is that you can get a registration with a .installing worker, but not active yet and start waiting. If the registration fails to install and goes away, though, you are stuck with ready pointing at this dead registration.

I don't think browsers do this. I think at least firefox and chrome probably wait for the first registration that gets a .active worker. I need to write a test for this.

[domenic]

I don't know the context here very well, but it's OK to change a property that returns a promise, if there's some kind of conceptual "reset" that makes the promise represent a new thing. For example, we use that pattern in streams: https://streams.spec.whatwg.org/#default-writer-ready

[mfalken]

it's OK to change a property that returns a promise,

The guidance from @annevk (and seemed also from @domenic) on #223 seemed to be that the promise shouldn't change: "always returning a new object from an attribute is a non-starter". Also see the comment on blink-dev https://groups.google.com/a/chromium.org/d/msg/blink-dev/jjh4KUS0cS0/c81n5M6c8qwJ

There might be confusion about what's a "stable promise". I think there's two possible definitions of stability: 1) whether the property can vend multiple promises:

x = navigator.serviceWorker.ready;
// ...
y = navigator.serviceWorker.ready; 
x === y; // true always

And 2), whether the promise can resolve/reject to different values over its lifetime.

x = navigator.serviceWorker.ready;
x.then(r => result = r);
// ...
x.then(r => result2 = r);
result === result2; // true always

@yutakahirano tells me stability 2) is always the case: once a given promise resolves/rejects, it always resolves/rejects to the same value. Is the stability 1) assumption no longer always true?

[domenic]

There's a difference between "always returning a new object" and "when some conceptual reset happens, returning a new object". The former breaks navigator.serviceWorker.ready === navigator.serviceWorker.ready, and is a non-starter. The latter is fine.

[mfalken]

I see. From an implementer POV it's easiest to just break .ready === .ready and have each access get a new Promise. If we try to keep the same promise, except when there was a change, it seems likely to cause races: We have to cache .ready in each context and then alert each context when there was a change (unregister or a new register took effect), and that alert might or might not reach the context before .ready resolves again.

If we can't break .ready === .ready, I'm kind of inclined to just keep the existing weird behavior where .ready can resolve to the non-active registration.

[annevk]

Please don't break it, such behavior is very confusing.

[jungkees]

The latter is fine.

.ready seems like the case of https://www.w3.org/2001/tag/doc/promises-guide#state-transitions to me. Don't we have any precedent in the platform? @annevk, wouldn't it be okay even if it's just for the "conceptual reset" with a clear note?

If we don't want to change it, developers would have to do something like:

let reg = await navigator.serviceWorker.ready;
const latestReg = await navigator.serviceWorker.getRegistration();
if (reg !== latestReg) {
  await waitForState(latestReg.installing, 'activating');
  reg = latestReg;
}

Or should we provide a method (e.g. ready()) that could eventually deprecate ready property?

[jungkees]

@wanderview,

The implication of the current spec text is that you can get a registration with a .installing worker, but not active yet and start waiting. If the registration fails to install and goes away, though, you are stuck with ready pointing at this dead registration.

I think the returned ready promise would be resolved by some later access to .ready that would end up a successful installation. But I agree that the step waiting for it to activate is flawed. I'm working on #1277 that would address this problem.

[annevk]

@jungkees @mattto suggested such a change transition was hard because there's currently no signaling of the change?

[jungkees]

Sure. I'd like to hear more from implementers about feasibility. Here's my thought on @mattto's comments.

We have to cache .ready in each context and then alert each context when there was a change (unregister or a new register took effect)

I think we're already maintaining .ready for each context. Yes, alerting each context from unregister and Activate would have to be added. We're already doing similar signaling for statechange events and controllerchange events.

, and that alert might or might not reach the context before .ready resolves again.

AFAICT, the matching service worker registration calls initiated from .ready and from Activate would be scheduled in the same thread. So either task would set the ready promise to a new promise that resolves with a more specific-scoped registration only when needed.

/cc @wanderview @aliams @cdumez

[wanderview]

I think we're already maintaining .ready for each context. Yes, alerting each context from unregister and Activate would have to be added. We're already doing similar signaling for statechange events and controllerchange events.

Well, there is a slight difference in we only have to signal statechange and controllerchange events where a ServiceWorker is actually controlling the client or already referenced by js.

From an implementation perspective its nice to be able to lazy create the .ready promise so we don't have to manage signaling activation to potentially every client open in the browser. Its doable, but its some memory/cpu overhead that it would be nice to avoid. Lazy creation allows us to only signal clients where js has touched the .ready promise.

[jungkees]

its nice to be able to lazy create the .ready promise so we don't have to manage signaling activation to potentially every client open in the browser. Its doable, but its some memory/cpu overhead that it would be nice to avoid.

I agree we should avoid looking at potentially every client. Would it be possible to confine signaling activation to those clients that already lazy-created the .ready promise? In the spec, we might be able to initialize the .ready promise to null, set it to a promise when accessed, and make Activate look at those clients whose ready promise isn't null.

[mfalken]

We're already doing similar signaling for statechange events and controllerchange events.

Right and it's a bit tricky in our implementation already to send the signals in the right order. For example we have to send the "change to active" message before the "resolve ready promise" message, to ensure inside the ready.then() that registration.active is populated. It'll be more complex to ensure .ready resolves at the right time when the registration changes (due to unregistration or new registartion).

It's all doable but it could be tricky to get it right, and there could be some unavoidable races.

[wanderview]

I agree we should avoid looking at potentially every client. Would it be possible to confine signaling activation to those clients that already lazy-created the .ready promise? In the spec, we might be able to initialize the .ready promise to null, set it to a promise when accessed, and make Activate look at those clients whose ready promise isn't null.

This matches what I plan to do in the implementation. The tricky bit is that you also have to handle cleaning up the promises if they never settle and the client is detached/destroyed. I have an implementation way to do that, but not sure about spec language.

[delapuente]

What should the second .ready promise access do? The promise is not settled yet, but the previous access is in a parallel bit of algorithm waiting for '/bar' to activate. Should the second access block on that somehow or start waiting on '/bar/baz' instead?

Honouring semantics, I would say it should start waiting on /bar/baz since that's the registration that controls the client and not the former one.

Another alternative is to reject a second .register() intent after calling .ready.

[jungkees]

Another alternative is to reject a second .register() intent after calling .ready

Please note that the second .register() can be called from other clients.

[delapuente]

Can not these clients keep track of their .register() calls and throw if they already register something and read ready?

[mfalken]

F2F: We discussed some options:

• Add .ready to ServiceWorkerRegistration
• Add something like .getReadyPromise() alongside with .ready
  There was agreement that if we were to do it over we'd have made .ready a function instead of a property.

IMO: We should just add .getReadyPromise() [with a pithier name] and leave .ready semantics as they are instead of trying to improve it while stuck as a property. And if there are no actual problems this is causing in the wild, just don't change anything for now.