Bug 1131327 - Patch 6 - SWM holds ServiceWorkerRegistrationListener for updatefound and invalidation notifications. r=baku

Splits up the listener interface to which SWM can hold rawptrs so that registration objects living on the worker thread can use awkward proxies to listen to these events.

dom/interfaces/base/nsIServiceWorkerManager.idl

@@ -33,7 +33,7 @@ interface nsIServiceWorkerInfo : nsISupports
   readonly attribute DOMString waitingCacheName;
 };
 
-[scriptable, builtinclass, uuid(7e3c44bd-112f-4668-83a6-37d1089bb1f8)]
+[scriptable, builtinclass, uuid(ff6e13ae-ae34-4941-a81e-a82f3e0e7b6b)]
 interface nsIServiceWorkerManager : nsISupports
 {
   /**
@@ -76,10 +76,6 @@ interface nsIServiceWorkerManager : nsISupports
   void dispatchFetchEvent(in nsIDocument aDoc, in nsIInterceptedChannel aChannel,
                           in boolean aIsReload);
 
-  // aTarget MUST be a ServiceWorkerRegistration.
-  [noscript] void AddRegistrationEventListener(in DOMString aScope, in nsIDOMEventTarget aTarget);
-  [noscript] void RemoveRegistrationEventListener(in DOMString aScope, in nsIDOMEventTarget aTarget);
-
   /**
    * Call this to request that document `aDoc` be controlled by a ServiceWorker
    * if a registration exists for it's scope.

dom/workers/ServiceWorkerManager.cpp

@@ -702,9 +702,11 @@ public:
 
     // Step 4.6 "Queue a task..." for updatefound.
     nsCOMPtr<nsIRunnable> upr =
-      NS_NewRunnableMethodWithArg<ServiceWorkerRegistrationInfo*>(swm,
+      NS_NewRunnableMethodWithArg<ServiceWorkerRegistrationInfo*>(
-                                                                  &ServiceWorkerManager::FireUpdateFound,
+        swm,
-                                                                  mRegistration);
+        &ServiceWorkerManager::FireUpdateFoundOnServiceWorkerRegistrations,
+        mRegistration);
+
     NS_DispatchToMainThread(upr);
 
     nsRefPtr<ServiceWorker> serviceWorker;
@@ -2184,63 +2186,59 @@ ServiceWorkerManager::GetScopeForUrl(const nsAString& aUrl, nsAString& aScope)
 }
 
 NS_IMETHODIMP
-ServiceWorkerManager::AddRegistrationEventListener(const nsAString& aScope, nsIDOMEventTarget* aListener)
+ServiceWorkerManager::AddRegistrationEventListener(const nsAString& aScope,
+                                                   ServiceWorkerRegistrationListener* aListener)
 {
   AssertIsOnMainThread();
-  nsAutoCString scope = NS_ConvertUTF16toUTF8(aScope);
+  MOZ_ASSERT(aListener);
-
-  // TODO: this is very very bad:
-  ServiceWorkerRegistrationBase* registration = static_cast<ServiceWorkerRegistrationBase*>(aListener);
-  MOZ_ASSERT(!mServiceWorkerRegistrations.Contains(registration));
 #ifdef DEBUG
   // Ensure a registration is only listening for it's own scope.
   nsAutoString regScope;
-  registration->GetScope(regScope);
+  aListener->GetScope(regScope);
   MOZ_ASSERT(!regScope.IsEmpty());
-  MOZ_ASSERT(scope.Equals(NS_ConvertUTF16toUTF8(regScope)));
+  MOZ_ASSERT(aScope.Equals(regScope));
 #endif
-  mServiceWorkerRegistrations.AppendElement(registration);
+
+  MOZ_ASSERT(!mServiceWorkerRegistrationListeners.Contains(aListener));
+  mServiceWorkerRegistrationListeners.AppendElement(aListener);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-ServiceWorkerManager::RemoveRegistrationEventListener(const nsAString& aScope, nsIDOMEventTarget* aListener)
+ServiceWorkerManager::RemoveRegistrationEventListener(const nsAString& aScope,
+                                                      ServiceWorkerRegistrationListener* aListener)
 {
   AssertIsOnMainThread();
-  nsCString scope = NS_ConvertUTF16toUTF8(aScope);
+  MOZ_ASSERT(aListener);
-  ServiceWorkerRegistrationBase* registration = static_cast<ServiceWorkerRegistrationBase*>(aListener);
-  MOZ_ASSERT(mServiceWorkerRegistrations.Contains(registration));
 #ifdef DEBUG
   // Ensure a registration is unregistering for it's own scope.
   nsAutoString regScope;
-  registration->GetScope(regScope);
+  aListener->GetScope(regScope);
   MOZ_ASSERT(!regScope.IsEmpty());
-  MOZ_ASSERT(scope.Equals(NS_ConvertUTF16toUTF8(regScope)));
+  MOZ_ASSERT(aScope.Equals(regScope));
 #endif
-  mServiceWorkerRegistrations.RemoveElement(registration);
+
+  MOZ_ASSERT(mServiceWorkerRegistrationListeners.Contains(aListener));
+  mServiceWorkerRegistrationListeners.RemoveElement(aListener);
   return NS_OK;
 }
 
 void
-ServiceWorkerManager::FireEventOnServiceWorkerRegistrations(
+ServiceWorkerManager::FireUpdateFoundOnServiceWorkerRegistrations(
-  ServiceWorkerRegistrationInfo* aRegistration,
+  ServiceWorkerRegistrationInfo* aRegistration)
-  const nsAString& aName)
 {
   AssertIsOnMainThread();
 
-  nsTObserverArray<ServiceWorkerRegistrationBase*>::ForwardIterator it(mServiceWorkerRegistrations);
+  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mServiceWorkerRegistrationListeners);
   while (it.HasMore()) {
-    nsRefPtr<ServiceWorkerRegistrationBase> target = it.GetNext();
+    nsRefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
     nsAutoString regScope;
     target->GetScope(regScope);
     MOZ_ASSERT(!regScope.IsEmpty());
 
     NS_ConvertUTF16toUTF8 utf8Scope(regScope);
     if (utf8Scope.Equals(aRegistration->mScope)) {
-      nsresult rv = target->DispatchTrustedEvent(aName);
+      target->UpdateFound();
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        // Warn only.
-      }
     }
   }
 }
@@ -2764,9 +2762,9 @@ ServiceWorkerManager::InvalidateServiceWorkerRegistrationWorker(ServiceWorkerReg
                                                                 WhichServiceWorker aWhichOnes)
 {
   AssertIsOnMainThread();
-  nsTObserverArray<ServiceWorkerRegistrationBase*>::ForwardIterator it(mServiceWorkerRegistrations);
+  nsTObserverArray<ServiceWorkerRegistrationListener*>::ForwardIterator it(mServiceWorkerRegistrationListeners);
   while (it.HasMore()) {
-    nsRefPtr<ServiceWorkerRegistrationBase> target = it.GetNext();
+    nsRefPtr<ServiceWorkerRegistrationListener> target = it.GetNext();
     nsAutoString regScope;
     target->GetScope(regScope);
     MOZ_ASSERT(!regScope.IsEmpty());
@@ -2774,7 +2772,7 @@ ServiceWorkerManager::InvalidateServiceWorkerRegistrationWorker(ServiceWorkerReg
     NS_ConvertUTF16toUTF8 utf8Scope(regScope);
 
     if (utf8Scope.Equals(aRegistration->mScope)) {
-      target->InvalidateWorkerReference(aWhichOnes);
+      target->InvalidateWorkers(aWhichOnes);
     }
   }
 }

dom/workers/ServiceWorkerManager.h

@@ -36,7 +36,7 @@ class BackgroundChild;
 
 namespace dom {
 
-class ServiceWorkerRegistrationBase;
+class ServiceWorkerRegistrationListener;
 
 namespace workers {
 
@@ -338,7 +338,7 @@ public:
   // The scope should be a fully qualified valid URL.
   nsRefPtrHashtable<nsCStringHashKey, ServiceWorkerRegistrationInfo> mServiceWorkerRegistrationInfos;
 
-  nsTObserverArray<ServiceWorkerRegistrationBase*> mServiceWorkerRegistrations;
+  nsTObserverArray<ServiceWorkerRegistrationListener*> mServiceWorkerRegistrationListeners;
 
   nsRefPtrHashtable<nsISupportsHashKey, ServiceWorkerRegistrationInfo> mControlledDocuments;
 
@@ -396,6 +396,13 @@ public:
  void LoadRegistrations(
                  const nsTArray<ServiceWorkerRegistrationData>& aRegistrations);
 
+  NS_IMETHOD
+  AddRegistrationEventListener(const nsAString& aScope,
+                               ServiceWorkerRegistrationListener* aListener);
+
+  NS_IMETHOD
+  RemoveRegistrationEventListener(const nsAString& aScope,
+                                  ServiceWorkerRegistrationListener* aListener);
 private:
   ServiceWorkerManager();
   ~ServiceWorkerManager();
@@ -455,15 +462,7 @@ private:
                                              const nsAString& aName);
 
   void
-  FireEventOnServiceWorkerRegistrations(ServiceWorkerRegistrationInfo* aRegistration,
+  FireUpdateFoundOnServiceWorkerRegistrations(ServiceWorkerRegistrationInfo* aRegistration);
-                                        const nsAString& aName);
-
-  void
-  FireUpdateFound(ServiceWorkerRegistrationInfo* aRegistration)
-  {
-    FireEventOnServiceWorkerRegistrations(aRegistration,
-                                          NS_LITERAL_STRING("updatefound"));
-  }
 
   void
   FireControllerChange(ServiceWorkerRegistrationInfo* aRegistration);

dom/workers/ServiceWorkerRegistration.cpp

@@ -142,7 +142,7 @@ ServiceWorkerRegistrationMainThread::StartListeningForEvents()
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(!mListeningForEvents);
-  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
+  nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
   if (swm) {
     swm->AddRegistrationEventListener(mScope, this);
     mListeningForEvents = true;
@@ -157,7 +157,7 @@ ServiceWorkerRegistrationMainThread::StopListeningForEvents()
     return;
   }
 
-  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
+  nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
   if (swm) {
     swm->RemoveRegistrationEventListener(mScope, this);
   }
@@ -208,7 +208,13 @@ ServiceWorkerRegistrationMainThread::GetActive()
 }
 
 void
-ServiceWorkerRegistrationMainThread::InvalidateWorkerReference(WhichServiceWorker aWhichOnes)
+ServiceWorkerRegistrationMainThread::UpdateFound()
+{
+  DispatchTrustedEvent(NS_LITERAL_STRING("updatefound"));
+}
+
+void
+ServiceWorkerRegistrationMainThread::InvalidateWorkers(WhichServiceWorker aWhichOnes)
 {
   AssertIsOnMainThread();
   if (aWhichOnes & WhichServiceWorker::INSTALLING_WORKER) {
@@ -602,12 +608,6 @@ ServiceWorkerRegistrationWorkerThread::GetActive()
   return nullptr;
 }
 
-void
-ServiceWorkerRegistrationWorkerThread::InvalidateWorkerReference(WhichServiceWorker aWhichOnes)
-{
-  MOZ_CRASH("FIXME");
-}
-
 void
 ServiceWorkerRegistrationWorkerThread::Update()
 {

dom/workers/ServiceWorkerRegistration.h

@@ -41,6 +41,24 @@ public:
 
 };
 
+// Used by ServiceWorkerManager to notify ServiceWorkerRegistrations of
+// updatefound event and invalidating ServiceWorker instances.
+class ServiceWorkerRegistrationListener
+{
+public:
+  NS_IMETHOD_(MozExternalRefCountType) AddRef() = 0;
+  NS_IMETHOD_(MozExternalRefCountType) Release() = 0;
+
+  virtual void
+  UpdateFound() = 0;
+
+  virtual void
+  InvalidateWorkers(WhichServiceWorker aWhichOnes) = 0;
+
+  virtual void
+  GetScope(nsAString& aScope) const = 0;
+};
+
 class ServiceWorkerRegistrationBase : public DOMEventTargetHelper
 {
 public:
@@ -65,16 +83,6 @@ public:
   virtual already_AddRefed<workers::ServiceWorker>
   GetActive() = 0;
 
-  void
-  GetScope(nsAString& aScope) const
-  {
-    aScope = mScope;
-  }
-
-  // Useful methods for ServiceWorkerManager:
-  virtual void
-  InvalidateWorkerReference(WhichServiceWorker aWhichOnes) = 0;
-
 protected:
   virtual ~ServiceWorkerRegistrationBase()
   { }
@@ -84,7 +92,8 @@ private:
   nsCOMPtr<nsISupports> mCCDummy;
 };
 
-class ServiceWorkerRegistrationMainThread final : public ServiceWorkerRegistrationBase
+class ServiceWorkerRegistrationMainThread final : public ServiceWorkerRegistrationBase,
+                                                  public ServiceWorkerRegistrationListener
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
@@ -115,9 +124,6 @@ public:
   already_AddRefed<PushManager>
   GetPushManager(ErrorResult& aRv);
 
-  void
-  InvalidateWorkerReference(WhichServiceWorker aWhichOnes) override;
-
   // DOMEventTargethelper
   void DisconnectFromOwner() override
   {
@@ -125,6 +131,19 @@ public:
     ServiceWorkerRegistrationBase::DisconnectFromOwner();
   }
 
+  // ServiceWorkerRegistrationListener
+  void
+  UpdateFound() override;
+
+  void
+  InvalidateWorkers(WhichServiceWorker aWhichOnes) override;
+
+  void
+  GetScope(nsAString& aScope) const override
+  {
+    aScope = mScope;
+  }
+
 private:
   ~ServiceWorkerRegistrationMainThread();
 
@@ -182,7 +201,10 @@ public:
   GetActive() override;
 
   void
-  InvalidateWorkerReference(WhichServiceWorker aWhichOnes) override;
+  GetScope(nsAString& aScope) const
+  {
+    aScope = mScope;
+  }
 
 private:
   ~ServiceWorkerRegistrationWorkerThread()