Bug 1672493 - P5 Move ServiceWorker Non-life cycle related operations from RemoteWorker to RemoteWorkerNonLifeCycleController, excepts FetchEventOp. r=asuth

Depends on D198022

Differential Revision: https://phabricator.services.mozilla.com/D198029

dom/serviceworkers/ServiceWorkerOp.cpp

@@ -50,6 +50,7 @@
 #include "mozilla/dom/PerformanceStorage.h"
 #include "mozilla/dom/PushEventBinding.h"
 #include "mozilla/dom/RemoteWorkerChild.h"
+#include "mozilla/dom/RemoteWorkerNonLifeCycleOpControllerChild.h"
 #include "mozilla/dom/RemoteWorkerService.h"
 #include "mozilla/dom/Request.h"
 #include "mozilla/dom/Response.h"
@@ -68,6 +69,8 @@ namespace mozilla::dom {
 
 using remoteworker::Canceled;
 using remoteworker::Killed;
+using remoteworker::Pending;
+using remoteworker::Running;
 
 namespace {
 
@@ -283,7 +286,6 @@ class ServiceWorkerOp::ServiceWorkerOpRunnable final
                           WorkerPrivate* aWorkerPrivate)
       : WorkerDebuggeeRunnable("ServiceWorkerOpRunnable"),
         mOwner(std::move(aOwner)) {
-    AssertIsOnMainThread();
     MOZ_ASSERT(mOwner);
     MOZ_ASSERT(aWorkerPrivate);
   }
@@ -309,6 +311,12 @@ class ServiceWorkerOp::ServiceWorkerOpRunnable final
     return rv;
   }
 
