Bug 1471631 Move ServiceWorkerManager::AddRegistrationEventListener() code to ServiceWorkerRegistrationInfo. r=mrbkap

dom/serviceworkers/ServiceWorkerManager.cpp

@@ -79,7 +79,6 @@
 #include "ServiceWorkerRegisterJob.h"
 #include "ServiceWorkerRegistrar.h"
 #include "ServiceWorkerRegistration.h"
-#include "ServiceWorkerRegistrationListener.h"
 #include "ServiceWorkerScriptCache.h"
 #include "ServiceWorkerEvents.h"
 #include "ServiceWorkerUnregisterJob.h"
@@ -1907,7 +1906,7 @@ ServiceWorkerManager::RemoveScopeAndRegistration(ServiceWorkerRegistrationInfo*
   swm->NotifyListenersOnUnregister(info);
 
   swm->MaybeRemoveRegistrationInfo(scopeKey);
-  swm->NotifyServiceWorkerRegistrationRemoved(aRegistration);
+  aRegistration->NotifyRemoved();
 }
 
 void
@@ -2007,64 +2006,6 @@ ServiceWorkerManager::GetScopeForUrl(nsIPrincipal* aPrincipal,
   return NS_OK;
 }
 
-NS_IMETHODIMP
-ServiceWorkerManager::AddRegistrationEventListener(const nsAString& aScope,
-                                                   ServiceWorkerRegistrationListener* aListener)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aListener);
-#ifdef DEBUG
-  // Ensure a registration is only listening for it's own scope.
-  nsAutoString regScope;
-  aListener->GetScope(regScope);
-  MOZ_ASSERT(!regScope.IsEmpty());
-  MOZ_ASSERT(aScope.Equals(regScope));
-#endif
-
-  MOZ_ASSERT(!mServiceWorkerRegistrationListeners.Contains(aListener));
-  mServiceWorkerRegistrationListeners.AppendElement(aListener);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-ServiceWorkerManager::RemoveRegistrationEventListener(const nsAString& aScope,
-                                                      ServiceWorkerRegistrationListener* aListener)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aListener);
-#ifdef DEBUG
-  // Ensure a registration is unregistering for it's own scope.
-  nsAutoString regScope;
-  aListener->GetScope(regScope);
-  MOZ_ASSERT(!regScope.IsEmpty());
-  MOZ_ASSERT(aScope.Equals(regScope));
-#endif
-
-  MOZ_ASSERT(mServiceWorkerRegistrationListeners.Contains(aListener));
-  mServiceWorkerRegistrationListeners.RemoveElement(aListener);
-  return NS_OK;
-}
-
-void
-ServiceWorkerManager::FireUpdateFoundOnServiceWorkerRegistrations(
-  ServiceWorkerRegistrationInfo* aRegistration)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mServiceWorkerRegistrationListeners);
-  while (it.HasMore()) {
-    RefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
-    nsAutoString regScope;
-    target->GetScope(regScope);
-    MOZ_ASSERT(!regScope.IsEmpty());
-
-    NS_ConvertUTF16toUTF8 utf8Scope(regScope);
-    if (utf8Scope.Equals(aRegistration->Scope())) {
-      target->UpdateFound();
-    }
-  }
-}
-
 namespace {
 
 class ContinueDispatchFetchEventRunnable : public Runnable
@@ -2366,38 +2307,6 @@ ServiceWorkerManager::GetClientRegistration(const ClientInfo& aClientInfo,
   return NS_OK;
 }
 
-void
-ServiceWorkerManager::UpdateRegistrationListeners(ServiceWorkerRegistrationInfo* aReg)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mServiceWorkerRegistrationListeners);
-  while (it.HasMore()) {
-    RefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
-    if (target->MatchesDescriptor(aReg->Descriptor())) {
-      target->UpdateState(aReg->Descriptor());
-    }
-  }
-}
-
-void
-ServiceWorkerManager::NotifyServiceWorkerRegistrationRemoved(ServiceWorkerRegistrationInfo* aRegistration)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mServiceWorkerRegistrationListeners);
-  while (it.HasMore()) {
-    RefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
-    nsAutoString regScope;
-    target->GetScope(regScope);
-    MOZ_ASSERT(!regScope.IsEmpty());
-
-    NS_ConvertUTF16toUTF8 utf8Scope(regScope);
-
-    if (utf8Scope.Equals(aRegistration->Scope())) {
-      target->RegistrationRemoved();
-    }
-  }
-}
-
 void
 ServiceWorkerManager::SoftUpdate(const OriginAttributes& aOriginAttributes,
                                  const nsACString& aScope)

dom/serviceworkers/ServiceWorkerManager.h

