diff --git a/dom/promise/PromiseDebugging.cpp b/dom/promise/PromiseDebugging.cpp index f3ec33e8b..3f9703901 100644 --- a/dom/promise/PromiseDebugging.cpp +++ b/dom/promise/PromiseDebugging.cpp @@ -218,8 +218,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(); } } @@ -227,15 +235,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; } } @@ -252,6 +265,7 @@ PromiseDebugging::FlushUncaughtRejectionsInternal() auto& uncaught = storage->mUncaughtRejections; auto& consumed = storage->mConsumedRejections; + auto& outstanding = storage->mOutstandingRejections; AutoJSAPI jsapi; jsapi.Init(); @@ -268,6 +282,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()); @@ -275,8 +293,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 5e5c850d6..53769ef0b 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 f1f7ed429..d5c04f805 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4996,6 +4996,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 232393b02..a26b1402d 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4667,6 +4667,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 02713e53f..220775d1a 100644 --- a/xpcom/base/CycleCollectedJSContext.cpp +++ b/xpcom/base/CycleCollectedJSContext.cpp @@ -1032,17 +1032,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) { @@ -1050,16 +1057,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); @@ -1071,6 +1081,9 @@ CycleCollectedJSContext::PromiseRejectionTrackerCallback(JSContext* aCx, asyncDispatcher->PostDOMEvent(); } } + + // Finally, notifiy debug observers + PromiseDebugging::AddConsumedRejection(aPromise); } } @@ -1863,7 +1876,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; @@ -1871,35 +1888,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; } @@ -1913,7 +1935,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 312f47713..6487ff7b0 100644 --- a/xpcom/base/CycleCollectedJSContext.h +++ b/xpcom/base/CycleCollectedJSContext.h @@ -435,6 +435,13 @@ public: // (because they were in the above list), but the rejection was handled // 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: @@ -485,25 +492,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,