Service worker: Altering ready test due to resurrection #17203
Conversation
|
I removed the timeout stuff due to https://web-platform-tests.org/writing-tests/testharness-api.html#timeouts-in-tests
|
|
@foolip I'm not sure how to interpret this failure. What did I break? |
|
Cause of Taskcluster failures are too hard to figure out , but if you know to look at the end of the logs, here it is: Unstable results
Does that failure with "https://web-platform.test:8443/service-workers/service-worker/resources/blank.html?ready-after-resolve-longer" seem like it could be a problem with the setup? Or is it a Firefox bug? |
|
Pretty sure this is a Firefox bug. It isn't any of the tests touched in this PR either. |
|
@jakearchibald can you file a Gecko bug or let the relevant maintainer know? Then I can admin merge this. |
|
|
||
| const readyReg = await readyPromise; | ||
|
|
||
| assert_equals(readyReg.active.scriptURL, reg1Active.scriptURL, 'Resolves with the first registration'); |
There was a problem hiding this comment.
@jakearchibald Could you explain where this assumption is coming from?
The way I'd interpret the spec here is that it should be the url of the new worker, which is also how Firefox seems to understand it [1].
In step 4 of the Unregister algorithm [2], we remove the registration from scope to registration map. Step 2.1 of serviceWorker.ready [3] invokes the Match Service Worker Registration algorithm [4], which only looks at registrations that are in scope to registration map. Since there is no non-uninstalled registration at the time we're calling serviceWorker.ready, the Promise should only resolve after the new worker is registered. Hence, we shouln't get the registration we intent to assert against here.
Am I missing something, or is this a bug in the test?
//cc @mattto
[1] https://wpt.fyi/results/service-workers/service-worker/ready.https.html?label=master&label=experimental&aligned&q=service-workers%2Fservice-worker%2Fready.https.html
[2] https://w3c.github.io/ServiceWorker/#unregister-algorithm
[3] https://w3c.github.io/ServiceWorker/#navigator-service-worker-ready
[4] https://w3c.github.io/ServiceWorker/#match-service-worker-registration
There was a problem hiding this comment.
Won't the promise be resolved earlier, in step 7.2 of https://w3c.github.io/ServiceWorker/#activation-algorithm?
There was a problem hiding this comment.
That'd be true if we move .ready before line 292.
.ready returns a new Promise every time it's called and runs https://w3c.github.io/ServiceWorker/#match-service-worker-registration to resolve it. Resolving it once after activating and return the same Promise for every call would contradict ready after a longer matched registration registered in this file.
There was a problem hiding this comment.
.readyreturns a new Promise every time it's called
That isn't how I interpret the spec. A service worker container has a ready promise, so the promise is created the same time the service worker container is created.
There was a problem hiding this comment.
Oof this isn't how Chrome implements .ready. Chrome doesn't try to resolve the promise until .ready is accessed. Then once it resolves, it always resolves with the same registration. I agree though the spec says to resolve each client's ready promise at activation time.
For the first link, the test matches the spec because frame is created after the activation of the first worker.
For the second link, I think you're right. frame is created, then the first activation occurs, so .ready should resolve to the first registration.
If browsers are all doing the "don't try to figure the value until the property is accessed thing", we could potentially change the spec for this.
There was a problem hiding this comment.
Filed w3c/ServiceWorker#1477
There was a problem hiding this comment.
I feel like the spec used to match the implementations. For example, I reference this behavior in this issue:
Found another resurrection test.
If I'm reading the spec correctly:
readyproperty is accessed, and there's a matching registration with an active service worker. This takes care of clients that were created after the registration was already 'ready'.readypromise.Chrome times outs on the first test, which I didn't expect. Firefox & Safari behave as I'd expect (given they implement resurrection).