1874782 - Stop using randomized test worker URL

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, task)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1861342#c14.

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

But await SWR.unregister() should make sure it waits until the actual unregistration, not sure how there can still be a registration after unregister()? I took a look how the function works and to me it seems the promise is properly only resolved during the processing of the unregistration queue, so I'm confused.

Andrew, any idea?

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

From the log in the comment I understand the issue to be that the SimpleTest infrastructure is seeing that we still have registered ServiceWorkers.

I see that this seems to be the first test after restarting Firefox, and that the last test in the preceding run also seems to be a push test dom/push/test/test_unregister.html.

The simplest explanation is that we are reusing the profile and the previous run shut down before having flushed serviceworker.txt to disk. (That said, we have a shutdown blocker that should block shutdown until we have flushed the data (calls-between diagram).) The next alternative is that a functional event or soft update check has gone and begun to reinstall the ServiceWorker. We do have delays in some update logic that use fairly simple runnables, so it wouldn't be shocking to find that these are responsible. Unfortunately, it seems like this all only happens on windows, so it's not easy to just get a pernosco trace.

The push tests seems to use Math.random() in the cache-busting idiom that likely was advisable back in the days of ServiceWorker resurrection, but is now unnecessary because, as you found, unregister() should be reliable. I would suggest changing the unregister cache-busting to be something deterministic like return "worker.js?test_unregister.html"; and similarly in test_permission_granted too. (Well, ideally, in all cases.) This should help make it clear what the source of the bad SW is.

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Thanks! (Yay that the resurrection is dead and makes things simple 🎉)

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1874782 - Stop using Math.random() for test worker URL r=asuth

Comment 5

[Kagami Rosylight [:saschanaz] (they/them)]

Actually we can open another bug for a real fix.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89e19e5ed13b
Stop using Math.random() for test worker URL r=asuth

Comment 7

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/89e19e5ed13b