1263307 - Centralize logic for handling service worker state updates

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently we have a long list of statements that have to be executed every time a worker changes its state.  That code is duplicated everywhere.  We should factor that code out into a central location to avoid future bugs.

This is code I wrote as a starting point in bug 1257977.  Moving it here since I'm not going to finish that bug soon.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Make ServiceWorkerRegistrationInfo::mScope const. r=jdm

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Make ServiceWorkerRegistrationInfo worker members private. r=jdm

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P3 Move ServiceWorker update logic into central place in ServiceWorkerRegistrationInfo methods. r=jdm

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

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P3 Move ServiceWorker update logic into central place in ServiceWorkerRegistrationInfo methods. r=jdm

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

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8739620 [details] [diff] [review]
P1 Make ServiceWorkerRegistrationInfo::mScope const. r=jdm

A simple refactoring to make a public member attribute const.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8739621 [details] [diff] [review]
P2 Make ServiceWorkerRegistrationInfo worker members private. r=jdm

Hide the ServiceWorkerInfo member attributes instead of exposing them as public.  This is setup so we can centralize the logic that needs to happen when they change.  That additional logic comes in the next patch.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8739631 [details] [diff] [review]
P3 Move ServiceWorker update logic into central place in ServiceWorkerRegistrationInfo methods. r=jdm

Centralize logic such that:

1) PurgeCache is always called when a ServiceWorkerInfo's state is updated to Redundant.  I believe we missed a few of these in the old code.
2) Trigger InvalidateServiceWorkerRegistrationWorker() directly from ServiceWorkerRegistrationInfo::NotifyListenersOnChange().  These always had to be called together, so we should just make the one do the other to avoid future bugs.
3) Create methods on ServiceWorkerRegistrationInfo that perform the state transitions.  This keeps all state transition logic in one file instead of spread across 3 different classes.  It also lets us enforce assertions better since we have more information about the operation that is being performed.

My ultimate goal is to replace or modify InvalidateServiceWorkerRegistrationWorker() in bug 1257977.  This refactoring to minimize the number of entry points to that method is a first step.

This should be the last of my service worker refactoring reviews for a while.  Thanks!

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d1bbc82b411
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b10d54f4288
https://hg.mozilla.org/integration/mozilla-inbound/rev/336a70a8dfe4

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9d1bbc82b411
https://hg.mozilla.org/mozilla-central/rev/6b10d54f4288
https://hg.mozilla.org/mozilla-central/rev/336a70a8dfe4