ServiceWorkerRegistration should not keep a reference to the window

RESOLVED FIXED in mozilla34

Status

()

defect
RESOLVED FIXED
5 years ago
3 months ago

People

(Reporter: baku, Assigned: nsm)

Tracking

Trunk
mozilla34
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

Reporter

Description

5 years ago
No description provided.
Reporter

Comment 1

5 years ago
Posted patch deth.patch (obsolete) — Splinter Review
Attachment #8478276 - Flags: review?(nsm.nikhil)
Attachment #8478276 - Flags: review?(nsm.nikhil) → review+
Reporter

Comment 2

5 years ago
Posted patch deth.patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=42b606e75e62
Attachment #8478276 - Attachment is obsolete: true
Reporter

Updated

5 years ago
Keywords: checkin-needed
Reporter

Comment 3

5 years ago
Posted patch deth.patchSplinter Review
rebased
Attachment #8478501 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/06168844e276
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
StopListeningForEvents() should be called in the dtor too (with appropriate checks), in case the registration is GCed before the window goes away.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: amarchesini → nsm.nikhil
Status: REOPENED → ASSIGNED
Reporter

Comment 8

5 years ago
Comment on attachment 8480111 [details] [diff] [review]
Patch 2 - StopListeningForEvents in destructor

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

We wrote the same patch :)
We should call RemoveReadyPromise() in the dtor of ServiceWorkerContainer as well.
Attachment #8480111 - Flags: review?(amarchesini) → review+
Reporter

Comment 9

5 years ago
Attachment #8480465 - Flags: review?(nsm.nikhil)
Comment on attachment 8480465 [details] [diff] [review]
patch 3 - ServiceWorkerContainer

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

Shouldn't this patch be attached to the .ready bug? Please update the commit message when landing.
Attachment #8480465 - Flags: review?(nsm.nikhil) → review+
https://hg.mozilla.org/mozilla-central/rev/b7d78e4bddde
Status: ASSIGNED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.