Replace the redundant MOZ_ASSERT + duplicate IsInOrBeyond check with a
cleaner if/else: call Cancel() in the shutdown case, and use
MOZ_ASSERT_UNREACHABLE in the impossible non-shutdown case.
Differential Revision: https://phabricator.services.mozilla.com/D305061
When timer creation fails in ResetTimer, mTimerActive stays false. The
IdleTaskRunnerTask previously added to the TaskController still fires,
calls Schedule(), sees !mTask, and enqueues another task — creating an
endless dispatch loop that prevents ShutdownXPCOM from draining the
event queue.
Fix by calling Cancel() when the timer fails and we are in or beyond
XPCOMShutdownThreads, which stops the runner from re-enqueuing itself.
Differential Revision: https://phabricator.services.mozilla.com/D305025
Promote TimedOut to a static member so it can clear mTimerActive before
calling Run(). This ties the flag strictly to the timer's lifecycle: set in
ResetTimer when the one-shot is armed, cleared in TimedOut when it fires.
Previously the else-if(now>=mDeadline) branch in Run() left mTimerActive==true
after the one-shot fired, causing SetTimerInternal to early-return and leaving
the runner with no wakeup source in environments where IdleTaskManager never
supplies an idle deadline (e.g. gtests).
Differential Revision: https://phabricator.services.mozilla.com/D303297
Timer initialization is fallible (in rare cases), but normally timers targeting the main thread should just work, modulo OOM crashes.
Differential Revision: https://phabricator.services.mozilla.com/D262918
This avoids the small overhead and potential for bugs from the previous
approach of assuming all `const char*` arguments passed to `nsITimer`
initializers are string literals, by instead requiring the argument to be a
`const nsACString&`. This involves changing all callsites to pass a `""_ns`
string literal down instead of a bare string literal.
It appears that while there were currently no buggy callers which
would've provided a dynamic string, there was at least one piece of code
(nsRepeatService::Start) which did pass down a raw char* which may be
dynamically allocated (all callers pass down `nsLiteralCString` so we're
actually OK here right now, but this was a footgun).
Differential Revision: https://phabricator.services.mozilla.com/D261032
This reverts commit 8c929cc018.
Revert "Bug 1979013 - Part 2: Require nsITimer API callers to provide a nsACString, r=xpcom-reviewers,media-playback-reviewers,webrtc-reviewers,profiler-reviewers,dom-storage-reviewers,toolkit-telemetry-reviewers,firefox-style-system-reviewers,mstange,emz,bwc,janv,emilio,padenot"
This reverts commit e01c0a6a32.
Revert "Bug 1979013 - Part 1: Eagerly fetch name for timer callbacks, r=xpcom-reviewers,mccr8"
This reverts commit 9f8e73c163.
This avoids the small overhead and potential for bugs from the previous
approach of assuming all `const char*` arguments passed to `nsITimer`
initializers are string literals, by instead requiring the argument to be a
`const nsACString&`. This involves changing all callsites to pass a `""_ns`
string literal down instead of a bare string literal.
It appears that while there were currently no buggy callers which
would've provided a dynamic string, there was at least one piece of code
(nsRepeatService::Start) which did pass down a raw char* which may be
dynamically allocated (all callers pass down `nsLiteralCString` so we're
actually OK here right now, but this was a footgun).
Differential Revision: https://phabricator.services.mozilla.com/D261032
Sorry this is not a particularly easy patch to review. But it should be
mostly straight-forward.
I kept Document::Dispatch mostly for convenience, but could be
cleaned-up too / changed by SchedulerGroup::Dispatch. Similarly maybe
that can just be NS_DispatchToMainThread if we add an NS_IsMainThread
check there or something (to preserve shutdown semantics).
Differential Revision: https://phabricator.services.mozilla.com/D190450
Classes that inherit from DiscardableRunnable are only promising that it is OK
for Run() to be skipped, rather than promising that Cancel() is effective.
Differential Revision: https://phabricator.services.mozilla.com/D98117