Bug 1409115 - Fix leaks by adding promise window proxy. r=bkelly

The service worker job queue can get stuck sometimes, so we don't want
the per-process service worker data structure to hold strong
references to content. Instead, I add a proxy that is only a weak
reference. The wrinkle is that we need to keep the promise alive as
long as the job and the window are otherwise alive. I solve this by
putting a cycle collected reference to the promise on the window
itself.

MozReview-Commit-ID: GbmCY4ZIQWk

--HG--
extra : rebase_source : 363ea865fd51bb44f3e727cd3469e316370e7cbc

dom/base/nsGlobalWindowInner.cpp

@@ -1515,6 +1515,8 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsGlobalWindowInner)
 
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChromeFields.mMessageManager)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mChromeFields.mGroupMessageManagers)
+
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingPromises)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGlobalWindowInner)
@@ -1609,6 +1611,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsGlobalWindowInner)
     NS_IMPL_CYCLE_COLLECTION_UNLINK(mChromeFields.mGroupMessageManagers)
   }
 
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mPendingPromises)
+
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -4947,6 +4951,19 @@ nsGlobalWindowInner::GetIndexedDB(ErrorResult& aError)
   return mIndexedDB;
 }
 
+void
+nsGlobalWindowInner::AddPendingPromise(mozilla::dom::Promise* aPromise)
+{
+  mPendingPromises.AppendElement(aPromise);
+}
+
+void
+nsGlobalWindowInner::RemovePendingPromise(mozilla::dom::Promise* aPromise)
+{
+  DebugOnly<bool> foundIt = mPendingPromises.RemoveElement(aPromise);
+  MOZ_ASSERT(foundIt, "tried to remove a non-existent element from mPendingPromises");
+}
+
 //*****************************************************************************
 // nsGlobalWindowInner::nsIInterfaceRequestor
 //*****************************************************************************
@@ -6736,6 +6753,9 @@ nsGlobalWindowInner::AddSizeOfIncludingThis(nsWindowSizes& aWindowSizes) const
     aWindowSizes.mDOMPerformanceResourceEntries =
       mPerformance->SizeOfResourceEntries(aWindowSizes.mState.mMallocSizeOf);
   }
+
+  aWindowSizes.mDOMOtherSize +=
+    mPendingPromises.ShallowSizeOfExcludingThis(aWindowSizes.mState.mMallocSizeOf);
 }
 
 void

dom/base/nsGlobalWindowInner.h

@@ -1164,6 +1164,10 @@ public:
   // Inner windows only.
   void UpdateCanvasFocus(bool aFocusChanged, nsIContent* aNewContent);
 
+  // See PromiseWindowProxy.h for an explanation.
+  void AddPendingPromise(mozilla::dom::Promise* aPromise);
+  void RemovePendingPromise(mozilla::dom::Promise* aPromise);
+
 public:
   virtual already_AddRefed<nsPIWindowRoot> GetTopWindowRoot() override;
 
@@ -1408,6 +1412,8 @@ protected:
 
   mozilla::UniquePtr<mozilla::dom::ClientSource> mClientSource;
 
+  nsTArray<RefPtr<mozilla::dom::Promise>> mPendingPromises;
+
   static InnerWindowByIdTable* sInnerWindowsById;
 
   // Members in the mChromeFields member should only be used in chrome windows.

dom/promise/PromiseWindowProxy.cpp

@@ -0,0 +1,54 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "mozilla/dom/PromiseWindowProxy.h"
+
+#include "nsGlobalWindowInner.h"
+#include "nsPIDOMWindow.h"
+#include "nsIWeakReference.h"
+#include "mozilla/dom/Promise.h"
+
+using namespace mozilla;
+using namespace dom;
+
+
+PromiseWindowProxy::PromiseWindowProxy(nsPIDOMWindowInner* aWindow, Promise* aPromise)
+  : mPromise(aPromise)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_DIAGNOSTIC_ASSERT(aWindow && aPromise);
+  auto* window = nsGlobalWindowInner::Cast(aWindow);
+  window->GetWeakReference(getter_AddRefs(mWindow));
+  window->AddPendingPromise(aPromise);
+}
+
+PromiseWindowProxy::~PromiseWindowProxy()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  nsCOMPtr<nsPIDOMWindowInner> window = GetWindow();
+  if (window && mPromise) {
+    nsGlobalWindowInner::Cast(window)->RemovePendingPromise(mPromise);
+  }
+}
+
+RefPtr<Promise>
+PromiseWindowProxy::Get() const
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  if (!mPromise) {
+    return nullptr;
+  }
+  RefPtr<Promise> promise(mPromise);
+  return promise;
+}
+
+nsCOMPtr<nsPIDOMWindowInner>
+PromiseWindowProxy::GetWindow() const
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  nsCOMPtr<nsPIDOMWindowInner> window = do_QueryReferent(mWindow);
+  return window;
+}

