From e1ed5804d7b3c5b4f5f3f7a3d4530731dcbc895e Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Thu, 22 Oct 2020 07:42:18 +0000 Subject: [PATCH] Bug 1660555 - Split AbortFollower::Abort into AbortFollower::RunAbortAlgorithm and AbortSignalImpl::SignalAbort functions for readability. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D92321 --- dom/abort/AbortController.cpp | 2 +- dom/abort/AbortSignal.cpp | 8 ++++---- dom/abort/AbortSignal.h | 33 ++++++++++++++++++++++---------- dom/base/BodyConsumer.cpp | 2 +- dom/base/BodyConsumer.h | 2 +- dom/fetch/Fetch.cpp | 15 ++++++++------- dom/fetch/Fetch.h | 2 +- dom/fetch/FetchDriver.cpp | 4 ++-- dom/fetch/FetchDriver.h | 2 +- dom/fetch/FetchObserver.cpp | 2 +- dom/fetch/FetchObserver.h | 3 ++- dom/webauthn/WebAuthnManager.cpp | 4 +++- dom/webauthn/WebAuthnManager.h | 2 +- 13 files changed, 49 insertions(+), 32 deletions(-) diff --git a/dom/abort/AbortController.cpp b/dom/abort/AbortController.cpp index e5fd8fc36cce..c7b683481394 100644 --- a/dom/abort/AbortController.cpp +++ b/dom/abort/AbortController.cpp @@ -61,7 +61,7 @@ void AbortController::Abort() { mAborted = true; if (mSignal) { - mSignal->Abort(); + mSignal->SignalAbort(); } } diff --git a/dom/abort/AbortSignal.cpp b/dom/abort/AbortSignal.cpp index fc2824464e14..c0de9600f9d2 100644 --- a/dom/abort/AbortSignal.cpp +++ b/dom/abort/AbortSignal.cpp @@ -19,7 +19,7 @@ AbortSignalImpl::AbortSignalImpl(bool aAborted) : mAborted(aAborted) {} bool AbortSignalImpl::Aborted() const { return mAborted; } -void AbortSignalImpl::Abort() { +void AbortSignalImpl::SignalAbort() { if (mAborted) { return; } @@ -28,7 +28,7 @@ void AbortSignalImpl::Abort() { // Let's inform the followers. for (RefPtr follower : mFollowers.ForwardRange()) { - follower->Abort(); + follower->RunAbortAlgorithm(); } } @@ -73,8 +73,8 @@ JSObject* AbortSignal::WrapObject(JSContext* aCx, return AbortSignal_Binding::Wrap(aCx, this, aGivenProto); } -void AbortSignal::Abort() { - AbortSignalImpl::Abort(); +void AbortSignal::SignalAbort() { + AbortSignalImpl::SignalAbort(); EventInit init; init.mBubbles = false; diff --git a/dom/abort/AbortSignal.h b/dom/abort/AbortSignal.h index 940ac237c7cf..1533c1674d58 100644 --- a/dom/abort/AbortSignal.h +++ b/dom/abort/AbortSignal.h @@ -23,7 +23,7 @@ class AbortFollower { public: NS_INLINE_DECL_PURE_VIRTUAL_REFCOUNTING - virtual void Abort() = 0; + virtual void RunAbortAlgorithm() = 0; void Follow(AbortSignalImpl* aSignal); @@ -39,18 +39,13 @@ class AbortFollower { RefPtr mFollowingSignal; }; -// Any subclass of this class must Traverse mFollowingSignal and call -// Unfollow() when Unlinking. -class AbortSignalImpl : public AbortFollower, public nsISupports { +class AbortSignalImpl : public nsISupports { public: - using nsISupports::AddRef; - using nsISupports::Release; - explicit AbortSignalImpl(bool aAborted); bool Aborted() const; - void Abort() override; + virtual void SignalAbort(); void AddFollower(AbortFollower* aFollower); @@ -66,7 +61,21 @@ class AbortSignalImpl : public AbortFollower, public nsISupports { bool mAborted; }; -class AbortSignal final : public DOMEventTargetHelper, public AbortSignalImpl { +// AbortSignal the spec concept includes the concept of a child signal +// "following" a parent signal -- internally, adding abort steps to the parent +// signal that will then signal abort on the child signal -- to propagate +// signaling abort from one signal to another. See +// . +// +// This requires that AbortSignal also inherit from AbortFollower. +// +// This ability to follow isn't directly exposed in the DOM; as of this writing +// it appears only to be used internally in the Fetch API. It might be a good +// idea to split AbortSignal into an implementation that can follow, and an +// implementation that can't, to provide this complexity only when it's needed. +class AbortSignal final : public DOMEventTargetHelper, + public AbortSignalImpl, + public AbortFollower { public: NS_DECL_ISUPPORTS_INHERITED NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(AbortSignal, DOMEventTargetHelper) @@ -78,7 +87,11 @@ class AbortSignal final : public DOMEventTargetHelper, public AbortSignalImpl { IMPL_EVENT_HANDLER(abort); - void Abort() override; + // AbortSignalImpl + void SignalAbort() override; + + // AbortFollower + void RunAbortAlgorithm() override { SignalAbort(); } private: ~AbortSignal() = default; diff --git a/dom/base/BodyConsumer.cpp b/dom/base/BodyConsumer.cpp index 109fc8219b98..d502d480d73a 100644 --- a/dom/base/BodyConsumer.cpp +++ b/dom/base/BodyConsumer.cpp @@ -803,7 +803,7 @@ NS_IMETHODIMP BodyConsumer::Observe(nsISupports* aSubject, const char* aTopic, return NS_OK; } -void BodyConsumer::Abort() { +void BodyConsumer::RunAbortAlgorithm() { AssertIsOnTargetThread(); ShutDownMainThreadConsuming(); ContinueConsumeBody(NS_ERROR_DOM_ABORT_ERR, 0, nullptr); diff --git a/dom/base/BodyConsumer.h b/dom/base/BodyConsumer.h index b56a6f4123bc..f75b33d5b992 100644 --- a/dom/base/BodyConsumer.h +++ b/dom/base/BodyConsumer.h @@ -90,7 +90,7 @@ class BodyConsumer final : public nsIObserver, } // AbortFollower - void Abort() override; + void RunAbortAlgorithm() override; private: BodyConsumer(nsIEventTarget* aMainThreadEventTarget, diff --git a/dom/fetch/Fetch.cpp b/dom/fetch/Fetch.cpp index d01ba8862b3a..a570aeced248 100644 --- a/dom/fetch/Fetch.cpp +++ b/dom/fetch/Fetch.cpp @@ -94,11 +94,11 @@ class AbortSignalMainThread final : public AbortSignalImpl { NS_IMPL_CYCLE_COLLECTION_CLASS(AbortSignalMainThread) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(AbortSignalMainThread) - tmp->Unfollow(); + // This is filled with new operations shortly. NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(AbortSignalMainThread) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFollowingSignal) + // This is filled with new operations shortly. NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(AbortSignalMainThread) @@ -135,7 +135,7 @@ class AbortSignalProxy final : public AbortFollower { MOZ_ASSERT(NS_IsMainThread()); AbortSignalImpl* signalImpl = mProxy->GetOrCreateSignalImplForMainThread(); - signalImpl->Abort(); + signalImpl->SignalAbort(); return NS_OK; } }; @@ -151,7 +151,8 @@ class AbortSignalProxy final : public AbortFollower { Follow(aSignalImpl); } - void Abort() override { + // AbortFollower + void RunAbortAlgorithm() override { RefPtr runnable = new AbortSignalProxyRunnable(this); mMainThreadEventTarget->Dispatch(runnable.forget(), NS_DISPATCH_NORMAL); @@ -1467,7 +1468,7 @@ template void FetchBody::MaybeTeeReadableStreamBody( ErrorResult& aRv); template -void FetchBody::Abort() { +void FetchBody::RunAbortAlgorithm() { if (!mReadableStreamBody) { return; } @@ -1484,9 +1485,9 @@ void FetchBody::Abort() { AbortStream(cx, body, result); } -template void FetchBody::Abort(); +template void FetchBody::RunAbortAlgorithm(); -template void FetchBody::Abort(); +template void FetchBody::RunAbortAlgorithm(); } // namespace dom } // namespace mozilla diff --git a/dom/fetch/Fetch.h b/dom/fetch/Fetch.h index 806b45c550f5..4aca8ead0224 100644 --- a/dom/fetch/Fetch.h +++ b/dom/fetch/Fetch.h @@ -210,7 +210,7 @@ class FetchBody : public BodyStreamHolder, public AbortFollower { virtual AbortSignalImpl* GetSignalImpl() const = 0; // AbortFollower - void Abort() override; + void RunAbortAlgorithm() override; already_AddRefed ConsumeBody(JSContext* aCx, BodyConsumer::ConsumeType aType, diff --git a/dom/fetch/FetchDriver.cpp b/dom/fetch/FetchDriver.cpp index a49c9dbf7e31..c2df017d9cc1 100644 --- a/dom/fetch/FetchDriver.cpp +++ b/dom/fetch/FetchDriver.cpp @@ -460,7 +460,7 @@ nsresult FetchDriver::Fetch(AbortSignalImpl* aSignalImpl, // the operation. if (aSignalImpl) { if (aSignalImpl->Aborted()) { - Abort(); + RunAbortAlgorithm(); return NS_OK; } @@ -1579,7 +1579,7 @@ void FetchDriver::SetRequestHeaders(nsIHttpChannel* aChannel, } } -void FetchDriver::Abort() { +void FetchDriver::RunAbortAlgorithm() { MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread()); if (mObserver) { diff --git a/dom/fetch/FetchDriver.h b/dom/fetch/FetchDriver.h index 5bb4fdd4dced..ddb24117a545 100644 --- a/dom/fetch/FetchDriver.h +++ b/dom/fetch/FetchDriver.h @@ -126,7 +126,7 @@ class FetchDriver final : public nsIStreamListener, } // AbortFollower - void Abort() override; + void RunAbortAlgorithm() override; private: nsCOMPtr mPrincipal; diff --git a/dom/fetch/FetchObserver.cpp b/dom/fetch/FetchObserver.cpp index 70d3deef2057..e9ae6c7774ba 100644 --- a/dom/fetch/FetchObserver.cpp +++ b/dom/fetch/FetchObserver.cpp @@ -43,7 +43,7 @@ JSObject* FetchObserver::WrapObject(JSContext* aCx, FetchState FetchObserver::State() const { return mState; } -void FetchObserver::Abort() { SetState(FetchState::Aborted); } +void FetchObserver::RunAbortAlgorithm() { SetState(FetchState::Aborted); } void FetchObserver::SetState(FetchState aState) { MOZ_ASSERT(mState < aState); diff --git a/dom/fetch/FetchObserver.h b/dom/fetch/FetchObserver.h index 2fc948e81b1d..ba63ed233bbb 100644 --- a/dom/fetch/FetchObserver.h +++ b/dom/fetch/FetchObserver.h @@ -30,7 +30,8 @@ class FetchObserver final : public DOMEventTargetHelper, public AbortFollower { IMPL_EVENT_HANDLER(requestprogress); IMPL_EVENT_HANDLER(responseprogress); - void Abort() override; + // AbortFollower + void RunAbortAlgorithm() override; void SetState(FetchState aState); diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp index 0ca9fff1cd1f..f058248468a7 100644 --- a/dom/webauthn/WebAuthnManager.cpp +++ b/dom/webauthn/WebAuthnManager.cpp @@ -826,7 +826,9 @@ void WebAuthnManager::RequestAborted(const uint64_t& aTransactionId, } } -void WebAuthnManager::Abort() { CancelTransaction(NS_ERROR_DOM_ABORT_ERR); } +void WebAuthnManager::RunAbortAlgorithm() { + CancelTransaction(NS_ERROR_DOM_ABORT_ERR); +} } // namespace dom } // namespace mozilla diff --git a/dom/webauthn/WebAuthnManager.h b/dom/webauthn/WebAuthnManager.h index a99f0df55184..f2d6a600f81b 100644 --- a/dom/webauthn/WebAuthnManager.h +++ b/dom/webauthn/WebAuthnManager.h @@ -104,7 +104,7 @@ class WebAuthnManager final : public WebAuthnManagerBase, public AbortFollower { // AbortFollower - void Abort() override; + void RunAbortAlgorithm() override; protected: // Cancels the current transaction (by sending a Cancel message to the