Closed Bug 1343877 Opened 7 years ago Closed 7 years ago

TimeoutManager::RunTimeout() accidentally allow one extra timer callback beyond pref setting

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox-esr45 --- unaffected
firefox51 --- unaffected
firefox52 --- wontfix
firefox-esr52 --- wontfix
firefox53 --- affected
firefox54 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(1 file)

In bug 1342854 I introduced a pref to control how many timer callbacks can be coalesced into a single runnable.  The code, however, allows "<prev value> + 1" callbacks to be coalesced.  Its not a critical problem, but we should fix it.
I realized yesterday that I implemented the pref based limit in TimeoutManager::RunTimeout() slightly wrong.  I need to update the count before checking against the limit.  As the code is now we always allow one additional timer callback to run.

Since this effectively lowers the limit by one I re-tested foofighters.com.  I couldn't measure a noticeable difference with the new limit.  So I propose we just leave the default 5 and we will no actually execute 5 instead of 6.
Attachment #8842888 - Flags: review?(bugs)
I'd like to uplift this to FF53, but not worth uplifting to FF52.
Blocks: 1343895
Comment on attachment 8842888 [details] [diff] [review]
Don't allow an extra timer callback beyond configured pref value. r=smaug

oops, sorry, I should have noticed.
numTimersToRun >= gTargetMaxConsecutiveCallbacks uses indeed >= and not >
Attachment #8842888 - Flags: review?(bugs) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/370ba5872bf6
Don't allow an extra timer callback beyond configured pref value. r=smaug
https://hg.mozilla.org/mozilla-central/rev/370ba5872bf6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.