From e0e154291395226b11eb0ac094e7fb33582dfde7 Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 1 Oct 2023 02:21:53 +0200 Subject: [PATCH 1/4] Issue #2321 - Fall back to 0 if setInterval interval not supplied --- dom/base/nsGlobalWindow.cpp | 27 +++------------------ dom/base/nsGlobalWindow.h | 4 +-- dom/webidl/WindowOrWorkerGlobalScope.webidl | 4 +-- dom/workers/WorkerScope.cpp | 14 +++-------- dom/workers/WorkerScope.h | 4 +-- 5 files changed, 14 insertions(+), 39 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index b0732ee51f..07920db6fd 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -12691,41 +12691,22 @@ nsGlobalWindow::SetTimeout(JSContext* aCx, const nsAString& aHandler, return SetTimeoutOrInterval(aCx, aHandler, aTimeout, false, aError); } -static bool -IsInterval(const Optional& aTimeout, int32_t& aResultTimeout) -{ - if (aTimeout.WasPassed()) { - aResultTimeout = aTimeout.Value(); - return true; - } - - // If no interval was specified, treat this like a timeout, to avoid setting - // an interval of 0 milliseconds. - aResultTimeout = 0; - return false; -} - int32_t nsGlobalWindow::SetInterval(JSContext* aCx, Function& aFunction, - const Optional& aTimeout, + const int32_t aTimeout, const Sequence& aArguments, ErrorResult& aError) { - int32_t timeout; - bool isInterval = IsInterval(aTimeout, timeout); - return SetTimeoutOrInterval(aCx, aFunction, timeout, aArguments, isInterval, - aError); + return SetTimeoutOrInterval(aCx, aFunction, aTimeout, aArguments, true, aError); } int32_t nsGlobalWindow::SetInterval(JSContext* aCx, const nsAString& aHandler, - const Optional& aTimeout, + const int32_t aTimeout, const Sequence& /* unused */, ErrorResult& aError) { - int32_t timeout; - bool isInterval = IsInterval(aTimeout, timeout); - return SetTimeoutOrInterval(aCx, aHandler, timeout, isInterval, aError); + return SetTimeoutOrInterval(aCx, aHandler, aTimeout, true, aError); } nsresult diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index 63bb574dd4..c59baee648 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -953,11 +953,11 @@ public: mozilla::ErrorResult& aError); void ClearTimeout(int32_t aHandle); int32_t SetInterval(JSContext* aCx, mozilla::dom::Function& aFunction, - const mozilla::dom::Optional& aTimeout, + const int32_t aTimeout, const mozilla::dom::Sequence& aArguments, mozilla::ErrorResult& aError); int32_t SetInterval(JSContext* aCx, const nsAString& aHandler, - const mozilla::dom::Optional& aTimeout, + const int32_t aTimeout, const mozilla::dom::Sequence& /* unused */, mozilla::ErrorResult& aError); void ClearInterval(int32_t aHandle); diff --git a/dom/webidl/WindowOrWorkerGlobalScope.webidl b/dom/webidl/WindowOrWorkerGlobalScope.webidl index 9e639db5f4..d378ba49ec 100644 --- a/dom/webidl/WindowOrWorkerGlobalScope.webidl +++ b/dom/webidl/WindowOrWorkerGlobalScope.webidl @@ -31,9 +31,9 @@ interface WindowOrWorkerGlobalScope { long setTimeout(DOMString handler, optional long timeout = 0, any... unused); void clearTimeout(optional long handle = 0); [Throws] - long setInterval(Function handler, optional long timeout, any... arguments); + long setInterval(Function handler, optional long timeout = 0, any... arguments); [Throws] - long setInterval(DOMString handler, optional long timeout, any... unused); + long setInterval(DOMString handler, optional long timeout = 0, any... unused); void clearInterval(optional long handle = 0); // microtask queuing diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index 637e441732..df75ff887a 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -299,28 +299,25 @@ WorkerGlobalScope::ClearTimeout(int32_t aHandle) int32_t WorkerGlobalScope::SetInterval(JSContext* aCx, Function& aHandler, - const Optional& aTimeout, + const int32_t aTimeout, const Sequence& aArguments, ErrorResult& aRv) { mWorkerPrivate->AssertIsOnWorkerThread(); - bool isInterval = aTimeout.WasPassed(); - int32_t timeout = aTimeout.WasPassed() ? aTimeout.Value() : 0; - nsCOMPtr handler = NS_CreateJSTimeoutHandler(aCx, mWorkerPrivate, aHandler, aArguments, aRv); if (NS_WARN_IF(aRv.Failed())) { return 0; } - return mWorkerPrivate->SetTimeout(aCx, handler, timeout, isInterval, aRv); + return mWorkerPrivate->SetTimeout(aCx, handler, aTimeout, true, aRv); } int32_t WorkerGlobalScope::SetInterval(JSContext* aCx, const nsAString& aHandler, - const Optional& aTimeout, + const int32_t aTimeout, const Sequence& /* unused */, ErrorResult& aRv) { @@ -328,12 +325,9 @@ WorkerGlobalScope::SetInterval(JSContext* aCx, Sequence dummy; - bool isInterval = aTimeout.WasPassed(); - int32_t timeout = aTimeout.WasPassed() ? aTimeout.Value() : 0; - nsCOMPtr handler = NS_CreateJSTimeoutHandler(aCx, mWorkerPrivate, aHandler); - return mWorkerPrivate->SetTimeout(aCx, handler, timeout, isInterval, aRv); + return mWorkerPrivate->SetTimeout(aCx, handler, aTimeout, true, aRv); } void diff --git a/dom/workers/WorkerScope.h b/dom/workers/WorkerScope.h index 01bae5cc11..81d2733efe 100644 --- a/dom/workers/WorkerScope.h +++ b/dom/workers/WorkerScope.h @@ -132,11 +132,11 @@ public: ClearTimeout(int32_t aHandle); int32_t SetInterval(JSContext* aCx, Function& aHandler, - const Optional& aTimeout, + const int32_t aTimeout, const Sequence& aArguments, ErrorResult& aRv); int32_t SetInterval(JSContext* aCx, const nsAString& aHandler, - const Optional& aTimeout, + const int32_t aTimeout, const Sequence& /* unused */, ErrorResult& aRv); void ClearInterval(int32_t aHandle); From c78ecee3f2c5eb9c63321590a6daa7027291cc9d Mon Sep 17 00:00:00 2001 From: Botond Ballo Date: Sun, 1 Oct 2023 19:37:49 +0200 Subject: [PATCH 2/4] Issue #2323 - Part 1: Add Min()/Max() methods to TimeDuration. Just some helper functions to make time comparisons in the correct data format easier. See BZ 1321885 --- mozglue/misc/TimeStamp.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/mozglue/misc/TimeStamp.h b/mozglue/misc/TimeStamp.h index 019393b284..5f5886abc7 100644 --- a/mozglue/misc/TimeStamp.h +++ b/mozglue/misc/TimeStamp.h @@ -7,6 +7,7 @@ #define mozilla_TimeStamp_h #include +#include // for std::min, std::max #include "mozilla/Assertions.h" #include "mozilla/Attributes.h" #include "mozilla/FloatingPoint.h" @@ -173,6 +174,17 @@ public: return FromTicks(ticks); } + static BaseTimeDuration Max(const BaseTimeDuration& aA, + const BaseTimeDuration& aB) + { + return FromTicks(std::max(aA.mValue, aB.mValue)); + } + static BaseTimeDuration Min(const BaseTimeDuration& aA, + const BaseTimeDuration& aB) + { + return FromTicks(std::min(aA.mValue, aB.mValue)); + } + private: // Block double multiplier (slower, imprecise if long duration) - Bug 853398. From 1b7df316c4f34150639a90c6f756ff4ec26aae9f Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 1 Oct 2023 19:38:52 +0200 Subject: [PATCH 3/4] Issue #2323 - Part 2: Implement timer nesting and clamping for workers. --- dom/workers/WorkerPrivate.cpp | 42 ++++++++++++++++++++++++++++++++--- dom/workers/WorkerPrivate.h | 15 +++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 2f2fa78d42..f0542e63ce 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -167,6 +167,9 @@ const nsIID kDEBUGWorkerEventTargetIID = { #endif +// The number of nested timeouts before we start clamping. HTML says 5. +const uint32_t kClampTimeoutNestingLevel = 5u; + template class AutoPtrComparator { @@ -1956,7 +1959,10 @@ NS_IMPL_QUERY_INTERFACE(WorkerLoadInfo::InterfaceRequestor, nsIInterfaceRequesto struct WorkerPrivate::TimeoutInfo { TimeoutInfo() - : mId(0), mIsInterval(false), mCanceled(false) + : mId(0) + , mNestingLevel(0) + , mIsInterval(false) + , mCanceled(false) { MOZ_COUNT_CTOR(mozilla::dom::workers::WorkerPrivate::TimeoutInfo); } @@ -1976,10 +1982,29 @@ struct WorkerPrivate::TimeoutInfo return mTargetTime < aOther.mTargetTime; } + void AccumulateNestingLevel(const uint32_t& aBaseLevel) { + if (aBaseLevel < kClampTimeoutNestingLevel) { + mNestingLevel = aBaseLevel + 1; + return; + } + mNestingLevel = kClampTimeoutNestingLevel; + } + + void CalculateTargetTime() { + auto target = mInterval; + if (mNestingLevel >= kClampTimeoutNestingLevel) { + target = TimeDuration::Max( + mInterval, + TimeDuration::FromMilliseconds(Preferences::GetInt("dom.min_timeout_value"))); + } + mTargetTime = TimeStamp::Now() + target; + } + nsCOMPtr mHandler; mozilla::TimeStamp mTargetTime; mozilla::TimeDuration mInterval; int32_t mId; + uint32_t mNestingLevel; bool mIsInterval; bool mCanceled; }; @@ -4130,6 +4155,7 @@ WorkerPrivate::WorkerPrivate(WorkerPrivate* aParent, , mMainThreadEventTarget(do_GetMainThread()) , mErrorHandlerRecursionCount(0) , mNextTimeoutId(1) + , mCurrentTimerNestingLevel(0) , mStatus(Pending) , mFrozen(false) , mTimerRunning(false) @@ -6076,6 +6102,7 @@ WorkerPrivate::SetTimeout(JSContext* aCx, nsAutoPtr newInfo(new TimeoutInfo()); newInfo->mIsInterval = aIsInterval; newInfo->mId = timerId; + newInfo->AccumulateNestingLevel(this->mCurrentTimerNestingLevel); if (MOZ_UNLIKELY(timerId == INT32_MAX)) { NS_WARNING("Timeout ids overflowed!"); @@ -6188,14 +6215,21 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx) // Guard against recursion. mRunningExpiredTimeouts = true; + MOZ_DIAGNOSTIC_ASSERT(data->mCurrentTimerNestingLevel == 0); + // Run expired timeouts. for (uint32_t index = 0; index < expiredTimeouts.Length(); index++) { TimeoutInfo*& info = expiredTimeouts[index]; + AutoRestore nestingLevel(this->mCurrentTimerNestingLevel); if (info->mCanceled) { continue; } + // Set current timer nesting level to current running timer handler's + // nesting level + this->mCurrentTimerNestingLevel = info->mNestingLevel; + LOG(TimeoutsLog(), ("Worker %p executing timeout with original delay %f ms.\n", this, info->mInterval.ToMilliseconds())); @@ -6273,8 +6307,10 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx) info->mCanceled) { if (info->mIsInterval && !info->mCanceled) { // Reschedule intervals. - info->mTargetTime = info->mTargetTime + info->mInterval; - // Don't resort the list here, we'll do that at the end. + // Reschedule a timeout and, if needed, increase the nesting level. + info->AccumulateNestingLevel(info->mNestingLevel); + info->CalculateTargetTime(); + // Don't re-sort the list here, we'll do that at the end. ++index; } else { diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 351f5458f1..018d14321a 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -17,6 +17,7 @@ #include "mozilla/Assertions.h" #include "mozilla/Attributes.h" +#include "mozilla/AutoRestore.h" #include "mozilla/CondVar.h" #include "mozilla/ConsoleReportCollector.h" #include "mozilla/DOMEventTargetHelper.h" @@ -1016,6 +1017,20 @@ class WorkerPrivate : public WorkerPrivateParent uint32_t mErrorHandlerRecursionCount; uint32_t mNextTimeoutId; Status mStatus; + + // Tracks the current setTimeout/setInterval nesting level. + // When there isn't a TimeoutHandler on the stack, this will be 0. + // Whenever setTimeout/setInterval are called, a new TimeoutInfo will be + // created with a nesting level one more than the current nesting level, + // saturating at the kClampTimeoutNestingLevel. + // + // When RunExpiredTimeouts is run, it sets this value to the + // TimeoutInfo::mNestingLevel for the duration of + // the WorkerScriptTimeoutHandler::Call which will explicitly trigger a + // microtask checkpoint so that any immediately-resolved promises will + // still see the nesting level. + uint32_t mCurrentTimerNestingLevel; + bool mFrozen; bool mTimerRunning; bool mRunningExpiredTimeouts; From 2ca5715149dadc7f6a599a6d222dd678850f0c98 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 1 Oct 2023 20:54:48 +0200 Subject: [PATCH 4/4] Issue #2323 - Part 3: Exclude chrome workers from worker timer clamping. --- dom/workers/WorkerPrivate.cpp | 6 +++++- dom/workers/WorkerPrivate.h | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index f0542e63ce..622a882a65 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -1963,6 +1963,7 @@ struct WorkerPrivate::TimeoutInfo , mNestingLevel(0) , mIsInterval(false) , mCanceled(false) + , mOnChromeWorker(false) { MOZ_COUNT_CTOR(mozilla::dom::workers::WorkerPrivate::TimeoutInfo); } @@ -1992,7 +1993,8 @@ struct WorkerPrivate::TimeoutInfo void CalculateTargetTime() { auto target = mInterval; - if (mNestingLevel >= kClampTimeoutNestingLevel) { + // Clamp timeout for workers, except chrome workers + if (mNestingLevel >= kClampTimeoutNestingLevel && !mOnChromeWorker) { target = TimeDuration::Max( mInterval, TimeDuration::FromMilliseconds(Preferences::GetInt("dom.min_timeout_value"))); @@ -2007,6 +2009,7 @@ struct WorkerPrivate::TimeoutInfo uint32_t mNestingLevel; bool mIsInterval; bool mCanceled; + bool mOnChromeWorker; }; class WorkerJSContextStats final : public JS::RuntimeStats @@ -6100,6 +6103,7 @@ WorkerPrivate::SetTimeout(JSContext* aCx, } nsAutoPtr newInfo(new TimeoutInfo()); + newInfo->mOnChromeWorker = mIsChromeWorker; newInfo->mIsInterval = aIsInterval; newInfo->mId = timerId; newInfo->AccumulateNestingLevel(this->mCurrentTimerNestingLevel); diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 018d14321a..9effdccc9a 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -213,6 +213,9 @@ protected: RefPtr mEventTarget; nsTArray> mPreStartRunnables; + // True if the worker is used in the UI + bool mIsChromeWorker; + private: WorkerPrivate* mParent; nsString mScriptURL; @@ -242,7 +245,6 @@ private: uint32_t mParentWindowPausedDepth; Status mParentStatus; bool mParentFrozen; - bool mIsChromeWorker; bool mMainThreadObjectsForgotten; // mIsSecureContext is set once in our constructor; after that it can be read // from various threads. We could make this const if we were OK with setting