@@ -32,7 +32,6 @@
 #include "nsDataHashtable.h"
 #include "nsRefPtrHashtable.h"
 #include "nsTArrayForwardDeclare.h"
-#include "nsTObserverArray.h"
 
 class nsIConsoleReportCollector;
 
@@ -51,7 +50,6 @@ class ServiceWorkerJobQueue;
 class ServiceWorkerManagerChild;
 class ServiceWorkerPrivate;
 class ServiceWorkerRegistrar;
-class ServiceWorkerRegistrationListener;
 
 class ServiceWorkerUpdateFinishCallback
 {
@@ -102,8 +100,6 @@ public:
   struct RegistrationDataPerPrincipal;
   nsClassHashtable<nsCStringHashKey, RegistrationDataPerPrincipal> mRegistrationInfos;
 
-  nsTObserverArray<ServiceWorkerRegistrationListener*> mServiceWorkerRegistrationListeners;
-
   struct ControlledClientData
   {
     RefPtr<ClientHandle> mClientHandle;
@@ -311,14 +307,6 @@ public:
   ForceUnregister(RegistrationDataPerPrincipal* aRegistrationData,
                   ServiceWorkerRegistrationInfo* aRegistration);
 
-  NS_IMETHOD
-  AddRegistrationEventListener(const nsAString& aScope,
-                               ServiceWorkerRegistrationListener* aListener);
-
-  NS_IMETHOD
-  RemoveRegistrationEventListener(const nsAString& aScope,
-                                  ServiceWorkerRegistrationListener* aListener);
-
   void
   MaybeCheckNavigationUpdate(const ClientInfo& aClientInfo);
 
@@ -393,12 +381,6 @@ private:
   ServiceWorkerInfo*
   GetActiveWorkerInfoForDocument(nsIDocument* aDocument);
 
-  void
-  UpdateRegistrationListeners(ServiceWorkerRegistrationInfo* aReg);
-
-  void
-  NotifyServiceWorkerRegistrationRemoved(ServiceWorkerRegistrationInfo* aRegistration);
-
   void
   StopControllingRegistration(ServiceWorkerRegistrationInfo* aRegistration);
 
@@ -447,9 +429,6 @@ private:
   QueueFireEventOnServiceWorkerRegistrations(ServiceWorkerRegistrationInfo* aRegistration,
                                              const nsAString& aName);
 
-  void
-  FireUpdateFoundOnServiceWorkerRegistrations(ServiceWorkerRegistrationInfo* aRegistration);
-
   void
   UpdateClientControllers(ServiceWorkerRegistrationInfo* aRegistration);
 

dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp

@@ -61,11 +61,17 @@ ServiceWorkerRegistrationMainThread::StartListeningForEvents()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mListeningForEvents);
+  MOZ_DIAGNOSTIC_ASSERT(!mInfo);
+
   RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-  if (swm) {
-    swm->AddRegistrationEventListener(mScope, this);
-    mListeningForEvents = true;
-  }
+  NS_ENSURE_TRUE_VOID(swm);
+
+  mInfo = swm->GetRegistration(mDescriptor.PrincipalInfo(),
+                               mDescriptor.Scope());
+  NS_ENSURE_TRUE_VOID(mInfo);
+
+  mInfo->AddInstance(this);
+  mListeningForEvents = true;
 }
 
 void
@@ -76,10 +82,10 @@ ServiceWorkerRegistrationMainThread::StopListeningForEvents()
     return;
   }
 
-  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-  if (swm) {
-    swm->RemoveRegistrationEventListener(mScope, this);
-  }
+  MOZ_DIAGNOSTIC_ASSERT(mInfo);
+  mInfo->RemoveInstance(this);
+  mInfo = nullptr;
+
   mListeningForEvents = false;
 }
 
