Bug 1231213 - Only spawn Service Workers in non-web-extensions processes that won't imminently shutdown. r=asuth

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

--HG--
extra : moz-landing-system : lando

dom/ipc/ContentParent.cpp

@@ -1382,6 +1382,32 @@ RemoteWindowContext::GetUsePrivateBrowsing(bool* aUsePrivateBrowsing) {
 
 }  // namespace
 
+void ContentParent::MaybeAsyncSendShutDownMessage() {
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(!TryToRecycle());
+
+#ifdef DEBUG
+  // Calling this below while the lock is acquired will deadlock.
+  bool shouldKeepProcessAlive = ShouldKeepProcessAlive();
+#endif
+
+  auto lock = mRemoteWorkerActorData.Lock();
+  MOZ_ASSERT_IF(!lock->mCount, !shouldKeepProcessAlive);
+
+  if (lock->mCount) {
+    return;
+  }
+
+  MOZ_ASSERT(!lock->mShutdownStarted);
+  lock->mShutdownStarted = true;
+
+  // In the case of normal shutdown, send a shutdown message to child to
+  // allow it to perform shutdown tasks.
+  MessageLoop::current()->PostTask(NewRunnableMethod<ShutDownMethod>(
+      "dom::ContentParent::ShutDownProcess", this,
+      &ContentParent::ShutDownProcess, SEND_SHUTDOWN_MESSAGE));
+}
+
 void ContentParent::ShutDownProcess(ShutDownMethod aMethod) {
   if (mScriptableHelper) {
     static_cast<ScriptableCPInfo*>(mScriptableHelper.get())->ProcessDied();
@@ -1726,14 +1752,17 @@ bool ContentParent::TryToRecycle() {
   return true;
 }
 
-bool ContentParent::ShouldKeepProcessAlive() const {
+bool ContentParent::ShouldKeepProcessAlive() {
   if (IsForJSPlugin()) {
     return true;
   }
 
   // If we have active workers, we need to stay alive.
-  if (mRemoteWorkerActors) {
-    return true;
+  {
+    const auto lock = mRemoteWorkerActorData.Lock();
+    if (lock->mCount) {
+      return true;
+    }
   }
 
   if (!sBrowserContentParents) {
@@ -1845,11 +1874,7 @@ void ContentParent::NotifyTabDestroyed(const TabId& aTabId,
   // us down.
   if (ManagedPBrowserParent().Count() == 1 && !ShouldKeepProcessAlive() &&
       !TryToRecycle()) {
-    // In the case of normal shutdown, send a shutdown message to child to
-    // allow it to perform shutdown tasks.
-    MessageLoop::current()->PostTask(NewRunnableMethod<ShutDownMethod>(
-        "dom::ContentParent::ShutDownProcess", this,
-        &ContentParent::ShutDownProcess, SEND_SHUTDOWN_MESSAGE));
+    MaybeAsyncSendShutDownMessage();
   }
 }
 
@@ -2274,7 +2299,7 @@ ContentParent::ContentParent(ContentParent* aOpener,
       mChildID(gContentChildID++),
       mGeolocationWatchID(-1),
       mJSPluginID(aJSPluginID),
-      mRemoteWorkerActors(0),
+      mRemoteWorkerActorData("ContentParent::mRemoteWorkerActorData"),
       mNumDestroyingTabs(0),
       mLifecycleState(LifecycleState::LAUNCHING),
       mIsForBrowser(!mRemoteType.IsEmpty()),
@@ -5909,23 +5934,25 @@ mozilla::ipc::IPCResult ContentParent::RecvRestoreBrowsingContextChildren(
   return IPC_OK();
 }
 
-void ContentParent::RegisterRemoteWorkerActor() { ++mRemoteWorkerActors; }
+void ContentParent::RegisterRemoteWorkerActor() {
+  auto lock = mRemoteWorkerActorData.Lock();
+  ++lock->mCount;
+}
 
 void ContentParent::UnregisterRemoveWorkerActor() {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (--mRemoteWorkerActors) {
-    return;
+  {
+    auto lock = mRemoteWorkerActorData.Lock();
+    if (--lock->mCount) {
+      return;
+    }
   }
 
   ContentProcessManager* cpm = ContentProcessManager::GetSingleton();
   if (!cpm->GetBrowserParentCountByProcessId(ChildID()) &&
       !ShouldKeepProcessAlive() && !TryToRecycle()) {
-    // In the case of normal shutdown, send a shutdown message to child to
-    // allow it to perform shutdown tasks.
-    MessageLoop::current()->PostTask(NewRunnableMethod<ShutDownMethod>(
-        "dom::ContentParent::ShutDownProcess", this,
-        &ContentParent::ShutDownProcess, SEND_SHUTDOWN_MESSAGE));
+    MaybeAsyncSendShutDownMessage();
   }
 }
 

dom/ipc/ContentParent.h

@@ -18,6 +18,7 @@
 #include "mozilla/ipc/PParentToChildStreamParent.h"
 #include "mozilla/ipc/PChildToParentStreamParent.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/DataMutex.h"
 #include "mozilla/FileUtils.h"
 #include "mozilla/HalTypes.h"
 #include "mozilla/LinkedList.h"
@@ -114,6 +115,7 @@ class MemoryReport;
 class TabContext;
 class GetFilesHelper;
 class MemoryReportRequestHost;
+class RemoteWorkerManager;
 struct CancelContentJSOptions;
 
 #define NS_CONTENTPARENT_IID                         \
@@ -145,6 +147,7 @@ class ContentParent final : public PContentParent,
 
   friend class mozilla::PreallocatedProcessManagerImpl;
   friend class PContentParent;
+  friend class mozilla::dom::RemoteWorkerManager;
 #ifdef FUZZING
   friend class mozilla::ipc::ProtocolFuzzerHelper;
 #endif
@@ -748,7 +751,7 @@ class ContentParent final : public PContentParent,
    * Decide whether the process should be kept alive even when it would normally
    * be shut down, for example when all its tabs are closed.
    */
-  bool ShouldKeepProcessAlive() const;
+  bool ShouldKeepProcessAlive();
 
   /**
    * Mark this ContentParent as "troubled". This means that it is still alive,
@@ -774,6 +777,8 @@ class ContentParent final : public PContentParent,
     CLOSE_CHANNEL_WITH_ERROR,
   };
 
+  void MaybeAsyncSendShutDownMessage();
+
   /**
    * Exit the subprocess and vamoose.  After this call IsAlive()
    * will return false and this ContentParent will not be returned
@@ -1236,11 +1241,24 @@ class ContentParent final : public PContentParent,
   // timer.
   nsCOMPtr<nsITimer> mForceKillTimer;
 
-  // Number of active remote workers. This value is increased when a
-  // RemoteWorkerParent actor is created for this ContentProcess and it is
-  // decreased when the actor is destroyed.
+  // `mCount` is increased when a RemoteWorkerParent actor is created for this
+  // ContentProcess and it is decreased when the actor is destroyed.
+  //
+  // `mShutdownStarted` is flipped to `true` when a runnable that calls
+  // `ShutDownProcess` is dispatched; it's needed because the corresponding
+  // Content Process may be shutdown if there's no remote worker actors, and
+  // decrementing `mCount` and the call to `ShutDownProcess` are async. So,
+  // when a worker is going to be spawned and we see that `mCount` is 0,
+  // we can decide whether or not to use that process based on the value of
+  // `mShutdownStarted.`
+  //
   // It's touched on PBackground thread and on main-thread.
-  Atomic<uint32_t> mRemoteWorkerActors;
+  struct RemoteWorkerActorData {
+    uint32_t mCount = 0;
+    bool mShutdownStarted = false;
+  };
+
+  DataMutex<RemoteWorkerActorData> mRemoteWorkerActorData;
 
   // How many tabs we're waiting to finish their destruction
   // sequence.  Precisely, how many BrowserParents have called

dom/workers/remoteworkers/RemoteWorkerManager.cpp

@@ -6,11 +6,19 @@
 
 #include "RemoteWorkerManager.h"
 
+#include <utility>
+
+#include "mozilla/RefPtr.h"
+#include "mozilla/ScopeExit.h"
+#include "mozilla/SystemGroup.h"
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/RemoteWorkerParent.h"
 #include "mozilla/ipc/BackgroundParent.h"
 #include "mozilla/ipc/PBackgroundParent.h"
+#include "nsCOMPtr.h"
 #include "nsIXULRuntime.h"
+#include "nsTArray.h"
+#include "nsThreadUtils.h"
 #include "RemoteWorkerServiceParent.h"
 
 namespace mozilla {
@@ -25,12 +33,17 @@ namespace {
 // actors.
 RemoteWorkerManager* sRemoteWorkerManager;
 
+bool IsServiceWorker(const RemoteWorkerData& aData) {
+  return aData.serviceWorkerData().type() ==
+         OptionalServiceWorkerData::TServiceWorkerData;
+}
+
 }  // namespace
 
 /* static */
 already_AddRefed<RemoteWorkerManager> RemoteWorkerManager::GetOrCreate() {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
 
   if (!sRemoteWorkerManager) {
     sRemoteWorkerManager = new RemoteWorkerManager();
@@ -41,21 +54,21 @@ already_AddRefed<RemoteWorkerManager> RemoteWorkerManager::GetOrCreate() {
 }
 
 RemoteWorkerManager::RemoteWorkerManager() : mParentActor(nullptr) {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(!sRemoteWorkerManager);
 }
 
 RemoteWorkerManager::~RemoteWorkerManager() {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(sRemoteWorkerManager == this);
   sRemoteWorkerManager = nullptr;
 }
 
 void RemoteWorkerManager::RegisterActor(RemoteWorkerServiceParent* aActor) {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(aActor);
 
   if (!BackgroundParent::IsOtherProcessActor(aActor->Manager())) {
@@ -83,8 +96,8 @@ void RemoteWorkerManager::RegisterActor(RemoteWorkerServiceParent* aActor) {
 }
 
 void RemoteWorkerManager::UnregisterActor(RemoteWorkerServiceParent* aActor) {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(aActor);
 
   if (aActor == mParentActor) {
@@ -98,8 +111,8 @@ void RemoteWorkerManager::UnregisterActor(RemoteWorkerServiceParent* aActor) {
 void RemoteWorkerManager::Launch(RemoteWorkerController* aController,
                                  const RemoteWorkerData& aData,
                                  base::ProcessId aProcessId) {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
 
   RemoteWorkerServiceParent* targetActor = SelectTargetActor(aData, aProcessId);
 
@@ -120,14 +133,20 @@ void RemoteWorkerManager::Launch(RemoteWorkerController* aController,
     return;
   }
 
-  LaunchInternal(aController, targetActor, aData);
+  /**
+   * If a target actor for a Service Worker has been selected, the actor has
+   * already been registered with the corresponding ContentParent (see
+   * `SelectTargetActorForServiceWorker()`).
+   */
+  LaunchInternal(aController, targetActor, aData, IsServiceWorker(aData));
 }
 
 void RemoteWorkerManager::LaunchInternal(
     RemoteWorkerController* aController,
-    RemoteWorkerServiceParent* aTargetActor, const RemoteWorkerData& aData) {
+    RemoteWorkerServiceParent* aTargetActor, const RemoteWorkerData& aData,
+    bool aRemoteWorkerAlreadyRegistered) {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(aController);
   MOZ_ASSERT(aTargetActor);
   MOZ_ASSERT(aTargetActor == mParentActor ||
@@ -140,7 +159,7 @@ void RemoteWorkerManager::LaunchInternal(
     return;
   }
 
-  workerActor->Initialize();
+  workerActor->Initialize(aRemoteWorkerAlreadyRegistered);
 
   // This makes the link better the 2 actors.
   aController->SetWorkerActor(workerActor);
@@ -157,10 +176,75 @@ void RemoteWorkerManager::AsyncCreationFailed(
   NS_DispatchToCurrentThread(r.forget());
 }
 
+/**
+ * Service Workers can spawn even when their registering page/script isn't
+ * active (e.g. push notifications), so we don't attempt to spawn the worker
+ * in its registering script's process. We search linearly and choose the
+ * search's starting position randomly.
+ *
+ * Spawning the workers in a random process makes the process selection criteria
+ * a little tricky, as a candidate process may imminently shutdown due to a
+ * remove worker actor unregistering
+ * (see `ContentParent::UnregisterRemoveWorkerActor`).
+ *
+ * In `ContentParent::MaybeAsyncSendShutDownMessage` we only dispatch a runnable
+ * to call `ContentParent::ShutDownProcess` if there are no registered remote
+ * worker actors, and we ensure that the check for the number of registered
+ * actors and the dispatching of the runnable are atomic. That happens on the
+ * main thread, so here on the background thread,  while
+ * `ContentParent::mRemoteWorkerActorData` is locked, if `mCount` > 0, we can
+ * register a remote worker actor "early" and guarantee that the corresponding
+ * content process will not shutdown.
+ */
+RemoteWorkerServiceParent*
+RemoteWorkerManager::SelectTargetActorForServiceWorker() const {
+  AssertIsInMainProcess();
+  AssertIsOnBackgroundThread();
+  MOZ_ASSERT(!mChildActors.IsEmpty());
+
+  nsTArray<RefPtr<ContentParent>> contentParents;
+
+  auto scopeExit = MakeScopeExit([&] {
+    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
+        __func__,
+        [doomed = std::move(contentParents)]() mutable { doomed.Clear(); });
+
+    SystemGroup::Dispatch(TaskCategory::Other, r.forget());
+  });
+
+  uint32_t random = uint32_t(rand() % mChildActors.Length());
+  uint32_t i = random;
+
+  do {
+    auto actor = mChildActors[i];
+    PBackgroundParent* bgParent = actor->Manager();
+    MOZ_ASSERT(bgParent);
+
+    RefPtr<ContentParent> contentParent =
+        BackgroundParent::GetContentParent(bgParent);
+
+    auto scopeExit = MakeScopeExit(
+        [&] { contentParents.AppendElement(std::move(contentParent)); });
+
+    if (contentParent->GetRemoteType().EqualsLiteral(DEFAULT_REMOTE_TYPE)) {
+      auto lock = contentParent->mRemoteWorkerActorData.Lock();
+
+      if (lock->mCount || !lock->mShutdownStarted) {
+        ++lock->mCount;
+        return actor;
+      }
+    }
+
+    i = (i + 1) % mChildActors.Length();
+  } while (i != random);
+
+  return nullptr;
+}
+
 RemoteWorkerServiceParent* RemoteWorkerManager::SelectTargetActor(
     const RemoteWorkerData& aData, base::ProcessId aProcessId) {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
 
   // System principal workers should run on the parent process.
   if (aData.principalInfo().type() == PrincipalInfo::TSystemPrincipalInfo) {
@@ -181,6 +265,10 @@ RemoteWorkerServiceParent* RemoteWorkerManager::SelectTargetActor(
     return nullptr;
   }
 
+  if (IsServiceWorker(aData)) {
+    return SelectTargetActorForServiceWorker();
+  }
+
   for (RemoteWorkerServiceParent* actor : mChildActors) {
     // Let's execute the RemoteWorker on the same process.
     if (actor->OtherPid() == aProcessId) {
@@ -194,8 +282,8 @@ RemoteWorkerServiceParent* RemoteWorkerManager::SelectTargetActor(
 }
 
 void RemoteWorkerManager::LaunchNewContentProcess() {
+  AssertIsInMainProcess();
   AssertIsOnBackgroundThread();
-  MOZ_ASSERT(XRE_IsParentProcess());
 
   // This runnable will spawn a new process if it doesn't exist yet.
   nsCOMPtr<nsIRunnable> r =

dom/workers/remoteworkers/RemoteWorkerManager.h

@@ -39,9 +39,12 @@ class RemoteWorkerManager final {
   RemoteWorkerServiceParent* SelectTargetActor(const RemoteWorkerData& aData,
                                                base::ProcessId aProcessId);
 
+  RemoteWorkerServiceParent* SelectTargetActorForServiceWorker() const;
+
   void LaunchInternal(RemoteWorkerController* aController,
                       RemoteWorkerServiceParent* aTargetActor,
-                      const RemoteWorkerData& aData);
+                      const RemoteWorkerData& aData,
+                      bool aRemoteWorkerAlreadyRegistered = false);
 
   void LaunchNewContentProcess();
 

dom/workers/remoteworkers/RemoteWorkerParent.cpp

@@ -53,12 +53,14 @@ RemoteWorkerParent::~RemoteWorkerParent() {
   MOZ_ASSERT(XRE_IsParentProcess());
 }
 
-void RemoteWorkerParent::Initialize() {
+void RemoteWorkerParent::Initialize(bool aAlreadyRegistered) {
   RefPtr<ContentParent> parent = BackgroundParent::GetContentParent(Manager());
 
   // Parent is null if the child actor runs on the parent process.
   if (parent) {
-    parent->RegisterRemoteWorkerActor();
+    if (!aAlreadyRegistered) {
+      parent->RegisterRemoteWorkerActor();
+    }
 
     nsCOMPtr<nsIEventTarget> target =
         SystemGroup::EventTargetFor(TaskCategory::Other);

dom/workers/remoteworkers/RemoteWorkerParent.h

@@ -22,7 +22,7 @@ class RemoteWorkerParent final : public PRemoteWorkerParent {
 
   RemoteWorkerParent();
 
-  void Initialize();
+  void Initialize(bool aAlreadyRegistered = false);
 
   void SetController(RemoteWorkerController* aController);