From f6b5ef8ffae4c314fada078c40f0121f095c01e5 Mon Sep 17 00:00:00 2001 From: Kagami Sascha Rosylight Date: Tue, 19 Oct 2021 12:01:51 +0000 Subject: [PATCH] Bug 1725941 - Add AbortSignal support for locks.request() r=smaug Differential Revision: https://phabricator.services.mozilla.com/D128000 --- dom/locks/Lock.cpp | 4 +- dom/locks/LockManager.cpp | 6 -- dom/locks/LockManagerChild.cpp | 2 +- dom/locks/LockRequestChild.cpp | 17 +++- dom/locks/LockRequestChild.h | 10 ++- dom/locks/LockRequestParent.cpp | 8 +- dom/locks/LockRequestParent.h | 2 +- dom/locks/PLockRequest.ipdl | 2 +- .../signal.tentative.https.any.js.ini | 83 ------------------- .../web-locks/signal.tentative.https.any.js | 16 ++++ 10 files changed, 51 insertions(+), 99 deletions(-) delete mode 100644 testing/web-platform/meta/web-locks/signal.tentative.https.any.js.ini diff --git a/dom/locks/Lock.cpp b/dom/locks/Lock.cpp index 6b5beab85bbd..ab29c7d8b207 100644 --- a/dom/locks/Lock.cpp +++ b/dom/locks/Lock.cpp @@ -50,7 +50,7 @@ Promise& Lock::GetWaitingPromise() { void Lock::ResolvedCallback(JSContext* aCx, JS::Handle aValue) { if (mLockRequestChild) { - locks::PLockRequestChild::Send__delete__(mLockRequestChild); + locks::PLockRequestChild::Send__delete__(mLockRequestChild, false); mLockRequestChild = nullptr; } mReleasedPromise->MaybeResolve(aValue); @@ -58,7 +58,7 @@ void Lock::ResolvedCallback(JSContext* aCx, JS::Handle aValue) { void Lock::RejectedCallback(JSContext* aCx, JS::Handle aValue) { if (mLockRequestChild) { - locks::PLockRequestChild::Send__delete__(mLockRequestChild); + locks::PLockRequestChild::Send__delete__(mLockRequestChild, false); mLockRequestChild = nullptr; } mReleasedPromise->MaybeReject(aValue); diff --git a/dom/locks/LockManager.cpp b/dom/locks/LockManager.cpp index 41b2e351bfcf..212b7786ff43 100644 --- a/dom/locks/LockManager.cpp +++ b/dom/locks/LockManager.cpp @@ -125,12 +125,6 @@ already_AddRefed LockManager::Request(const nsAString& aName, return nullptr; } - if (aOptions.mSignal.WasPassed()) { - // TODO(krosylight): Bug 1725941 - aRv.ThrowNotSupportedError("AbortSignal support is not implemented yet"); - return nullptr; - } - if (!mActor) { // TODO: https://github.com/WICG/web-locks/issues/78 aRv.ThrowInvalidStateError( diff --git a/dom/locks/LockManagerChild.cpp b/dom/locks/LockManagerChild.cpp index 3cbfec705ac6..e8938d2f075e 100644 --- a/dom/locks/LockManagerChild.cpp +++ b/dom/locks/LockManagerChild.cpp @@ -15,7 +15,7 @@ NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(LockManagerChild, Release) void LockManagerChild::RequestLock(const LockRequest& aRequest, const LockOptions& aOptions) { - auto requestActor = MakeRefPtr(aRequest); + auto requestActor = MakeRefPtr(aRequest, aOptions.mSignal); SendPLockRequestConstructor( requestActor, IPCLockRequest(nsString(aRequest.mName), aOptions.mMode, aOptions.mIfAvailable, aOptions.mSteal)); diff --git a/dom/locks/LockRequestChild.cpp b/dom/locks/LockRequestChild.cpp index 86435c922f93..6ef5a96c497c 100644 --- a/dom/locks/LockRequestChild.cpp +++ b/dom/locks/LockRequestChild.cpp @@ -14,6 +14,8 @@ namespace mozilla::dom::locks { using IPCResult = mozilla::ipc::IPCResult; +NS_IMPL_ISUPPORTS(LockRequestChild, nsISupports) + // XXX: should be MOZ_CAN_RUN_SCRIPT, but not sure how to call it from closures MOZ_CAN_RUN_SCRIPT_BOUNDARY static void RunCallbackAndSettlePromise( LockGrantedCallback& aCallback, mozilla::dom::Lock* lock, @@ -37,8 +39,13 @@ MOZ_CAN_RUN_SCRIPT_BOUNDARY static void RunCallbackAndSettlePromise( MOZ_ASSERT(!rv.Failed()); } -LockRequestChild::LockRequestChild(const LockRequest& aRequest) +LockRequestChild::LockRequestChild( + const LockRequest& aRequest, + const Optional>& aSignal) : mRequest(aRequest) { + if (aSignal.WasPassed()) { + Follow(&aSignal.Value()); + } if (!NS_IsMainThread()) { mWorkerRef = StrongWorkerRef::Create( GetCurrentThreadWorkerPrivate(), "LockManager", @@ -48,6 +55,8 @@ LockRequestChild::LockRequestChild(const LockRequest& aRequest) IPCResult LockRequestChild::RecvResolve(const LockMode& aLockMode, bool aIsAvailable) { + Unfollow(); + RefPtr lock; RefPtr promise; if (aIsAvailable) { @@ -75,8 +84,14 @@ IPCResult LockRequestChild::RecvResolve(const LockMode& aLockMode, } IPCResult LockRequestChild::RecvAbort() { + Unfollow(); mRequest.mPromise->MaybeRejectWithAbortError("The lock request is aborted"); return IPC_OK(); } +void LockRequestChild::RunAbortAlgorithm() { + RecvAbort(); + Send__delete__(this, true); +} + } // namespace mozilla::dom::locks diff --git a/dom/locks/LockRequestChild.h b/dom/locks/LockRequestChild.h index ce32f8044c9c..88fce5133912 100644 --- a/dom/locks/LockRequestChild.h +++ b/dom/locks/LockRequestChild.h @@ -10,6 +10,7 @@ #include "mozilla/dom/locks/PLockRequestChild.h" #include "mozilla/dom/Lock.h" #include "mozilla/dom/WorkerRef.h" +#include "nsISupportsImpl.h" namespace mozilla::dom::locks { @@ -20,17 +21,22 @@ struct LockRequest { }; class LockRequestChild final : public PLockRequestChild, + public AbortFollower, public SupportsWeakPtr { using IPCResult = mozilla::ipc::IPCResult; - NS_INLINE_DECL_REFCOUNTING(LockRequestChild) + NS_DECL_ISUPPORTS public: - explicit LockRequestChild(const LockRequest& aRequest); + explicit LockRequestChild( + const LockRequest& aRequest, + const Optional>& aSignal); IPCResult RecvResolve(const LockMode& aLockMode, bool aIsAvailable); IPCResult RecvAbort(); + void RunAbortAlgorithm() final; + private: ~LockRequestChild() = default; diff --git a/dom/locks/LockRequestParent.cpp b/dom/locks/LockRequestParent.cpp index a47bc9c978d0..60afa0679a5d 100644 --- a/dom/locks/LockRequestParent.cpp +++ b/dom/locks/LockRequestParent.cpp @@ -11,15 +11,19 @@ namespace mozilla::dom::locks { -mozilla::ipc::IPCResult LockRequestParent::Recv__delete__() { +mozilla::ipc::IPCResult LockRequestParent::Recv__delete__(bool aAborted) { RefPtr manager = static_cast(Manager()); ManagedLocks& managed = manager->Locks(); DebugOnly unheld = managed.mHeldLocks.RemoveElement(this); - MOZ_ASSERT(unheld, "No held lock?"); + MOZ_ASSERT_IF(!aAborted, unheld); if (auto queue = managed.mQueueMap.Lookup(mRequest.name())) { + if (aAborted) { + DebugOnly dequeued = queue.Data().RemoveElement(this); + MOZ_ASSERT_IF(!unheld, dequeued); + } manager->ProcessRequestQueue(queue.Data()); if (queue.Data().IsEmpty()) { // Remove if empty, to prevent the queue map from growing forever diff --git a/dom/locks/LockRequestParent.h b/dom/locks/LockRequestParent.h index 48428745cc77..cdb4fc112740 100644 --- a/dom/locks/LockRequestParent.h +++ b/dom/locks/LockRequestParent.h @@ -22,7 +22,7 @@ class LockRequestParent final : public PLockRequestParent { const IPCLockRequest& Data() { return mRequest; } - mozilla::ipc::IPCResult Recv__delete__() final; + mozilla::ipc::IPCResult Recv__delete__(bool aAborted); private: ~LockRequestParent() = default; diff --git a/dom/locks/PLockRequest.ipdl b/dom/locks/PLockRequest.ipdl index 74909f403cdf..0a7266f3205f 100644 --- a/dom/locks/PLockRequest.ipdl +++ b/dom/locks/PLockRequest.ipdl @@ -21,7 +21,7 @@ protocol PLockRequest { async Abort(); parent: - async __delete__(); + async __delete__(bool aAborted); }; } // namespace cache diff --git a/testing/web-platform/meta/web-locks/signal.tentative.https.any.js.ini b/testing/web-platform/meta/web-locks/signal.tentative.https.any.js.ini deleted file mode 100644 index af58ec251d82..000000000000 --- a/testing/web-platform/meta/web-locks/signal.tentative.https.any.js.ini +++ /dev/null @@ -1,83 +0,0 @@ -[signal.tentative.https.any.html] - expected: ERROR - [Abort after a timeout] - expected: FAIL - - [Synchronously signaled abort] - expected: FAIL - - [Abort signaled after lock released] - expected: NOTRUN - - [Signal that is not aborted] - expected: FAIL - - [Abort signaled after lock granted] - expected: TIMEOUT - - [An aborted request results in AbortError] - expected: FAIL - - -[signal.tentative.https.any.worker.html] - expected: TIMEOUT - [Abort after a timeout] - expected: FAIL - - [Synchronously signaled abort] - expected: FAIL - - [Abort signaled after lock released] - expected: NOTRUN - - [Signal that is not aborted] - expected: FAIL - - [Abort signaled after lock granted] - expected: TIMEOUT - - [An aborted request results in AbortError] - expected: FAIL - - -[signal.tentative.https.any.serviceworker.html] - expected: TIMEOUT - [Abort after a timeout] - expected: FAIL - - [Synchronously signaled abort] - expected: FAIL - - [Abort signaled after lock released] - expected: NOTRUN - - [Signal that is not aborted] - expected: FAIL - - [Abort signaled after lock granted] - expected: TIMEOUT - - [An aborted request results in AbortError] - expected: FAIL - - -[signal.tentative.https.any.sharedworker.html] - expected: TIMEOUT - [Abort after a timeout] - expected: FAIL - - [Synchronously signaled abort] - expected: FAIL - - [Abort signaled after lock released] - expected: NOTRUN - - [Signal that is not aborted] - expected: FAIL - - [Abort signaled after lock granted] - expected: TIMEOUT - - [An aborted request results in AbortError] - expected: FAIL - diff --git a/testing/web-platform/tests/web-locks/signal.tentative.https.any.js b/testing/web-platform/tests/web-locks/signal.tentative.https.any.js index 146a6c701561..c4509de637b8 100644 --- a/testing/web-platform/tests/web-locks/signal.tentative.https.any.js +++ b/testing/web-platform/tests/web-locks/signal.tentative.https.any.js @@ -190,3 +190,19 @@ promise_test(async t => { 'Lock released promise should not reject'); }, 'Abort signaled after lock released'); + +promise_test(async t => { + const res = uniqueName(t); + + const controller = new AbortController(); + const first = requestLockAndHold(t, res, { signal: controller.signal }); + const next = navigator.locks.request(res, () => "resolved"); + controller.abort(); + + await promise_rejects_dom(t, "AbortError", first, "Request should abort"); + assert_equals( + await next, + "resolved", + "The next request is processed after abort" + ); +}, "Abort should process the next pending lock request");