@@ -627,7 +633,8 @@ ServiceWorkerRegistrationMainThread::Unregister(ServiceWorkerBoolCallback&& aSuc
 
 class WorkerListener final : public ServiceWorkerRegistrationListener
 {
-  const nsString mScope;
+  ServiceWorkerRegistrationDescriptor mDescriptor;
+  nsMainThreadPtrHandle<ServiceWorkerRegistrationInfo> mInfo;
   bool mListeningForEvents;
 
   // Set and unset on worker thread, used on main-thread and protected by mutex.
@@ -639,8 +646,8 @@ public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(WorkerListener, override)
 
   WorkerListener(ServiceWorkerRegistrationWorkerThread* aReg,
-                 const nsAString& aScope)
-    : mScope(aScope)
+                 const ServiceWorkerRegistrationDescriptor& aDescriptor)
+    : mDescriptor(aDescriptor)
     , mListeningForEvents(false)
     , mRegistration(aReg)
     , mMutex("WorkerListener::mMutex")
@@ -653,13 +660,21 @@ public:
   StartListeningForEvents()
   {
     MOZ_ASSERT(NS_IsMainThread());
-    MOZ_ASSERT(!mListeningForEvents);
+    MOZ_DIAGNOSTIC_ASSERT(!mListeningForEvents);
+    MOZ_DIAGNOSTIC_ASSERT(!mInfo);
+
     RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-    if (swm) {
-      // FIXME(nsm): Maybe the function shouldn't take an explicit scope.
-      swm->AddRegistrationEventListener(mScope, this);
-      mListeningForEvents = true;
-    }
+    NS_ENSURE_TRUE_VOID(swm);
+
+    RefPtr<ServiceWorkerRegistrationInfo> info =
+      swm->GetRegistration(mDescriptor.PrincipalInfo(), mDescriptor.Scope());
+    NS_ENSURE_TRUE_VOID(info);
+
+    mInfo = new nsMainThreadPtrHolder<ServiceWorkerRegistrationInfo>(
+      "WorkerListener::mInfo", info);
+
+    mInfo->AddInstance(this);
+    mListeningForEvents = true;
   }
 
   void
@@ -671,13 +686,9 @@ public:
       return;
     }
 
-    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-
-    if (swm) {
-      // FIXME(nsm): Maybe the function shouldn't take an explicit scope.
-      swm->RemoveRegistrationEventListener(mScope, this);
-      mListeningForEvents = false;
-    }
+    MOZ_DIAGNOSTIC_ASSERT(mInfo);
+    mInfo->RemoveInstance(this);
+    mListeningForEvents = false;
   }
 
   // ServiceWorkerRegistrationListener
@@ -688,6 +699,7 @@ public:
   UpdateState(const ServiceWorkerRegistrationDescriptor& aDescriptor) override
   {
     MOZ_ASSERT(NS_IsMainThread());
+    mDescriptor = aDescriptor;
     // TODO: Not implemented
   }
 
@@ -697,7 +709,7 @@ public:
   void
   GetScope(nsAString& aScope) const override
   {
-    aScope = mScope;
+    CopyUTF8toUTF16(mDescriptor.Scope(), aScope);
   }
 
   bool
@@ -903,7 +915,7 @@ ServiceWorkerRegistrationWorkerThread::InitListener()
     return;
   }
 
-  mListener = new WorkerListener(this, mScope);
+  mListener = new WorkerListener(this, mDescriptor);
 
   nsCOMPtr<nsIRunnable> r =
     NewRunnableMethod("dom::WorkerListener::StartListeningForEvents",

dom/serviceworkers/ServiceWorkerRegistrationImpl.h

@@ -83,6 +83,7 @@ private:
 
   ServiceWorkerRegistration* mOuter;
   ServiceWorkerRegistrationDescriptor mDescriptor;
+  RefPtr<ServiceWorkerRegistrationInfo> mInfo;
   const nsString mScope;
   bool mListeningForEvents;
 };

dom/serviceworkers/ServiceWorkerRegistrationInfo.cpp

@@ -8,6 +8,7 @@
 
 #include "ServiceWorkerManager.h"
 #include "ServiceWorkerPrivate.h"
+#include "ServiceWorkerRegistrationListener.h"
 
 namespace mozilla {
 namespace dom {
@@ -113,6 +114,22 @@ ServiceWorkerRegistrationInfo::~ServiceWorkerRegistrationInfo()
   MOZ_DIAGNOSTIC_ASSERT(!IsControllingClients());
 }
 
+void
+ServiceWorkerRegistrationInfo::AddInstance(ServiceWorkerRegistrationListener* aInstance)
+{
+  MOZ_DIAGNOSTIC_ASSERT(aInstance);
+  MOZ_ASSERT(!mInstanceList.Contains(aInstance));
+  mInstanceList.AppendElement(aInstance);
+}
+
+void
+ServiceWorkerRegistrationInfo::RemoveInstance(ServiceWorkerRegistrationListener* aInstance)
+{
+  MOZ_DIAGNOSTIC_ASSERT(aInstance);
+  DebugOnly<bool> removed = mInstanceList.RemoveElement(aInstance);
+  MOZ_ASSERT(removed);
+}
+
 const nsCString&
 ServiceWorkerRegistrationInfo::Scope() const
 {
@@ -418,10 +435,11 @@ ServiceWorkerRegistrationInfo::UpdateRegistrationState()
 
   mDescriptor.SetWorkers(mInstallingWorker, mWaitingWorker, mActiveWorker);
 
-  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-  NS_ENSURE_TRUE_VOID(swm);
-
-  swm->UpdateRegistrationListeners(this);
+  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mInstanceList);
+  while (it.HasMore()) {
+    RefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
+    target->UpdateState(mDescriptor);
+  }
 }
 
 void
