Bug 1333112 - Fix windowClient.Navigate leak. r=bkelly a=jcristau
authorCatalin Badea <catalin.badea392@gmail.com>
Thu, 26 Jan 2017 21:37:17 +0200
changeset 375789 f80dc144cdd03a4999b4c527d13538316eb19fdc
parent 375788 4d570c752fc9249899dac5a9ae98cc35e4061084
child 375790 49268ecb72fbc8daf6d28e9be88fa5c4a55c2223
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly, jcristau
bugs1333112
milestone53.0a2
Bug 1333112 - Fix windowClient.Navigate leak. r=bkelly a=jcristau
dom/promise/Promise.cpp
dom/promise/PromiseWorkerProxy.h
dom/workers/ServiceWorkerWindowClient.cpp
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -821,26 +821,16 @@ PromiseWorkerProxy::WorkerPromise() cons
   MOZ_ASSERT(worker);
   worker->AssertIsOnWorkerThread();
 #endif
   MOZ_ASSERT(mWorkerPromise);
   return mWorkerPromise;
 }
 
 void
-PromiseWorkerProxy::StoreISupports(nsISupports* aSupports)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  nsMainThreadPtrHandle<nsISupports> supports(
-    new nsMainThreadPtrHolder<nsISupports>(aSupports));
-  mSupportsArray.AppendElement(supports);
-}
-
-void
 PromiseWorkerProxy::RunCallback(JSContext* aCx,
                                 JS::Handle<JS::Value> aValue,
                                 RunCallbackFunc aFunc)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   MutexAutoLock lock(Lock());
   // If the worker thread's been cancelled we don't need to resolve the Promise.
--- a/dom/promise/PromiseWorkerProxy.h
+++ b/dom/promise/PromiseWorkerProxy.h
@@ -142,18 +142,16 @@ public:
   // Worker thread callers, this will assert that the proxy has not been cleaned
   // up.
   workers::WorkerPrivate* GetWorkerPrivate() const;
 
   // This should only be used within WorkerRunnable::WorkerRun() running on the
   // worker thread! Do not call this after calling CleanUp().
   Promise* WorkerPromise() const;
 
-  void StoreISupports(nsISupports* aSupports);
-
   // Worker thread only. Calling this invalidates several assumptions, so be
   // sure this is the last thing you do.
   // 1. WorkerPrivate() will no longer return a valid worker.
   // 2. WorkerPromise() will crash!
   void CleanUp();
 
   Mutex& Lock()
   {
@@ -212,20 +210,16 @@ private:
 
   // Modified on the worker thread.
   // It is ok to *read* this without a lock on the worker.
   // Main thread must always acquire a lock.
   bool mCleanedUp; // To specify if the cleanUp() has been done.
 
   const PromiseWorkerProxyStructuredCloneCallbacks* mCallbacks;
 
-  // Aimed to keep objects alive when doing the structured-clone read/write,
-  // which can be added by calling StoreISupports() on the main thread.
-  nsTArray<nsMainThreadPtrHandle<nsISupports>> mSupportsArray;
-
   // Ensure the worker and the main thread won't race to access |mCleanedUp|.
   Mutex mCleanUpLock;
 
   UniquePtr<workers::WorkerHolder> mWorkerHolder;
 };
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/workers/ServiceWorkerWindowClient.cpp
+++ b/dom/workers/ServiceWorkerWindowClient.cpp
@@ -199,39 +199,44 @@ class WebProgressListener final : public
                                   public nsSupportsWeakReference
 {
 public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(WebProgressListener,
                                            nsIWebProgressListener)
 
   WebProgressListener(PromiseWorkerProxy* aPromiseProxy,
+                      ServiceWorkerPrivate* aServiceWorkerPrivate,
                       nsPIDOMWindowOuter* aWindow, nsIURI* aBaseURI)
     : mPromiseProxy(aPromiseProxy)
+    , mServiceWorkerPrivate(aServiceWorkerPrivate)
     , mWindow(aWindow)
     , mBaseURI(aBaseURI)
   {
     MOZ_ASSERT(aPromiseProxy);
+    MOZ_ASSERT(aServiceWorkerPrivate);
     MOZ_ASSERT(aWindow);
     MOZ_ASSERT(aWindow->IsOuterWindow());
     MOZ_ASSERT(aBaseURI);
     AssertIsOnMainThread();
 
-    mPromiseProxy->StoreISupports(static_cast<nsIWebProgressListener*>(this));
+    mServiceWorkerPrivate->StoreISupports(static_cast<nsIWebProgressListener*>(this));
   }
 
   NS_IMETHOD
   OnStateChange(nsIWebProgress* aWebProgress, nsIRequest* aRequest,
                 uint32_t aStateFlags, nsresult aStatus) override
   {
     if (!(aStateFlags & STATE_IS_DOCUMENT) ||
         !(aStateFlags & (STATE_STOP | STATE_TRANSFERRING))) {
       return NS_OK;
     }
 
+    // This is safe because our caller holds a strong ref.
+    mServiceWorkerPrivate->RemoveISupports(static_cast<nsIWebProgressListener*>(this));
     aWebProgress->RemoveProgressListener(this);
 
     WorkerPrivate* workerPrivate;
 
     {
       MutexAutoLock lock(mPromiseProxy->Lock());
       if (mPromiseProxy->CleanedUp()) {
         return NS_OK;
@@ -303,43 +308,47 @@ public:
     MOZ_CRASH("Unexpected notification.");
     return NS_OK;
   }
 
 private:
   ~WebProgressListener() {}
 
   RefPtr<PromiseWorkerProxy> mPromiseProxy;
+  RefPtr<ServiceWorkerPrivate> mServiceWorkerPrivate;
   nsCOMPtr<nsPIDOMWindowOuter> mWindow;
   nsCOMPtr<nsIURI> mBaseURI;
 };
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(WebProgressListener)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(WebProgressListener)
 NS_IMPL_CYCLE_COLLECTION(WebProgressListener, mPromiseProxy,
-                         mWindow)
+                         mServiceWorkerPrivate, mWindow)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(WebProgressListener)
   NS_INTERFACE_MAP_ENTRY(nsIWebProgressListener)
   NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference)
 NS_INTERFACE_MAP_END
 
 class ClientNavigateRunnable final : public Runnable
 {
   uint64_t mWindowId;
   nsString mUrl;
   nsCString mBaseUrl;
+  nsString mScope;
   RefPtr<PromiseWorkerProxy> mPromiseProxy;
   MOZ_INIT_OUTSIDE_CTOR WorkerPrivate* mWorkerPrivate;
 
 public:
   ClientNavigateRunnable(uint64_t aWindowId, const nsAString& aUrl,
+                         const nsAString& aScope,
                          PromiseWorkerProxy* aPromiseProxy)
     : mWindowId(aWindowId)
     , mUrl(aUrl)
+    , mScope(aScope)
     , mPromiseProxy(aPromiseProxy)
     , mWorkerPrivate(nullptr)
   {
     MOZ_ASSERT(aPromiseProxy);
     MOZ_ASSERT(aPromiseProxy->GetWorkerPrivate());
     aPromiseProxy->GetWorkerPrivate()->AssertIsOnWorkerThread();
   }
 
@@ -355,16 +364,17 @@ public:
       if (mPromiseProxy->CleanedUp()) {
         return NS_OK;
       }
 
       mWorkerPrivate = mPromiseProxy->GetWorkerPrivate();
       WorkerPrivate::LocationInfo& info = mWorkerPrivate->GetLocationInfo();
       mBaseUrl = info.mHref;
       principal = mWorkerPrivate->GetPrincipal();
+      MOZ_DIAGNOSTIC_ASSERT(principal);
     }
 
     nsCOMPtr<nsIURI> baseUrl;
     nsCOMPtr<nsIURI> url;
     nsresult rv = ParseUrl(getter_AddRefs(baseUrl), getter_AddRefs(url));
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return RejectPromise(NS_ERROR_TYPE_ERR);
@@ -382,18 +392,35 @@ public:
     }
 
     nsCOMPtr<nsIDocShell> docShell = window->GetDocShell();
     nsCOMPtr<nsIWebProgress> webProgress = do_GetInterface(docShell);
     if (NS_WARN_IF(!webProgress)) {
       return NS_ERROR_FAILURE;
     }
 
