Service workers: Resurrection is no longer a thing #17139
Conversation
service-workers/service-worker/unregister-then-register-new-script.https.html
Show resolved
Hide resolved
| try { | ||
| await navigator.serviceWorker.register('this-will-404', { scope }); | ||
| assert_unreached('register should reject'); | ||
| } catch (err) {} |
There was a problem hiding this comment.
We also have promise_rejects() but I'm not sure which is actually easier to read. I guess promise_rejects lets you test the exception type.
There was a problem hiding this comment.
promise_rejects is better, as it also ensures it's a promise that rejects, and not a function that throws.
service-workers/service-worker/unregister-then-register-new-script.https.html
Outdated
Show resolved
Hide resolved
service-workers/service-worker/unregister-then-register-new-script.https.html
Show resolved
Hide resolved
| promise_test(async function(t) { | ||
| const scope = 'resources/scope/re-register-while-old-registration-in-use'; | ||
| const registration = await service_worker_unregister_and_register(t, worker_url, scope); | ||
| t.add_cleanup(() => service_worker_unregister(t, scope)); |
There was a problem hiding this comment.
It's nice to also remove the iframe in the cleanup before unregistering, as it helps unregister really unregister.
| assert_equals(newRegistration, iframeRegistration, 'iFrame uses new registration'); | ||
| }, 'Unregister then register does not resurrect the registration'); | ||
| // Service worker used to 'ressurect' registrations, so registration and new_registration would be equal. | ||
| // This has since been changed. |
There was a problem hiding this comment.
Do we have tests for getRegistration() after unregister() in a controlled frame? I guess it can be added to getregistration.https.html. Looks like we already have coverage for getRegistrations() in getregistrations.https.html.
|
@mattto changes we pretty significant, want to take another look? |
| await wait_for_state(t, registration.installing, 'activated'); | ||
|
|
||
| const iframe = await with_iframe(scope); | ||
| t.add_cleanup(() => iframe.remove()); |
There was a problem hiding this comment.
I usually collapse all the cleanups into one, so you can remove the frames first then do the unregister for the cleanest cleanup.
Although I suppose with this spec change that becomes less important (OTOH browsers won't implement this yet).
| }, 'Registering a new script URL that 404s does not resurrect unregistered registration'); | ||
|
|
||
| promise_test(async function(t) { | ||
| const scope = 'resources/scope/unregister-then-register-new-script-that-404s'; |
There was a problem hiding this comment.
that-404s -> that-rejects-in-install
(but we don't really need these unique scopes anymore, they were just there because we used async_tests which run in parallel and caused collisions)
There was a problem hiding this comment.
Ugh, silly copy-paste error. Thanks for spotting.
| assert_true(!!newIframe.contentWindow.navigator.serviceWorker.controller, 'iFrame has controller'); | ||
|
|
||
| assert_not_equals(registration, newRegistration, 'Registrations are different'); | ||
| assert_equals(newRegistration, iframeRegistration, 'iFrame uses new registration'); |
There was a problem hiding this comment.
I don't think this holds because newRegistration and iframeRegistration are from different windows so have different prototypes.
There was a problem hiding this comment.
lol indeed. Will fix.
* Updating tests now we've removed resurrection * Async test to promise test * Proper cleanup after tests * More feedback fixes * Removing comments * Adding test for controlled iframe * Fixing tests following feedback
w3c/ServiceWorker#1415