Centralize logic for handling service worker state updates

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

(Blocks 1 bug)

Trunk
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

(Whiteboard: btpp-active)

Attachments

(3 attachments, 1 obsolete attachment)

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.
Summary: Central logic for handling service worker state updates → Centralize logic for handling service worker state updates
Comment on attachment 8739620 [details] [diff] [review]
P1 Make ServiceWorkerRegistrationInfo::mScope const. r=jdm

A simple refactoring to make a public member attribute const.
Attachment #8739620 - Flags: review?(josh)
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.
Attachment #8739621 - Flags: review?(josh)
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!
Attachment #8739631 - Flags: review?(josh)
Whiteboard: btpp-active
Attachment #8739620 - Flags: review?(josh) → review+
Attachment #8739621 - Flags: review?(josh) → review+
Attachment #8739631 - Flags: review?(josh) → review+
You need to log in before you can comment on or make changes to this bug.