Most observers are not removed on shutdown

RESOLVED FIXED

Status

RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: jminta, Assigned: jminta)

Tracking

Details

(Whiteboard: [good first bug])

Attachments

(2 attachments)

(Assignee)

Description

13 years ago
Much of the calendar code requires observers of one kind or another.  Although all components which implement an addObserver function also implement a removeObserver function, most callers do not take advantage of this.  The easy cases to fix are the front-end cases, like the views, which can just use destructors or onunload listeners to tidy up (most don't even do this!).  The tougher cases are where backend (component) code is adding observers.  These are probably going to need shutdown() methods added to them.

(possibly incomplete) list of unremoved observers:
calendar-multiday-view's mObserver
calendar-month-view's mObserver
agendaTreeView's calendarObserver
calendar-todo-list's mOperationListener
ltnCompositeCalendarObserver
ltnCalendarManagerObserver
calCalendarManagerObserver

All error-announcer observers
All helper-observers in the composite calendar
ics calendar's observer on the memory calendar

For the most part, I'm working off of http://landfill.mozilla.org/mxr-test/mozilla/search?string=addObserver&find=calendar&filter=
Whiteboard: [good first bug]
The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!
Assignee: base → nobody
(Assignee)

Comment 2

12 years ago
Created attachment 231032 [details] [diff] [review]
sunbird tested patch

I'll ask for review here once Lightning is unbroken and I can test there too.  These are some major leaks on Mac and on Firefox Lightning that I'd like to fix.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
(Assignee)

Comment 3

12 years ago
Created attachment 234954 [details] [diff] [review]
unbitrotted

Unbitrots the previous patch.  Now tested on Lightning.
Attachment #234954 - Flags: second-review?(dmose)
Attachment #234954 - Flags: first-review?(mattwillis)
Comment on attachment 234954 [details] [diff] [review]
unbitrotted

Yay! Less leaking!

r1=lilmatt
Attachment #234954 - Flags: first-review?(mattwillis) → first-review+
Comment on attachment 234954 [details] [diff] [review]
unbitrotted

r=dmose
Attachment #234954 - Flags: second-review?(dmose) → second-review+
(Assignee)

Comment 6

12 years ago
patch checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.