1452244 - don't write broken waiting worker records to serviceworker.txt

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, enhancement, P2)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently we call StoreRegistration() in ServiceWorkerRegistrationInfo::TransitionInstallingToWaiting().  This is in anticipation of actually implementating bug 1426401.  Until we implement that bug, though, calling StoreRegistration() is just a waste of resources.  The entries it produces are not correct and won't result anything being usefully read.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Avoid empty entries in serviceworker.txt. r=asuth

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3462dd8679adc5ea41456cc05f199fd9f30b0bc2

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Avoid empty entries in serviceworker.txt. r=asuth

https://treeherder.mozilla.org/#/jobs?repo=try&revision=53b14b926bf0dfc3f920e8d9792916541e4f4a10

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc3702636b75e5d7439531cf0319371ebcb0bea6

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Avoid empty entries in serviceworker.txt. r=asuth

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a900f398f81e4104e24b507d273a13645e2090a

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8966213 [details] [diff] [review]
Avoid empty entries in serviceworker.txt. r=asuth

Andrew, I realized recently that we are stacking up a lot of service worker changes at once.  Each one tries to trigger its own write to the serviceworker.txt file.  This is probably exacerbating write failures on windows.

This patch is a first step to trying to calm the situation down.  Currently we try to write out some entries that can't be useful read in yet.  In particular:

1. We are writing out when installing transitions to waiting.  We haven't implemented bug 1426401, though, so we don't really handle this case yet and the entry in the file doesn't have a script URL.
2. When we resurrect a service worker we write out the file again to reflect that the registration is back.  We don't check to see if we are resurrecting a registration with an active worker, though.  In theory it could just have an installing/waiting worker.

This patch tries to close these loop holes and add some debug-only assertions.  I also added some read/write validation code as a safe guard to avoid actually writing this stuff to disk and to immediately ignore it if its read.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Stacking up a lot of writes is bad because of the overlapping runnables on STS.  See bug 1452373.

Comment 7

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

Comment on attachment 8966213 [details] [diff] [review]
Avoid empty entries in serviceworker.txt. r=asuth

Review of attachment 8966213 [details] [diff] [review]:
-----------------------------------------------------------------

Great cleanup, and great catch on the STS snafu in bug 1452373... I feel like anytime DOM code uses it without a TaskQueue we're usually mis-using...

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c913924064a5
Avoid empty entries in serviceworker.txt. r=asuth

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/c913924064a5