dom/promise/PromiseWindowProxy.h

@@ -0,0 +1,57 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_dom_PromiseWindowProxy_h
+#define mozilla_dom_PromiseWindowProxy_h
+
+#include "nsCOMPtr.h"
+#include "mozilla/WeakPtr.h"
+
+class nsIWeakReference;
+class nsPIDOMWindowInner;
+
+namespace mozilla {
+namespace dom {
+
+class Promise;
+
+// When a data structure is waiting to resolve a promise from a
+// window, but that data structure is not owned by that window, there
+// exists a dilemma:
+//
+// 1) If the promise is never resolved, and the non-window data
+// structure is keeping alive the promise, the promise's window will
+// be leaked.
+//
+// 2) Something has to keep the promise alive from native code in case
+// script has dropped all direct references to it, but the promise is
+// later resolved.
+//
+// PromiseWindowProxy solves this dilemma for the non-window data
+// structure. It solves (1) by only keeping a weak reference to the
+// promise. It solves (2) by adding a strong reference to the promise
+// on the window itself. This ensures that as long as both the
+// PromiseWindowProxy and the window are still alive that the promise
+// will remain alive. This strong reference is cycle collected, so it
+// won't cause the window to leak.
+class PromiseWindowProxy final
+{
+public:
+  PromiseWindowProxy(nsPIDOMWindowInner* aWindow, mozilla::dom::Promise* aPromise);
+  ~PromiseWindowProxy();
+
+  RefPtr<Promise> Get() const;
+  nsCOMPtr<nsPIDOMWindowInner> GetWindow() const;
+
+private:
+  nsCOMPtr<nsIWeakReference> mWindow;
+  WeakPtr<Promise> mPromise;
+};
+
+} // namespace dom
+} // namespace mozilla
+
+#endif // mozilla_dom_PromiseWindowProxy_h

dom/promise/moz.build

