From ee9adaed00d5a8eac2c9c2b1935dc82539556c53 Mon Sep 17 00:00:00 2001 From: Nikhil Marathe Date: Wed, 22 Apr 2015 16:34:21 -0700 Subject: [PATCH] Bug 1058695 - Add member to nsIGlobalObject to detect it is going away. Make promises use it. r=bholley --- dom/base/nsGlobalWindow.cpp | 2 ++ dom/base/nsIGlobalObject.h | 34 +++++++++++++++++++++++++++++ dom/promise/Promise.cpp | 24 +++++++++++++++++++-- dom/promise/PromiseCallback.cpp | 36 +++++++++++++++++-------------- dom/promise/PromiseCallback.h | 20 ++++++++--------- js/src/jsapi.h | 1 - js/src/jsfriendapi.h | 2 ++ js/xpconnect/src/XPCJSRuntime.cpp | 7 +++++- 8 files changed, 96 insertions(+), 30 deletions(-) diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index b944165c5a4..60a392115b0 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -1438,6 +1438,8 @@ nsGlobalWindow::CleanUp() return; mCleanedUp = true; + StartDying(); + mEventTargetObjects.EnumerateEntries(DisconnectEventTargetObjects, nullptr); mEventTargetObjects.Clear(); diff --git a/dom/base/nsIGlobalObject.h b/dom/base/nsIGlobalObject.h index c5b28041acc..35794d49ea2 100644 --- a/dom/base/nsIGlobalObject.h +++ b/dom/base/nsIGlobalObject.h @@ -17,13 +17,47 @@ class nsIPrincipal; class nsIGlobalObject : public nsISupports { + bool mIsDying; + +protected: + nsIGlobalObject() + : mIsDying(false) + {} + public: NS_DECLARE_STATIC_IID_ACCESSOR(NS_IGLOBALOBJECT_IID) + /** + * This check is added to deal with Promise microtask queues. On the main + * thread, we do not impose restrictions about when script stops running or + * when runnables can no longer be dispatched to the main thread. This means + * it is possible for a Promise chain to keep resolving an infinite chain of + * promises, preventing the browser from shutting down. See Bug 1058695. To + * prevent this, the nsGlobalWindow subclass sets this flag when it is + * closed. The Promise implementation checks this and prohibits new runnables + * from being dispatched. + * + * We pair this with checks during processing the promise microtask queue + * that pops up the slow script dialog when the Promise queue is preventing + * a window from going away. + */ + bool + IsDying() const + { + return mIsDying; + } + virtual JSObject* GetGlobalJSObject() = 0; // This method is not meant to be overridden. nsIPrincipal* PrincipalOrNull(); + +protected: + void + StartDying() + { + mIsDying = true; + } }; NS_DEFINE_STATIC_IID_ACCESSOR(nsIGlobalObject, diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index b323e78491c..a29db245754 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -263,7 +263,7 @@ protected: // console though, for debugging. } - return NS_OK; + return rv.ErrorCode(); } private: @@ -489,13 +489,24 @@ Promise::PerformMicroTaskCheckpoint() return false; } + Maybe cx; + if (NS_IsMainThread()) { + cx.emplace(); + } + do { nsCOMPtr runnable = microtaskQueue.ElementAt(0); MOZ_ASSERT(runnable); // This function can re-enter, so we remove the element before calling. microtaskQueue.RemoveElementAt(0); - runnable->Run(); + nsresult rv = runnable->Run(); + if (NS_WARN_IF(NS_FAILED(rv))) { + return false; + } + if (cx.isSome()) { + JS::CheckForJSInterrupt(cx.ref()); + } } while (!microtaskQueue.IsEmpty()); return true; @@ -1071,6 +1082,10 @@ void Promise::AppendCallbacks(PromiseCallback* aResolveCallback, PromiseCallback* aRejectCallback) { + if (mGlobal->IsDying()) { + return; + } + MOZ_ASSERT(aResolveCallback); MOZ_ASSERT(aRejectCallback); @@ -1297,7 +1312,12 @@ Promise::RejectInternal(JSContext* aCx, void Promise::Settle(JS::Handle aValue, PromiseState aState) { + if (mGlobal->IsDying()) { + return; + } + mSettlementTimestamp = TimeStamp::Now(); + SetResult(aValue); SetState(aState); diff --git a/dom/promise/PromiseCallback.cpp b/dom/promise/PromiseCallback.cpp index eff07757fe7..195beba7b49 100644 --- a/dom/promise/PromiseCallback.cpp +++ b/dom/promise/PromiseCallback.cpp @@ -71,7 +71,7 @@ ResolvePromiseCallback::~ResolvePromiseCallback() DropJSObjects(this); } -void +nsresult ResolvePromiseCallback::Call(JSContext* aCx, JS::Handle aValue) { @@ -81,10 +81,11 @@ ResolvePromiseCallback::Call(JSContext* aCx, JS::Rooted value(aCx, aValue); if (!JS_WrapValue(aCx, &value)) { NS_WARNING("Failed to wrap value into the right compartment."); - return; + return NS_ERROR_FAILURE; } mPromise->ResolveInternal(aCx, value); + return NS_OK; } // RejectPromiseCallback @@ -129,7 +130,7 @@ RejectPromiseCallback::~RejectPromiseCallback() DropJSObjects(this); } -void +nsresult RejectPromiseCallback::Call(JSContext* aCx, JS::Handle aValue) { @@ -139,11 +140,12 @@ RejectPromiseCallback::Call(JSContext* aCx, JS::Rooted value(aCx, aValue); if (!JS_WrapValue(aCx, &value)) { NS_WARNING("Failed to wrap value into the right compartment."); - return; + return NS_ERROR_FAILURE; } mPromise->RejectInternal(aCx, value); + return NS_OK; } // WrapperPromiseCallback @@ -190,7 +192,7 @@ WrapperPromiseCallback::~WrapperPromiseCallback() DropJSObjects(this); } -void +nsresult WrapperPromiseCallback::Call(JSContext* aCx, JS::Handle aValue) { @@ -198,7 +200,7 @@ WrapperPromiseCallback::Call(JSContext* aCx, JS::Rooted value(aCx, aValue); if (!JS_WrapValue(aCx, &value)) { NS_WARNING("Failed to wrap value into the right compartment."); - return; + return NS_ERROR_FAILURE; } ErrorResult rv; @@ -219,7 +221,7 @@ WrapperPromiseCallback::Call(JSContext* aCx, if (!JS_WrapValue(aCx, &value)) { NS_WARNING("Failed to wrap value into the right compartment."); - return; + return NS_ERROR_FAILURE; } } else { // Convert the ErrorResult to a JS exception object that we can reject @@ -232,7 +234,7 @@ WrapperPromiseCallback::Call(JSContext* aCx, } mNextPromise->RejectInternal(aCx, value); - return; + return NS_OK; } // If the return value is the same as the promise itself, throw TypeError. @@ -270,7 +272,7 @@ WrapperPromiseCallback::Call(JSContext* aCx, if (!fn) { // Out of memory. Promise will stay unresolved. JS_ClearPendingException(aCx); - return; + return NS_ERROR_OUT_OF_MEMORY; } JS::Rooted message(aCx, @@ -279,7 +281,7 @@ WrapperPromiseCallback::Call(JSContext* aCx, if (!message) { // Out of memory. Promise will stay unresolved. JS_ClearPendingException(aCx); - return; + return NS_ERROR_OUT_OF_MEMORY; } JS::Rooted typeError(aCx); @@ -287,21 +289,22 @@ WrapperPromiseCallback::Call(JSContext* aCx, nullptr, message, &typeError)) { // Out of memory. Promise will stay unresolved. JS_ClearPendingException(aCx); - return; + return NS_ERROR_OUT_OF_MEMORY; } mNextPromise->RejectInternal(aCx, typeError); - return; + return NS_OK; } } // Otherwise, run resolver's resolve with value. if (!JS_WrapValue(aCx, &retValue)) { NS_WARNING("Failed to wrap value into the right compartment."); - return; + return NS_ERROR_FAILURE; } mNextPromise->ResolveInternal(aCx, retValue); + return NS_OK; } // NativePromiseCallback @@ -327,21 +330,22 @@ NativePromiseCallback::~NativePromiseCallback() { } -void +nsresult NativePromiseCallback::Call(JSContext* aCx, JS::Handle aValue) { if (mState == Promise::Resolved) { mHandler->ResolvedCallback(aCx, aValue); - return; + return NS_OK; } if (mState == Promise::Rejected) { mHandler->RejectedCallback(aCx, aValue); - return; + return NS_OK; } NS_NOTREACHED("huh?"); + return NS_ERROR_FAILURE; } /* static */ PromiseCallback* diff --git a/dom/promise/PromiseCallback.h b/dom/promise/PromiseCallback.h index 64e870b1703..1802556410b 100644 --- a/dom/promise/PromiseCallback.h +++ b/dom/promise/PromiseCallback.h @@ -26,8 +26,8 @@ public: PromiseCallback(); - virtual void Call(JSContext* aCx, - JS::Handle aValue) = 0; + virtual nsresult Call(JSContext* aCx, + JS::Handle aValue) = 0; // Return the Promise that this callback will end up resolving or // rejecting, if any. @@ -54,8 +54,8 @@ public: NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(WrapperPromiseCallback, PromiseCallback) - void Call(JSContext* aCx, - JS::Handle aValue) override; + nsresult Call(JSContext* aCx, + JS::Handle aValue) override; Promise* GetDependentPromise() override { @@ -82,8 +82,8 @@ public: NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(ResolvePromiseCallback, PromiseCallback) - void Call(JSContext* aCx, - JS::Handle aValue) override; + nsresult Call(JSContext* aCx, + JS::Handle aValue) override; Promise* GetDependentPromise() override { @@ -108,8 +108,8 @@ public: NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(RejectPromiseCallback, PromiseCallback) - void Call(JSContext* aCx, - JS::Handle aValue) override; + nsresult Call(JSContext* aCx, + JS::Handle aValue) override; Promise* GetDependentPromise() override { @@ -133,8 +133,8 @@ public: NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(NativePromiseCallback, PromiseCallback) - void Call(JSContext* aCx, - JS::Handle aValue) override; + nsresult Call(JSContext* aCx, + JS::Handle aValue) override; Promise* GetDependentPromise() override { diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 61d6654b50a..b226380f341 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -3941,7 +3941,6 @@ extern JS_PUBLIC_API(bool) Construct(JSContext* cx, JS::HandleValue fun, const JS::HandleValueArray& args, MutableHandleValue rval); - } /* namespace JS */ /* diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index ade724c5928..539d43a7c68 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -2742,6 +2742,8 @@ extern JS_FRIEND_API(bool) DefineOwnProperty(JSContext* cx, JSObject* objArg, jsid idArg, JS::Handle descriptor, JS::ObjectOpResult& result); +extern JS_FRIEND_API(bool) +CheckForInterrupt(JSContext* cx); } /* namespace js */ extern JS_FRIEND_API(void) diff --git a/js/xpconnect/src/XPCJSRuntime.cpp b/js/xpconnect/src/XPCJSRuntime.cpp index d1ac68b4e0f..84f02d95f14 100644 --- a/js/xpconnect/src/XPCJSRuntime.cpp +++ b/js/xpconnect/src/XPCJSRuntime.cpp @@ -1461,8 +1461,13 @@ XPCJSRuntime::InterruptCallback(JSContext* cx) win = WindowGlobalOrNull(proto); } } - if (!win) + + if (!win) { + NS_WARNING("No active window"); return true; + } + + MOZ_ASSERT(!win->IsDying()); if (win->GetIsPrerendered()) { // We cannot display a dialog if the page is being prerendered, so