From e12fcacd5350348e76d8101ee5ec7f0577b4a750 Mon Sep 17 00:00:00 2001 From: Ben Kelly Date: Tue, 6 Sep 2016 14:38:13 -0700 Subject: [PATCH] Bug 1299887 Use a PromiseNativeHandler shim to ensure real PromiseNativeHandlers are released when the Promise settles. r=bz --- dom/promise/Promise.cpp | 64 +++++++++++++++++++++++++++- dom/promise/PromiseNativeHandler.cpp | 26 ----------- dom/promise/PromiseNativeHandler.h | 6 --- dom/promise/moz.build | 1 - 4 files changed, 62 insertions(+), 35 deletions(-) delete mode 100644 dom/promise/PromiseNativeHandler.cpp diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index 9fb4cce84fe2..06b6ed980753 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -740,7 +740,7 @@ NativeHandlerCallback(JSContext* aCx, unsigned aArgc, JS::Value* aVp) SLOT_NATIVEHANDLER)); MOZ_ASSERT(v.isObject()); - PromiseNativeHandler* handler; + PromiseNativeHandler* handler = nullptr; if (NS_FAILED(UNWRAP_OBJECT(PromiseNativeHandler, &v.toObject(), handler))) { return Throw(aCx, NS_ERROR_UNEXPECTED); @@ -781,6 +781,59 @@ CreateNativeHandlerFunction(JSContext* aCx, JS::Handle aHolder, return obj; } +namespace { + +class PromiseNativeHandlerShim final : public PromiseNativeHandler +{ + RefPtr mInner; + + ~PromiseNativeHandlerShim() + { + } + +public: + explicit PromiseNativeHandlerShim(PromiseNativeHandler* aInner) + : mInner(aInner) + { + MOZ_ASSERT(mInner); + } + + void + ResolvedCallback(JSContext* aCx, JS::Handle aValue) override + { + mInner->ResolvedCallback(aCx, aValue); + mInner = nullptr; + } + + void + RejectedCallback(JSContext* aCx, JS::Handle aValue) override + { + mInner->RejectedCallback(aCx, aValue); + mInner = nullptr; + } + + bool + WrapObject(JSContext* aCx, JS::Handle aGivenProto, + JS::MutableHandle aWrapper) + { + return PromiseNativeHandlerBinding::Wrap(aCx, this, aGivenProto, aWrapper); + } + + NS_DECL_CYCLE_COLLECTING_ISUPPORTS + NS_DECL_CYCLE_COLLECTION_CLASS(PromiseNativeHandlerShim) +}; + +NS_IMPL_CYCLE_COLLECTION(PromiseNativeHandlerShim, mInner) + +NS_IMPL_CYCLE_COLLECTING_ADDREF(PromiseNativeHandlerShim) +NS_IMPL_CYCLE_COLLECTING_RELEASE(PromiseNativeHandlerShim) + +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(PromiseNativeHandlerShim) + NS_INTERFACE_MAP_ENTRY(nsISupports) +NS_INTERFACE_MAP_END + +} // anonymous namespace + void Promise::AppendNativeHandler(PromiseNativeHandler* aRunnable) { @@ -793,11 +846,18 @@ Promise::AppendNativeHandler(PromiseNativeHandler* aRunnable) return; } + // The self-hosted promise js may keep the object we pass to it alive + // for quite a while depending on when GC runs. Therefore, pass a shim + // object instead. The shim will free its inner PromiseNativeHandler + // after the promise has settled just like our previous c++ promises did. + RefPtr shim = + new PromiseNativeHandlerShim(aRunnable); + JSContext* cx = jsapi.cx(); JS::Rooted handlerWrapper(cx); // Note: PromiseNativeHandler is NOT wrappercached. So we can't use // ToJSValue here, because it will try to do XPConnect wrapping on it, sadly. - if (NS_WARN_IF(!aRunnable->WrapObject(cx, nullptr, &handlerWrapper))) { + if (NS_WARN_IF(!shim->WrapObject(cx, nullptr, &handlerWrapper))) { // Again, no way to report errors. jsapi.ClearException(); return; diff --git a/dom/promise/PromiseNativeHandler.cpp b/dom/promise/PromiseNativeHandler.cpp deleted file mode 100644 index 50d0e1b96c5b..000000000000 --- a/dom/promise/PromiseNativeHandler.cpp +++ /dev/null @@ -1,26 +0,0 @@ -/* -*- 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/PromiseNativeHandler.h" -#include "mozilla/dom/PromiseBinding.h" - -namespace mozilla { -namespace dom { - -#ifdef SPIDERMONKEY_PROMISE -bool -PromiseNativeHandler::WrapObject(JSContext* aCx, - JS::Handle aGivenProto, - JS::MutableHandle aWrapper) -{ - return PromiseNativeHandlerBinding::Wrap(aCx, this, aGivenProto, aWrapper); -} -#endif // SPIDERMONKEY_PROMISE - -} // namespace dom -} // namespace mozilla - - diff --git a/dom/promise/PromiseNativeHandler.h b/dom/promise/PromiseNativeHandler.h index 6e91f745c9b0..6ba7142aa138 100644 --- a/dom/promise/PromiseNativeHandler.h +++ b/dom/promise/PromiseNativeHandler.h @@ -30,12 +30,6 @@ public: virtual void RejectedCallback(JSContext* aCx, JS::Handle aValue) = 0; - -#ifdef SPIDERMONKEY_PROMISE - bool WrapObject(JSContext* aCx, JS::Handle aGivenProto, - JS::MutableHandle aWrapper); -#endif // SPIDERMONKEY_PROMISE - }; } // namespace dom diff --git a/dom/promise/moz.build b/dom/promise/moz.build index b1cdbcf5c019..11d2a7496242 100644 --- a/dom/promise/moz.build +++ b/dom/promise/moz.build @@ -15,7 +15,6 @@ UNIFIED_SOURCES += [ 'Promise.cpp', 'PromiseCallback.cpp', 'PromiseDebugging.cpp', - 'PromiseNativeHandler.cpp', ] LOCAL_INCLUDES += [