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
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -1510,16 +1510,18 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mExternal)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMozSelfSupport)
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mIntlUtils)
tmp->TraverseHostObjectURIs(cb);
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)
tmp->CleanupCachedXBLHandlers();
NS_IMPL_CYCLE_COLLECTION_UNLINK(mNavigator)
NS_IMPL_CYCLE_COLLECTION_UNLINK(mPerformance)
@@ -1604,16 +1606,18 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ns
static_cast<nsFrameMessageManager*>(
tmp->mChromeFields.mMessageManager.get())->Disconnect();
NS_IMPL_CYCLE_COLLECTION_UNLINK(mChromeFields.mMessageManager)
}
tmp->DisconnectAndClearGroupMessageManagers();
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
#ifdef DEBUG
void
nsGlobalWindowInner::RiskyUnlink()
{
NS_CYCLE_COLLECTION_INNERNAME.Unlink(this);
@@ -4942,16 +4946,29 @@ nsGlobalWindowInner::GetIndexedDB(ErrorR
// This may keep mIndexedDB null without setting an error.
aError = IDBFactory::CreateForWindow(this,
getter_AddRefs(mIndexedDB));
}
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
//*****************************************************************************
NS_IMETHODIMP
nsGlobalWindowInner::GetInterface(const nsIID & aIID, void **aSink)
{
nsGlobalWindowOuter* outer = GetOuterWindowInternal();
@@ -6731,16 +6748,19 @@ nsGlobalWindowInner::AddSizeOfIncludingT
}
if (mPerformance) {
aWindowSizes.mDOMPerformanceUserEntries =
mPerformance->SizeOfUserEntries(aWindowSizes.mState.mMallocSizeOf);
aWindowSizes.mDOMPerformanceResourceEntries =
mPerformance->SizeOfResourceEntries(aWindowSizes.mState.mMallocSizeOf);
}
+
+ aWindowSizes.mDOMOtherSize +=
+ mPendingPromises.ShallowSizeOfExcludingThis(aWindowSizes.mState.mMallocSizeOf);
}
void
nsGlobalWindowInner::AddGamepad(uint32_t aIndex, Gamepad* aGamepad)
{
// Create the index we will present to content based on which indices are
// already taken, as required by the spec.
// https://w3c.github.io/gamepad/gamepad.html#widl-Gamepad-index
--- a/dom/base/nsGlobalWindowInner.h
+++ b/dom/base/nsGlobalWindowInner.h
@@ -1159,16 +1159,20 @@ public:
virtual uint32_t GetFocusMethod() override;
virtual bool ShouldShowFocusRing() override;
// 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;
protected:
static void NotifyDOMWindowDestroyed(nsGlobalWindowInner* aWindow);
void NotifyWindowIDDestroyed(const char* aTopic);
static void NotifyDOMWindowFrozen(nsGlobalWindowInner* aWindow);
@@ -1403,16 +1407,18 @@ protected:
RefPtr<mozilla::dom::VREventObserver> mVREventObserver;
int64_t mBeforeUnloadListenerCount;
RefPtr<mozilla::dom::IntlUtils> mIntlUtils;
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.
// All accesses to this field should be guarded by a check of mIsChrome.
struct ChromeFields {
ChromeFields()
: mGroupMessageManagers(1)
{}
new file mode 100644
--- /dev/null
+++ b/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;
+}
new file mode 100644
--- /dev/null
+++ b/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
--- a/dom/promise/moz.build
+++ b/dom/promise/moz.build
@@ -6,22 +6,24 @@
with Files("**"):
BUG_COMPONENT = ("Core", "DOM")
EXPORTS.mozilla.dom += [
'Promise.h',
'PromiseDebugging.h',
'PromiseNativeHandler.h',
+ 'PromiseWindowProxy.h',
'PromiseWorkerProxy.h',
]
UNIFIED_SOURCES += [
'Promise.cpp',
'PromiseDebugging.cpp',
+ 'PromiseWindowProxy.cpp',
]
LOCAL_INCLUDES += [
'../base',
'../ipc',
'../workers',
]
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -41,16 +41,17 @@
#include "mozilla/dom/ClientManager.h"
#include "mozilla/dom/ContentParent.h"
#include "mozilla/dom/ErrorEvent.h"
#include "mozilla/dom/Headers.h"
#include "mozilla/dom/InternalHeaders.h"
#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"
#include "mozilla/ipc/BackgroundChild.h"
#include "mozilla/ipc/PBackgroundChild.h"
#include "mozilla/ipc/PBackgroundSharedTypes.h"
#include "mozilla/dom/ScriptLoader.h"
#include "mozilla/Unused.h"
@@ -358,50 +359,57 @@ ServiceWorkerManager::MaybeStartShutdown
RefPtr<TeardownRunnable> runnable = new TeardownRunnable(mActor);
nsresult rv = NS_DispatchToMainThread(runnable);
Unused << NS_WARN_IF(NS_FAILED(rv));
mActor = nullptr;
}
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()
{}
virtual void
JobFinished(ServiceWorkerJob* aJob, ErrorResult& aStatus) override
{
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;
}
MOZ_ASSERT(aJob->GetType() == ServiceWorkerJob::Type::Register);
RefPtr<ServiceWorkerRegisterJob> registerJob =
static_cast<ServiceWorkerRegisterJob*>(aJob);
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)
};
namespace {
class PropagateSoftUpdateRunnable final : public Runnable
--- a/dom/workers/ServiceWorkerRegistration.cpp
+++ b/dom/workers/ServiceWorkerRegistration.cpp
@@ -4,16 +4,17 @@
* 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 "ServiceWorkerRegistration.h"
#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"
#include "mozilla/dom/ServiceWorkerRegistrationBinding.h"
#include "mozilla/Preferences.h"
#include "mozilla/Services.h"
#include "mozilla/Unused.h"
#include "nsCycleCollectionParticipant.h"
@@ -408,38 +409,43 @@ UpdateInternal(nsIPrincipal* aPrincipal,
return;
}
swm->Update(aPrincipal, NS_ConvertUTF16toUTF8(aScope), aCallback);
}
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();
}
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);
+ }
}
};
class UpdateResultRunnable final : public WorkerRunnable
{
RefPtr<PromiseWorkerProxy> mPromiseProxy;
IPC::Message mSerializedErrorResult;
@@ -565,41 +571,46 @@ private:
{}
RefPtr<PromiseWorkerProxy> mPromiseProxy;
const nsString mScope;
};
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;
}
NS_IMETHOD
UnregisterFailed() override
{
AssertIsOnMainThread();
- mPromise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
+ if (RefPtr<Promise> promise = mPromise.Get()) {
+ promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
+ }
return NS_OK;
}
private:
~UnregisterCallback()
{ }
};
@@ -759,17 +770,17 @@ ServiceWorkerRegistrationMainThread::Upd
if (NS_WARN_IF(aRv.Failed())) {
return nullptr;
}
nsCOMPtr<nsIDocument> doc = GetOwner()->GetExtantDoc();
MOZ_ASSERT(doc);
RefPtr<MainThreadUpdateCallback> cb =
- new MainThreadUpdateCallback(promise);
+ new MainThreadUpdateCallback(GetOwner(), promise);
UpdateInternal(doc->NodePrincipal(), mScope, cb);
return promise.forget();
}
already_AddRefed<Promise>
ServiceWorkerRegistrationMainThread::Unregister(ErrorResult& aRv)
{
@@ -817,17 +828,17 @@ ServiceWorkerRegistrationMainThread::Unr
nsCOMPtr<nsIServiceWorkerManager> swm =
mozilla::services::GetServiceWorkerManager();
RefPtr<Promise> promise = Promise::Create(go, aRv);
if (NS_WARN_IF(aRv.Failed())) {
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);
if (aRv.Failed()) {
return nullptr;
}
return promise.forget();