Bug 1058043 - ServiceWorkerRegistration should not keep a reference to the window, r=nsm
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 26 Aug 2014 09:17:36 +0100
changeset 223255 06168844e276351a29996224b3cbc28bec3c1a53
parent 223254 e48a1782de89eca7b4de7f96b2bbe00cc29b3230
child 223256 75805f95cfbeb4c251d8734b72fadfd5173fcdeb
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnsm
bugs1058043
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1058043 - ServiceWorkerRegistration should not keep a reference to the window, r=nsm
dom/workers/ServiceWorkerRegistration.cpp
dom/workers/ServiceWorkerRegistration.h
--- a/dom/workers/ServiceWorkerRegistration.cpp
+++ b/dom/workers/ServiceWorkerRegistration.cpp
@@ -8,64 +8,57 @@
 
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/ServiceWorkerRegistrationBinding.h"
 #include "mozilla/Services.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsServiceManagerUtils.h"
 #include "ServiceWorker.h"
 
-#include "nsIObserverService.h"
 #include "nsIServiceWorkerManager.h"
 #include "nsISupportsPrimitives.h"
 #include "nsPIDOMWindow.h"
 
 using namespace mozilla::dom::workers;
 
 namespace mozilla {
 namespace dom {
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(ServiceWorkerRegistration)
-  NS_INTERFACE_MAP_ENTRY(nsIObserver)
 NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
 
 NS_IMPL_ADDREF_INHERITED(ServiceWorkerRegistration, DOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(ServiceWorkerRegistration, DOMEventTargetHelper)
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(ServiceWorkerRegistration,
                                    DOMEventTargetHelper,
-                                   mWindow,
                                    mInstallingWorker,
                                    mWaitingWorker,
                                    mActiveWorker)
 
 ServiceWorkerRegistration::ServiceWorkerRegistration(nsPIDOMWindow* aWindow,
                                                      const nsAString& aScope)
-  : mWindow(aWindow)
+  : DOMEventTargetHelper(aWindow)
   , mScope(aScope)
-  , mInnerID(0)
-  , mIsListeningForEvents(false)
 {
   MOZ_ASSERT(aWindow);
   MOZ_ASSERT(aWindow->IsInnerWindow());
 
-  SetIsDOMBinding();
   StartListeningForEvents();
-
-  mInnerID = aWindow->WindowID();
-
-  nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
-  if (obs) {
-    obs->AddObserver(this, "inner-window-destroyed", false);
-  }
 }
 
 ServiceWorkerRegistration::~ServiceWorkerRegistration()
 {
+}
+
+void
+ServiceWorkerRegistration::DisconnectFromOwner()
+{
   StopListeningForEvents();
+  DOMEventTargetHelper::DisconnectFromOwner();
 }
 
 JSObject*
 ServiceWorkerRegistration::WrapObject(JSContext* aCx)
 {
   return ServiceWorkerRegistrationBinding::Wrap(aCx, this);
 }
 
@@ -133,23 +126,23 @@ ServiceWorkerRegistration::GetWorkerRefe
     do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
 
   nsCOMPtr<nsISupports> serviceWorker;
   switch(aWhichOne) {
     case WhichServiceWorker::INSTALLING_WORKER:
-      rv = swm->GetInstalling(mWindow, getter_AddRefs(serviceWorker));
+      rv = swm->GetInstalling(GetOwner(), getter_AddRefs(serviceWorker));
       break;
     case WhichServiceWorker::WAITING_WORKER:
-      rv = swm->GetWaiting(mWindow, getter_AddRefs(serviceWorker));
+      rv = swm->GetWaiting(GetOwner(), getter_AddRefs(serviceWorker));
       break;
     case WhichServiceWorker::ACTIVE_WORKER:
-      rv = swm->GetActive(mWindow, getter_AddRefs(serviceWorker));
+      rv = swm->GetActive(GetOwner(), getter_AddRefs(serviceWorker));
       break;
     default:
       MOZ_CRASH("Invalid enum value");
   }
 
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
@@ -175,79 +168,32 @@ ServiceWorkerRegistration::InvalidateWor
   }
 }
 
 // XXXnsm, maybe this can be optimized to only add when a event handler is
 // registered.
 void
 ServiceWorkerRegistration::StartListeningForEvents()
 {
-  MOZ_ASSERT(!mIsListeningForEvents);
-
   nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
-  MOZ_ASSERT(mWindow);
-
   if (swm) {
     swm->AddRegistrationEventListener(GetDocumentURI(), this);
   }
-
-  mIsListeningForEvents = true;
 }
 
 void
 ServiceWorkerRegistration::StopListeningForEvents()
 {
-  if (!mIsListeningForEvents) {
-    return;
-  }
-
   nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
-
-  // StopListeningForEvents is called in the dtor, and it can happen that
-  // SnowWhite had already set to null mWindow.
-  if (swm && mWindow) {
+  if (swm) {
     swm->RemoveRegistrationEventListener(GetDocumentURI(), this);
   }
-
-  mIsListeningForEvents = false;
 }
 
 nsIURI*
 ServiceWorkerRegistration::GetDocumentURI() const
 {
-  MOZ_ASSERT(mWindow);
-  return mWindow->GetDocumentURI();
+  MOZ_ASSERT(GetOwner());
+  return GetOwner()->GetDocumentURI();
 }
 
-NS_IMETHODIMP
-ServiceWorkerRegistration::Observe(nsISupports* aSubject, const char* aTopic,
-                                   const char16_t* aData)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  if (strcmp(aTopic, "inner-window-destroyed")) {
-    return NS_OK;
-  }
-
-  if (!mIsListeningForEvents) {
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsISupportsPRUint64> wrapper = do_QueryInterface(aSubject);
-  NS_ENSURE_TRUE(wrapper, NS_ERROR_FAILURE);
-
-  uint64_t innerID;
-  nsresult rv = wrapper->GetData(&innerID);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (innerID == mInnerID) {
-    nsCOMPtr<nsIObserverService> obs = services::GetObserverService();
-    if (obs) {
-      obs->RemoveObserver(this, "inner-window-destroyed");
-    }
-
-    StopListeningForEvents();
-  }
-
-  return NS_OK;
-}
 } // dom namespace
 } // mozilla namespace
