From 518c66d0ddc91d347c55bf6f7348394e5f6bdffa Mon Sep 17 00:00:00 2001 From: Butkovits Atila Date: Mon, 3 Mar 2025 23:28:43 +0200 Subject: [PATCH] Backed out changeset 873b1f683e95 (bug 1945470) for causing high frequency failures at test_ext_native_messaging_perf.js. CLOSED TREE --- dom/base/TimeoutBudgetManager.cpp | 5 ++ dom/base/TimeoutBudgetManager.h | 8 ++- dom/base/TimeoutManager.cpp | 59 ++++++------------ dom/base/TimeoutManager.h | 28 +++------ dom/base/moz.build | 1 - dom/base/nsGlobalWindowInner.cpp | 6 +- dom/base/nsIGlobalObject.h | 2 - dom/workers/WorkerPrivate.cpp | 61 +------------------ dom/workers/WorkerScope.cpp | 6 +- dom/workers/WorkerScope.h | 4 -- modules/libpref/init/StaticPrefList.yaml | 2 +- .../test/xpcshell/test_subprocess.js | 8 +-- 12 files changed, 49 insertions(+), 141 deletions(-) diff --git a/dom/base/TimeoutBudgetManager.cpp b/dom/base/TimeoutBudgetManager.cpp index 1e812bd4426c..0fb9597df86f 100644 --- a/dom/base/TimeoutBudgetManager.cpp +++ b/dom/base/TimeoutBudgetManager.cpp @@ -10,6 +10,11 @@ namespace mozilla::dom { +/* static */ TimeoutBudgetManager& TimeoutBudgetManager::Get() { + static TimeoutBudgetManager gTimeoutBudgetManager; + return gTimeoutBudgetManager; +} + void TimeoutBudgetManager::StartRecording(const TimeStamp& aNow) { mStart = aNow; } diff --git a/dom/base/TimeoutBudgetManager.h b/dom/base/TimeoutBudgetManager.h index 35c281f8093f..2287537c24b7 100644 --- a/dom/base/TimeoutBudgetManager.h +++ b/dom/base/TimeoutBudgetManager.h @@ -8,18 +8,22 @@ #define mozilla_dom_timeoutbudgetmanager_h #include "mozilla/TimeStamp.h" -#include "mozilla/dom/Timeout.h" namespace mozilla::dom { +class Timeout; + class TimeoutBudgetManager { public: + static TimeoutBudgetManager& Get(); void StartRecording(const TimeStamp& aNow); void StopRecording(); TimeDuration RecordExecution(const TimeStamp& aNow, const Timeout* aTimeout); private: - TimeStamp mStart{}; + TimeoutBudgetManager() = default; + + TimeStamp mStart; }; } // namespace mozilla::dom diff --git a/dom/base/TimeoutManager.cpp b/dom/base/TimeoutManager.cpp index 1ed4c8a1222c..269219e65bd3 100644 --- a/dom/base/TimeoutManager.cpp +++ b/dom/base/TimeoutManager.cpp @@ -21,6 +21,7 @@ #include "mozilla/dom/ContentChild.h" #include "mozilla/dom/TimeoutHandler.h" #include "TimeoutExecutor.h" +#include "TimeoutBudgetManager.h" #include "mozilla/net/WebSocketEventService.h" #include "mozilla/MediaManager.h" #include "mozilla/dom/WorkerScope.h" @@ -30,14 +31,12 @@ using namespace mozilla::dom; LazyLogModule gTimeoutLog("Timeout"); -TimeoutBudgetManager TimeoutManager::sBudgetManager{}; +static int32_t gRunningTimeoutDepth = 0; // static const uint32_t TimeoutManager::InvalidFiringId = 0; namespace { -static int32_t gRunningTimeoutDepth = 0; - double GetRegenerationFactor(bool aIsBackground) { // Lookup function for "dom.timeout.{background, // foreground}_budget_regeneration_rate". @@ -334,9 +333,8 @@ TimeDuration TimeoutManager::CalculateDelay(Timeout* aTimeout) const { void TimeoutManager::RecordExecution(Timeout* aRunningTimeout, Timeout* aTimeout) { + TimeoutBudgetManager& budgetManager = TimeoutBudgetManager::Get(); TimeStamp now = TimeStamp::Now(); - TimeoutBudgetManager& budgetManager{GetInnerWindow() ? sBudgetManager - : mBudgetManager}; if (aRunningTimeout) { // If we're running a timeout callback, record any execution until @@ -490,12 +488,6 @@ nsresult TimeoutManager::SetTimeout(TimeoutHandler* aHandler, int32_t interval, } } - auto scopeExit = MakeScopeExit([&] { - if (!mGlobalObject.GetAsInnerWindow() && !HasTimeouts()) { - mGlobalObject.TriggerUpdateCCFlag(); - } - }); - // Disallow negative intervals. interval = std::max(0, interval); @@ -517,20 +509,13 @@ nsresult TimeoutManager::SetTimeout(TimeoutHandler* aHandler, int32_t interval, timeout->mScriptHandler = aHandler; timeout->mReason = aReason; - if (GetInnerWindow()) { - // No popups from timeouts by default - timeout->mPopupState = PopupBlocker::openAbused; - } + // No popups from timeouts by default + timeout->mPopupState = PopupBlocker::openAbused; // XXX: Does eIdleCallbackTimeout need clamping? if (aReason == Timeout::Reason::eTimeoutOrInterval || aReason == Timeout::Reason::eIdleCallbackTimeout) { - uint32_t nestingLevel{}; - if (GetInnerWindow()) { - nestingLevel = GetNestingLevelForWindow(); - } else { - nestingLevel = GetNestingLevelForWorker(); - } + const uint32_t nestingLevel{GetNestingLevel()}; timeout->mNestingLevel = nestingLevel < StaticPrefs::dom_clamp_timeout_nesting_level_AtStartup() ? nestingLevel + 1 @@ -550,20 +535,18 @@ nsresult TimeoutManager::SetTimeout(TimeoutHandler* aHandler, int32_t interval, } } - if (GetInnerWindow()) { - if (gRunningTimeoutDepth == 0 && - PopupBlocker::GetPopupControlState() < PopupBlocker::openBlocked) { - // This timeout is *not* set from another timeout and it's set - // while popups are enabled. Propagate the state to the timeout if - // its delay (interval) is equal to or less than what - // "dom.disable_open_click_delay" is set to (in ms). + if (gRunningTimeoutDepth == 0 && + PopupBlocker::GetPopupControlState() < PopupBlocker::openBlocked) { + // This timeout is *not* set from another timeout and it's set + // while popups are enabled. Propagate the state to the timeout if + // its delay (interval) is equal to or less than what + // "dom.disable_open_click_delay" is set to (in ms). - // This is checking |interval|, not realInterval, on purpose, - // because our lower bound for |realInterval| could be pretty high - // in some cases. - if (interval <= StaticPrefs::dom_disable_open_click_delay()) { - timeout->mPopupState = PopupBlocker::GetPopupControlState(); - } + // This is checking |interval|, not realInterval, on purpose, + // because our lower bound for |realInterval| could be pretty high + // in some cases. + if (interval <= StaticPrefs::dom_disable_open_click_delay()) { + timeout->mPopupState = PopupBlocker::GetPopupControlState(); } } @@ -1127,18 +1110,14 @@ void TimeoutManager::Timeouts::Insert(Timeout* aTimeout, SortBy aSortBy) { Timeout* TimeoutManager::BeginRunningTimeout(Timeout* aTimeout) { Timeout* currentTimeout = mRunningTimeout; mRunningTimeout = aTimeout; - if (GetInnerWindow()) { - ++gRunningTimeoutDepth; - } + ++gRunningTimeoutDepth; RecordExecution(currentTimeout, aTimeout); return currentTimeout; } void TimeoutManager::EndRunningTimeout(Timeout* aTimeout) { - if (GetInnerWindow()) { - --gRunningTimeoutDepth; - } + --gRunningTimeoutDepth; RecordExecution(mRunningTimeout, aTimeout); mRunningTimeout = aTimeout; diff --git a/dom/base/TimeoutManager.h b/dom/base/TimeoutManager.h index 447fe4ab1e4a..105c084253e2 100644 --- a/dom/base/TimeoutManager.h +++ b/dom/base/TimeoutManager.h @@ -10,7 +10,6 @@ #include "mozilla/dom/Timeout.h" #include "nsTArray.h" #include "nsISerialEventTarget.h" -#include "mozilla/dom/TimeoutBudgetManager.h" class nsIEventTarget; class nsITimer; @@ -37,23 +36,16 @@ class TimeoutManager final { bool IsRunningTimeout() const; - uint32_t GetNestingLevelForWorker() { - MOZ_DIAGNOSTIC_ASSERT(!NS_IsMainThread()); - return mNestingLevel; - } - uint32_t GetNestingLevelForWindow() { - MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread()); - return sNestingLevel; + uint32_t GetNestingLevel() { + return mGlobalObject.GetAsInnerWindow() ? sNestingLevel : mNestingLevel; } - void SetNestingLevelForWorker(uint32_t aLevel) { - MOZ_DIAGNOSTIC_ASSERT(!NS_IsMainThread()); - mNestingLevel = aLevel; - } - - void SetNestingLevelForWindow(uint32_t aLevel) { - MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread()); - sNestingLevel = aLevel; + void SetNestingLevel(uint32_t aLevel) { + if (mGlobalObject.GetAsInnerWindow()) { + sNestingLevel = aLevel; + } else { + mNestingLevel = aLevel; + } } bool HasTimeouts() const { @@ -269,11 +261,7 @@ class TimeoutManager final { nsCOMPtr mEventTarget; uint32_t mNestingLevel{0}; - static uint32_t sNestingLevel; - - TimeoutBudgetManager mBudgetManager; - static TimeoutBudgetManager sBudgetManager; }; } // namespace dom diff --git a/dom/base/moz.build b/dom/base/moz.build index 957cf6d1858f..1d57a568a10b 100644 --- a/dom/base/moz.build +++ b/dom/base/moz.build @@ -282,7 +282,6 @@ EXPORTS.mozilla.dom += [ "TestUtils.h", "Text.h", "Timeout.h", - "TimeoutBudgetManager.h", "TimeoutHandler.h", "TimeoutManager.h", "TreeIterator.h", diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp index d949962634a2..9b04d26e06e0 100644 --- a/dom/base/nsGlobalWindowInner.cpp +++ b/dom/base/nsGlobalWindowInner.cpp @@ -6274,8 +6274,8 @@ bool nsGlobalWindowInner::RunTimeoutHandler(Timeout* aTimeout) { // timeouts from repeatedly opening poups. timeout->mPopupState = PopupBlocker::openAbused; - uint32_t nestingLevel = mTimeoutManager->GetNestingLevelForWindow(); - mTimeoutManager->SetNestingLevelForWindow(timeout->mNestingLevel); + uint32_t nestingLevel = mTimeoutManager->GetNestingLevel(); + mTimeoutManager->SetNestingLevel(timeout->mNestingLevel); const char* reason = GetTimeoutReasonString(timeout); @@ -6326,7 +6326,7 @@ bool nsGlobalWindowInner::RunTimeoutHandler(Timeout* aTimeout) { // point anyway, and the script context should have already reported // the script error in the usual way - so we just drop it. - mTimeoutManager->SetNestingLevelForWindow(nestingLevel); + mTimeoutManager->SetNestingLevel(nestingLevel); mTimeoutManager->EndRunningTimeout(last_running_timeout); timeout->mRunning = false; diff --git a/dom/base/nsIGlobalObject.h b/dom/base/nsIGlobalObject.h index 2fcbecc35fcd..d40db19528ce 100644 --- a/dom/base/nsIGlobalObject.h +++ b/dom/base/nsIGlobalObject.h @@ -250,8 +250,6 @@ class nsIGlobalObject : public nsISupports { // nullptr otherwise. nsPIDOMWindowInner* GetAsInnerWindow(); - virtual void TriggerUpdateCCFlag() {} - void QueueMicrotask(mozilla::dom::VoidFunction& aCallback); void RegisterReportingObserver(mozilla::dom::ReportingObserver* aObserver, diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 169a9c40affd..5d918a82708a 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -4558,16 +4558,6 @@ void WorkerPrivate::PropagateStorageAccessPermissionGrantedInternal() { void WorkerPrivate::TraverseTimeouts(nsCycleCollectionTraversalCallback& cb) { auto data = mWorkerThreadAccessible.Access(); - if (StaticPrefs::dom_workers_timeoutmanager_AtStartup()) { - auto* timeoutManager = - data->mScope ? data->mScope->GetTimeoutManager() : nullptr; - if (timeoutManager) { - timeoutManager->ForEachUnorderedTimeout([&cb](Timeout* timeout) { - cb.NoteNativeChild(timeout, NS_CYCLE_COLLECTION_PARTICIPANT(Timeout)); - }); - } - return; - } for (uint32_t i = 0; i < data->mTimeouts.Length(); ++i) { // TODO(erahm): No idea what's going on here. TimeoutInfo* tmp = data->mTimeouts[i].get(); @@ -4577,17 +4567,6 @@ void WorkerPrivate::TraverseTimeouts(nsCycleCollectionTraversalCallback& cb) { void WorkerPrivate::UnlinkTimeouts() { auto data = mWorkerThreadAccessible.Access(); - if (StaticPrefs::dom_workers_timeoutmanager_AtStartup()) { - auto* timeoutManager = - data->mScope ? data->mScope->GetTimeoutManager() : nullptr; - if (timeoutManager) { - timeoutManager->ClearAllTimeouts(); - if (!timeoutManager->HasTimeouts()) { - UpdateCCFlag(CCFlag::EligibleForTimeout); - } - } - return; - } data->mTimeouts.Clear(); } @@ -4803,22 +4782,10 @@ void WorkerPrivate::UpdateCCFlag(const CCFlag aFlag) { break; } case CCFlag::EligibleForTimeout: { - if (StaticPrefs::dom_workers_timeoutmanager_AtStartup()) { - auto* timeoutManager = - data->mScope ? data->mScope->GetTimeoutManager() : nullptr; - MOZ_ASSERT(timeoutManager && !timeoutManager->HasTimeouts()); - break; - } MOZ_ASSERT(data->mTimeouts.IsEmpty()); break; } case CCFlag::IneligibleForTimeout: { - if (StaticPrefs::dom_workers_timeoutmanager_AtStartup()) { - auto* timeoutManager = - data->mScope ? data->mScope->GetTimeoutManager() : nullptr; - MOZ_ASSERT(!timeoutManager || timeoutManager->HasTimeouts()); - break; - } MOZ_ASSERT(!data->mTimeouts.IsEmpty()); break; } @@ -4848,17 +4815,8 @@ void WorkerPrivate::UpdateCCFlag(const CCFlag aFlag) { return totalCount > nonblockingActorCount; }; - bool noTimeouts{data->mTimeouts.IsEmpty()}; - - if (StaticPrefs::dom_workers_timeoutmanager_AtStartup()) { - auto* timeoutManager = - data->mScope ? data->mScope->GetTimeoutManager() : nullptr; - if (timeoutManager) { - noTimeouts = !timeoutManager->HasTimeouts(); - } - } - - bool eligibleForCC = data->mChildWorkers.IsEmpty() && noTimeouts && + bool eligibleForCC = data->mChildWorkers.IsEmpty() && + data->mTimeouts.IsEmpty() && !data->mNumWorkerRefsPreventingShutdownStart; // Only checking BackgroundActors when no strong WorkerRef, ChildWorker, and @@ -4912,9 +4870,6 @@ void WorkerPrivate::CancelAllTimeouts() { data->mScope ? data->mScope->GetTimeoutManager() : nullptr; if (timeoutManager) { timeoutManager->ClearAllTimeouts(); - if (!timeoutManager->HasTimeouts()) { - UpdateCCFlag(CCFlag::EligibleForTimeout); - } } return; } @@ -5635,18 +5590,12 @@ int32_t WorkerPrivate::SetTimeout(JSContext* aCx, TimeoutHandler* aHandler, if (StaticPrefs::dom_workers_timeoutmanager_AtStartup()) { WorkerGlobalScope* globalScope = GlobalScope(); - MOZ_DIAGNOSTIC_ASSERT(globalScope); auto* timeoutManager = globalScope->GetTimeoutManager(); int32_t timerId = -1; - bool hadTimeouts = timeoutManager->HasTimeouts(); nsresult rv = timeoutManager->SetTimeout(aHandler, aTimeout, aIsInterval, aReason, &timerId); if (NS_FAILED(rv)) { aRv.Throw(NS_ERROR_FAILURE); - return timerId; - } - if (!hadTimeouts) { - UpdateCCFlag(CCFlag::IneligibleForTimeout); } return timerId; } @@ -5733,12 +5682,8 @@ void WorkerPrivate::ClearTimeout(int32_t aId, Timeout::Reason aReason) { "This timeout reason doesn't support cancellation."); if (StaticPrefs::dom_workers_timeoutmanager_AtStartup()) { WorkerGlobalScope* globalScope = GlobalScope(); - MOZ_DIAGNOSTIC_ASSERT(globalScope); auto* timeoutManager = globalScope->GetTimeoutManager(); timeoutManager->ClearTimeout(aId, aReason); - if (!timeoutManager->HasTimeouts()) { - UpdateCCFlag(CCFlag::EligibleForTimeout); - } return; } @@ -6070,7 +6015,7 @@ uint32_t WorkerPrivate::GetCurrentTimerNestingLevel() const { auto* timeoutManager = data->mScope ? data->mScope->GetTimeoutManager() : nullptr; if (timeoutManager) { - return timeoutManager->GetNestingLevelForWorker(); + return timeoutManager->GetNestingLevel(); } return data->mCurrentTimerNestingLevel; diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index 4b20e0ad8b96..499885030241 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -308,8 +308,8 @@ bool WorkerGlobalScopeBase::RunTimeoutHandler(mozilla::dom::Timeout* aTimeout) { Timeout* last_running_timeout = mTimeoutManager->BeginRunningTimeout(timeout); timeout->mRunning = true; - uint32_t nestingLevel = mTimeoutManager->GetNestingLevelForWorker(); - mTimeoutManager->SetNestingLevelForWorker(timeout->mNestingLevel); + uint32_t nestingLevel = mTimeoutManager->GetNestingLevel(); + mTimeoutManager->SetNestingLevel(timeout->mNestingLevel); const char* reason = workerinternals::GetTimeoutReasonString(timeout); @@ -343,7 +343,7 @@ bool WorkerGlobalScopeBase::RunTimeoutHandler(mozilla::dom::Timeout* aTimeout) { // point anyway, and the script context should have already reported // the script error in the usual way - so we just drop it. - mTimeoutManager->SetNestingLevelForWorker(nestingLevel); + mTimeoutManager->SetNestingLevel(nestingLevel); mTimeoutManager->EndRunningTimeout(last_running_timeout); timeout->mRunning = false; diff --git a/dom/workers/WorkerScope.h b/dom/workers/WorkerScope.h index 5014e14f2231..43d1de000e16 100644 --- a/dom/workers/WorkerScope.h +++ b/dom/workers/WorkerScope.h @@ -206,10 +206,6 @@ class WorkerGlobalScopeBase : public DOMEventTargetHelper, return mWorkerPrivate->IsFrozenForWorkerThread(); } - void TriggerUpdateCCFlag() override { - mWorkerPrivate->UpdateCCFlag(WorkerPrivate::CCFlag::EligibleForTimeout); - } - static WorkerGlobalScopeBase* Cast(nsIGlobalObject* obj) { return static_cast(obj); } diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index f13f32ed1f7a..ddf5ac5adc6a 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -4502,7 +4502,7 @@ - name: dom.workers.timeoutmanager type: bool - value: @IS_NIGHTLY_BUILD@ + value: false mirror: once - name: dom.workers.serialized-sab-access diff --git a/toolkit/modules/subprocess/test/xpcshell/test_subprocess.js b/toolkit/modules/subprocess/test/xpcshell/test_subprocess.js index 9b9ae10dfaa8..51c9956d0d91 100644 --- a/toolkit/modules/subprocess/test/xpcshell/test_subprocess.js +++ b/toolkit/modules/subprocess/test/xpcshell/test_subprocess.js @@ -5,13 +5,7 @@ const { setTimeout } = ChromeUtils.importESModule( "resource://gre/modules/Timer.sys.mjs" ); -let max_round_trip_time_ms = AppConstants.DEBUG || AppConstants.ASAN ? 18 : 9; -if (Services.prefs.getBoolPref("dom.workers.timeoutmanager") === true) { - max_round_trip_time_ms = 90; -} - -const MAX_ROUND_TRIP_TIME_MS = max_round_trip_time_ms; - +const MAX_ROUND_TRIP_TIME_MS = AppConstants.DEBUG || AppConstants.ASAN ? 18 : 9; const MAX_RETRIES = 5; let PYTHON;