Make nsITimer.TYPE_REPEATING_PRECISE a synonym for nsITimer.TYPE_REPEATING_PRECISE_CAN_SKIP

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

({addon-compat, dev-doc-complete})

unspecified
mozilla42
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed, firefox-esr38- affected, thunderbird_esr3841+ fixed)

Details

Attachments

(2 attachments)

The docs for TYPE_REPEATING_PRECISE say "Use this timer type with extreme caution. Chances are, this is not what you want."

Bug 650379 introduced TYPE_REPEATING_PRECISE_CAN_SKIP as a safer alternative. That bug also converted all uses of TYPE_REPEATING_PRECISE in the tree to TYPE_REPEATING_PRECISE_CAN_SKIP.

Since then, one use of TYPE_REPEATING_PRECISE has been added to the tree relating to network stats (https://dxr.mozilla.org/mozilla-central/source/dom/network/NetworkStatsService.jsm#130).

The value is exposed via IDL so add-ons might use it. Making it equivalent to TYPE_REPEATING_PRECISE_CAN_SKIP might be a reasonable thing to do.
The patch is straightforward. The hard part is deciding if this is the right
thing to do. froydnj, please feel free to muster additional troops for the
review if appropriate; there's no rush on this.
Attachment #8642887 - Flags: review?(nfroyd)
Comment on attachment 8642887 [details] [diff] [review]
Remove nsITimer.TYPE_REPEATING_PRECISE

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

::: xpcom/threads/nsITimer.idl
@@ +92,5 @@
>     * guarantees that it will not queue up new events to fire the callback
>     * until the previous callback event finishes firing.  If the callback
>     * takes a long time, then the next callback will be scheduled immediately
> +   * afterward, but only once.  This is the only non-slack timer.
> +   * to use.

Nit: this "to use" should be wrapped with the line above, which also means removing the period after "timer".
Attachment #8642887 - Flags: review?(nfroyd) → review+
I'll update the MDN docs once this has stuck.
https://hg.mozilla.org/mozilla-central/rev/e14cb4eb7c0b
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8642887 [details] [diff] [review]
Remove nsITimer.TYPE_REPEATING_PRECISE

Approval Request Comment
[Feature/regressing bug #]: N/A (to support landing bug 1197152)

[User impact if declined]: Can't uplift bug 1197152 to beta

[Describe test coverage new/current, TreeHerder]: Lots of stuff uses timers...

[Risks and why]: In nightly and aurora since 8/7.  Low risk to uplift

[String/UUID change made/needed]: none
Attachment #8642887 - Flags: approval-mozilla-beta?
Comment on attachment 8642887 [details] [diff] [review]
Remove nsITimer.TYPE_REPEATING_PRECISE

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: See request in bug 1197152

User impact if declined: Can't land bug 1197152.  

Fix Landed on Version: 42

Risk to taking this patch (and alternatives if risky): Pretty low risk, and been in for a long while.

String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8642887 - Flags: approval-mozilla-esr38?
Comment on attachment 8642887 [details] [diff] [review]
Remove nsITimer.TYPE_REPEATING_PRECISE

We need this bug to be uplifted to fix bug 1197152. Beta41+
Attachment #8642887 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Nicholas, is manual QA required here? If yes, please let us know what we can do to help verify this fix.
Flags: needinfo?(n.nethercote)
> Nicholas, is manual QA required here? If yes, please let us know what we can
> do to help verify this fix.

There's no useful QA that I can think of.
Flags: needinfo?(n.nethercote)
(In reply to Nicholas Nethercote [:njn] from comment #13)
> > Nicholas, is manual QA required here? If yes, please let us know what we can
> > do to help verify this fix.
> 
> There's no useful QA that I can think of.

Thank you for the prompt reply! Setting qe-verify- based on Comment 13.
Flags: qe-verify-
Comment on attachment 8642887 [details] [diff] [review]
Remove nsITimer.TYPE_REPEATING_PRECISE

After discussion with the rest of the team, we're not taking this for ESR but can take it on a thunderbird relbranch.
Attachment #8642887 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38-
https://hg.mozilla.org/releases/mozilla-esr38/rev/9e87ca58e3f5 for THUNDERBIRD_38_VERBRANCH

(I did not roll the idl since this was a documentation change only).
You need to log in before you can comment on or make changes to this bug.