+    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+    if (!swm) {
+      return NS_ERROR_FAILURE;
+    }
+
+    RefPtr<ServiceWorkerRegistrationInfo> registration =
+      swm->GetRegistration(principal, NS_ConvertUTF16toUTF8(mScope));
+    if (NS_WARN_IF(!registration)) {
+      return NS_ERROR_FAILURE;
+    }
+    RefPtr<ServiceWorkerInfo> serviceWorkerInfo =
+      registration->GetServiceWorkerInfoById(mWorkerPrivate->ServiceWorkerID());
+    if (NS_WARN_IF(!serviceWorkerInfo)) {
+      return NS_ERROR_FAILURE;
+    }
+
     nsCOMPtr<nsIWebProgressListener> listener =
-      new WebProgressListener(mPromiseProxy, window->GetOuterWindow(), baseUrl);
+      new WebProgressListener(mPromiseProxy, serviceWorkerInfo->WorkerPrivate(),
+                              window->GetOuterWindow(), baseUrl);
 
     rv = webProgress->AddProgressListener(
       listener, nsIWebProgress::NOTIFY_STATE_DOCUMENT);
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return RejectPromise(rv);
     }
 
@@ -507,20 +534,25 @@ ServiceWorkerWindowClient::Navigate(cons
     return nullptr;
   }
 
   if (aUrl.EqualsLiteral("about:blank")) {
     promise->MaybeReject(NS_ERROR_TYPE_ERR);
     return promise.forget();
   }
 
+  ServiceWorkerGlobalScope* globalScope =
+    static_cast<ServiceWorkerGlobalScope*>(workerPrivate->GlobalScope());
+  nsString scope;
+  globalScope->GetScope(scope);
+
   RefPtr<PromiseWorkerProxy> promiseProxy =
     PromiseWorkerProxy::Create(workerPrivate, promise);
   if (promiseProxy) {
     RefPtr<ClientNavigateRunnable> r =
-      new ClientNavigateRunnable(mWindowId, aUrl, promiseProxy);
+      new ClientNavigateRunnable(mWindowId, aUrl, scope, promiseProxy);
     MOZ_ALWAYS_SUCCEEDS(workerPrivate->DispatchToMainThread(r.forget()));
   } else {
     promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
   }
 
   return promise.forget();
 }