Bug 1588152 - Eliminate redundant profile-change-teardown observer. r=asuth

Under parent intercept, we had both an nsIAsyncShutdownClient and a manual
observer notification used by ServiceWorkerManager for shutdown.  (Under
child intercept, the observer notification used a different phase, but we no
longer support child intercept.)  If the observer notification didn't fire
prior to the shutdown blocker, deadlock would result at shutdown.

This patch addresses the problem by changing us to only use the shutdown blocker
for profile-change-teardown, with it taking on the responsibility of calling
the relevant ServiceWorkerManager shutdown logic.

We do leave a finish shutdown observer notification in place.  It's added
dynamically during shutdown (as long as we can get the observer service)
which I believe should allow the observer to run prior to the QuotaManager
profile-before-change-qm receiving its notification due to the reverse
observer notification.  But that isn't required to avoid deadlocking.

As part of these changes, I also removed the configurable logic around shutdown
that existed to support child intercept.  Note that this is not a comprehensive
cleanup, just a normalization of shutdown to require/assume parent intercept.
The most notable aspect of this change is explicitly bailing out of
ServiceWorkerManager initialization if we are not in the parent process.

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

dom/serviceworkers/ServiceWorkerManager.cpp

@@ -237,27 +237,11 @@ class TeardownRunnable final : public Runnable {
   RefPtr<ServiceWorkerManagerChild> mActor;
 };
 