@@ -740,6 +758,26 @@ ServiceWorkerRegistrationInfo::GetUpdateDelay()
   return delay;
 }
 
+void
+ServiceWorkerRegistrationInfo::FireUpdateFound()
+{
+  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mInstanceList);
+  while (it.HasMore()) {
+    RefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
+    target->UpdateFound();
+  }
+}
+
+void
+ServiceWorkerRegistrationInfo::NotifyRemoved()
+{
+  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mInstanceList);
+  while (it.HasMore()) {
+    RefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
+    target->RegistrationRemoved();
+  }
+}
+
 // static
 uint64_t
 ServiceWorkerRegistrationInfo::GetNextId()

dom/serviceworkers/ServiceWorkerRegistrationInfo.h

@@ -11,16 +11,20 @@
 #include "mozilla/dom/ServiceWorkerRegistrationBinding.h"
 #include "mozilla/dom/ServiceWorkerRegistrationDescriptor.h"
 #include "nsProxyRelease.h"
+#include "nsTObserverArray.h"
 
 namespace mozilla {
 namespace dom {
 
+class ServiceWorkerRegistrationListener;
+
 class ServiceWorkerRegistrationInfo final
   : public nsIServiceWorkerRegistrationInfo
 {
   nsCOMPtr<nsIPrincipal> mPrincipal;
   ServiceWorkerRegistrationDescriptor mDescriptor;
   nsTArray<nsCOMPtr<nsIServiceWorkerRegistrationInfoListener>> mListeners;
+  nsTObserverArray<ServiceWorkerRegistrationListener*> mInstanceList;
 
   uint32_t mControlledClientsCounter;
   uint32_t mDelayMultiplier;
@@ -60,6 +64,12 @@ public:
                                 nsIPrincipal* aPrincipal,
                                 ServiceWorkerUpdateViaCache aUpdateViaCache);
 
+  void
+  AddInstance(ServiceWorkerRegistrationListener* aInstance);
+
+  void
+  RemoveInstance(ServiceWorkerRegistrationListener* aInstance);
+
   const nsCString&
   Scope() const;
 
@@ -227,6 +237,12 @@ public:
   uint32_t
   GetUpdateDelay();
 
+  void
+  FireUpdateFound();
+
+  void
+  NotifyRemoved();
+
 private:
   // Roughly equivalent to [[Update Registration State algorithm]]. Make sure
   // this is called *before* updating SW instances' state, otherwise they

dom/serviceworkers/ServiceWorkerUpdateJob.cpp

@@ -515,15 +515,14 @@ ServiceWorkerUpdateJob::ContinueUpdateAfterScriptEval(bool aScriptEvaluationResu
     return;
   }
 
-  Install(swm);
+  Install();
 }
 
 void
-ServiceWorkerUpdateJob::Install(ServiceWorkerManager* aSWM)
+ServiceWorkerUpdateJob::Install()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(!Canceled());
-  MOZ_DIAGNOSTIC_ASSERT(aSWM);
 
   MOZ_ASSERT(!mRegistration->GetInstalling());
 
@@ -540,13 +539,10 @@ ServiceWorkerUpdateJob::Install(ServiceWorkerManager* aSWM)
   // still fail; e.g. if the install event handler throws, etc.
 
   // fire the updatefound event
-  nsCOMPtr<nsIRunnable> upr =
-    NewRunnableMethod<RefPtr<ServiceWorkerRegistrationInfo>>(
-      "dom::ServiceWorkerManager::"
-      "FireUpdateFoundOnServiceWorkerRegistrations",
-      aSWM,
-      &ServiceWorkerManager::FireUpdateFoundOnServiceWorkerRegistrations,
-      mRegistration);
+  nsCOMPtr<nsIRunnable> upr = NewRunnableMethod(
+      "ServiceWorkerRegistrationInfo::FireUpdateFound",
+      mRegistration,
+      &ServiceWorkerRegistrationInfo::FireUpdateFound);
   NS_DispatchToMainThread(upr);
 
   nsMainThreadPtrHandle<ServiceWorkerUpdateJob> handle(

dom/serviceworkers/ServiceWorkerUpdateJob.h

@@ -99,7 +99,7 @@ private:
 
   // Utility method corresponding to the spec Install algorithm.
   void
-  Install(ServiceWorkerManager* aSWM);
+  Install();
 
   // Utility method called after the install event is handled.
   void