From 73b14eae31b41f92d8ac5197269aeeb3192388b9 Mon Sep 17 00:00:00 2001 From: Martok Date: Fri, 5 Jan 2024 17:12:34 +0100 Subject: [PATCH 1/6] Issue #2435 - Make CycleCollectedJSContext.h dependency explicit --- dom/animation/Animation.cpp | 1 + dom/base/CustomElementRegistry.cpp | 1 - dom/base/CustomElementRegistry.h | 1 + dom/bindings/BindingUtils.h | 1 - dom/geolocation/nsGeolocation.cpp | 5 +++-- dom/indexedDB/ActorsChild.cpp | 1 + dom/indexedDB/ActorsParent.cpp | 1 + dom/plugins/base/nsNPAPIPlugin.cpp | 1 + dom/script/ScriptLoader.cpp | 1 + dom/workers/WorkerPrivate.cpp | 1 + 10 files changed, 10 insertions(+), 4 deletions(-) diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp index dc33c67493..1c62011f1c 100644 --- a/dom/animation/Animation.cpp +++ b/dom/animation/Animation.cpp @@ -11,6 +11,7 @@ #include "mozilla/AnimationTarget.h" #include "mozilla/AutoRestore.h" #include "mozilla/AsyncEventDispatcher.h" // For AsyncEventDispatcher +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/Maybe.h" // For Maybe #include "nsAnimationManager.h" // For CSSAnimation #include "nsDOMMutationObserver.h" // For nsAutoAnimationMutationBatch diff --git a/dom/base/CustomElementRegistry.cpp b/dom/base/CustomElementRegistry.cpp index 431d78856d..f7745a5a81 100644 --- a/dom/base/CustomElementRegistry.cpp +++ b/dom/base/CustomElementRegistry.cpp @@ -5,7 +5,6 @@ #include "mozilla/dom/CustomElementRegistry.h" -#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/dom/CustomElementRegistryBinding.h" #include "mozilla/dom/HTMLElementBinding.h" #include "mozilla/dom/WebComponentsBinding.h" diff --git a/dom/base/CustomElementRegistry.h b/dom/base/CustomElementRegistry.h index afbdc2812f..06e5947dbc 100644 --- a/dom/base/CustomElementRegistry.h +++ b/dom/base/CustomElementRegistry.h @@ -9,6 +9,7 @@ #include "js/GCHashTable.h" #include "js/TypeDecls.h" #include "mozilla/Attributes.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/ErrorResult.h" #include "mozilla/dom/BindingDeclarations.h" #include "mozilla/dom/FunctionBinding.h" diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h index 467b134864..4011e8c338 100644 --- a/dom/bindings/BindingUtils.h +++ b/dom/bindings/BindingUtils.h @@ -13,7 +13,6 @@ #include "mozilla/Alignment.h" #include "mozilla/Array.h" #include "mozilla/Assertions.h" -#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/DeferredFinalize.h" #include "mozilla/dom/BindingDeclarations.h" #include "mozilla/dom/CallbackObject.h" diff --git a/dom/geolocation/nsGeolocation.cpp b/dom/geolocation/nsGeolocation.cpp index 0b447dfa7a..826af388ba 100644 --- a/dom/geolocation/nsGeolocation.cpp +++ b/dom/geolocation/nsGeolocation.cpp @@ -18,10 +18,11 @@ #include "nsIObserverService.h" #include "nsPIDOMWindow.h" #include "nsThreadUtils.h" +#include "mozilla/ClearOnShutdown.h" +#include "mozilla/CycleCollectedJSContext.h" +#include "mozilla/Preferences.h" #include "mozilla/Services.h" #include "mozilla/Unused.h" -#include "mozilla/Preferences.h" -#include "mozilla/ClearOnShutdown.h" #include "mozilla/WeakPtr.h" #include "mozilla/dom/PermissionMessageUtils.h" diff --git a/dom/indexedDB/ActorsChild.cpp b/dom/indexedDB/ActorsChild.cpp index 31437ea059..eeaaf05266 100644 --- a/dom/indexedDB/ActorsChild.cpp +++ b/dom/indexedDB/ActorsChild.cpp @@ -18,6 +18,7 @@ #include "IndexedDatabase.h" #include "IndexedDatabaseInlines.h" #include "mozilla/BasicEvents.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/Maybe.h" #include "mozilla/TypeTraits.h" #include "mozilla/dom/Element.h" diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp index 0c6bd1ee1d..d80b900e76 100644 --- a/dom/indexedDB/ActorsParent.cpp +++ b/dom/indexedDB/ActorsParent.cpp @@ -23,6 +23,7 @@ #include "mozilla/AutoRestore.h" #include "mozilla/Casting.h" #include "mozilla/CheckedInt.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/EndianUtils.h" #include "mozilla/ErrorNames.h" #include "mozilla/LazyIdleThread.h" diff --git a/dom/plugins/base/nsNPAPIPlugin.cpp b/dom/plugins/base/nsNPAPIPlugin.cpp index 4b02440e20..5f138a6c71 100644 --- a/dom/plugins/base/nsNPAPIPlugin.cpp +++ b/dom/plugins/base/nsNPAPIPlugin.cpp @@ -22,6 +22,7 @@ #include "nsPluginStreamListenerPeer.h" #include "nsIServiceManager.h" #include "nsThreadUtils.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/Preferences.h" #include "nsPluginInstanceOwner.h" diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 06fcee7aca..c07c4b184f 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -52,6 +52,7 @@ #include "ImportManager.h" #include "mozilla/dom/EncodingUtils.h" #include "mozilla/ConsoleReportCollector.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/Attributes.h" #include "mozilla/Unused.h" diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 98f153ea89..89a30efea3 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -40,6 +40,7 @@ #include "mozilla/Assertions.h" #include "mozilla/Attributes.h" #include "mozilla/ContentEvents.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/EventDispatcher.h" #include "mozilla/Likely.h" #include "mozilla/LoadContext.h" From f059bb0a59b8cce62937ec867afc5898f41dcb58 Mon Sep 17 00:00:00 2001 From: Martok Date: Fri, 5 Jan 2024 17:19:42 +0100 Subject: [PATCH 2/6] Issue #2240 - Align Microtasks and promises scheduling with spec Microtasks, resolved Promises and Observers are handled after the sync task that caused them, in the order they were generated. Also simplifies reentrancy handling. Based-on: m-c 1193394 --- dom/animation/Animation.cpp | 38 +++++- dom/animation/Animation.h | 7 +- dom/animation/DocumentTimeline.cpp | 7 + dom/base/CustomElementRegistry.cpp | 2 +- dom/base/nsContentUtils.cpp | 17 ++- dom/base/nsContentUtils.h | 15 +- dom/base/nsDOMMutationObserver.cpp | 7 +- dom/base/nsGlobalWindow.cpp | 6 - dom/base/nsIGlobalObject.cpp | 2 +- dom/bindings/CallbackObject.cpp | 16 +-- dom/events/EventListenerManager.cpp | 14 +- dom/indexedDB/ActorsChild.cpp | 16 ++- dom/indexedDB/IDBFileHandle.cpp | 2 +- dom/indexedDB/IDBTransaction.cpp | 6 +- dom/promise/Promise.cpp | 104 -------------- dom/promise/Promise.h | 8 -- dom/script/ScriptSettings.cpp | 11 +- dom/script/ScriptSettings.h | 2 +- dom/workers/RuntimeService.cpp | 30 ++-- dom/workers/WorkerPrivate.cpp | 31 +++-- js/xpconnect/src/XPCJSContext.cpp | 15 -- xpcom/base/CycleCollectedJSContext.cpp | 182 ++++++++++++++++--------- xpcom/base/CycleCollectedJSContext.h | 67 +++------ 23 files changed, 259 insertions(+), 346 deletions(-) diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp index 1c62011f1c..17b01807a5 100644 --- a/dom/animation/Animation.cpp +++ b/dom/animation/Animation.cpp @@ -11,7 +11,6 @@ #include "mozilla/AnimationTarget.h" #include "mozilla/AutoRestore.h" #include "mozilla/AsyncEventDispatcher.h" // For AsyncEventDispatcher -#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/Maybe.h" // For Maybe #include "nsAnimationManager.h" // For CSSAnimation #include "nsDOMMutationObserver.h" // For nsAutoAnimationMutationBatch @@ -1384,6 +1383,30 @@ Animation::GetRenderedDocument() const return mEffect->AsKeyframeEffect()->GetRenderedDocument(); } +class AsyncFinishNotification : public MicroTaskRunnable +{ +public: + explicit AsyncFinishNotification(Animation* aAnimation) + : MicroTaskRunnable() + , mAnimation(aAnimation) + {} + + virtual void Run(AutoSlowOperation& aAso) override + { + mAnimation->DoFinishNotificationImmediately(this); + mAnimation = nullptr; + } + + virtual bool Suppressed() override + { + nsIGlobalObject* global = mAnimation->GetOwnerGlobal(); + return global && global->IsInSyncOperation(); + } + +private: + RefPtr mAnimation; +}; + void Animation::DoFinishNotification(SyncNotifyFlag aSyncNotifyFlag) { @@ -1391,9 +1414,8 @@ Animation::DoFinishNotification(SyncNotifyFlag aSyncNotifyFlag) if (aSyncNotifyFlag == SyncNotifyFlag::Sync) { DoFinishNotificationImmediately(); - } else if (!mFinishNotificationTask.IsPending()) { - RefPtr> runnable = - NewRunnableMethod(this, &Animation::DoFinishNotificationImmediately); + } else if (!mFinishNotificationTask) { + RefPtr runnable = new AsyncFinishNotification(this); context->DispatchToMicroTask(do_AddRef(runnable)); mFinishNotificationTask = runnable.forget(); } @@ -1416,9 +1438,13 @@ Animation::MaybeResolveFinishedPromise() } void -Animation::DoFinishNotificationImmediately() +Animation::DoFinishNotificationImmediately(MicroTaskRunnable* aAsync) { - mFinishNotificationTask.Revoke(); + if (aAsync && aAsync != mFinishNotificationTask) { + return; + } + + mFinishNotificationTask = nullptr; if (PlayState() != AnimationPlayState::Finished) { return; diff --git a/dom/animation/Animation.h b/dom/animation/Animation.h index a63579c704..d640dd6ff4 100644 --- a/dom/animation/Animation.h +++ b/dom/animation/Animation.h @@ -9,6 +9,7 @@ #include "nsWrapperCache.h" #include "nsCycleCollectionParticipant.h" #include "mozilla/Attributes.h" +#include "mozilla/CycleCollectedJSContext.h" #include "mozilla/DOMEventTargetHelper.h" #include "mozilla/EffectCompositor.h" // For EffectCompositor::CascadeLevel #include "mozilla/LinkedList.h" @@ -42,6 +43,7 @@ class AnimValuesStyleRule; namespace dom { +class AsyncFinishNotification; class CSSAnimation; class CSSTransition; @@ -378,7 +380,8 @@ protected: void ResetFinishedPromise(); void MaybeResolveFinishedPromise(); void DoFinishNotification(SyncNotifyFlag aSyncNotifyFlag); - void DoFinishNotificationImmediately(); + friend class AsyncFinishNotification; + void DoFinishNotificationImmediately(MicroTaskRunnable* aAsync = nullptr); void DispatchPlaybackEvent(const nsAString& aName); /** @@ -446,7 +449,7 @@ protected: // getAnimations() list. bool mIsRelevant; - nsRevocableEventPtr> mFinishNotificationTask; + RefPtr mFinishNotificationTask; // True if mFinished is resolved or would be resolved if mFinished has // yet to be created. This is not set when mFinished is rejected since // in that case mFinished is immediately reset to represent a new current diff --git a/dom/animation/DocumentTimeline.cpp b/dom/animation/DocumentTimeline.cpp index f6609b5262..2900739f92 100644 --- a/dom/animation/DocumentTimeline.cpp +++ b/dom/animation/DocumentTimeline.cpp @@ -155,6 +155,13 @@ DocumentTimeline::WillRefresh(mozilla::TimeStamp aTime) bool needsTicks = false; nsTArray animationsToRemove(mAnimations.Count()); + // https://drafts.csswg.org/web-animations-1/#update-animations-and-send-events, + // step2. + // Note that this should be done before nsAutoAnimationMutationBatch. If + // PerformMicroTaskCheckpoint was called before nsAutoAnimationMutationBatch + // is destroyed, some mutation records might not be delivered in this + // checkpoint. + nsAutoMicroTask mt; nsAutoAnimationMutationBatch mb(mDocument); for (Animation* animation = mAnimationOrder.getFirst(); animation; diff --git a/dom/base/CustomElementRegistry.cpp b/dom/base/CustomElementRegistry.cpp index f7745a5a81..11ba39a67e 100644 --- a/dom/base/CustomElementRegistry.cpp +++ b/dom/base/CustomElementRegistry.cpp @@ -1152,7 +1152,7 @@ CustomElementReactionsStack::Enqueue(Element* aElement, CycleCollectedJSContext* context = CycleCollectedJSContext::Get(); RefPtr bqmt = new BackupQueueMicroTask(this); - context->DispatchMicroTaskRunnable(bqmt.forget()); + context->DispatchToMicroTask(bqmt.forget()); } void diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 29cc1bce47..b246132de2 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -5418,10 +5418,10 @@ nsContentUtils::RunInStableState(already_AddRefed aRunnable) /* static */ void -nsContentUtils::RunInMetastableState(already_AddRefed aRunnable) +nsContentUtils::AddPendingIDBTransaction(already_AddRefed aTransaction) { MOZ_ASSERT(CycleCollectedJSContext::Get(), "Must be on a script thread!"); - CycleCollectedJSContext::Get()->RunInMetastableState(Move(aRunnable)); + CycleCollectedJSContext::Get()->AddPendingIDBTransaction(Move(aTransaction)); } /* @@ -6472,9 +6472,16 @@ nsContentUtils::IsSubDocumentTabbable(nsIContent* aContent) contentViewer->GetPreviousViewer(getter_AddRefs(zombieViewer)); // If there are 2 viewers for the current docshell, that - // means the current document is a zombie document. - // Only navigate into the subdocument if it's not a zombie. - return !zombieViewer; + // means the current document may be a zombie document. + // While load and pageshow events are dispatched, zombie viewer is the old, + // to be hidden document. + if (zombieViewer) { + bool inOnLoad = false; + docShell->GetIsExecutingOnLoadHandler(&inOnLoad); + return inOnLoad; + } + + return true; } bool diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 6e9f23054a..74239c8036 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -1766,17 +1766,12 @@ public: */ static void RunInStableState(already_AddRefed aRunnable); - /* Add a "synchronous section", in the form of an nsIRunnable run once the - * event loop has reached a "metastable state". |aRunnable| must not cause any - * queued events to be processed (i.e. must not spin the event loop). - * We've reached a metastable state when the currently executing task or - * microtask has finished. This is not specced at this time. - * In practice this runs aRunnable once the currently executing task or - * microtask finishes. If called multiple times per microtask, all the - * runnables will be executed, in the order in which RunInMetastableState() - * was called + /* Add a pending IDBTransaction to be cleaned up at the end of performing a + * microtask checkpoint. + * See the step of "Cleanup Indexed Database Transactions" in + * https://html.spec.whatwg.org/multipage/webappapis.html#perform-a-microtask-checkpoint */ - static void RunInMetastableState(already_AddRefed aRunnable); + static void AddPendingIDBTransaction(already_AddRefed aTransaction); /* Process viewport META data. This gives us information for the scale * and zoom of a page on mobile devices. We stick the information in diff --git a/dom/base/nsDOMMutationObserver.cpp b/dom/base/nsDOMMutationObserver.cpp index fdad6ee907..732d57c905 100644 --- a/dom/base/nsDOMMutationObserver.cpp +++ b/dom/base/nsDOMMutationObserver.cpp @@ -623,7 +623,7 @@ nsDOMMutationObserver::QueueMutationObserverMicroTask() RefPtr momt = new MutationObserverMicroTask(); - ccjs->DispatchMicroTaskRunnable(momt.forget()); + ccjs->DispatchToMicroTask(momt.forget()); } void @@ -644,9 +644,8 @@ nsDOMMutationObserver::RescheduleForRun() return; } - RefPtr momt = - new MutationObserverMicroTask(); - ccjs->DispatchMicroTaskRunnable(momt.forget()); + RefPtr momt = new MutationObserverMicroTask(); + ccjs->DispatchToMicroTask(momt.forget()); sScheduledMutationObservers = new AutoTArray, 4>; } diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index f264aa50d5..75d3c1861a 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -12999,12 +12999,6 @@ nsGlobalWindow::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. - // Since we might be processing more timeouts, go ahead and flush the promise - // queue now before we do that. We need to do that while we're still in our - // "running JS is safe" state (e.g. mRunningTimeout is set, timeout->mRunning - // is false). - Promise::PerformMicroTaskCheckpoint(); - if (trackNestingLevel) { sNestingLevel = nestingLevel; } diff --git a/dom/base/nsIGlobalObject.cpp b/dom/base/nsIGlobalObject.cpp index 7a6bab8b4b..a3d22aafa4 100644 --- a/dom/base/nsIGlobalObject.cpp +++ b/dom/base/nsIGlobalObject.cpp @@ -145,6 +145,6 @@ void nsIGlobalObject::QueueMicrotask(VoidFunction& aCallback) { CycleCollectedJSContext* context = CycleCollectedJSContext::Get(); if (context) { RefPtr mt = new QueuedMicrotask(this, aCallback); - context->DispatchMicroTaskRunnable(mt.forget()); + context->DispatchToMicroTask(mt.forget()); } } \ No newline at end of file diff --git a/dom/bindings/CallbackObject.cpp b/dom/bindings/CallbackObject.cpp index eb4b828ffe..3034f2b019 100644 --- a/dom/bindings/CallbackObject.cpp +++ b/dom/bindings/CallbackObject.cpp @@ -78,11 +78,9 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback, , mExceptionHandling(aExceptionHandling) , mIsMainThread(NS_IsMainThread()) { - if (mIsMainThread) { - CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); - if (ccjs) { - ccjs->EnterMicroTask(); - } + CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); + if (ccjs) { + ccjs->EnterMicroTask(); } // Compute the caller's subject principal (if necessary) early, before we @@ -290,11 +288,9 @@ CallbackObject::CallSetup::~CallSetup() // It is important that this is the last thing we do, after leaving the // compartment and undoing all our entry/incumbent script changes - if (mIsMainThread) { - CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); - if (ccjs) { - ccjs->LeaveMicroTask(); - } + CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); + if (ccjs) { + ccjs->LeaveMicroTask(); } } diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index 058e6916b5..7d68394850 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -1064,12 +1064,8 @@ EventListenerManager::HandleEventSubType(Listener* aListener, } if (NS_SUCCEEDED(result)) { - if (mIsMainThreadELM) { - CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); - if (ccjs) { - ccjs->EnterMicroTask(); - } - } + nsAutoMicroTask mt; + // nsIDOMEvent::currentTarget is set in EventDispatcher. if (listenerHolder.HasWebIDLCallback()) { ErrorResult rv; @@ -1079,12 +1075,6 @@ EventListenerManager::HandleEventSubType(Listener* aListener, } else { result = listenerHolder.GetXPCOMCallback()->HandleEvent(aDOMEvent); } - if (mIsMainThreadELM) { - CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); - if (ccjs) { - ccjs->LeaveMicroTask(); - } - } } return result; diff --git a/dom/indexedDB/ActorsChild.cpp b/dom/indexedDB/ActorsChild.cpp index eeaaf05266..6ec99e58f6 100644 --- a/dom/indexedDB/ActorsChild.cpp +++ b/dom/indexedDB/ActorsChild.cpp @@ -871,22 +871,26 @@ DispatchSuccessEvent(ResultHelper* aResultHelper, IDB_LOG_STRINGIFY(aEvent, kSuccessEventType)); } + MOZ_ASSERT_IF(transaction, + transaction->IsOpen() && !transaction->IsAborted()); + bool dummy; nsresult rv = request->DispatchEvent(aEvent, &dummy); if (NS_WARN_IF(NS_FAILED(rv))) { return; } - MOZ_ASSERT_IF(transaction, - transaction->IsOpen() || transaction->IsAborted()); - WidgetEvent* internalEvent = aEvent->WidgetEventPtr(); MOZ_ASSERT(internalEvent); if (transaction && - transaction->IsOpen() && - internalEvent->mFlags.mExceptionWasRaised) { - transaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR); + transaction->IsOpen()) { + if (internalEvent->mFlags.mExceptionWasRaised) { + transaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR); + } else { + // To handle upgrade transaction. + transaction->Run(); + } } } diff --git a/dom/indexedDB/IDBFileHandle.cpp b/dom/indexedDB/IDBFileHandle.cpp index 823e643998..3ee284386c 100644 --- a/dom/indexedDB/IDBFileHandle.cpp +++ b/dom/indexedDB/IDBFileHandle.cpp @@ -55,7 +55,7 @@ IDBFileHandle::Create(IDBMutableFile* aMutableFile, MOZ_ASSERT(NS_IsMainThread(), "This won't work on non-main threads!"); nsCOMPtr runnable = do_QueryObject(fileHandle); - nsContentUtils::RunInMetastableState(runnable.forget()); + nsContentUtils::AddPendingIDBTransaction(runnable.forget()); fileHandle->SetCreating(); diff --git a/dom/indexedDB/IDBTransaction.cpp b/dom/indexedDB/IDBTransaction.cpp index 4ee6239a25..8034588d31 100644 --- a/dom/indexedDB/IDBTransaction.cpp +++ b/dom/indexedDB/IDBTransaction.cpp @@ -190,13 +190,9 @@ IDBTransaction::CreateVersionChange( transaction->SetScriptOwner(aDatabase->GetScriptOwner()); - nsCOMPtr runnable = do_QueryObject(transaction); - nsContentUtils::RunInMetastableState(runnable.forget()); - transaction->mBackgroundActor.mVersionChangeBackgroundActor = aActor; transaction->mNextObjectStoreId = aNextObjectStoreId; transaction->mNextIndexId = aNextIndexId; - transaction->mCreating = true; aDatabase->RegisterTransaction(transaction); transaction->mRegistered = true; @@ -226,7 +222,7 @@ IDBTransaction::Create(JSContext* aCx, IDBDatabase* aDatabase, transaction->SetScriptOwner(aDatabase->GetScriptOwner()); nsCOMPtr runnable = do_QueryObject(transaction); - nsContentUtils::RunInMetastableState(runnable.forget()); + nsContentUtils::AddPendingIDBTransaction(runnable.forget()); transaction->mCreating = true; diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index 0e1349d087..fbaa8e1eaa 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -508,110 +508,6 @@ Promise::ReportRejectedPromise(JSContext* aCx, JS::HandleObject aPromise) NS_DispatchToMainThread(new AsyncErrorReporter(xpcReport)); } -bool -Promise::PerformMicroTaskCheckpoint() -{ - MOZ_ASSERT(NS_IsMainThread(), "Wrong thread!"); - - CycleCollectedJSContext* context = CycleCollectedJSContext::Get(); - - // On the main thread, we always use the main promise micro task queue. - std::queue>& microtaskQueue = - context->GetPromiseMicroTaskQueue(); - - if (microtaskQueue.empty()) { - return false; - } - - AutoSlowOperation aso; - - do { - nsCOMPtr runnable = microtaskQueue.front().forget(); - MOZ_ASSERT(runnable); - - // This function can re-enter, so we remove the element before calling. - microtaskQueue.pop(); - nsresult rv = runnable->Run(); - if (NS_WARN_IF(NS_FAILED(rv))) { - return false; - } - aso.CheckForInterrupt(); - context->AfterProcessMicrotask(); - } while (!microtaskQueue.empty()); - - return true; -} - -void -Promise::PerformWorkerMicroTaskCheckpoint() -{ - MOZ_ASSERT(!NS_IsMainThread(), "Wrong thread!"); - - CycleCollectedJSContext* context = CycleCollectedJSContext::Get(); - if (!context) { - return; - } - - for (;;) { - // For a normal microtask checkpoint, we try to use the debugger microtask - // queue first. If the debugger queue is empty, we use the normal microtask - // queue instead. - std::queue>* microtaskQueue = - &context->GetDebuggerPromiseMicroTaskQueue(); - - if (microtaskQueue->empty()) { - microtaskQueue = &context->GetPromiseMicroTaskQueue(); - if (microtaskQueue->empty()) { - break; - } - } - - nsCOMPtr runnable = microtaskQueue->front().forget(); - MOZ_ASSERT(runnable); - - // This function can re-enter, so we remove the element before calling. - microtaskQueue->pop(); - nsresult rv = runnable->Run(); - if (NS_WARN_IF(NS_FAILED(rv))) { - return; - } - context->AfterProcessMicrotask(); - } -} - -void -Promise::PerformWorkerDebuggerMicroTaskCheckpoint() -{ - MOZ_ASSERT(!NS_IsMainThread(), "Wrong thread!"); - - CycleCollectedJSContext* context = CycleCollectedJSContext::Get(); - if (!context) { - return; - } - - for (;;) { - // For a debugger microtask checkpoint, we always use the debugger microtask - // queue. - std::queue>* microtaskQueue = - &context->GetDebuggerPromiseMicroTaskQueue(); - - if (microtaskQueue->empty()) { - break; - } - - nsCOMPtr runnable = microtaskQueue->front().forget(); - MOZ_ASSERT(runnable); - - // This function can re-enter, so we remove the element before calling. - microtaskQueue->pop(); - nsresult rv = runnable->Run(); - if (NS_WARN_IF(NS_FAILED(rv))) { - return; - } - context->AfterProcessMicrotask(); - } -} - JSObject* Promise::GlobalJSObject() const { diff --git a/dom/promise/Promise.h b/dom/promise/Promise.h index 31f7bcae9e..dfbb82d489 100644 --- a/dom/promise/Promise.h +++ b/dom/promise/Promise.h @@ -104,14 +104,6 @@ public: // specializations in the .cpp for // the T values we support. - // Called by DOM to let us execute our callbacks. May be called recursively. - // Returns true if at least one microtask was processed. - static bool PerformMicroTaskCheckpoint(); - - static void PerformWorkerMicroTaskCheckpoint(); - - static void PerformWorkerDebuggerMicroTaskCheckpoint(); - // WebIDL nsIGlobalObject* GetParentObject() const diff --git a/dom/script/ScriptSettings.cpp b/dom/script/ScriptSettings.cpp index 790394de65..d315243531 100644 --- a/dom/script/ScriptSettings.cpp +++ b/dom/script/ScriptSettings.cpp @@ -828,8 +828,6 @@ AutoSafeJSContext::AutoSafeJSContext(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMP AutoSlowOperation::AutoSlowOperation(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL) : AutoJSAPI() { - MOZ_ASSERT(NS_IsMainThread()); - MOZ_GUARD_OBJECT_NOTIFIER_INIT; Init(); @@ -838,9 +836,12 @@ AutoSlowOperation::AutoSlowOperation(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMP void AutoSlowOperation::CheckForInterrupt() { - // JS_CheckForInterrupt expects us to be in a compartment. - JSAutoCompartment ac(cx(), xpc::UnprivilegedJunkScope()); - JS_CheckForInterrupt(cx()); + // For now we support only main thread! + if (mIsMainThread) { + // JS_CheckForInterrupt expects us to be in a compartment. + JSAutoCompartment ac(cx(), xpc::UnprivilegedJunkScope()); + JS_CheckForInterrupt(cx()); + } } } // namespace mozilla diff --git a/dom/script/ScriptSettings.h b/dom/script/ScriptSettings.h index f2e12f0be9..11181dda36 100644 --- a/dom/script/ScriptSettings.h +++ b/dom/script/ScriptSettings.h @@ -298,7 +298,6 @@ protected: // AutoJSAPI, so Init must NOT be called on subclasses that use this. AutoJSAPI(nsIGlobalObject* aGlobalObject, bool aIsMainThread, Type aType); -private: mozilla::Maybe mAutoRequest; mozilla::Maybe mAutoNullableCompartment; JSContext *mCx; @@ -307,6 +306,7 @@ private: bool mIsMainThread; Maybe mOldWarningReporter; +private: void InitInternal(nsIGlobalObject* aGlobalObject, JSObject* aGlobal, JSContext* aCx, bool aIsMainThread); diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp index 6138a3c6de..199f6ea56a 100644 --- a/dom/workers/RuntimeService.cpp +++ b/dom/workers/RuntimeService.cpp @@ -1007,6 +1007,10 @@ public: : mWorkerPrivate(aWorkerPrivate) { MOZ_ASSERT(aWorkerPrivate); + // Magical number 2. Workers have the base recursion depth 1, and normal + // runnables run at level 2, and we don't want to process microtasks + // at any other level. + SetTargetedMicroTaskRecursionDepth(2); } ~WorkerJSContext() @@ -1092,26 +1096,14 @@ public: } } - virtual void AfterProcessTask(uint32_t aRecursionDepth) override + virtual void DispatchToMicroTask(already_AddRefed aRunnable) override { - // Only perform the Promise microtask checkpoint on the outermost event - // loop. Don't run it, for example, during sync XHR or importScripts. - if (aRecursionDepth == 2) { - CycleCollectedJSContext::AfterProcessTask(aRecursionDepth); - } else if (aRecursionDepth > 2) { - AutoDisableMicroTaskCheckpoint disableMicroTaskCheckpoint; - CycleCollectedJSContext::AfterProcessTask(aRecursionDepth); - } - } - - virtual void DispatchToMicroTask(already_AddRefed aRunnable) override - { - RefPtr runnable(aRunnable); + RefPtr runnable(aRunnable); MOZ_ASSERT(!NS_IsMainThread()); MOZ_ASSERT(runnable); - std::queue>* microTaskQueue = nullptr; + std::queue>* microTaskQueue = nullptr; JSContext* cx = GetCurrentThreadJSContext(); NS_ASSERTION(cx, "This should never be null!"); @@ -1120,15 +1112,15 @@ public: NS_ASSERTION(global, "This should never be null!"); // On worker threads, if the current global is the worker global, we use the - // main promise micro task queue. Otherwise, the current global must be + // main micro task queue. Otherwise, the current global must be // either the debugger global or a debugger sandbox, and we use the debugger - // promise micro task queue instead. + // micro task queue instead. if (IsWorkerGlobal(global)) { - microTaskQueue = &mPromiseMicroTaskQueue; + microTaskQueue = &GetMicroTaskQueue(); } else { MOZ_ASSERT(IsDebuggerGlobal(global) || IsDebuggerSandbox(global)); - microTaskQueue = &mDebuggerPromiseMicroTaskQueue; + microTaskQueue = &GetDebuggerMicroTaskQueue(); } microTaskQueue->push(runnable.forget()); diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 89a30efea3..5e211c1085 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -4816,8 +4816,10 @@ WorkerPrivate::DoRunLoop(JSContext* aCx) static_cast(runnable)->Run(); runnable->Release(); - // Flush the promise queue. - Promise::PerformWorkerDebuggerMicroTaskCheckpoint(); + CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); + if (ccjs) { + ccjs->PerformDebuggerMicroTaskCheckpoint(); + } if (debuggerRunnablesPending) { WorkerDebuggerGlobalScope* globalScope = DebuggerGlobalScope(); @@ -5803,8 +5805,12 @@ WorkerPrivate::EnterDebuggerEventLoop() { MutexAutoLock lock(mMutex); + CycleCollectedJSContext* context = CycleCollectedJSContext::Get(); + std::queue>& debuggerMtQueue = + context->GetDebuggerMicroTaskQueue(); while (mControlQueue.IsEmpty() && - !(debuggerRunnablesPending = !mDebuggerQueue.IsEmpty())) { + !(debuggerRunnablesPending = !mDebuggerQueue.IsEmpty()) && + debuggerMtQueue.empty()) { WaitForWorkerEvents(); } @@ -5813,6 +5819,11 @@ WorkerPrivate::EnterDebuggerEventLoop() // XXXkhuey should we abort JS on the stack here if we got Abort above? } + CycleCollectedJSContext* context = CycleCollectedJSContext::Get(); + if (context) { + context->PerformDebuggerMicroTaskCheckpoint(); + } + if (debuggerRunnablesPending) { // Start the periodic GC timer if it is not already running. SetGCTimerMode(PeriodicTimer); @@ -5829,8 +5840,10 @@ WorkerPrivate::EnterDebuggerEventLoop() static_cast(runnable)->Run(); runnable->Release(); - // Flush the promise queue. - Promise::PerformWorkerDebuggerMicroTaskCheckpoint(); + CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get(); + if (ccjs) { + ccjs->PerformDebuggerMicroTaskCheckpoint(); + } // Now *might* be a good time to GC. Let the JS engine make the decision. if (JS::CurrentGlobalOrNull(cx)) { @@ -6257,8 +6270,8 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx) RefPtr callback = info->mHandler->GetCallback(); if (!callback) { - // scope for the AutoEntryScript, so it comes off the stack before we do - // Promise::PerformMicroTaskCheckpoint. + nsAutoMicroTask mt; + AutoEntryScript aes(global, reason, false); // Evaluate the timeout expression. @@ -6293,10 +6306,6 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx) rv.SuppressException(); } - // Since we might be processing more timeouts, go ahead and flush - // the promise queue now before we do that. - Promise::PerformWorkerMicroTaskCheckpoint(); - NS_ASSERTION(mRunningExpiredTimeouts, "Someone changed this!"); } diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index be9a75f799..d45f9f49eb 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -3485,21 +3485,6 @@ XPCJSContext::BeforeProcessTask(bool aMightBlock) { MOZ_ASSERT(NS_IsMainThread()); - // If ProcessNextEvent was called during a Promise "then" callback, we - // must process any pending microtasks before blocking in the event loop, - // otherwise we may deadlock until an event enters the queue later. - if (aMightBlock) { - if (Promise::PerformMicroTaskCheckpoint()) { - // If any microtask was processed, we post a dummy event in order to - // force the ProcessNextEvent call not to block. This is required - // to support nested event loops implemented using a pattern like - // "while (condition) thread.processNextEvent(true)", in case the - // condition is triggered here by a Promise "then" callback. - - NS_DispatchToMainThread(new Runnable()); - } - } - // Start the slow script timer. mSlowScriptCheckpoint = mozilla::TimeStamp::NowLoRes(); mSlowScriptSecondHalf = false; diff --git a/xpcom/base/CycleCollectedJSContext.cpp b/xpcom/base/CycleCollectedJSContext.cpp index a28b1ed3ba..bbf8bca623 100644 --- a/xpcom/base/CycleCollectedJSContext.cpp +++ b/xpcom/base/CycleCollectedJSContext.cpp @@ -437,7 +437,7 @@ CycleCollectedJSContext::CycleCollectedJSContext() , mPrevGCNurseryCollectionCallback(nullptr) , mJSHolders(256) , mDoingStableStates(false) - , mDisableMicroTaskCheckpoint(false) + , mTargetedMicroTaskRecursionDepth(0) , mMicroTaskLevel(0) , mMicroTaskRecursionDepth(0) , mOutOfMemoryState(OOMState::OK) @@ -458,8 +458,8 @@ CycleCollectedJSContext::~CycleCollectedJSContext() MOZ_ASSERT(!mDeferredFinalizerTable.Count()); // Last chance to process any events. - ProcessMetastableStateQueue(mBaseRecursionDepth); - MOZ_ASSERT(mMetastableStateEvents.IsEmpty()); + CleanupIDBTransactions(mBaseRecursionDepth); + MOZ_ASSERT(mPendingIDBTransactions.IsEmpty()); ProcessStableStateQueue(); MOZ_ASSERT(mStableStateEvents.IsEmpty()); @@ -467,8 +467,8 @@ CycleCollectedJSContext::~CycleCollectedJSContext() // Clear mPendingException first, since it might be cycle collected. mPendingException = nullptr; - MOZ_ASSERT(mDebuggerPromiseMicroTaskQueue.empty()); - MOZ_ASSERT(mPromiseMicroTaskQueue.empty()); + MOZ_ASSERT(mDebuggerMicroTaskQueue.empty()); + MOZ_ASSERT(mPendingMicroTaskRunnables.empty()); mUncaughtRejections.reset(); mConsumedRejections.reset(); @@ -921,7 +921,7 @@ CycleCollectedJSContext::LargeAllocationFailureCallback(void* aData) self->OnLargeAllocationFailure(); } -class PromiseJobRunnable final : public Runnable +class PromiseJobRunnable final : public MicroTaskRunnable { public: PromiseJobRunnable(JS::HandleObject aCallback, JS::HandleObject aAllocationSite, @@ -935,14 +935,20 @@ public: } protected: - NS_IMETHOD - Run() override + virtual void Run(AutoSlowOperation& aAso) override { nsIGlobalObject* global = xpc::NativeGlobal(mCallback->CallbackPreserveColor()); if (global && !global->IsDying()) { mCallback->Call("promise callback"); + aAso.CheckForInterrupt(); } - return NS_OK; + } + + virtual bool Suppressed() override + { + nsIGlobalObject* global = + xpc::NativeGlobal(mCallback->CallbackPreserveColor()); + return global && global->IsInSyncOperation(); } private: @@ -976,7 +982,7 @@ CycleCollectedJSContext::EnqueuePromiseJobCallback(JSContext* aCx, if (aIncumbentGlobal) { global = xpc::NativeGlobal(aIncumbentGlobal); } - nsCOMPtr runnable = new PromiseJobRunnable(aJob, aAllocationSite, global); + RefPtr runnable = new PromiseJobRunnable(aJob, aAllocationSite, global); self->DispatchToMicroTask(runnable.forget()); return true; } @@ -1175,18 +1181,18 @@ CycleCollectedJSContext::SetPendingException(nsIException* aException) mPendingException = aException; } -std::queue>& -CycleCollectedJSContext::GetPromiseMicroTaskQueue() +std::queue>& +CycleCollectedJSContext::GetMicroTaskQueue() { MOZ_ASSERT(mJSContext); - return mPromiseMicroTaskQueue; + return mPendingMicroTaskRunnables; } -std::queue>& -CycleCollectedJSContext::GetDebuggerPromiseMicroTaskQueue() +std::queue>& +CycleCollectedJSContext::GetDebuggerMicroTaskQueue() { MOZ_ASSERT(mJSContext); - return mDebuggerPromiseMicroTaskQueue; + return mDebuggerMicroTaskQueue; } nsCycleCollectionParticipant* @@ -1345,24 +1351,24 @@ CycleCollectedJSContext::ProcessStableStateQueue() } void -CycleCollectedJSContext::ProcessMetastableStateQueue(uint32_t aRecursionDepth) +CycleCollectedJSContext::CleanupIDBTransactions(uint32_t aRecursionDepth) { MOZ_ASSERT(mJSContext); MOZ_RELEASE_ASSERT(!mDoingStableStates); mDoingStableStates = true; - nsTArray localQueue = Move(mMetastableStateEvents); + nsTArray localQueue = Move(mPendingIDBTransactions); for (uint32_t i = 0; i < localQueue.Length(); ++i) { - RunInMetastableStateData& data = localQueue[i]; + PendingIDBTransactionData& data = localQueue[i]; if (data.mRecursionDepth != aRecursionDepth) { continue; } { - nsCOMPtr runnable = data.mRunnable.forget(); - runnable->Run(); + nsCOMPtr transaction = data.mTransaction.forget(); + transaction->Run(); } localQueue.RemoveElementAt(i--); @@ -1370,11 +1376,27 @@ CycleCollectedJSContext::ProcessMetastableStateQueue(uint32_t aRecursionDepth) // If the queue has events in it now, they were added from something we called, // so they belong at the end of the queue. - localQueue.AppendElements(mMetastableStateEvents); - localQueue.SwapElements(mMetastableStateEvents); + localQueue.AppendElements(mPendingIDBTransactions); + localQueue.SwapElements(mPendingIDBTransactions); mDoingStableStates = false; } +void +CycleCollectedJSContext::BeforeProcessTask(bool aMightBlock) +{ + // If ProcessNextEvent was called during a microtask callback, we + // must process any pending microtasks before blocking in the event loop, + // otherwise we may deadlock until an event enters the queue later. + if (aMightBlock && PerformMicroTaskCheckPoint()) { + // If any microtask was processed, we post a dummy event in order to + // force the ProcessNextEvent call not to block. This is required + // to support nested event loops implemented using a pattern like + // "while (condition) thread.processNextEvent(true)", in case the + // condition is triggered here by a Promise "then" callback. + NS_DispatchToMainThread(new Runnable()); + } +} + void CycleCollectedJSContext::AfterProcessTask(uint32_t aRecursionDepth) { @@ -1382,39 +1404,20 @@ CycleCollectedJSContext::AfterProcessTask(uint32_t aRecursionDepth) // See HTML 6.1.4.2 Processing model - // Execute any events that were waiting for a microtask to complete. - // This is not (yet) in the spec. - ProcessMetastableStateQueue(aRecursionDepth); - // Step 4.1: Execute microtasks. - if (!mDisableMicroTaskCheckpoint) { - PerformMicroTaskCheckPoint(); - if (NS_IsMainThread()) { - Promise::PerformMicroTaskCheckpoint(); - } else { - Promise::PerformWorkerMicroTaskCheckpoint(); - } - } + PerformMicroTaskCheckPoint(); // Step 4.2 Execute any events that were waiting for a stable state. ProcessStableStateQueue(); } void -CycleCollectedJSContext::AfterProcessMicrotask() +CycleCollectedJSContext::AfterProcessMicrotasks() { MOZ_ASSERT(mJSContext); - AfterProcessMicrotask(RecursionDepth()); -} - -void -CycleCollectedJSContext::AfterProcessMicrotask(uint32_t aRecursionDepth) -{ - MOZ_ASSERT(mJSContext); - - // Between microtasks, execute any events that were waiting for a microtask - // to complete. - ProcessMetastableStateQueue(aRecursionDepth); + // Cleanup Indexed Database transactions: + // https://html.spec.whatwg.org/multipage/webappapis.html#perform-a-microtask-checkpoint + CleanupIDBTransactions(RecursionDepth()); } uint32_t @@ -1431,12 +1434,12 @@ CycleCollectedJSContext::RunInStableState(already_AddRefed&& aRunna } void -CycleCollectedJSContext::RunInMetastableState(already_AddRefed&& aRunnable) +CycleCollectedJSContext::AddPendingIDBTransaction(already_AddRefed&& aTransaction) { MOZ_ASSERT(mJSContext); - RunInMetastableStateData data; - data.mRunnable = aRunnable; + PendingIDBTransactionData data; + data.mTransaction = aTransaction; MOZ_ASSERT(mOwningThread); data.mRecursionDepth = RecursionDepth(); @@ -1453,7 +1456,7 @@ CycleCollectedJSContext::RunInMetastableState(already_AddRefed&& aR } #endif - mMetastableStateEvents.AppendElement(Move(data)); + mPendingIDBTransactions.AppendElement(Move(data)); } IncrementalFinalizeRunnable::IncrementalFinalizeRunnable(CycleCollectedJSContext* aCx, @@ -1659,14 +1662,14 @@ CycleCollectedJSContext::PrepareWaitingZonesForGC() } void -CycleCollectedJSContext::DispatchToMicroTask(already_AddRefed aRunnable) +CycleCollectedJSContext::DispatchToMicroTask(already_AddRefed aRunnable) { - RefPtr runnable(aRunnable); + RefPtr runnable(aRunnable); MOZ_ASSERT(NS_IsMainThread()); MOZ_ASSERT(runnable); - mPromiseMicroTaskQueue.push(runnable.forget()); + mPendingMicroTaskRunnables.push(runnable.forget()); } class AsyncMutationHandler final : public mozilla::Runnable @@ -1682,41 +1685,61 @@ public: } }; -void +bool CycleCollectedJSContext::PerformMicroTaskCheckPoint() { - if (mPendingMicroTaskRunnables.empty()) { + if (mPendingMicroTaskRunnables.empty() && mDebuggerMicroTaskQueue.empty()) { + AfterProcessMicrotasks(); // Nothing to do, return early. - return; + return false; } uint32_t currentDepth = RecursionDepth(); if (mMicroTaskRecursionDepth >= currentDepth) { // We are already executing microtasks for the current recursion depth. - return; + return false; + } + + if (mTargetedMicroTaskRecursionDepth != 0 && + mTargetedMicroTaskRecursionDepth != currentDepth) { + return false; } if (NS_IsMainThread() && !nsContentUtils::IsSafeToRunScript()) { // Special case for main thread where DOM mutations may happen when // it is not safe to run scripts. nsContentUtils::AddScriptRunner(new AsyncMutationHandler()); - return; + return false; } mozilla::AutoRestore restore(mMicroTaskRecursionDepth); MOZ_ASSERT(currentDepth > 0); mMicroTaskRecursionDepth = currentDepth; + bool didProcess = false; AutoSlowOperation aso; std::queue> suppressed; - while (!mPendingMicroTaskRunnables.empty()) { - RefPtr runnable = - mPendingMicroTaskRunnables.front().forget(); - mPendingMicroTaskRunnables.pop(); + for (;;) { + RefPtr runnable; + if (!mDebuggerMicroTaskQueue.empty()) { + runnable = mDebuggerMicroTaskQueue.front().forget(); + mDebuggerMicroTaskQueue.pop(); + } else if (!mPendingMicroTaskRunnables.empty()) { + runnable = mPendingMicroTaskRunnables.front().forget(); + mPendingMicroTaskRunnables.pop(); + } else { + break; + } + if (runnable->Suppressed()) { + // Microtasks in worker shall never be suppressed. + // Otherwise, mPendingMicroTaskRunnables will be replaced later with + // all suppressed tasks in mDebuggerMicroTaskQueue unexpectedly. + MOZ_ASSERT(NS_IsMainThread()); suppressed.push(runnable); } else { + didProcess = true; runnable->Run(aso); } } @@ -1726,13 +1749,38 @@ CycleCollectedJSContext::PerformMicroTaskCheckPoint() // for some time, but no longer than spinning the event loop nestedly // (sync XHR, alert, etc.) mPendingMicroTaskRunnables.swap(suppressed); + + AfterProcessMicrotasks(); + + return didProcess; } void -CycleCollectedJSContext::DispatchMicroTaskRunnable( - already_AddRefed aRunnable) -{ - mPendingMicroTaskRunnables.push(aRunnable); +CycleCollectedJSContext::PerformDebuggerMicroTaskCheckpoint() + { + // Don't do normal microtask handling checks here, since whoever is calling + // this method is supposed to know what they are doing. + + AutoSlowOperation aso; + for (;;) { + // For a debugger microtask checkpoint, we always use the debugger microtask + // queue. + std::queue>* microtaskQueue = + &GetDebuggerMicroTaskQueue(); + + if (microtaskQueue->empty()) { + break; + } + + RefPtr runnable = microtaskQueue->front().forget(); + MOZ_ASSERT(runnable); + + // This function can re-enter, so we remove the element before calling. + microtaskQueue->pop(); + runnable->Run(aso); + } + + AfterProcessMicrotasks(); } void diff --git a/xpcom/base/CycleCollectedJSContext.h b/xpcom/base/CycleCollectedJSContext.h index b9fc8e6045..d106d54ae9 100644 --- a/xpcom/base/CycleCollectedJSContext.h +++ b/xpcom/base/CycleCollectedJSContext.h @@ -170,9 +170,6 @@ protected: virtual void CustomOutOfMemoryCallback() {} virtual void CustomLargeAllocationFailureCallback() {} - std::queue> mPromiseMicroTaskQueue; - std::queue> mDebuggerPromiseMicroTaskQueue; - private: void DescribeGCThing(bool aIsMarked, JS::GCCellPtr aThing, @@ -243,11 +240,11 @@ private: virtual void TraceNativeBlackRoots(JSTracer* aTracer) { }; void TraceNativeGrayRoots(JSTracer* aTracer); - void AfterProcessMicrotask(uint32_t aRecursionDepth); + void AfterProcessMicrotasks(); public: void ProcessStableStateQueue(); private: - void ProcessMetastableStateQueue(uint32_t aRecursionDepth); + void CleanupIDBTransactions(uint32_t aRecursionDepth); public: enum DeferredFinalizeType { @@ -306,8 +303,8 @@ public: already_AddRefed GetPendingException() const; void SetPendingException(nsIException* aException); - std::queue>& GetPromiseMicroTaskQueue(); - std::queue>& GetDebuggerPromiseMicroTaskQueue(); + std::queue>& GetMicroTaskQueue(); + std::queue>& GetDebuggerMicroTaskQueue(); nsCycleCollectionParticipant* GCThingParticipant(); nsCycleCollectionParticipant* ZoneParticipant(); @@ -346,53 +343,25 @@ public: return JS::RootingContext::get(mJSContext); } - bool MicroTaskCheckpointDisabled() const + void SetTargetedMicroTaskRecursionDepth(uint32_t aDepth) { - return mDisableMicroTaskCheckpoint; + mTargetedMicroTaskRecursionDepth = aDepth; } - void DisableMicroTaskCheckpoint(bool aDisable) - { - mDisableMicroTaskCheckpoint = aDisable; - } - - class MOZ_RAII AutoDisableMicroTaskCheckpoint - { - public: - AutoDisableMicroTaskCheckpoint() - : mCCJSCX(CycleCollectedJSContext::Get()) - { - mOldValue = mCCJSCX->MicroTaskCheckpointDisabled(); - mCCJSCX->DisableMicroTaskCheckpoint(true); - } - - ~AutoDisableMicroTaskCheckpoint() - { - mCCJSCX->DisableMicroTaskCheckpoint(mOldValue); - } - - CycleCollectedJSContext* mCCJSCX; - bool mOldValue; - }; - protected: JSContext* MaybeContext() const { return mJSContext; } public: // nsThread entrypoints - virtual void BeforeProcessTask(bool aMightBlock) { }; + virtual void BeforeProcessTask(bool aMightBlock); virtual void AfterProcessTask(uint32_t aRecursionDepth); - // microtask processor entry point - void AfterProcessMicrotask(); - uint32_t RecursionDepth(); // Run in stable state (call through nsContentUtils) void RunInStableState(already_AddRefed&& aRunnable); - // This isn't in the spec at all yet, but this gets the behavior we want for IDB. - // Runs after the current microtask completes. - void RunInMetastableState(already_AddRefed&& aRunnable); + + void AddPendingIDBTransaction(already_AddRefed&& aTransaction); // Get the current thread's CycleCollectedJSContext. Returns null if there // isn't one. @@ -411,7 +380,7 @@ public: void PrepareWaitingZonesForGC(); // Queue an async microtask to the current main or worker thread. - virtual void DispatchToMicroTask(already_AddRefed aRunnable); + virtual void DispatchToMicroTask(already_AddRefed aRunnable); // Call EnterMicroTask when you're entering JS execution. // Usually the best way to do this is to use nsAutoMicroTask. @@ -442,9 +411,9 @@ public: mMicroTaskLevel = aLevel; } - void PerformMicroTaskCheckPoint(); + bool PerformMicroTaskCheckPoint(); - void DispatchMicroTaskRunnable(already_AddRefed aRunnable); + void PerformDebuggerMicroTaskCheckpoint(); // Storage for watching rejected promises waiting for some client to // consume their rejection. @@ -483,21 +452,25 @@ private: nsCOMPtr mPendingException; nsThread* mOwningThread; // Manual refcounting to avoid include hell. - struct RunInMetastableStateData + struct PendingIDBTransactionData { - nsCOMPtr mRunnable; + nsCOMPtr mTransaction; uint32_t mRecursionDepth; }; nsTArray> mStableStateEvents; - nsTArray mMetastableStateEvents; + nsTArray mPendingIDBTransactions; uint32_t mBaseRecursionDepth; bool mDoingStableStates; - bool mDisableMicroTaskCheckpoint; + // If set to none 0, microtasks will be processed only when recursion depth + // is the set value. + uint32_t mTargetedMicroTaskRecursionDepth; uint32_t mMicroTaskLevel; + std::queue> mPendingMicroTaskRunnables; + std::queue> mDebuggerMicroTaskQueue; uint32_t mMicroTaskRecursionDepth; From 511a9376e32fdb992487c9512f52ee8846f39d5a Mon Sep 17 00:00:00 2001 From: Martok Date: Tue, 2 Jan 2024 01:56:12 +0100 Subject: [PATCH 3/6] Issue #2435 - Declare PromiseRejectionEvent WebIDL Based-on: m-c 1338059 --- dom/bindings/Codegen.py | 2 +- dom/webidl/PromiseRejectionEvent.webidl | 22 ++++++++++++++++++++++ dom/webidl/moz.build | 1 + 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 dom/webidl/PromiseRejectionEvent.webidl diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index b7caaad7bb..3094a98c41 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -80,7 +80,7 @@ def idlTypeNeedsCycleCollection(type): type.isObject() or type.isSpiderMonkeyInterface()): return False - elif type.isCallback() or type.isGeckoInterface(): + elif type.isCallback() or type.isPromise() or type.isGeckoInterface(): return True elif type.isUnion(): return any(idlTypeNeedsCycleCollection(t) for t in type.flatMemberTypes) diff --git a/dom/webidl/PromiseRejectionEvent.webidl b/dom/webidl/PromiseRejectionEvent.webidl new file mode 100644 index 0000000000..3c489f3db4 --- /dev/null +++ b/dom/webidl/PromiseRejectionEvent.webidl @@ -0,0 +1,22 @@ +/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. + * + * The origin of this IDL file is + * https://html.spec.whatwg.org/multipage/webappapis.html#the-promiserejectionevent-interface + */ + +[Constructor(DOMString type, PromiseRejectionEventInit eventInitDict), + Exposed=(Window,Worker)] +interface PromiseRejectionEvent : Event +{ + [BinaryName="rejectedPromise"] + readonly attribute Promise promise; + readonly attribute any reason; +}; + +dictionary PromiseRejectionEventInit : EventInit { + required Promise promise; + any reason; +}; diff --git a/dom/webidl/moz.build b/dom/webidl/moz.build index 0f47aabae4..c1f5ad1b76 100644 --- a/dom/webidl/moz.build +++ b/dom/webidl/moz.build @@ -695,6 +695,7 @@ GENERATED_EVENTS_WEBIDL_FILES = [ 'PopStateEvent.webidl', 'PopupBlockedEvent.webidl', 'ProgressEvent.webidl', + 'PromiseRejectionEvent.webidl', 'RecordErrorEvent.webidl', 'ScrollViewChangeEvent.webidl', 'ServiceWorkerMessageEvent.webidl', From 65d72c568275c4bbe5a547d61da7112b284f7b5a Mon Sep 17 00:00:00 2001 From: Martok Date: Tue, 2 Jan 2024 02:10:31 +0100 Subject: [PATCH 4/6] Issue #2435 - Add onrejectionhandled and onunhandledrejection EventHandler Based-on: m-c 1362272 --- dom/base/nsGkAtomList.h | 2 ++ dom/events/EventNameList.h | 8 ++++---- dom/webidl/EventHandler.webidl | 2 ++ dom/webidl/WorkerGlobalScope.webidl | 2 ++ dom/workers/WorkerScope.h | 2 ++ widget/EventMessageList.h | 2 ++ 6 files changed, 14 insertions(+), 4 deletions(-) diff --git a/dom/base/nsGkAtomList.h b/dom/base/nsGkAtomList.h index af25f97e1c..9622b21d3e 100644 --- a/dom/base/nsGkAtomList.h +++ b/dom/base/nsGkAtomList.h @@ -908,6 +908,7 @@ GK_ATOM(onreadsuccess, "onreadsuccess") GK_ATOM(onready, "onready") GK_ATOM(onreadystatechange, "onreadystatechange") GK_ATOM(onreceived, "onreceived") +GK_ATOM(onrejectionhandled, "onrejectionhandled") GK_ATOM(onremoteheld, "onremoteheld") GK_ATOM(onremoteresumed, "onremoteresumed") GK_ATOM(onrequestprogress, "onrequestprogress") @@ -951,6 +952,7 @@ GK_ATOM(ontransitionend, "ontransitionend") GK_ATOM(ontransitionrun, "ontransitionrun") GK_ATOM(ontransitionstart, "ontransitionstart") GK_ATOM(onunderflow, "onunderflow") +GK_ATOM(onunhandledrejection, "onunhandledrejection") GK_ATOM(onunload, "onunload") GK_ATOM(onupdatefound, "onupdatefound") GK_ATOM(onupdateready, "onupdateready") diff --git a/dom/events/EventNameList.h b/dom/events/EventNameList.h index c4edc1fe2a..0a17b5f147 100644 --- a/dom/events/EventNameList.h +++ b/dom/events/EventNameList.h @@ -578,14 +578,14 @@ WINDOW_EVENT(popstate, ePopState, EventNameType_XUL | EventNameType_HTMLBodyOrFramesetOnly, eBasicEventClass) -// Not supported yet -// WINDOW_EVENT(redo) +WINDOW_EVENT(rejectionhandled, eRejectionHandled, + EventNameType_HTMLBodyOrFramesetOnly, eBasicEventClass) WINDOW_EVENT(storage, eStorage, EventNameType_HTMLBodyOrFramesetOnly, eBasicEventClass) -// Not supported yet -// WINDOW_EVENT(undo) +WINDOW_EVENT(unhandledrejection, eUnhandledRejection, + EventNameType_HTMLBodyOrFramesetOnly, eBasicEventClass) WINDOW_EVENT(unload, eUnload, (EventNameType_XUL | EventNameType_SVGSVG | diff --git a/dom/webidl/EventHandler.webidl b/dom/webidl/EventHandler.webidl index f7acb66ef4..853067bd6f 100644 --- a/dom/webidl/EventHandler.webidl +++ b/dom/webidl/EventHandler.webidl @@ -160,7 +160,9 @@ interface WindowEventHandlers { attribute EventHandler onpagehide; attribute EventHandler onpageshow; attribute EventHandler onpopstate; + attribute EventHandler onrejectionhandled; attribute EventHandler onstorage; + attribute EventHandler onunhandledrejection; attribute EventHandler onunload; }; diff --git a/dom/webidl/WorkerGlobalScope.webidl b/dom/webidl/WorkerGlobalScope.webidl index 25cfe9e294..7afba6c367 100644 --- a/dom/webidl/WorkerGlobalScope.webidl +++ b/dom/webidl/WorkerGlobalScope.webidl @@ -25,6 +25,8 @@ interface WorkerGlobalScope : EventTarget { attribute EventHandler onoffline; attribute EventHandler ononline; + attribute EventHandler onrejectionhandled; + attribute EventHandler onunhandledrejection; // also has additional members in a partial interface }; diff --git a/dom/workers/WorkerScope.h b/dom/workers/WorkerScope.h index 81d2733efe..ffbe4fca75 100644 --- a/dom/workers/WorkerScope.h +++ b/dom/workers/WorkerScope.h @@ -151,6 +151,8 @@ public: IMPL_EVENT_HANDLER(online) IMPL_EVENT_HANDLER(offline) + IMPL_EVENT_HANDLER(rejectionhandled) + IMPL_EVENT_HANDLER(unhandledrejection) void Dump(const Optional& aString) const; diff --git a/widget/EventMessageList.h b/widget/EventMessageList.h index 7ff16c4841..03ae940dc8 100644 --- a/widget/EventMessageList.h +++ b/widget/EventMessageList.h @@ -118,7 +118,9 @@ NS_EVENT_MESSAGE(eImageAbort) NS_EVENT_MESSAGE(eLoadError) NS_EVENT_MESSAGE(eLoadEnd) NS_EVENT_MESSAGE(ePopState) +NS_EVENT_MESSAGE(eRejectionHandled) NS_EVENT_MESSAGE(eStorage) +NS_EVENT_MESSAGE(eUnhandledRejection) NS_EVENT_MESSAGE(eBeforeUnload) NS_EVENT_MESSAGE(eReadyStateChange) From 9126a4836f94a51eb2a420ffd24e03bf8a4200d9 Mon Sep 17 00:00:00 2001 From: Martok Date: Tue, 2 Jan 2024 17:23:39 +0100 Subject: [PATCH 5/6] Issue #2435 - Implement notifying of rejected promises Based-on: m-c 1362272 --- js/src/jsapi.cpp | 7 ++ js/src/jsapi.h | 9 ++ xpcom/base/CycleCollectedJSContext.cpp | 109 ++++++++++++++++++++++++- xpcom/base/CycleCollectedJSContext.h | 42 ++++++++++ 4 files changed, 165 insertions(+), 2 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 1be4e39113..bb143a4998 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4949,6 +4949,13 @@ JS::GetPromiseResult(JS::HandleObject promiseObj) return promise->state() == JS::PromiseState::Fulfilled ? promise->value() : promise->reason(); } +JS_PUBLIC_API(bool) +JS::GetPromiseIsHandled(JS::HandleObject promise) +{ + PromiseObject* promiseObj = &promise->as(); + return !promiseObj->isUnhandled(); +} + JS_PUBLIC_API(JSObject*) JS::GetPromiseAllocationSite(JS::HandleObject promise) { diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 444b9e2ed2..97c085b330 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4611,6 +4611,15 @@ GetPromiseID(JS::HandleObject promise); extern JS_PUBLIC_API(JS::Value) GetPromiseResult(JS::HandleObject promise); +/** + * Returns whether the given promise's rejection is already handled or not. + * + * The caller must check the given promise is rejected before checking it's + * handled or not. + */ +extern JS_PUBLIC_API(bool) +GetPromiseIsHandled(JS::HandleObject promise); + /** * Returns a js::SavedFrame linked list of the stack that lead to the given * Promise's allocation. diff --git a/xpcom/base/CycleCollectedJSContext.cpp b/xpcom/base/CycleCollectedJSContext.cpp index bbf8bca623..9d033c33b8 100644 --- a/xpcom/base/CycleCollectedJSContext.cpp +++ b/xpcom/base/CycleCollectedJSContext.cpp @@ -56,6 +56,7 @@ #include "mozilla/CycleCollectedJSContext.h" #include #include "mozilla/ArrayUtils.h" +#include "mozilla/AsyncEventDispatcher.h" #include "mozilla/AutoRestore.h" #include "mozilla/Move.h" #include "mozilla/MemoryReporting.h" @@ -69,6 +70,8 @@ #include "mozilla/dom/Promise.h" #include "mozilla/dom/PromiseBinding.h" #include "mozilla/dom/PromiseDebugging.h" +#include "mozilla/dom/PromiseRejectionEvent.h" +#include "mozilla/dom/PromiseRejectionEventBinding.h" #include "mozilla/dom/ScriptSettings.h" #include "jsprf.h" #include "js/Debug.h" @@ -994,16 +997,53 @@ CycleCollectedJSContext::PromiseRejectionTrackerCallback(JSContext* aCx, PromiseRejectionHandlingState state, void* aData) { -#ifdef DEBUG CycleCollectedJSContext* self = static_cast(aData); -#endif // DEBUG + MOZ_ASSERT(aCx == self->Context()); MOZ_ASSERT(Get() == self); + // TODO: Bug 1549351 - Promise rejection event should not be sent for + // cross-origin scripts + + PromiseArray& aboutToBeNotified = self->mAboutToBeNotifiedRejectedPromises; + PromiseHashtable& unhandled = self->mPendingUnhandledRejections; + uint64_t promiseID = JS::GetPromiseID(aPromise); + if (state == PromiseRejectionHandlingState::Unhandled) { PromiseDebugging::AddUncaughtRejection(aPromise); + RefPtr promise = Promise::CreateFromExisting(xpc::NativeGlobal(aPromise), aPromise); + aboutToBeNotified.AppendElement(promise); + unhandled.Put(promiseID, promise); } else { PromiseDebugging::AddConsumedRejection(aPromise); + for (size_t i = 0; i < aboutToBeNotified.Length(); i++) { + if (aboutToBeNotified[i] && + aboutToBeNotified[i]->PromiseObj() == aPromise) { + // To avoid large amounts of memmoves, we don't shrink the vector + // here. Instead, we filter out nullptrs when iterating over the + // vector later. + aboutToBeNotified[i] = nullptr; + MOZ_ASSERT(unhandled.Get(promiseID, nullptr)); + unhandled.Remove(promiseID); + return; + } + } + RefPtr promise; + unhandled.Remove(promiseID, getter_AddRefs(promise)); + if (!promise) { + nsIGlobalObject* global = xpc::NativeGlobal(aPromise); + if (nsCOMPtr owner = do_QueryInterface(global)) { + PromiseRejectionEventInit init; + init.mPromise = Promise::CreateFromExisting(global, aPromise); + init.mReason = JS::GetPromiseResult(aPromise); + + RefPtr event = PromiseRejectionEvent::Constructor(owner, + NS_LITERAL_STRING("rejectionhandled"), init); + + RefPtr asyncDispatcher = new AsyncEventDispatcher(owner, event); + asyncDispatcher->PostDOMEvent(); + } + } } } @@ -1415,6 +1455,13 @@ void CycleCollectedJSContext::AfterProcessMicrotasks() { MOZ_ASSERT(mJSContext); + // Notify unhandled promise rejections: + // https://html.spec.whatwg.org/multipage/webappapis.html#notify-about-rejected-promises + if (mAboutToBeNotifiedRejectedPromises.Length()) { + RefPtr runnable = new NotifyUnhandledRejections( + this, std::move(mAboutToBeNotifiedRejectedPromises)); + NS_DispatchToCurrentThread(runnable); + } // Cleanup Indexed Database transactions: // https://html.spec.whatwg.org/multipage/webappapis.html#perform-a-microtask-checkpoint CleanupIDBTransactions(RecursionDepth()); @@ -1783,6 +1830,64 @@ CycleCollectedJSContext::PerformDebuggerMicroTaskCheckpoint() AfterProcessMicrotasks(); } +NS_IMETHODIMP +CycleCollectedJSContext::NotifyUnhandledRejections::Run() +{ + for (size_t i = 0; i < mUnhandledRejections.Length(); ++i) { + RefPtr& promise = mUnhandledRejections[i]; + if (!promise) + continue; + + JS::RootedObject promiseObj(mCx->RootingCx(), promise->PromiseObj()); + MOZ_ASSERT(JS::IsPromiseObject(promiseObj)); + + // Only fire unhandledrejection if the promise is still not handled; + uint64_t promiseID = JS::GetPromiseID(promiseObj); + if (!JS::GetPromiseIsHandled(promiseObj)) { + if (nsCOMPtr target = + do_QueryInterface(promise->GetParentObject())) { + PromiseRejectionEventInit init; + init.mPromise = promise; + init.mReason = JS::GetPromiseResult(promiseObj); + init.mCancelable = true; + + RefPtr event = + PromiseRejectionEvent::Constructor( + target, NS_LITERAL_STRING("unhandledrejection"), init); + // We don't use the result of dispatching event here to check whether to + // report the Promise to console. + bool dummy = true; + target->DispatchEvent(event, &dummy); + } + } + + if (!JS::GetPromiseIsHandled(promiseObj)) { + MOZ_ASSERT(mCx->mPendingUnhandledRejections.Get(promiseID, nullptr)); + mCx->mPendingUnhandledRejections.Remove(promiseID); + } + + // If a rejected promise is being handled in "unhandledrejection" event + // handler, it should be removed from the table in + // PromiseRejectionTrackerCallback. + MOZ_ASSERT(!mCx->mPendingUnhandledRejections.Get(promiseID, nullptr)); + } + return NS_OK; +} + +nsresult +CycleCollectedJSContext::NotifyUnhandledRejections::Cancel() +{ + for (size_t i = 0; i < mUnhandledRejections.Length(); ++i) { + RefPtr& promise = mUnhandledRejections[i]; + if (!promise) + continue; + + JS::RootedObject promiseObj(mCx->RootingCx(), promise->PromiseObj()); + mCx->mPendingUnhandledRejections.Remove(JS::GetPromiseID(promiseObj)); + } + return NS_OK; +} + void CycleCollectedJSContext::EnvironmentPreparer::invoke(JS::HandleObject scope, js::ScriptEnvironmentPreparer::Closure& closure) diff --git a/xpcom/base/CycleCollectedJSContext.h b/xpcom/base/CycleCollectedJSContext.h index d106d54ae9..63669bffa7 100644 --- a/xpcom/base/CycleCollectedJSContext.h +++ b/xpcom/base/CycleCollectedJSContext.h @@ -12,6 +12,7 @@ #include "mozilla/mozalloc.h" #include "mozilla/MemoryReporting.h" #include "mozilla/SegmentedVector.h" +#include "mozilla/dom/Promise.h" #include "jsapi.h" #include "jsfriendapi.h" @@ -474,6 +475,47 @@ private: uint32_t mMicroTaskRecursionDepth; + // This implements about-to-be-notified rejected promises list in the spec. + // https://html.spec.whatwg.org/multipage/webappapis.html#about-to-be-notified-rejected-promises-list + typedef nsTArray> PromiseArray; + PromiseArray mAboutToBeNotifiedRejectedPromises; + + // This is for the "outstanding rejected promises weak set" in the spec, + // https://html.spec.whatwg.org/multipage/webappapis.html#outstanding-rejected-promises-weak-set + // We use different data structure and opposite logic here to achieve the same + // effect. Basically this is used for tracking the rejected promise that does + // NOT need firing a rejectionhandled event. We will check the table to see if + // firing rejectionhandled event is required when a rejected promise is being + // handled. + // + // The rejected promise will be stored in the table if + // - it is unhandled, and + // - The unhandledrejection is not yet fired. + // + // And be removed when + // - it is handled, or + // - A unhandledrejection is fired and it isn't being handled in event + // handler. + typedef nsRefPtrHashtable PromiseHashtable; + PromiseHashtable mPendingUnhandledRejections; + + class NotifyUnhandledRejections final : public CancelableRunnable { + public: + NotifyUnhandledRejections(CycleCollectedJSContext* aCx, + PromiseArray&& aPromises) + : CancelableRunnable(), + mCx(aCx), + mUnhandledRejections(std::move(aPromises)) {} + + NS_IMETHOD Run() final; + + nsresult Cancel() final; + + private: + CycleCollectedJSContext* mCx; + PromiseArray mUnhandledRejections; + }; + OOMState mOutOfMemoryState; OOMState mLargeAllocationFailureState; From 90eb83819c6819b2bcbe62d47654b44c7a345270 Mon Sep 17 00:00:00 2001 From: Martok Date: Thu, 4 Jan 2024 13:23:53 +0100 Subject: [PATCH 6/6] Issue #2435 - Re-Implement rejected Promises events according to spec Own implementation; upstream gets several things wrong. --- dom/promise/PromiseDebugging.cpp | 36 +++++++++-- js/src/builtin/Promise.h | 4 ++ js/src/jsapi.cpp | 12 ++++ js/src/jsapi.h | 12 ++++ xpcom/base/CycleCollectedJSContext.cpp | 88 ++++++++++++++++---------- xpcom/base/CycleCollectedJSContext.h | 25 ++------ 6 files changed, 118 insertions(+), 59 deletions(-) diff --git a/dom/promise/PromiseDebugging.cpp b/dom/promise/PromiseDebugging.cpp index 14ad69bdc8..c0f3f534dc 100644 --- a/dom/promise/PromiseDebugging.cpp +++ b/dom/promise/PromiseDebugging.cpp @@ -217,8 +217,16 @@ PromiseDebugging::RemoveUncaughtRejectionObserver(GlobalObject&, /* static */ void PromiseDebugging::AddUncaughtRejection(JS::HandleObject aPromise) { + CycleCollectedJSContext* storage = CycleCollectedJSContext::Get(); + auto& uncaught = storage->mUncaughtRejections; + auto& outstanding = storage->mOutstandingRejections; + + // See 8.1.4.7 Unhandled promise rejections, Step 5.1.4. + RefPtr promise = Promise::CreateFromExisting(xpc::NativeGlobal(aPromise), aPromise); + uint64_t promiseID = JS::GetPromiseID(aPromise); + outstanding.Put(promiseID, promise); // This might OOM, but won't set a pending exception, so we'll just ignore it. - if (CycleCollectedJSContext::Get()->mUncaughtRejections.append(aPromise)) { + if (uncaught.append(aPromise)) { FlushRejections::DispatchNeeded(); } } @@ -226,15 +234,20 @@ PromiseDebugging::AddUncaughtRejection(JS::HandleObject aPromise) /* void */ void PromiseDebugging::AddConsumedRejection(JS::HandleObject aPromise) { + CycleCollectedJSContext* storage = CycleCollectedJSContext::Get(); + auto& uncaught = storage->mUncaughtRejections; + auto& outstanding = storage->mOutstandingRejections; + // If the promise is in our list of uncaught rejections, we haven't yet // reported it as unhandled. In that case, just remove it from the list // and don't add it to the list of consumed rejections. - auto& uncaughtRejections = CycleCollectedJSContext::Get()->mUncaughtRejections; - for (size_t i = 0; i < uncaughtRejections.length(); i++) { - if (uncaughtRejections[i] == aPromise) { + uint64_t promiseID = JS::GetPromiseID(aPromise); + outstanding.Remove(promiseID); + for (size_t i = 0; i < uncaught.length(); i++) { + if (uncaught[i] == aPromise) { // To avoid large amounts of memmoves, we don't shrink the vector here. // Instead, we filter out nullptrs when iterating over the vector later. - uncaughtRejections[i].set(nullptr); + uncaught[i].set(nullptr); return; } } @@ -251,6 +264,7 @@ PromiseDebugging::FlushUncaughtRejectionsInternal() auto& uncaught = storage->mUncaughtRejections; auto& consumed = storage->mConsumedRejections; + auto& outstanding = storage->mOutstandingRejections; AutoJSAPI jsapi; jsapi.Init(); @@ -267,6 +281,10 @@ PromiseDebugging::FlushUncaughtRejectionsInternal() continue; } + // Clean up outstanding rejected promises weak set + uint64_t promiseID = JS::GetPromiseID(promise); + outstanding.Remove(promiseID); + for (size_t j = 0; j < observers.Length(); ++j) { RefPtr obs = static_cast(observers[j].get()); @@ -274,8 +292,12 @@ PromiseDebugging::FlushUncaughtRejectionsInternal() IgnoredErrorResult err; obs->OnLeftUncaught(promise, err); } - JSAutoCompartment ac(cx, promise); - Promise::ReportRejectedPromise(cx, promise); + // report error to console, unless marked by unhandledrejection event + bool reportError = !JS::GetPromiseIsReported(promise); + if (reportError) { + JSAutoCompartment ac(cx, promise); + Promise::ReportRejectedPromise(cx, promise); + } } storage->mUncaughtRejections.clear(); diff --git a/js/src/builtin/Promise.h b/js/src/builtin/Promise.h index 5e5c850d60..53769ef0bf 100644 --- a/js/src/builtin/Promise.h +++ b/js/src/builtin/Promise.h @@ -100,6 +100,10 @@ class PromiseObject : public NativeObject int32_t flags = getFixedSlot(PromiseSlot_Flags).toInt32(); setFixedSlot(PromiseSlot_Flags, Int32Value(flags | PROMISE_FLAG_REPORTED)); } + bool isReported() { + MOZ_ASSERT(isUnhandled()); + return flags() & PROMISE_FLAG_REPORTED; + } }; /** diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index bb143a4998..5aa38917f5 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4962,6 +4962,18 @@ JS::GetPromiseAllocationSite(JS::HandleObject promise) return promise->as().allocationSite(); } +JS_PUBLIC_API(bool) +JS::GetPromiseIsReported(JS::HandleObject promise) +{ + return promise->as().isReported(); +} + +JS_PUBLIC_API(void) +JS::MarkPromiseRejectionReported(JS::HandleObject promise) +{ + return promise->as().markAsReported(); +} + JS_PUBLIC_API(JSObject*) JS::GetPromiseResolutionSite(JS::HandleObject promise) { diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 97c085b330..f1e71f9e7a 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4620,6 +4620,18 @@ GetPromiseResult(JS::HandleObject promise); extern JS_PUBLIC_API(bool) GetPromiseIsHandled(JS::HandleObject promise); +/** + * Returns whether the given promise's rejection is reported or not. + */ +extern JS_PUBLIC_API(bool) +GetPromiseIsReported(JS::HandleObject promise); + +/** + * Mark the given promise's rejection as already reported. + */ +extern JS_PUBLIC_API(void) +MarkPromiseRejectionReported(JS::HandleObject promise); + /** * Returns a js::SavedFrame linked list of the stack that lead to the given * Promise's allocation. diff --git a/xpcom/base/CycleCollectedJSContext.cpp b/xpcom/base/CycleCollectedJSContext.cpp index 9d033c33b8..d92283fb20 100644 --- a/xpcom/base/CycleCollectedJSContext.cpp +++ b/xpcom/base/CycleCollectedJSContext.cpp @@ -1005,17 +1005,24 @@ CycleCollectedJSContext::PromiseRejectionTrackerCallback(JSContext* aCx, // TODO: Bug 1549351 - Promise rejection event should not be sent for // cross-origin scripts + + // See HTML 8.1.6.3 HostPromiseRejectionTracker + PromiseArray& aboutToBeNotified = self->mAboutToBeNotifiedRejectedPromises; - PromiseHashtable& unhandled = self->mPendingUnhandledRejections; + PromiseHashtable& outstanding = self->mOutstandingRejections; uint64_t promiseID = JS::GetPromiseID(aPromise); if (state == PromiseRejectionHandlingState::Unhandled) { - PromiseDebugging::AddUncaughtRejection(aPromise); + // Step 5. + // Unhandled Promises first go to the about-to-be-notified queue, processed + // after Microtasks as NotifyUnhandledRejections RefPtr promise = Promise::CreateFromExisting(xpc::NativeGlobal(aPromise), aPromise); aboutToBeNotified.AppendElement(promise); - unhandled.Put(promiseID, promise); } else { - PromiseDebugging::AddConsumedRejection(aPromise); + // Unhandled-then-handled Promises (ie.e Promises where the unhandledrejection event + // handler attached a handler) may fire the rejectionhandled event. + + // Step 6.1. for (size_t i = 0; i < aboutToBeNotified.Length(); i++) { if (aboutToBeNotified[i] && aboutToBeNotified[i]->PromiseObj() == aPromise) { @@ -1023,16 +1030,19 @@ CycleCollectedJSContext::PromiseRejectionTrackerCallback(JSContext* aCx, // here. Instead, we filter out nullptrs when iterating over the // vector later. aboutToBeNotified[i] = nullptr; - MOZ_ASSERT(unhandled.Get(promiseID, nullptr)); - unhandled.Remove(promiseID); + MOZ_ASSERT(!outstanding.Get(promiseID, nullptr)); return; } } + // Step 6.2. and 6.3. RefPtr promise; - unhandled.Remove(promiseID, getter_AddRefs(promise)); - if (!promise) { + if (outstanding.Remove(promiseID, getter_AddRefs(promise))) { + // Step 6.4. nsIGlobalObject* global = xpc::NativeGlobal(aPromise); if (nsCOMPtr owner = do_QueryInterface(global)) { + // Step 6.5. + // The spec says to do this in a global task, but we're just queuing an event, + // which will execute in the next event loop either way. PromiseRejectionEventInit init; init.mPromise = Promise::CreateFromExisting(global, aPromise); init.mReason = JS::GetPromiseResult(aPromise); @@ -1044,6 +1054,9 @@ CycleCollectedJSContext::PromiseRejectionTrackerCallback(JSContext* aCx, asyncDispatcher->PostDOMEvent(); } } + + // Finally, notifiy debug observers + PromiseDebugging::AddConsumedRejection(aPromise); } } @@ -1833,7 +1846,11 @@ CycleCollectedJSContext::PerformDebuggerMicroTaskCheckpoint() NS_IMETHODIMP CycleCollectedJSContext::NotifyUnhandledRejections::Run() { + // See HTML 8.1.4.7 Unhandled promise rejections + // with 'mUnhandledRejections' == copy of 'about-to-be-notified rejected' + for (size_t i = 0; i < mUnhandledRejections.Length(); ++i) { + // Step 5.1. RefPtr& promise = mUnhandledRejections[i]; if (!promise) continue; @@ -1841,35 +1858,40 @@ CycleCollectedJSContext::NotifyUnhandledRejections::Run() JS::RootedObject promiseObj(mCx->RootingCx(), promise->PromiseObj()); MOZ_ASSERT(JS::IsPromiseObject(promiseObj)); - // Only fire unhandledrejection if the promise is still not handled; + // Step 5.1.1. + if (JS::GetPromiseIsHandled(promiseObj)) + continue; + uint64_t promiseID = JS::GetPromiseID(promiseObj); - if (!JS::GetPromiseIsHandled(promiseObj)) { - if (nsCOMPtr target = - do_QueryInterface(promise->GetParentObject())) { - PromiseRejectionEventInit init; - init.mPromise = promise; - init.mReason = JS::GetPromiseResult(promiseObj); - init.mCancelable = true; - RefPtr event = - PromiseRejectionEvent::Constructor( - target, NS_LITERAL_STRING("unhandledrejection"), init); - // We don't use the result of dispatching event here to check whether to - // report the Promise to console. - bool dummy = true; - target->DispatchEvent(event, &dummy); - } + // Step 5.1.2. + bool raiseError = true; + if (nsCOMPtr target = + do_QueryInterface(promise->GetParentObject())) { + PromiseRejectionEventInit init; + init.mPromise = promise; + init.mReason = JS::GetPromiseResult(promiseObj); + init.mCancelable = true; + + RefPtr event = + PromiseRejectionEvent::Constructor( + target, NS_LITERAL_STRING("unhandledrejection"), init); + + // Step 5.1.4. (reordered) + // In contrast to the letter of the spec, this must be done before + // the event handler is called or the PromiseRejectionTrackerCallback + // will not see the correct state. + PromiseDebugging::AddUncaughtRejection(promiseObj); + target->DispatchEvent(event, &raiseError); } - if (!JS::GetPromiseIsHandled(promiseObj)) { - MOZ_ASSERT(mCx->mPendingUnhandledRejections.Get(promiseID, nullptr)); - mCx->mPendingUnhandledRejections.Remove(promiseID); + // Step 5.1.3. (implied) + // If the Promise became handled, PromiseRejectionTrackerCallback will have marked it. + // If not, keep it in the "uncaught" list for PromiseDebugging, and set the flag if + // preventDefault indicated we don't want it reported to console. + if (!JS::GetPromiseIsHandled(promiseObj) && !raiseError) { + JS::MarkPromiseRejectionReported(promiseObj); } - - // If a rejected promise is being handled in "unhandledrejection" event - // handler, it should be removed from the table in - // PromiseRejectionTrackerCallback. - MOZ_ASSERT(!mCx->mPendingUnhandledRejections.Get(promiseID, nullptr)); } return NS_OK; } @@ -1883,7 +1905,7 @@ CycleCollectedJSContext::NotifyUnhandledRejections::Cancel() continue; JS::RootedObject promiseObj(mCx->RootingCx(), promise->PromiseObj()); - mCx->mPendingUnhandledRejections.Remove(JS::GetPromiseID(promiseObj)); + mCx->mOutstandingRejections.Remove(JS::GetPromiseID(promiseObj)); } return NS_OK; } diff --git a/xpcom/base/CycleCollectedJSContext.h b/xpcom/base/CycleCollectedJSContext.h index 63669bffa7..4e387bae1a 100644 --- a/xpcom/base/CycleCollectedJSContext.h +++ b/xpcom/base/CycleCollectedJSContext.h @@ -430,6 +430,12 @@ public: // in the last turn of the event loop. JS::PersistentRooted> mConsumedRejections; + // This is for the "outstanding rejected promises weak set" in the spec, + // https://html.spec.whatwg.org/multipage/webappapis.html#outstanding-rejected-promises-weak-set + typedef nsRefPtrHashtable PromiseHashtable; + PromiseHashtable mOutstandingRejections; + + nsTArray> mUncaughtRejectionObservers; private: @@ -480,25 +486,6 @@ private: typedef nsTArray> PromiseArray; PromiseArray mAboutToBeNotifiedRejectedPromises; - // This is for the "outstanding rejected promises weak set" in the spec, - // https://html.spec.whatwg.org/multipage/webappapis.html#outstanding-rejected-promises-weak-set - // We use different data structure and opposite logic here to achieve the same - // effect. Basically this is used for tracking the rejected promise that does - // NOT need firing a rejectionhandled event. We will check the table to see if - // firing rejectionhandled event is required when a rejected promise is being - // handled. - // - // The rejected promise will be stored in the table if - // - it is unhandled, and - // - The unhandledrejection is not yet fired. - // - // And be removed when - // - it is handled, or - // - A unhandledrejection is fired and it isn't being handled in event - // handler. - typedef nsRefPtrHashtable PromiseHashtable; - PromiseHashtable mPendingUnhandledRejections; - class NotifyUnhandledRejections final : public CancelableRunnable { public: NotifyUnhandledRejections(CycleCollectedJSContext* aCx,