@@ -11,12 +11,14 @@ EXPORTS.mozilla.dom += [
     'Promise.h',
     'PromiseDebugging.h',
     'PromiseNativeHandler.h',
+    'PromiseWindowProxy.h',
     'PromiseWorkerProxy.h',
 ]
 
 UNIFIED_SOURCES += [
     'Promise.cpp',
     'PromiseDebugging.cpp',
+    'PromiseWindowProxy.cpp',
 ]
 
 LOCAL_INCLUDES += [

dom/workers/ServiceWorkerManager.cpp

@@ -46,6 +46,7 @@
 #include "mozilla/dom/Navigator.h"
 #include "mozilla/dom/NotificationEvent.h"
 #include "mozilla/dom/PromiseNativeHandler.h"
+#include "mozilla/dom/PromiseWindowProxy.h"
 #include "mozilla/dom/Request.h"
 #include "mozilla/dom/RootedDictionary.h"
 #include "mozilla/dom/TypedArray.h"
@@ -363,10 +364,9 @@ ServiceWorkerManager::MaybeStartShutdown()
 
 class ServiceWorkerResolveWindowPromiseOnRegisterCallback final : public ServiceWorkerJob::Callback
 {
-  RefPtr<nsPIDOMWindowInner> mWindow;
   // The promise "returned" by the call to Update up to
   // navigator.serviceWorker.register().
-  RefPtr<Promise> mPromise;
+  PromiseWindowProxy mPromise;
 
   ~ServiceWorkerResolveWindowPromiseOnRegisterCallback()
   {}
@@ -376,9 +376,18 @@ class ServiceWorkerResolveWindowPromiseOnRegisterCallback final : public Service
   {
     AssertIsOnMainThread();
     MOZ_ASSERT(aJob);
+    RefPtr<Promise> promise = mPromise.Get();
+    if (!promise) {
+      return;
+    }
 
     if (aStatus.Failed()) {
-      mPromise->MaybeReject(aStatus);
+      promise->MaybeReject(aStatus);
+      return;
+    }
+
+    nsCOMPtr<nsPIDOMWindowInner> window = mPromise.GetWindow();
+    if (!window) {
       return;
     }
 
@@ -388,15 +397,14 @@ class ServiceWorkerResolveWindowPromiseOnRegisterCallback final : public Service
     RefPtr<ServiceWorkerRegistrationInfo> reg = registerJob->GetRegistration();
 
     RefPtr<ServiceWorkerRegistration> swr =
-      mWindow->GetServiceWorkerRegistration(NS_ConvertUTF8toUTF16(reg->mScope));
-    mPromise->MaybeResolve(swr);
+      window->GetServiceWorkerRegistration(NS_ConvertUTF8toUTF16(reg->mScope));
+    promise->MaybeResolve(swr);
   }
 
 public:
   ServiceWorkerResolveWindowPromiseOnRegisterCallback(nsPIDOMWindowInner* aWindow,
                                                       Promise* aPromise)
-    : mWindow(aWindow)
-    , mPromise(aPromise)
+    : mPromise(aWindow, aPromise)
   {}
 
   NS_INLINE_DECL_REFCOUNTING(ServiceWorkerResolveWindowPromiseOnRegisterCallback, override)

dom/workers/ServiceWorkerRegistration.cpp

@@ -9,6 +9,7 @@
 #include "ipc/ErrorIPCUtils.h"
 #include "mozilla/dom/Notification.h"
 #include "mozilla/dom/Promise.h"
+#include "mozilla/dom/PromiseWindowProxy.h"
 #include "mozilla/dom/PromiseWorkerProxy.h"
 #include "mozilla/dom/PushManagerBinding.h"
 #include "mozilla/dom/PushManager.h"
@@ -413,14 +414,15 @@ UpdateInternal(nsIPrincipal* aPrincipal,
 
 class MainThreadUpdateCallback final : public ServiceWorkerUpdateFinishCallback
 {
-  RefPtr<Promise> mPromise;
+  PromiseWindowProxy mPromise;
 
   ~MainThreadUpdateCallback()
   { }
 
 public:
-  explicit MainThreadUpdateCallback(Promise* aPromise)
-    : mPromise(aPromise)
+  explicit MainThreadUpdateCallback(nsPIDOMWindowInner* aWindow,
+                                    Promise* aPromise)
+    : mPromise(aWindow, aPromise)
   {
     AssertIsOnMainThread();
   }
@@ -428,13 +430,17 @@ public:
   void
   UpdateSucceeded(ServiceWorkerRegistrationInfo* aRegistration) override
   {
-    mPromise->MaybeResolveWithUndefined();
+    if (RefPtr<Promise> promise = mPromise.Get()) {
+      promise->MaybeResolveWithUndefined();
+    }
   }
 
   void
   UpdateFailed(ErrorResult& aStatus) override
   {
-    mPromise->MaybeReject(aStatus);
+    if (RefPtr<Promise> promise = mPromise.Get()) {
+      promise->MaybeReject(aStatus);
+    }
   }
 };
 
@@ -570,22 +576,25 @@ private:
 
 class UnregisterCallback final : public nsIServiceWorkerUnregisterCallback
 {
-  RefPtr<Promise> mPromise;
+  PromiseWindowProxy mPromise;
 
 public:
   NS_DECL_ISUPPORTS
 
-  explicit UnregisterCallback(Promise* aPromise)
-    : mPromise(aPromise)
+  explicit UnregisterCallback(nsPIDOMWindowInner* aWindow,
+                              Promise* aPromise)
+    : mPromise(aWindow, aPromise)
   {
-    MOZ_ASSERT(mPromise);
+    MOZ_ASSERT(aPromise);
   }
 
   NS_IMETHOD
   UnregisterSucceeded(bool aState) override
   {
     AssertIsOnMainThread();
-    mPromise->MaybeResolve(aState);
+    if (RefPtr<Promise> promise = mPromise.Get()) {
+      promise->MaybeResolve(aState);
+    }
     return NS_OK;
   }
 
@@ -594,7 +603,9 @@ public:
   {
     AssertIsOnMainThread();
 
-    mPromise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
+    if (RefPtr<Promise> promise = mPromise.Get()) {
+      promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
+    }
     return NS_OK;
   }
 
@@ -764,7 +775,7 @@ ServiceWorkerRegistrationMainThread::Update(ErrorResult& aRv)
   MOZ_ASSERT(doc);
 
   RefPtr<MainThreadUpdateCallback> cb =
-    new MainThreadUpdateCallback(promise);
+    new MainThreadUpdateCallback(GetOwner(), promise);
   UpdateInternal(doc->NodePrincipal(), mScope, cb);
 
   return promise.forget();
@@ -822,7 +833,7 @@ ServiceWorkerRegistrationMainThread::Unregister(ErrorResult& aRv)
     return nullptr;
   }
 
-  RefPtr<UnregisterCallback> cb = new UnregisterCallback(promise);
+  RefPtr<UnregisterCallback> cb = new UnregisterCallback(GetOwner(), promise);
 
   NS_ConvertUTF8toUTF16 scope(uriSpec);
   aRv = swm->Unregister(documentPrincipal, cb, scope);