Audit references across XPCOM borders

RESOLVED FIXED

Status

Cloud Services
Firefox Sync: Backend
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: philikon, Assigned: philikon)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Two common sources of leaks are getter closures hanging on to XPCOM services and strong-ref'ed observers.
Created attachment 462790 [details] [diff] [review]
part 1: nuke refs to pref service
Assignee: nobody → philipp
Attachment #462790 - Flags: review?(mconnor)
Created attachment 462791 [details] [diff] [review]
part 2: weak ref history and bookmark observers
Attachment #462791 - Flags: review?(mconnor)
Created attachment 462796 [details] [diff] [review]
part 2 (v2): weak ref history, bookmark, window observers

Also weakref the window observers in the TabTracker.
Attachment #462791 - Attachment is obsolete: true
Attachment #462796 - Flags: review?(mconnor)
Attachment #462791 - Flags: review?(mconnor)
Created attachment 462798 [details] [diff] [review]
part 2 (v2.1): weak ref history, bookmark, window observers

Actually, while nsIWindowWatcher.registerNotification() registers the observer for both "domwindowopened" and "domwindowclosed", we only need the former here.
Attachment #462796 - Attachment is obsolete: true
Attachment #462798 - Flags: review?(mconnor)
Attachment #462796 - Flags: review?(mconnor)
Comment on attachment 462798 [details] [diff] [review]
part 2 (v2.1): weak ref history, bookmark, window observers

>   // Register as an observer so we can catch windows opening and closing:
>-  Svc.WinWatcher.registerNotification(this);
>+  Svc.Obs.add("domwindowopened", this);

Adjust the comment here? r=me
Attachment #462798 - Flags: review?(mconnor) → review+

Updated

8 years ago
Attachment #462790 - Flags: review?(mconnor) → review+
part 1: http://hg.mozilla.org/services/fx-sync/rev/ad85963610eb
part 2: http://hg.mozilla.org/services/fx-sync/rev/7ee899185089

I've looked for other cases of holding on to XPCOM services throughout the source but couldn't find any. Resolving this.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Blocks: 584481
You need to log in before you can comment on or make changes to this bug.