From 712c0d58c7e48142ceaf8d1c0356b477649f6eb3 Mon Sep 17 00:00:00 2001 From: Kagami Sascha Rosylight Date: Mon, 13 Dec 2021 21:08:55 +0000 Subject: [PATCH] Bug 1737771 - Implement AbortSignal.reason r=mgaudet,smaug Differential Revision: https://phabricator.services.mozilla.com/D132281 --- dom/abort/AbortController.cpp | 32 ++++++-- dom/abort/AbortController.h | 5 +- dom/abort/AbortFollower.h | 18 +++-- dom/abort/AbortSignal.cpp | 76 ++++++++++++++++--- dom/abort/AbortSignal.h | 16 ++-- dom/fetch/Fetch.cpp | 16 +++- dom/fetch/Request.cpp | 6 +- dom/webidl/AbortController.webidl | 4 +- dom/webidl/AbortSignal.webidl | 3 +- .../meta/dom/abort/event.any.js.ini | 44 ----------- .../web-platform/tests/dom/abort/event.any.js | 29 +++++++ .../tests/dom/abort/reason-constructor.html | 12 +++ 12 files changed, 180 insertions(+), 81 deletions(-) delete mode 100644 testing/web-platform/meta/dom/abort/event.any.js.ini create mode 100644 testing/web-platform/tests/dom/abort/reason-constructor.html diff --git a/dom/abort/AbortController.cpp b/dom/abort/AbortController.cpp index c5b4a743ee347..837d11185b6de 100644 --- a/dom/abort/AbortController.cpp +++ b/dom/abort/AbortController.cpp @@ -6,12 +6,27 @@ #include "AbortController.h" #include "AbortSignal.h" +#include "js/Value.h" #include "mozilla/dom/AbortControllerBinding.h" +#include "mozilla/dom/DOMException.h" #include "mozilla/dom/WorkerPrivate.h" namespace mozilla::dom { -NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(AbortController, mGlobal, mSignal) +NS_IMPL_CYCLE_COLLECTION_CLASS(AbortController) +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(AbortController) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mGlobal, mSignal) + tmp->mReason.setUndefined(); + NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER +NS_IMPL_CYCLE_COLLECTION_UNLINK_END +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(AbortController) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mGlobal, mSignal) +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END + +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(AbortController) + NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mReason) +NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTING_ADDREF(AbortController) NS_IMPL_CYCLE_COLLECTING_RELEASE(AbortController) @@ -35,7 +50,9 @@ already_AddRefed AbortController::Constructor( } AbortController::AbortController(nsIGlobalObject* aGlobal) - : mGlobal(aGlobal), mAborted(false) {} + : mGlobal(aGlobal), mAborted(false), mReason(JS::UndefinedHandleValue) { + mozilla::HoldJSObjects(this); +} JSObject* AbortController::WrapObject(JSContext* aCx, JS::Handle aGivenProto) { @@ -46,13 +63,14 @@ nsIGlobalObject* AbortController::GetParentObject() const { return mGlobal; } AbortSignal* AbortController::Signal() { if (!mSignal) { - mSignal = new AbortSignal(mGlobal, mAborted); + JS::Rooted reason(RootingCx(), mReason); + mSignal = new AbortSignal(mGlobal, mAborted, reason); } return mSignal; } -void AbortController::Abort() { +void AbortController::Abort(JSContext* aCx, JS::Handle aReason) { if (mAborted) { return; } @@ -60,8 +78,12 @@ void AbortController::Abort() { mAborted = true; if (mSignal) { - mSignal->SignalAbort(); + mSignal->SignalAbort(aReason); + } else { + mReason = aReason; } } +AbortController::~AbortController() { mozilla::DropJSObjects(this); } + } // namespace mozilla::dom diff --git a/dom/abort/AbortController.h b/dom/abort/AbortController.h index c653f2a768b52..846496fe08a24 100644 --- a/dom/abort/AbortController.h +++ b/dom/abort/AbortController.h @@ -38,15 +38,16 @@ class AbortController final : public nsISupports, public nsWrapperCache { AbortSignal* Signal(); - void Abort(); + void Abort(JSContext* aCx, JS::Handle aReason); private: - ~AbortController() = default; + ~AbortController(); nsCOMPtr mGlobal; RefPtr mSignal; bool mAborted; + JS::Heap mReason; }; } // namespace dom diff --git a/dom/abort/AbortFollower.h b/dom/abort/AbortFollower.h index 8c0e46d803e30..a83b4083becbc 100644 --- a/dom/abort/AbortFollower.h +++ b/dom/abort/AbortFollower.h @@ -7,6 +7,7 @@ #ifndef mozilla_dom_AbortFollower_h #define mozilla_dom_AbortFollower_h +#include "jsapi.h" #include "nsISupportsImpl.h" #include "nsTObserverArray.h" @@ -47,11 +48,16 @@ class AbortFollower : public nsISupports { class AbortSignalImpl : public nsISupports { public: - explicit AbortSignalImpl(bool aAborted); + explicit AbortSignalImpl(bool aAborted, JS::Handle aReason); bool Aborted() const; - virtual void SignalAbort(); + // Web IDL Layer + void GetReason(JSContext* aCx, JS::MutableHandle aReason); + // Helper for other DOM code + JS::Value RawReason() const; + + virtual void SignalAbort(JS::Handle aReason); protected: // Subclasses of this class must call these Traverse and Unlink functions @@ -59,15 +65,17 @@ class AbortSignalImpl : public nsISupports { static void Traverse(AbortSignalImpl* aSignal, nsCycleCollectionTraversalCallback& cb); - static void Unlink(AbortSignalImpl* aSignal) { - // To be filled in shortly. - } + static void Unlink(AbortSignalImpl* aSignal); virtual ~AbortSignalImpl() = default; + JS::Heap mReason; + private: friend class AbortFollower; + void MaybeAssignAbortError(JSContext* aCx); + // Raw pointers. |AbortFollower::Follow| adds to this array, and // |AbortFollower::Unfollow| (also callbed by the destructor) will remove // from this array. Finally, calling |SignalAbort()| will (after running all diff --git a/dom/abort/AbortSignal.cpp b/dom/abort/AbortSignal.cpp index 40ded23571d39..342e5448e7010 100644 --- a/dom/abort/AbortSignal.cpp +++ b/dom/abort/AbortSignal.cpp @@ -6,22 +6,39 @@ #include "AbortSignal.h" +#include "mozilla/dom/DOMException.h" #include "mozilla/dom/Event.h" #include "mozilla/dom/EventBinding.h" #include "mozilla/dom/AbortSignalBinding.h" +#include "mozilla/dom/ToJSValue.h" #include "mozilla/RefPtr.h" +#include "nsCycleCollectionParticipant.h" namespace mozilla::dom { // AbortSignalImpl // ---------------------------------------------------------------------------- -AbortSignalImpl::AbortSignalImpl(bool aAborted) : mAborted(aAborted) {} +AbortSignalImpl::AbortSignalImpl(bool aAborted, JS::Handle aReason) + : mReason(aReason), mAborted(aAborted) { + MOZ_ASSERT_IF(!mReason.isUndefined(), mAborted); +} bool AbortSignalImpl::Aborted() const { return mAborted; } +void AbortSignalImpl::GetReason(JSContext* aCx, + JS::MutableHandle aReason) { + if (!mAborted) { + return; + } + MaybeAssignAbortError(aCx); + aReason.set(mReason); +} + +JS::Value AbortSignalImpl::RawReason() const { return mReason.get(); } + // https://dom.spec.whatwg.org/#abortsignal-signal-abort steps 1-4 -void AbortSignalImpl::SignalAbort() { +void AbortSignalImpl::SignalAbort(JS::Handle aReason) { // Step 1. if (mAborted) { return; @@ -29,6 +46,7 @@ void AbortSignalImpl::SignalAbort() { // Step 2. mAborted = true; + mReason = aReason; // Step 3. // When there are multiple followers, the follower removal algorithm @@ -48,11 +66,31 @@ void AbortSignalImpl::SignalAbort() { mFollowers.Clear(); } -/* static */ void AbortSignalImpl::Traverse( - AbortSignalImpl* aSignal, nsCycleCollectionTraversalCallback& cb) { +void AbortSignalImpl::Traverse(AbortSignalImpl* aSignal, + nsCycleCollectionTraversalCallback& cb) { // To be filled in shortly. } +void AbortSignalImpl::Unlink(AbortSignalImpl* aSignal) { + aSignal->mReason.setUndefined(); +} + +void AbortSignalImpl::MaybeAssignAbortError(JSContext* aCx) { + MOZ_ASSERT(mAborted); + if (!mReason.isUndefined()) { + return; + } + + JS::Rooted exception(aCx); + RefPtr dom = DOMException::Create(NS_ERROR_DOM_ABORT_ERR); + + if (NS_WARN_IF(!ToJSValue(aCx, dom, &exception))) { + return; + } + + mReason.set(exception); +} + // AbortSignal // ---------------------------------------------------------------------------- @@ -73,27 +111,38 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(AbortSignal) NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper) +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(AbortSignal, + DOMEventTargetHelper) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mReason) +NS_IMPL_CYCLE_COLLECTION_TRACE_END + NS_IMPL_ADDREF_INHERITED(AbortSignal, DOMEventTargetHelper) NS_IMPL_RELEASE_INHERITED(AbortSignal, DOMEventTargetHelper) -AbortSignal::AbortSignal(nsIGlobalObject* aGlobalObject, bool aAborted) - : DOMEventTargetHelper(aGlobalObject), AbortSignalImpl(aAborted) {} +AbortSignal::AbortSignal(nsIGlobalObject* aGlobalObject, bool aAborted, + JS::Handle aReason) + : DOMEventTargetHelper(aGlobalObject), AbortSignalImpl(aAborted, aReason) { + mozilla::HoldJSObjects(this); +} JSObject* AbortSignal::WrapObject(JSContext* aCx, JS::Handle aGivenProto) { return AbortSignal_Binding::Wrap(aCx, this, aGivenProto); } -already_AddRefed AbortSignal::Abort(GlobalObject& aGlobal) { +already_AddRefed AbortSignal::Abort(GlobalObject& aGlobal, + JS::Handle aReason, + ErrorResult& aRv) { nsCOMPtr global = do_QueryInterface(aGlobal.GetAsSupports()); - RefPtr abortSignal = new AbortSignal(global, true); + + RefPtr abortSignal = new AbortSignal(global, true, aReason); return abortSignal.forget(); } // https://dom.spec.whatwg.org/#abortsignal-signal-abort -void AbortSignal::SignalAbort() { +void AbortSignal::SignalAbort(JS::Handle aReason) { // Steps 1-4. - AbortSignalImpl::SignalAbort(); + AbortSignalImpl::SignalAbort(aReason); // Step 5. EventInit init; @@ -106,6 +155,13 @@ void AbortSignal::SignalAbort() { DispatchEvent(*event); } +void AbortSignal::RunAbortAlgorithm() { + JS::Rooted reason(RootingCx(), Signal()->RawReason()); + SignalAbort(reason); +} + +AbortSignal::~AbortSignal() { mozilla::DropJSObjects(this); } + // AbortFollower // ---------------------------------------------------------------------------- diff --git a/dom/abort/AbortSignal.h b/dom/abort/AbortSignal.h index 7b32084ea1dac..53c86acf40709 100644 --- a/dom/abort/AbortSignal.h +++ b/dom/abort/AbortSignal.h @@ -30,25 +30,29 @@ class AbortSignal final : public DOMEventTargetHelper, public AbortFollower { public: NS_DECL_ISUPPORTS_INHERITED - NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(AbortSignal, DOMEventTargetHelper) + NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(AbortSignal, + DOMEventTargetHelper) - AbortSignal(nsIGlobalObject* aGlobalObject, bool aAborted); + AbortSignal(nsIGlobalObject* aGlobalObject, bool aAborted, + JS::Handle aReason); JSObject* WrapObject(JSContext* aCx, JS::Handle aGivenProto) override; IMPL_EVENT_HANDLER(abort); - static already_AddRefed Abort(GlobalObject& aGlobal); + static already_AddRefed Abort(GlobalObject& aGlobal, + JS::Handle aReason, + ErrorResult& aRv); // AbortSignalImpl - void SignalAbort() override; + void SignalAbort(JS::Handle aReason) override; // AbortFollower - void RunAbortAlgorithm() override { SignalAbort(); } + void RunAbortAlgorithm() override; private: - ~AbortSignal() = default; + ~AbortSignal(); }; } // namespace dom diff --git a/dom/fetch/Fetch.cpp b/dom/fetch/Fetch.cpp index 39c2a9662f801..b8f909e6f04d5 100644 --- a/dom/fetch/Fetch.cpp +++ b/dom/fetch/Fetch.cpp @@ -6,6 +6,7 @@ #include "Fetch.h" +#include "js/Value.h" #include "mozilla/dom/Document.h" #include "mozilla/ipc/PBackgroundSharedTypes.h" #include "nsIGlobalObject.h" @@ -81,12 +82,15 @@ void AbortStream(JSContext* aCx, JS::Handle aStream, class AbortSignalMainThread final : public AbortSignalImpl { public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS - NS_DECL_CYCLE_COLLECTION_CLASS(AbortSignalMainThread) + NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(AbortSignalMainThread) - explicit AbortSignalMainThread(bool aAborted) : AbortSignalImpl(aAborted) {} + explicit AbortSignalMainThread(bool aAborted) + : AbortSignalImpl(aAborted, JS::UndefinedHandleValue) { + mozilla::HoldJSObjects(this); + } private: - ~AbortSignalMainThread() = default; + ~AbortSignalMainThread() { mozilla::DropJSObjects(this); }; }; NS_IMPL_CYCLE_COLLECTION_CLASS(AbortSignalMainThread) @@ -99,6 +103,10 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(AbortSignalMainThread) AbortSignalImpl::Traverse(static_cast(tmp), cb); NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END +NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(AbortSignalMainThread) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mReason) +NS_IMPL_CYCLE_COLLECTION_TRACE_END + NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(AbortSignalMainThread) NS_INTERFACE_MAP_ENTRY(nsISupports) NS_INTERFACE_MAP_END @@ -212,7 +220,7 @@ NS_IMPL_ISUPPORTS0(AbortSignalProxy) NS_IMETHODIMP WorkerSignalFollower::AbortSignalProxyRunnable::Run() { MOZ_ASSERT(NS_IsMainThread()); AbortSignalImpl* signalImpl = mProxy->GetOrCreateSignalImplForMainThread(); - signalImpl->SignalAbort(); + signalImpl->SignalAbort(JS::UndefinedHandleValue); return NS_OK; } diff --git a/dom/fetch/Request.cpp b/dom/fetch/Request.cpp index 62041cdc10a9d..ef517b43cd381 100644 --- a/dom/fetch/Request.cpp +++ b/dom/fetch/Request.cpp @@ -6,6 +6,7 @@ #include "Request.h" +#include "js/Value.h" #include "nsIURI.h" #include "nsNetUtil.h" #include "nsPIDOMWindow.h" @@ -63,7 +64,8 @@ Request::Request(nsIGlobalObject* aOwner, SafeRefPtr aRequest, if (aSignal) { // If we don't have a signal as argument, we will create it when required by // content, otherwise the Request's signal must follow what has been passed. - mSignal = new AbortSignal(aOwner, aSignal->Aborted()); + JS::Rooted reason(RootingCx(), aSignal->RawReason()); + mSignal = new AbortSignal(aOwner, aSignal->Aborted(), reason); if (!mSignal->Aborted()) { mSignal->Follow(aSignal); } @@ -651,7 +653,7 @@ Headers* Request::Headers_() { AbortSignal* Request::GetOrCreateSignal() { if (!mSignal) { - mSignal = new AbortSignal(mOwner, false); + mSignal = new AbortSignal(mOwner, false, JS::UndefinedHandleValue); } return mSignal; diff --git a/dom/webidl/AbortController.webidl b/dom/webidl/AbortController.webidl index be3659cfd2975..6f4b1443cad7b 100644 --- a/dom/webidl/AbortController.webidl +++ b/dom/webidl/AbortController.webidl @@ -12,7 +12,7 @@ interface AbortController { [Throws] constructor(); - readonly attribute AbortSignal signal; + [SameObject] readonly attribute AbortSignal signal; - void abort(); + void abort(optional any reason); }; diff --git a/dom/webidl/AbortSignal.webidl b/dom/webidl/AbortSignal.webidl index 372328808fef1..4f96ef06df6ad 100644 --- a/dom/webidl/AbortSignal.webidl +++ b/dom/webidl/AbortSignal.webidl @@ -9,9 +9,10 @@ [Exposed=(Window,Worker)] interface AbortSignal : EventTarget { - [NewObject] static AbortSignal abort(); + [NewObject, Throws] static AbortSignal abort(optional any reason); readonly attribute boolean aborted; + readonly attribute any reason; attribute EventHandler onabort; }; diff --git a/testing/web-platform/meta/dom/abort/event.any.js.ini b/testing/web-platform/meta/dom/abort/event.any.js.ini deleted file mode 100644 index a03b294520af8..0000000000000 --- a/testing/web-platform/meta/dom/abort/event.any.js.ini +++ /dev/null @@ -1,44 +0,0 @@ -[event.any.html] - [AbortController abort() should fire event synchronously] - expected: FAIL - - [AbortController abort(reason) should set signal.reason] - expected: FAIL - - [aborting AbortController without reason creates an "AbortError" DOMException] - expected: FAIL - - [AbortController abort(undefined) creates an "AbortError" DOMException] - expected: FAIL - - [static aborting signal should have right properties] - expected: FAIL - - [static aborting signal with reason should set signal.reason] - expected: FAIL - - [AbortController abort(null) should set signal.reason] - expected: FAIL - - -[event.any.worker.html] - [AbortController abort() should fire event synchronously] - expected: FAIL - - [AbortController abort(reason) should set signal.reason] - expected: FAIL - - [aborting AbortController without reason creates an "AbortError" DOMException] - expected: FAIL - - [AbortController abort(undefined) creates an "AbortError" DOMException] - expected: FAIL - - [static aborting signal should have right properties] - expected: FAIL - - [static aborting signal with reason should set signal.reason] - expected: FAIL - - [AbortController abort(null) should set signal.reason] - expected: FAIL diff --git a/testing/web-platform/tests/dom/abort/event.any.js b/testing/web-platform/tests/dom/abort/event.any.js index cb85250b3be1e..5db8379dca524 100644 --- a/testing/web-platform/tests/dom/abort/event.any.js +++ b/testing/web-platform/tests/dom/abort/event.any.js @@ -139,4 +139,33 @@ test(t => { assert_equals(signal.reason, reason, "signal.reason"); }, "static aborting signal with reason should set signal.reason"); +test(t => { + const signal = AbortSignal.abort(); + + assert_true( + signal.reason instanceof DOMException, + "signal.reason is a DOMException" + ); + assert_equals( + signal.reason, + signal.reason, + "signal.reason returns the same DOMException" + ); +}, "AbortSignal.reason returns the same DOMException"); + +test(t => { + const controller = new AbortController(); + controller.abort(); + + assert_true( + controller.signal.reason instanceof DOMException, + "signal.reason is a DOMException" + ); + assert_equals( + controller.signal.reason, + controller.signal.reason, + "signal.reason returns the same DOMException" + ); +}, "AbortController.signal.reason returns the same DOMException"); + done(); diff --git a/testing/web-platform/tests/dom/abort/reason-constructor.html b/testing/web-platform/tests/dom/abort/reason-constructor.html new file mode 100644 index 0000000000000..0515165a0f678 --- /dev/null +++ b/testing/web-platform/tests/dom/abort/reason-constructor.html @@ -0,0 +1,12 @@ + + +AbortSignal.reason constructor + + + +