+  // Silent PreDispatch and PostDispatch, since ServiceWorkerOpRunnable can be
+  // from the main thread or from the worker thread.
+  bool PreDispatch(WorkerPrivate* WorkerPrivate) override { return true; }
+  void PostDispatch(WorkerPrivate* WorkerPrivate,
+                    bool aDispatchResult) override {}
+
   nsresult Cancel() override {
     MOZ_ASSERT(mOwner);
 
@@ -387,6 +395,7 @@ bool ServiceWorkerOp::MaybeStart(RemoteWorkerChild* aOwner,
 }
 
 void ServiceWorkerOp::StartOnMainThread(RefPtr<RemoteWorkerChild>& aOwner) {
+  AssertIsOnMainThread();
   MaybeReportServiceWorkerShutdownProgress(mArgs);
 
   {
@@ -414,14 +423,49 @@ void ServiceWorkerOp::StartOnMainThread(RefPtr<RemoteWorkerChild>& aOwner) {
   }
 }
 
+void ServiceWorkerOp::Start(RemoteWorkerNonLifeCycleOpControllerChild* aOwner,
+                            RemoteWorkerState& aState) {
+  MOZ_ASSERT(!mStarted);
+  MOZ_ASSERT(aOwner);
+
+  if (NS_WARN_IF(!aOwner->CanSend())) {
+    RejectAll(NS_ERROR_DOM_ABORT_ERR);
+    mStarted = true;
+    return;
+  }
+
+  // NonLifeCycle related operations would never start at Pending state.
+  MOZ_ASSERT(!aState.is<Pending>());
+
+  if (NS_WARN_IF(aState.is<Canceled>()) || NS_WARN_IF(aState.is<Killed>())) {
+    RejectAll(NS_ERROR_DOM_INVALID_STATE_ERR);
+    mStarted = true;
+    return;
+  }
+
+  MOZ_ASSERT(aState.is<Running>());
+
+  WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
+
+  MOZ_ASSERT_DEBUG_OR_FUZZING(workerPrivate);
+
+  RefPtr<WorkerThreadRunnable> workerRunnable = GetRunnable(workerPrivate);
+
+  if (NS_WARN_IF(!workerRunnable->Dispatch(workerPrivate))) {
+    RejectAll(NS_ERROR_FAILURE);
+  }
+
+  mStarted = true;
+}
+
 void ServiceWorkerOp::Cancel() { RejectAll(NS_ERROR_DOM_ABORT_ERR); }
 
 ServiceWorkerOp::ServiceWorkerOp(
     ServiceWorkerOpArgs&& aArgs,
     std::function<void(const ServiceWorkerOpResult&)>&& aCallback)
     : mArgs(std::move(aArgs)) {
-  MOZ_ASSERT(RemoteWorkerService::Thread()->IsOnCurrentThread());
-
+  // Can be on the Worker Launcher thread for Terminate operations or on the
+  // Worker thread for other operations
   RefPtr<ServiceWorkerOpPromise> promise = mPromiseHolder.Ensure(__func__);
 
   promise->Then(
@@ -445,7 +489,6 @@ ServiceWorkerOp::~ServiceWorkerOp() {
 
 bool ServiceWorkerOp::Started() const {
   MOZ_ASSERT(RemoteWorkerService::Thread()->IsOnCurrentThread());
-
   return mStarted;
 }
 
@@ -456,7 +499,6 @@ bool ServiceWorkerOp::IsTerminationOp() const {
 
 RefPtr<WorkerThreadRunnable> ServiceWorkerOp::GetRunnable(
     WorkerPrivate* aWorkerPrivate) {
-  AssertIsOnMainThread();
   MOZ_ASSERT(aWorkerPrivate);
 
   return new ServiceWorkerOpRunnable(this, aWorkerPrivate);
@@ -518,17 +560,27 @@ class UpdateServiceWorkerStateOp final : public ServiceWorkerOp {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(UpdateServiceWorkerStateOp, override);
 
  private:
-  class UpdateStateOpRunnable final : public MainThreadWorkerControlRunnable {
+  class UpdateStateOpRunnable final : public WorkerControlRunnable {
    public:
     NS_DECL_ISUPPORTS_INHERITED
 
     UpdateStateOpRunnable(RefPtr<UpdateServiceWorkerStateOp> aOwner,
                           WorkerPrivate* aWorkerPrivate)
-        : MainThreadWorkerControlRunnable("UpdateStateOpRunnable"),
+        : WorkerControlRunnable("UpdateStateOpRunnable"),
           mOwner(std::move(aOwner)) {
-      AssertIsOnMainThread();
       MOZ_ASSERT(mOwner);
       MOZ_ASSERT(aWorkerPrivate);
+      aWorkerPrivate->AssertIsOnWorkerThread();
+    }
+
+    virtual bool PreDispatch(WorkerPrivate* aWorkerPrivate) override {
+      aWorkerPrivate->AssertIsOnWorkerThread();
+      return true;
+    }
+
+    virtual void PostDispatch(WorkerPrivate* aWorkerPrivate,
+                              bool aDispatchResult) override {
+      aWorkerPrivate->AssertIsOnWorkerThread();
     }
 
    private:
@@ -563,8 +615,8 @@ class UpdateServiceWorkerStateOp final : public ServiceWorkerOp {
 
   RefPtr<WorkerThreadRunnable> GetRunnable(
       WorkerPrivate* aWorkerPrivate) override {
-    AssertIsOnMainThread();
     MOZ_ASSERT(aWorkerPrivate);
+    aWorkerPrivate->IsOnWorkerThread();
     MOZ_ASSERT(mArgs.type() ==
                ServiceWorkerOpArgs::TServiceWorkerUpdateStateOpArgs);
 
@@ -588,7 +640,7 @@ class UpdateServiceWorkerStateOp final : public ServiceWorkerOp {
 };
 
 NS_IMPL_ISUPPORTS_INHERITED0(UpdateServiceWorkerStateOp::UpdateStateOpRunnable,
-                             MainThreadWorkerControlRunnable)
+                             WorkerControlRunnable)
 
 void ExtendableEventOp::FinishedWithResult(ExtendableEventResult aResult) {
   MOZ_ASSERT(IsCurrentThreadRunningWorker());
@@ -1889,8 +1941,8 @@ class ExtensionAPIEventOp final : public ServiceWorkerOp {
 /* static */ already_AddRefed<ServiceWorkerOp> ServiceWorkerOp::Create(
     ServiceWorkerOpArgs&& aArgs,
     std::function<void(const ServiceWorkerOpResult&)>&& aCallback) {
-  MOZ_ASSERT(RemoteWorkerService::Thread()->IsOnCurrentThread());
-
+  // Can be on the Worker Launcher thread for Terminate operations or on the
+  // Worker thread for other operations.
   RefPtr<ServiceWorkerOp> op;
 
   switch (aArgs.type()) {

dom/serviceworkers/ServiceWorkerOp.h

@@ -55,7 +55,7 @@ class ServiceWorkerOp : public RemoteWorkerOp {
   void StartOnMainThread(RefPtr<RemoteWorkerChild>& aOwner) final;
 
   void Start(RemoteWorkerNonLifeCycleOpControllerChild* aOwner,
-             RemoteWorkerState& aState) final {}
+             RemoteWorkerState& aState) final;
 
   void Cancel() final;
 

dom/workers/remoteworkers/PRemoteWorkerNonLifeCycleOpController.ipdl

@@ -3,6 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 include RemoteWorkerTypes;
+include ServiceWorkerOpArgs;
 include SharedWorkerOpArgs;
 
 namespace mozilla {
@@ -20,6 +21,9 @@ parent:
 child:
   async ExecOp(SharedWorkerOpArgs aOpArgs);
 
+  async ExecServiceWorkerOp(ServiceWorkerOpArgs aOpArgs)
+      returns (ServiceWorkerOpResult aResult) ;
+
   async Shutdown();
 };
 

dom/workers/remoteworkers/RemoteWorkerController.cpp

@@ -121,7 +121,12 @@ void RemoteWorkerController::CreationSucceeded() {
   }
 
   MOZ_ASSERT(mActor);
-  MOZ_ASSERT(mNonLifeCycleOpController);
+
+  // mNonLifeCycleOpController could be already shutdown at the this moment.
+  // So no need to assert its existence.
+  // op->MaybeStart() will return true to ensure the op will not be in the
+  // mPendingOps
+
   mState = eReady;
 
   mObserver->CreationSucceeded();
@@ -184,9 +189,7 @@ void RemoteWorkerController::Shutdown() {
   CancelAllPendingOps();
 
   if (mNonLifeCycleOpController) {
-    if (mNonLifeCycleOpController->CanSend()) {
-      Unused << mNonLifeCycleOpController->SendShutdown();
-    }
+    mNonLifeCycleOpController->Shutdown();
     mNonLifeCycleOpController = nullptr;
   }
 
@@ -395,7 +398,14 @@ bool RemoteWorkerController::PendingSharedWorkerOp::MaybeStart(
       Unused << aOwner->mActor->SendExecOp(SharedWorkerThawOpArgs());
       break;
     case ePortIdentifier:
-      MOZ_ASSERT(aOwner->mNonLifeCycleOpController);
+      // mNonLifeCycleOpController can be nullptr if the Worker is in "Killing."
+      // RemoteWorkerNonLifeCycleOpControllerChild switches to the Killed status
+      // earlier than RemoteWorkerChild since it switches the status on the
+      // worker thread, not the main thread.
+      if (!aOwner->mNonLifeCycleOpController) {
+        Cancel();
+        return true;
+      }
       if (!aOwner->mNonLifeCycleOpController->CanSend()) {
         return false;
       }
@@ -486,21 +496,51 @@ bool RemoteWorkerController::PendingServiceWorkerOp::MaybeStart(
     return false;
   }
 
-  MaybeReportServiceWorkerShutdownProgress(mArgs);
+  switch (mArgs.type()) {
+    case ServiceWorkerOpArgs::TServiceWorkerTerminateWorkerOpArgs:
+    case ServiceWorkerOpArgs::TParentToChildServiceWorkerFetchEventOpArgs: {
+      MaybeReportServiceWorkerShutdownProgress(mArgs);
 
-  aOwner->mActor->SendExecServiceWorkerOp(mArgs)->Then(
-      GetCurrentSerialEventTarget(), __func__,
-      [promise = std::move(mPromise)](
-          PRemoteWorkerParent::ExecServiceWorkerOpPromise::
-              ResolveOrRejectValue&& aResult) {
-        if (NS_WARN_IF(aResult.IsReject())) {
-          promise->Reject(NS_ERROR_DOM_ABORT_ERR, __func__);
-          return;
-        }
+      aOwner->mActor->SendExecServiceWorkerOp(mArgs)->Then(
+          GetCurrentSerialEventTarget(), __func__,
+          [promise = std::move(mPromise)](
+              PRemoteWorkerParent::ExecServiceWorkerOpPromise::
+                  ResolveOrRejectValue&& aResult) {
+            if (NS_WARN_IF(aResult.IsReject())) {
+              promise->Reject(NS_ERROR_DOM_ABORT_ERR, __func__);
+              return;
+            }
 
-        promise->Resolve(std::move(aResult.ResolveValue()), __func__);
-      });
+            promise->Resolve(std::move(aResult.ResolveValue()), __func__);
+          });
+      break;
+    }
+    default: {
+      // mNonLifeCycleOpController can be nullptr if the Worker is in "Killing."
+      // RemoteWorkerNonLifeCycleOpControllerChild switches to the Killed status
+      // earlier than RemoteWorkerChild since it switches the status on the
+      // worker thread, not the main thread.
+      if (!aOwner->mNonLifeCycleOpController) {
+        Cancel();
+        return true;
+      }
+      if (!aOwner->mNonLifeCycleOpController->CanSend()) {
+        return false;
+      }
+      aOwner->mNonLifeCycleOpController->SendExecServiceWorkerOp(mArgs)->Then(
+          GetCurrentSerialEventTarget(), __func__,
+          [promise = std::move(mPromise)](
+              PRemoteWorkerParent::ExecServiceWorkerOpPromise::
+                  ResolveOrRejectValue&& aResult) {
+            if (NS_WARN_IF(aResult.IsReject())) {
+              promise->Reject(NS_ERROR_DOM_ABORT_ERR, __func__);
+              return;
+            }
 
+            promise->Resolve(std::move(aResult.ResolveValue()), __func__);
+          });
+    }
+  }
   return true;
 }
 

dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.cpp

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "RemoteWorkerNonLifeCycleOpControllerChild.h"
+#include "mozilla/dom/ServiceWorkerOp.h"
 #include "mozilla/dom/SharedWorkerOp.h"
 #include "mozilla/dom/WorkerCommon.h"
 #include "mozilla/dom/WorkerPrivate.h"
@@ -38,18 +39,20 @@ void RemoteWorkerNonLifeCycleOpControllerChild::TransistionStateToCanceled() {
   MOZ_ASSERT(lock->is<Running>());
 
   /*Canceling pending/processing operations here*/
-
   *lock = VariantType<Canceled>();
+
+  // SendTerminated() here, because after entering Canceling status, no
+  // nonlife-cycle related ServiceWorkerOp can be executed.
+  if (!CanSend()) {
+    return;
+  }
+  Unused << SendTerminated();
 }
 
 void RemoteWorkerNonLifeCycleOpControllerChild::TransistionStateToKilled() {
   auto lock = mState.Lock();
   MOZ_ASSERT(lock->is<Canceled>());
   *lock = VariantType<Killed>();
-  if (!CanSend()) {
-    return;
-  }
-  Unused << SendTerminated();
 }
 
 void RemoteWorkerNonLifeCycleOpControllerChild::ErrorPropagation(
@@ -77,6 +80,21 @@ IPCResult RemoteWorkerNonLifeCycleOpControllerChild::RecvExecOp(
   return IPC_OK();
 }
 
+IPCResult RemoteWorkerNonLifeCycleOpControllerChild::RecvExecServiceWorkerOp(
+    ServiceWorkerOpArgs&& aOpArgs, ExecServiceWorkerOpResolver&& aResolve) {
+  MOZ_ASSERT(
+      aOpArgs.type() !=
+          ServiceWorkerOpArgs::TParentToChildServiceWorkerFetchEventOpArgs,
+      "FetchEvent operations should be sent via PFetchEventOp(Proxy) actors!");
+
+  MOZ_ASSERT(aOpArgs.type() !=
+                 ServiceWorkerOpArgs::TServiceWorkerTerminateWorkerOpArgs,
+             "Terminate operations should be sent via PRemoteWorker actros!");
+
+  StartOp(ServiceWorkerOp::Create(std::move(aOpArgs), std::move(aResolve)));
+  return IPC_OK();
+}
+
 IPCResult RemoteWorkerNonLifeCycleOpControllerChild::RecvShutdown() {
   return IPC_OK();
 }

dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerChild.h

@@ -18,9 +18,12 @@ namespace mozilla::dom {
 
 using remoteworker::RemoteWorkerState;
 
+class ServiceWorkerOp;
+
 class RemoteWorkerNonLifeCycleOpControllerChild final
     : public PRemoteWorkerNonLifeCycleOpControllerChild {
   friend class PRemoteWorkerNonLifeCycleOpControllerChild;
+  friend class ServiceWorkerOp;
 
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(
@@ -32,6 +35,9 @@ class RemoteWorkerNonLifeCycleOpControllerChild final
 
   IPCResult RecvExecOp(SharedWorkerOpArgs&& aOpArgs);
 
+  IPCResult RecvExecServiceWorkerOp(ServiceWorkerOpArgs&& aOpArgs,
+                                    ExecServiceWorkerOpResolver&& aResolve);
+
   IPCResult RecvShutdown();
 
   void TransistionStateToCanceled();

dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.cpp

@@ -17,12 +17,18 @@ RemoteWorkerNonLifeCycleOpControllerParent::
 RemoteWorkerNonLifeCycleOpControllerParent::
     ~RemoteWorkerNonLifeCycleOpControllerParent() = default;
 
-IPCResult RemoteWorkerNonLifeCycleOpControllerParent::RecvTerminated() {
-  Unused << SendShutdown();
+void RemoteWorkerNonLifeCycleOpControllerParent::Shutdown() {
+  if (CanSend()) {
+    Unused << SendShutdown();
+  }
+  mController = nullptr;
+}
 
+IPCResult RemoteWorkerNonLifeCycleOpControllerParent::RecvTerminated() {
   // mController could be nullptr when the controller had already shutted down.
   if (mController) {
     mController->mNonLifeCycleOpController = nullptr;
+    mController = nullptr;
   }
 
   return IPC_OK();
@@ -35,4 +41,12 @@ IPCResult RemoteWorkerNonLifeCycleOpControllerParent::RecvError(
   return IPC_OK();
 }
 
+void RemoteWorkerNonLifeCycleOpControllerParent::ActorDestroy(
+    IProtocol::ActorDestroyReason) {
+  if (mController) {
+    mController->mNonLifeCycleOpController = nullptr;
+    mController = nullptr;
+  }
+}
+
 }  // namespace mozilla::dom

dom/workers/remoteworkers/RemoteWorkerNonLifeCycleOpControllerParent.h

@@ -26,6 +26,10 @@ class RemoteWorkerNonLifeCycleOpControllerParent final
 
   IPCResult RecvError(const ErrorValue& aError);
 
+  void ActorDestroy(IProtocol::ActorDestroyReason) override;
+
+  void Shutdown();
+
  private:
   ~RemoteWorkerNonLifeCycleOpControllerParent();
 

tools/fuzzing/ipc/IPCFuzzController.cpp

@@ -80,6 +80,7 @@ IPCFuzzController::IPCFuzzController()
   portNameToIndex["PRemoteLazyInputStream"] = 9;
   portNameToIndex["PRemoteWorkerService"] = 10;
   portNameToIndex["PBackgroundLSDatabase"] = 11;
+  portNameToIndex["PRemoteWorkerNonLifeCycleOpController"] = 12;
 
   // Used to select the n-th trigger message as a starting point for fuzzing
   // in single message mode. A value of 1 will skip the first matching message