Closed Bug 1350471 Opened 3 years ago Closed 3 years ago

add unregisterTimer method to nsIUpdateTimerManager

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: rhelmer, Assigned: rhelmer)

Details

Attachments

(2 files)

Currently nsIUpdateTimerManager has a `registerTimer` method, but does not provide a method to unregister:

http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/toolkit/components/timermanager/nsIUpdateTimerManager.idl

To be useful from hot-reloadable code like add-ons, we should add an "unregisterTimer" method.
Assignee: nobody → rhelmer
Status: NEW → ASSIGNED
Comment on attachment 8851331 [details]
Bug 1350471 - provide an unregisterTimer function for nsIUpdateTimerManager

https://reviewboard.mozilla.org/r/123654/#review126424

Well that was easy!
Attachment #8851331 - Flags: review?(dtownsend) → review+
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0a4d80866b40
provide an unregisterTimer function for nsIUpdateTimerManager r=mossop
https://hg.mozilla.org/mozilla-central/rev/0a4d80866b40
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Approval Request Comment
[Feature/Bug causing the regression]: bug 1350471
[User impact if declined]: enable add-ons to use built-in update timer. without this patch, add-ons will leak on upgrade since they cannot unregister timers.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: trivial change (new "unregister" function)
[String changes made/needed]: none
Attachment #8852665 - Flags: approval-mozilla-beta?
Attachment #8852665 - Flags: approval-mozilla-aurora?
Sounds like this affects all current branches and has been an issue for a while.
Comment on attachment 8851331 [details]
Bug 1350471 - provide an unregisterTimer function for nsIUpdateTimerManager

The uplift request should be for this patch. This should help fix potential memory leaks from system add-ons.
Attachment #8851331 - Flags: approval-mozilla-beta+
Attachment #8851331 - Flags: approval-mozilla-aurora+
Attachment #8852665 - Flags: approval-mozilla-beta?
Attachment #8852665 - Flags: approval-mozilla-beta-
Attachment #8852665 - Flags: approval-mozilla-aurora?
Attachment #8852665 - Flags: approval-mozilla-aurora-
Setting qe-verify- based on Robert's assessment on manual testing needs (see Comment 6) and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.