Closed
Bug 1263307
Opened 7 years ago
Closed 7 years ago
Centralize logic for handling service worker state updates
Categories
(Core :: DOM: Service Workers, defect)
Core
DOM: Service Workers
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: bkelly, Assigned: bkelly)
References
(Blocks 1 open bug)
Details
(Whiteboard: btpp-active)
Attachments
(3 files, 1 obsolete file)
910 bytes,
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
21.37 KB,
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
17.09 KB,
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•7 years ago
|
Summary: Central logic for handling service worker state updates → Centralize logic for handling service worker state updates
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8c4caac7876
Assignee | ||
Comment 4•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0cf4642ae5f5
Attachment #8739622 -
Attachment is obsolete: true
Assignee | ||
Comment 5•7 years ago
|
||
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)
Assignee | ||
Comment 6•7 years ago
|
||
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)
Assignee | ||
Comment 7•7 years ago
|
||
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)
Updated•7 years ago
|
Whiteboard: btpp-active
Updated•7 years ago
|
Attachment #8739620 -
Flags: review?(josh) → review+
Updated•7 years ago
|
Attachment #8739621 -
Flags: review?(josh) → review+
Updated•7 years ago
|
Attachment #8739631 -
Flags: review?(josh) → review+
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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9d1bbc82b411 https://hg.mozilla.org/mozilla-central/rev/6b10d54f4288 https://hg.mozilla.org/mozilla-central/rev/336a70a8dfe4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•