From 46942c1113d0d2facb2469c090beb8f3c54c8ec0 Mon Sep 17 00:00:00 2001 From: Eden Chuang Date: Tue, 12 Nov 2024 14:28:42 +0000 Subject: [PATCH] Bug 1672493 - P4 Move ShareWorker Non-life cycle related operations from RemoteWorker to RemoteWorkerNonLifeCycleOpController. r=asuth Depends on D198005 Differential Revision: https://phabricator.services.mozilla.com/D198022 --- dom/serviceworkers/ServiceWorkerOp.h | 3 + dom/workers/WorkerPrivate.cpp | 6 +- ...PRemoteWorkerNonLifeCycleOpController.ipdl | 9 ++- .../remoteworkers/RemoteWorkerChild.cpp | 8 -- dom/workers/remoteworkers/RemoteWorkerChild.h | 3 - .../remoteworkers/RemoteWorkerController.cpp | 13 ++- .../remoteworkers/RemoteWorkerManager.cpp | 2 +- ...oteWorkerNonLifeCycleOpControllerChild.cpp | 32 +++++++- ...emoteWorkerNonLifeCycleOpControllerChild.h | 9 ++- ...teWorkerNonLifeCycleOpControllerParent.cpp | 19 ++++- ...moteWorkerNonLifeCycleOpControllerParent.h | 9 ++- dom/workers/remoteworkers/RemoteWorkerOp.h | 4 + dom/workers/sharedworkers/SharedWorkerOp.cpp | 81 +++++++++++++------ dom/workers/sharedworkers/SharedWorkerOp.h | 3 + 14 files changed, 153 insertions(+), 48 deletions(-) diff --git a/dom/serviceworkers/ServiceWorkerOp.h b/dom/serviceworkers/ServiceWorkerOp.h index 45fee90d1257..e2a2d414a183 100644 --- a/dom/serviceworkers/ServiceWorkerOp.h +++ b/dom/serviceworkers/ServiceWorkerOp.h @@ -54,6 +54,9 @@ class ServiceWorkerOp : public RemoteWorkerOp { void StartOnMainThread(RefPtr& aOwner) final; + void Start(RemoteWorkerNonLifeCycleOpControllerChild* aOwner, + RemoteWorkerState& aState) final {} + void Cancel() final; protected: diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 0595ec11dfb5..fb5745ba459f 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -3389,9 +3389,8 @@ void WorkerPrivate::DoRunLoop(JSContext* aCx) { if (mChildEp.IsValid()) { mRemoteWorkerNonLifeCycleOpController = RemoteWorkerNonLifeCycleOpControllerChild::Create(); - if (mRemoteWorkerNonLifeCycleOpController) { - mChildEp.Bind(mRemoteWorkerNonLifeCycleOpController); - } + MOZ_ASSERT_DEBUG_OR_FUZZING(mRemoteWorkerNonLifeCycleOpController); + mChildEp.Bind(mRemoteWorkerNonLifeCycleOpController); } // Now that we've done that, we can go ahead and set up our AutoJSAPI. We @@ -5341,6 +5340,7 @@ bool WorkerPrivate::NotifyInternal(WorkerStatus aStatus) { if (aStatus == Killing && mRemoteWorkerNonLifeCycleOpController) { mRemoteWorkerNonLifeCycleOpController->TransistionStateToKilled(); + mRemoteWorkerNonLifeCycleOpController = nullptr; } // If the worker script never ran, or failed to compile, we don't need to do diff --git a/dom/workers/remoteworkers/PRemoteWorkerNonLifeCycleOpController.ipdl b/dom/workers/remoteworkers/PRemoteWorkerNonLifeCycleOpController.ipdl index 0d91649517cd..8e571c95b422 100644 --- a/dom/workers/remoteworkers/PRemoteWorkerNonLifeCycleOpController.ipdl +++ b/dom/workers/remoteworkers/PRemoteWorkerNonLifeCycleOpController.ipdl @@ -2,6 +2,8 @@ * 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/. */ +include RemoteWorkerTypes; +include SharedWorkerOpArgs; namespace mozilla { namespace dom{ @@ -13,9 +15,12 @@ protocol PRemoteWorkerNonLifeCycleOpController parent: async Terminated(); -child: - async Shutdown(); + async Error(ErrorValue aValue); +child: + async ExecOp(SharedWorkerOpArgs aOpArgs); + + async Shutdown(); }; } // namespace dom diff --git a/dom/workers/remoteworkers/RemoteWorkerChild.cpp b/dom/workers/remoteworkers/RemoteWorkerChild.cpp index da1b8d5b4d41..239745aac2d5 100644 --- a/dom/workers/remoteworkers/RemoteWorkerChild.cpp +++ b/dom/workers/remoteworkers/RemoteWorkerChild.cpp @@ -811,14 +811,6 @@ void RemoteWorkerChild::ExceptionalErrorTransitionDuringExecWorker() { } } -void RemoteWorkerChild::AddPortIdentifier( - JSContext* aCx, WorkerPrivate* aWorkerPrivate, - UniqueMessagePortId& aPortIdentifier) { - if (NS_WARN_IF(!aWorkerPrivate->ConnectMessagePort(aCx, aPortIdentifier))) { - ErrorPropagationDispatch(NS_ERROR_FAILURE); - } -} - void RemoteWorkerChild::CancelAllPendingOps(RemoteWorkerState& aState) { MOZ_ASSERT(aState.is()); diff --git a/dom/workers/remoteworkers/RemoteWorkerChild.h b/dom/workers/remoteworkers/RemoteWorkerChild.h index 2160094c9aaa..ed611eb87dcc 100644 --- a/dom/workers/remoteworkers/RemoteWorkerChild.h +++ b/dom/workers/remoteworkers/RemoteWorkerChild.h @@ -78,9 +78,6 @@ class RemoteWorkerChild final : public PRemoteWorkerChild { void FlushReportsOnMainThread(nsIConsoleReportCollector* aReporter); - void AddPortIdentifier(JSContext* aCx, WorkerPrivate* aWorkerPrivate, - UniqueMessagePortId& aPortIdentifier); - RefPtr GetTerminationPromise(); RefPtr MaybeSendSetServiceWorkerSkipWaitingFlag(); diff --git a/dom/workers/remoteworkers/RemoteWorkerController.cpp b/dom/workers/remoteworkers/RemoteWorkerController.cpp index 4c1e3f8225eb..1eb7be91b5a4 100644 --- a/dom/workers/remoteworkers/RemoteWorkerController.cpp +++ b/dom/workers/remoteworkers/RemoteWorkerController.cpp @@ -97,6 +97,7 @@ void RemoteWorkerController::CreationFailed() { if (mState == eTerminated) { MOZ_ASSERT(!mActor); + MOZ_ASSERT(!mNonLifeCycleOpController); MOZ_ASSERT(mPendingOps.IsEmpty()); // Nothing to do. return; @@ -113,12 +114,14 @@ void RemoteWorkerController::CreationSucceeded() { if (mState == eTerminated) { MOZ_ASSERT(!mActor); + MOZ_ASSERT(!mNonLifeCycleOpController); MOZ_ASSERT(mPendingOps.IsEmpty()); // Nothing to do. return; } MOZ_ASSERT(mActor); + MOZ_ASSERT(mNonLifeCycleOpController); mState = eReady; mObserver->CreationSucceeded(); @@ -181,7 +184,9 @@ void RemoteWorkerController::Shutdown() { CancelAllPendingOps(); if (mNonLifeCycleOpController) { - Unused << mNonLifeCycleOpController->SendShutdown(); + if (mNonLifeCycleOpController->CanSend()) { + Unused << mNonLifeCycleOpController->SendShutdown(); + } mNonLifeCycleOpController = nullptr; } @@ -390,7 +395,11 @@ bool RemoteWorkerController::PendingSharedWorkerOp::MaybeStart( Unused << aOwner->mActor->SendExecOp(SharedWorkerThawOpArgs()); break; case ePortIdentifier: - Unused << aOwner->mActor->SendExecOp( + MOZ_ASSERT(aOwner->mNonLifeCycleOpController); + if (!aOwner->mNonLifeCycleOpController->CanSend()) { + return false; + } + Unused << aOwner->mNonLifeCycleOpController->SendExecOp( SharedWorkerPortIdentifierOpArgs(mPortIdentifier)); break; case eAddWindowID: diff --git a/dom/workers/remoteworkers/RemoteWorkerManager.cpp b/dom/workers/remoteworkers/RemoteWorkerManager.cpp index 9661a13a17db..ce457c33d9a0 100644 --- a/dom/workers/remoteworkers/RemoteWorkerManager.cpp +++ b/dom/workers/remoteworkers/RemoteWorkerManager.cpp @@ -357,7 +357,7 @@ void RemoteWorkerManager::LaunchInternal( MOZ_ASSERT(!aController->mNonLifeCycleOpController); aController->mNonLifeCycleOpController = - MakeAndAddRef(); + MakeAndAddRef(aController); parentEp.Bind(aController->mNonLifeCycleOpController); diff --git a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.cpp b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.cpp index bddf7735e124..cb984c0fd323 100644 --- a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.cpp +++ b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.cpp @@ -3,7 +3,9 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "RemoteWorkerNonLifeCycleOpControllerChild.h" +#include "mozilla/dom/SharedWorkerOp.h" #include "mozilla/dom/WorkerCommon.h" +#include "mozilla/dom/WorkerPrivate.h" #include "mozilla/dom/WorkerRef.h" namespace mozilla::dom { @@ -43,8 +45,36 @@ void RemoteWorkerNonLifeCycleOpControllerChild::TransistionStateToCanceled() { void RemoteWorkerNonLifeCycleOpControllerChild::TransistionStateToKilled() { auto lock = mState.Lock(); MOZ_ASSERT(lock->is()); - Unused << SendTerminated(); *lock = VariantType(); + if (!CanSend()) { + return; + } + Unused << SendTerminated(); +} + +void RemoteWorkerNonLifeCycleOpControllerChild::ErrorPropagation( + nsresult aError) { + if (!CanSend()) { + return; + } + Unused << SendError(aError); +} + +void RemoteWorkerNonLifeCycleOpControllerChild::StartOp( + RefPtr&& aOp) { + MOZ_ASSERT(aOp); + auto lock = mState.Lock(); + // ServiceWorkerOp/SharedWorkerOp handles the Canceled/Killed state cases. + aOp->Start(this, lock.ref()); +} + +IPCResult RemoteWorkerNonLifeCycleOpControllerChild::RecvExecOp( + SharedWorkerOpArgs&& aOpArgs) { + MOZ_ASSERT(aOpArgs.type() == + SharedWorkerOpArgs::TSharedWorkerPortIdentifierOpArgs); + StartOp(new SharedWorkerOp(std::move(aOpArgs))); + + return IPC_OK(); } IPCResult RemoteWorkerNonLifeCycleOpControllerChild::RecvShutdown() { diff --git a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.h b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.h index da73d96e3074..2a5db51f58dd 100644 --- a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.h +++ b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.h @@ -30,12 +30,17 @@ class RemoteWorkerNonLifeCycleOpControllerChild final RemoteWorkerNonLifeCycleOpControllerChild(); - void TransistionStateToCanceled(); - void TransistionStateToKilled(); + IPCResult RecvExecOp(SharedWorkerOpArgs&& aOpArgs); IPCResult RecvShutdown(); + void TransistionStateToCanceled(); + void TransistionStateToKilled(); + + void ErrorPropagation(nsresult aError); + private: + void StartOp(RefPtr&& aOp); ~RemoteWorkerNonLifeCycleOpControllerChild(); DataMutex mState; diff --git a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.cpp b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.cpp index 4fce363fcba3..ffae9a87ef56 100644 --- a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.cpp +++ b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.cpp @@ -8,13 +8,30 @@ namespace mozilla::dom { RemoteWorkerNonLifeCycleOpControllerParent:: - RemoteWorkerNonLifeCycleOpControllerParent() {} + RemoteWorkerNonLifeCycleOpControllerParent( + RemoteWorkerController* aController) + : mController(aController) { + MOZ_ASSERT(mController); +} RemoteWorkerNonLifeCycleOpControllerParent:: ~RemoteWorkerNonLifeCycleOpControllerParent() = default; IPCResult RemoteWorkerNonLifeCycleOpControllerParent::RecvTerminated() { Unused << SendShutdown(); + + // mController could be nullptr when the controller had already shutted down. + if (mController) { + mController->mNonLifeCycleOpController = nullptr; + } + + return IPC_OK(); +} + +IPCResult RemoteWorkerNonLifeCycleOpControllerParent::RecvError( + const ErrorValue& aError) { + MOZ_ASSERT(mController); + mController->ErrorPropagation(aError); return IPC_OK(); } diff --git a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.h b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.h index c4b0272136f1..93f675e8ecc4 100644 --- a/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.h +++ b/dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.h @@ -11,18 +11,25 @@ using mozilla::ipc::IPCResult; namespace mozilla::dom { +class RemoteWorkerController; + class RemoteWorkerNonLifeCycleOpControllerParent final : public PRemoteWorkerNonLifeCycleOpControllerParent { public: NS_INLINE_DECL_REFCOUNTING(RemoteWorkerNonLifeCycleOpControllerParent, override); - RemoteWorkerNonLifeCycleOpControllerParent(); + explicit RemoteWorkerNonLifeCycleOpControllerParent( + RemoteWorkerController* aRemoteWorkerController); IPCResult RecvTerminated(); + IPCResult RecvError(const ErrorValue& aError); + private: ~RemoteWorkerNonLifeCycleOpControllerParent(); + + RefPtr mController; }; } // namespace mozilla::dom diff --git a/dom/workers/remoteworkers/RemoteWorkerOp.h b/dom/workers/remoteworkers/RemoteWorkerOp.h index df02bc8495c5..c4056205ce4d 100644 --- a/dom/workers/remoteworkers/RemoteWorkerOp.h +++ b/dom/workers/remoteworkers/RemoteWorkerOp.h @@ -12,6 +12,7 @@ namespace mozilla::dom { class RemoteWorkerChild; +class RemtoeWorkerNonfLifeCycleOpControllerChild; class RemoteWorkerOp; // class RemoteWorkerNonLifeCycleOpControllerChild; @@ -83,6 +84,9 @@ class RemoteWorkerOp { virtual void StartOnMainThread(RefPtr& aOwner) = 0; + virtual void Start(RemoteWorkerNonLifeCycleOpControllerChild* aOwner, + remoteworker::RemoteWorkerState& aState) = 0; + virtual void Cancel() = 0; }; diff --git a/dom/workers/sharedworkers/SharedWorkerOp.cpp b/dom/workers/sharedworkers/SharedWorkerOp.cpp index 0def457a5cf0..ac96e09a4d8f 100644 --- a/dom/workers/sharedworkers/SharedWorkerOp.cpp +++ b/dom/workers/sharedworkers/SharedWorkerOp.cpp @@ -5,6 +5,7 @@ #include "SharedWorkerOp.h" #include "mozilla/dom/MessagePort.h" #include "mozilla/dom/RemoteWorkerChild.h" +#include "mozilla/dom/RemoteWorkerNonLifeCycleOpControllerChild.h" #include "mozilla/dom/WorkerPrivate.h" #include "mozilla/dom/WorkerRunnable.h" #include "mozilla/dom/WorkerScope.h" @@ -13,15 +14,18 @@ namespace mozilla::dom { using remoteworker::Canceled; using remoteworker::Killed; +using remoteworker::Pending; +using remoteworker::Running; namespace { // Normal runnable because AddPortIdentifier() is going to exec JS code. -class MessagePortIdentifierRunnable final : public WorkerThreadRunnable { +class MessagePortIdentifierRunnable final : public WorkerSameThreadRunnable { public: - MessagePortIdentifierRunnable(RemoteWorkerChild* aActor, - const MessagePortIdentifier& aPortIdentifier) - : WorkerThreadRunnable("MessagePortIdentifierRunnable"), + MessagePortIdentifierRunnable( + RemoteWorkerNonLifeCycleOpControllerChild* aActor, + const MessagePortIdentifier& aPortIdentifier) + : WorkerSameThreadRunnable("MessagePortIdentifierRunnable"), mActor(aActor), mPortIdentifier(aPortIdentifier) {} @@ -31,11 +35,13 @@ class MessagePortIdentifierRunnable final : public WorkerThreadRunnable { mPortIdentifier.ForceClose(); return true; } - mActor->AddPortIdentifier(aCx, aWorkerPrivate, mPortIdentifier); + if (!aWorkerPrivate->ConnectMessagePort(aCx, mPortIdentifier)) { + mActor->ErrorPropagation(NS_ERROR_FAILURE); + } return true; } - RefPtr mActor; + RefPtr mActor; UniqueMessagePortId mPortIdentifier; }; @@ -71,11 +77,6 @@ bool SharedWorkerOp::MaybeStart(RemoteWorkerChild* aOwner, #ifdef DEBUG mStarted = true; #endif - if (mOpArgs.type() == - SharedWorkerOpArgs::TSharedWorkerPortIdentifierOpArgs) { - MessagePort::ForceClose( - mOpArgs.get_SharedWorkerPortIdentifierOpArgs().portIdentifier()); - } return true; } @@ -91,13 +92,6 @@ bool SharedWorkerOp::MaybeStart(RemoteWorkerChild* aOwner, if (NS_WARN_IF(lock->is() || lock->is())) { self->Cancel(); - // Worker has already canceled, force close the MessagePort. - if (self->mOpArgs.type() == - SharedWorkerOpArgs::TSharedWorkerPortIdentifierOpArgs) { - MessagePort::ForceClose( - self->mOpArgs.get_SharedWorkerPortIdentifierOpArgs() - .portIdentifier()); - } return; } } @@ -143,12 +137,9 @@ void SharedWorkerOp::StartOnMainThread(RefPtr& aOwner) { workerPrivate->Thaw(nullptr); } else if (mOpArgs.type() == SharedWorkerOpArgs::TSharedWorkerPortIdentifierOpArgs) { - RefPtr r = new MessagePortIdentifierRunnable( - aOwner, - mOpArgs.get_SharedWorkerPortIdentifierOpArgs().portIdentifier()); - if (NS_WARN_IF(!r->Dispatch(workerPrivate))) { - aOwner->ErrorPropagationDispatch(NS_ERROR_FAILURE); - } + MOZ_CRASH( + "PortIdentifierOpArgs should not be processed by " + "StartOnMainThread!!!"); } else if (mOpArgs.type() == SharedWorkerOpArgs::TSharedWorkerAddWindowIDOpArgs) { aOwner->mWindowIDs.AppendElement( @@ -162,6 +153,48 @@ void SharedWorkerOp::StartOnMainThread(RefPtr& aOwner) { } } +void SharedWorkerOp::Start(RemoteWorkerNonLifeCycleOpControllerChild* aOwner, + RemoteWorkerState& aState) { + MOZ_ASSERT(!mStarted); + MOZ_ASSERT(aOwner); + // Thread: We are on the Worker thread. + + // Only PortIdentifierOp is NonLifeCycle related opertaion. + MOZ_ASSERT(mOpArgs.type() == + SharedWorkerOpArgs::TSharedWorkerPortIdentifierOpArgs); + + // Should never be Pending state. + MOZ_ASSERT(!aState.is()); + + // If the worker is already shutting down (which should be unexpected + // because we should be told new operations after a termination op), just + // return directly. + if (aState.is() || aState.is()) { +#ifdef DEBUG + mStarted = true; +#endif + MessagePort::ForceClose( + mOpArgs.get_SharedWorkerPortIdentifierOpArgs().portIdentifier()); + return; + } + + MOZ_ASSERT(aState.is()); + + // RefPtr workerPrivate = aState.as().mWorkerPrivate; + WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate(); + + RefPtr r = new MessagePortIdentifierRunnable( + aOwner, mOpArgs.get_SharedWorkerPortIdentifierOpArgs().portIdentifier()); + + if (NS_WARN_IF(!r->Dispatch(workerPrivate))) { + aOwner->ErrorPropagation(NS_ERROR_FAILURE); + } + +#ifdef DEBUG + mStarted = true; +#endif +} + bool SharedWorkerOp::IsTerminationOp() const { return mOpArgs.type() == SharedWorkerOpArgs::TSharedWorkerTerminateOpArgs; } diff --git a/dom/workers/sharedworkers/SharedWorkerOp.h b/dom/workers/sharedworkers/SharedWorkerOp.h index 6b41e6045b26..96fdc7e8ac77 100644 --- a/dom/workers/sharedworkers/SharedWorkerOp.h +++ b/dom/workers/sharedworkers/SharedWorkerOp.h @@ -25,6 +25,9 @@ class SharedWorkerOp : public RemoteWorkerOp { void StartOnMainThread(RefPtr& aOwner) final; + void Start(RemoteWorkerNonLifeCycleOpControllerChild* aOwner, + RemoteWorkerState& aState) final; + void Cancel() override; private: