Closed Bug 1266857 Opened 8 years ago Closed 8 years ago

Clients.claim() should not maintain a separate document list in ServiceWorkerManager

Categories

(Core :: DOM: Service Workers, defect)

48 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

(Whiteboard: btpp-backlog)

Attachments

(1 file)

We currently maintain two different lists of documents for service workers:

* Each document registers with the observer service when they become active.
* ServiceWorkerManager maintains a mAllDocuments hashtable of all active documents.

We don't need the secondary hashtable.  Clients.claim() should instead just use the observer service list.
Whiteboard: btpp-backlog
This removes the duplicate data we are storing in SWM for tracking active documents.  Remove the mAllDocuments hashtable and instead just make Clients.claim() use the observer list like Clients.matchAll().

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f540b5909174
Attachment #8744488 - Flags: review?(bzbarsky)
Comment on attachment 8744488 [details] [diff] [review]
Make Clients.claim() use observer document list instead of secondary hashtable. r=bz

r=me
Attachment #8744488 - Flags: review?(bzbarsky) → review+
If anything, I would have expected a leak like that to come from bug 1265771, not this bug.  This bug removes strong refs and doesn't add any new ones.  I don't think service workers are directly used in that test.

Anyway, I will look at it.
Test failure is fixed in new patch in bug 1265771.  Marking checkin-needed since the trees are closed atm.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5b7355654e6e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.