--- a/dom/workers/ServiceWorkerRegistration.h
+++ b/dom/workers/ServiceWorkerRegistration.h
@@ -17,35 +17,27 @@ namespace dom {
 
 class Promise;
 
 namespace workers {
 class ServiceWorker;
 }
 
 class ServiceWorkerRegistration MOZ_FINAL : public DOMEventTargetHelper
-                                          , public nsIObserver
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
-  NS_DECL_NSIOBSERVER
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ServiceWorkerRegistration,
                                            DOMEventTargetHelper)
 
   IMPL_EVENT_HANDLER(updatefound)
 
   ServiceWorkerRegistration(nsPIDOMWindow* aWindow,
                             const nsAString& aScope);
 
-  nsPIDOMWindow*
-  GetParentObject() const
-  {
-    return mWindow;
-  }
-
   JSObject*
   WrapObject(JSContext* aCx);
 
   already_AddRefed<workers::ServiceWorker>
   GetInstalling();
 
   already_AddRefed<workers::ServiceWorker>
   GetWaiting();
@@ -65,40 +57,38 @@ public:
   // Useful methods for ServiceWorkerManager:
 
   nsIURI*
   GetDocumentURI() const;
 
   void
   InvalidateWorkerReference(WhichServiceWorker aWhichOnes);
 
+  // DOMEventTargethelper
+  virtual void DisconnectFromOwner() MOZ_OVERRIDE;
+
 private:
   ~ServiceWorkerRegistration();
 
   already_AddRefed<workers::ServiceWorker>
   GetWorkerReference(WhichServiceWorker aWhichOne);
 
   void
   StartListeningForEvents();
 
   void
   StopListeningForEvents();
 
-  nsCOMPtr<nsPIDOMWindow> mWindow;
-
   // The following properties are cached here to ensure JS equality is satisfied
   // instead of acquiring a new worker instance from the ServiceWorkerManager
   // for every access. A null value is considered a cache miss.
   // These three may change to a new worker at any time.
   nsRefPtr<workers::ServiceWorker> mInstallingWorker;
   nsRefPtr<workers::ServiceWorker> mWaitingWorker;
   nsRefPtr<workers::ServiceWorker> mActiveWorker;
 
   const nsString mScope;
-
-  uint64_t mInnerID;
-  bool mIsListeningForEvents;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif /* mozilla_dom_ServiceWorkerRegistration_h */