Closed Bug 1343877 Opened 3 years ago Closed 3 years ago

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


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

Not set



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


(Reporter: bkelly, Assigned: bkelly)




(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  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
Don't allow an extra timer callback beyond configured pref value. r=smaug
Closed: 3 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.