-bool ServiceWorkersAreCrossProcess() {
-  return ServiceWorkerParentInterceptEnabled() && XRE_IsE10sParentProcess();
-}
-
-const char* GetStartShutdownTopic() {
-  if (ServiceWorkersAreCrossProcess()) {
-    return "profile-change-teardown";
-  }
-
-  return NS_XPCOM_SHUTDOWN_OBSERVER_ID;
-}
-
 constexpr char kFinishShutdownTopic[] = "profile-before-change-qm";
 
 already_AddRefed<nsIAsyncShutdownClient> GetAsyncShutdownBarrier() {
   AssertIsOnMainThread();
 
-  if (!ServiceWorkersAreCrossProcess()) {
-    return nullptr;
-  }
-
   nsCOMPtr<nsIAsyncShutdownService> svc = services::GetAsyncShutdownService();
   MOZ_ASSERT(svc);
 
@@ -431,10 +415,6 @@ ServiceWorkerManager::~ServiceWorkerManager() {
   // The map will assert if it is not empty when destroyed.
   mRegistrationInfos.Clear();
 
-  if (!ServiceWorkersAreCrossProcess()) {
-    MOZ_ASSERT(!mActor);
-  }
-
   // This can happen if the browser is started up in ProfileManager mode, in
   // which case XPCOM will startup and shutdown, but there won't be any
   // profile-* topic notifications. The shutdown blocker expects to be in a
@@ -450,11 +430,6 @@ void ServiceWorkerManager::BlockShutdownOn(GenericNonExclusivePromise* aPromise,
                                            uint32_t aShutdownStateId) {
   AssertIsOnMainThread();
 
-  // This may be called when in non-e10s mode with parent-intercept enabled.
-  if (!ServiceWorkersAreCrossProcess()) {
-    return;
-  }
-
   MOZ_ASSERT(mShutdownBlocker);
   MOZ_ASSERT(aPromise);
 
@@ -462,28 +437,32 @@ void ServiceWorkerManager::BlockShutdownOn(GenericNonExclusivePromise* aPromise,
 }
 
 void ServiceWorkerManager::Init(ServiceWorkerRegistrar* aRegistrar) {
+  // ServiceWorkers now only support parent intercept.  In parent intercept
+  // mode, only the parent process ServiceWorkerManager has any state or does
+  // anything.
+  //
+  // It is our goal to completely eliminate support for content process
+  // ServiceWorkerManager instances and make getting a SWM instance trigger a
+  // fatal assertion.  But until we've reached that point, we make
+  // initialization a no-op so that content process ServiceWorkerManager
+  // instances will simply have no state and no registrations.
+  if (!XRE_IsParentProcess()) {
+    return;
+  }
+
   nsCOMPtr<nsIAsyncShutdownClient> shutdownBarrier = GetAsyncShutdownBarrier();
 
   if (shutdownBarrier) {
-    mShutdownBlocker =
-        ServiceWorkerShutdownBlocker::CreateAndRegisterOn(shutdownBarrier);
+    mShutdownBlocker = ServiceWorkerShutdownBlocker::CreateAndRegisterOn(
+        *shutdownBarrier, *this);
     MOZ_ASSERT(mShutdownBlocker);
   }
 
-  nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
-  if (obs) {
-    DebugOnly<nsresult> rv;
-    rv = obs->AddObserver(this, GetStartShutdownTopic(), false /* ownsWeak */);
-    MOZ_ASSERT(NS_SUCCEEDED(rv));
-  }
+  MOZ_DIAGNOSTIC_ASSERT(aRegistrar);
 
-  if (XRE_IsParentProcess()) {
-    MOZ_DIAGNOSTIC_ASSERT(aRegistrar);
-
-    nsTArray<ServiceWorkerRegistrationData> data;
-    aRegistrar->GetRegistrations(data);
-    LoadRegistrations(data);
-  }
+  nsTArray<ServiceWorkerRegistrationData> data;
+  aRegistrar->GetRegistrations(data);
+  LoadRegistrations(data);
 
   PBackgroundChild* actorChild = BackgroundChild::GetOrCreateForCurrentThread();
   if (NS_WARN_IF(!actorChild)) {
@@ -643,12 +622,8 @@ void ServiceWorkerManager::MaybeStartShutdown() {
 
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (obs) {
-    obs->RemoveObserver(this, GetStartShutdownTopic());
-
-    if (ServiceWorkersAreCrossProcess()) {
-      obs->AddObserver(this, kFinishShutdownTopic, false);
-      return;
-    }
+    obs->AddObserver(this, kFinishShutdownTopic, false);
+    return;
   }
 
   MaybeFinishShutdown();
@@ -3168,11 +3143,6 @@ ServiceWorkerManager::RemoveListener(
 NS_IMETHODIMP
 ServiceWorkerManager::Observe(nsISupports* aSubject, const char* aTopic,
                               const char16_t* aData) {
-  if (strcmp(aTopic, GetStartShutdownTopic()) == 0) {
-    MaybeStartShutdown();
-    return NS_OK;
-  }
-
   if (strcmp(aTopic, kFinishShutdownTopic) == 0) {
     MaybeFinishShutdown();
     return NS_OK;

dom/serviceworkers/ServiceWorkerManager.h

@@ -104,6 +104,7 @@ class ServiceWorkerManager final : public nsIServiceWorkerManager,
   friend class GetRegistrationRunnable;
   friend class ServiceWorkerJob;
   friend class ServiceWorkerRegistrationInfo;
+  friend class ServiceWorkerShutdownBlocker;
   friend class ServiceWorkerUnregisterJob;
   friend class ServiceWorkerUpdateJob;
   friend class UpdateTimerCallback;

dom/serviceworkers/ServiceWorkerShutdownBlocker.cpp

@@ -14,6 +14,7 @@
 #include "nsError.h"
 #include "nsIWritablePropertyBag2.h"
 #include "nsThreadUtils.h"
+#include "ServiceWorkerManager.h"
 
 #include "mozilla/Assertions.h"
 #include "mozilla/RefPtr.h"
@@ -33,9 +34,13 @@ NS_IMETHODIMP
 ServiceWorkerShutdownBlocker::BlockShutdown(nsIAsyncShutdownClient* aClient) {
   AssertIsOnMainThread();
   MOZ_ASSERT(!mShutdownClient);
+  MOZ_ASSERT(mServiceWorkerManager);
 
   mShutdownClient = aClient;
 
+  (*mServiceWorkerManager)->MaybeStartShutdown();
+  mServiceWorkerManager.destroy();
+
   MaybeUnblockShutdown();
   MaybeInitUnblockShutdownTimer();
 
@@ -87,14 +92,14 @@ NS_IMETHODIMP ServiceWorkerShutdownBlocker::GetState(nsIPropertyBag** aBagOut) {
 
 /* static */ already_AddRefed<ServiceWorkerShutdownBlocker>
 ServiceWorkerShutdownBlocker::CreateAndRegisterOn(
-    nsIAsyncShutdownClient* aShutdownBarrier) {
+    nsIAsyncShutdownClient& aShutdownBarrier,
+    ServiceWorkerManager& aServiceWorkerManager) {
   AssertIsOnMainThread();
-  MOZ_ASSERT(aShutdownBarrier);
 
   RefPtr<ServiceWorkerShutdownBlocker> blocker =
-      new ServiceWorkerShutdownBlocker();
+      new ServiceWorkerShutdownBlocker(aServiceWorkerManager);
 
-  nsresult rv = aShutdownBarrier->AddBlocker(
+  nsresult rv = aShutdownBarrier.AddBlocker(
       blocker.get(), NS_LITERAL_STRING_FROM_CSTRING(__FILE__), __LINE__,
       u"Service Workers shutdown"_ns);
 
@@ -170,8 +175,10 @@ void ServiceWorkerShutdownBlocker::ReportShutdownProgress(
   }
 }
 
-ServiceWorkerShutdownBlocker::ServiceWorkerShutdownBlocker()
-    : mState(VariantType<AcceptingPromises>()) {
+ServiceWorkerShutdownBlocker::ServiceWorkerShutdownBlocker(
+    ServiceWorkerManager& aServiceWorkerManager)
+    : mState(VariantType<AcceptingPromises>()),
+      mServiceWorkerManager(WrapNotNull(&aServiceWorkerManager)) {
   AssertIsOnMainThread();
 }
 
@@ -179,6 +186,7 @@ ServiceWorkerShutdownBlocker::~ServiceWorkerShutdownBlocker() {
   MOZ_ASSERT(!IsAcceptingPromises());
   MOZ_ASSERT(!GetPendingPromises());
   MOZ_ASSERT(!mShutdownClient);
+  MOZ_ASSERT(!mServiceWorkerManager);
 }
 
 void ServiceWorkerShutdownBlocker::MaybeUnblockShutdown() {

dom/serviceworkers/ServiceWorkerShutdownBlocker.h

@@ -13,12 +13,16 @@
 #include "nsITimer.h"
 
 #include "ServiceWorkerShutdownState.h"
+#include "mozilla/InitializedOnce.h"
 #include "mozilla/MozPromise.h"
+#include "mozilla/NotNull.h"
 #include "mozilla/HashTable.h"
 
 namespace mozilla {
 namespace dom {
 
+class ServiceWorkerManager;
+
 /**
  * Main thread only.
  *
@@ -52,7 +56,8 @@ class ServiceWorkerShutdownBlocker final : public nsIAsyncShutdownBlocker,
    * nullptr otherwise.
    */
   static already_AddRefed<ServiceWorkerShutdownBlocker> CreateAndRegisterOn(
-      nsIAsyncShutdownClient* aShutdownBarrier);
+      nsIAsyncShutdownClient& aShutdownBarrier,
+      ServiceWorkerManager& aServiceWorkerManager);
 
   /**
    * Blocks shutdown until `aPromise` settles.
@@ -86,7 +91,8 @@ class ServiceWorkerShutdownBlocker final : public nsIAsyncShutdownBlocker,
   void ReportShutdownProgress(uint32_t aShutdownStateId, Progress aProgress);
 
  private:
-  ServiceWorkerShutdownBlocker();
+  explicit ServiceWorkerShutdownBlocker(
+      ServiceWorkerManager& aServiceWorkerManager);
 
   ~ServiceWorkerShutdownBlocker();
 
@@ -140,6 +146,9 @@ class ServiceWorkerShutdownBlocker final : public nsIAsyncShutdownBlocker,
   HashMap<uint32_t, ServiceWorkerShutdownState> mShutdownStates;
 
   nsCOMPtr<nsITimer> mTimer;
+  LazyInitializedOnceEarlyDestructible<
+      const NotNull<RefPtr<ServiceWorkerManager>>>
+      mServiceWorkerManager;
 };
 
 }  // namespace dom