From a0ee72689ecac156fc7c6e4c73e06e6f23c5e783 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 12 Apr 2018 17:03:49 -0400 Subject: [PATCH] Bug 1453339 - Make it harder to mess up Promise::All. r=peterv MozReview-Commit-ID: UO4wssYHj7 --- dom/base/nsFrameLoader.cpp | 4 +--- dom/cache/Cache.cpp | 2 +- dom/promise/Promise.cpp | 27 +++++++++++++++------------ dom/promise/Promise.h | 19 +++++++++++-------- layout/style/FontFaceSet.cpp | 12 +----------- 5 files changed, 29 insertions(+), 35 deletions(-) diff --git a/dom/base/nsFrameLoader.cpp b/dom/base/nsFrameLoader.cpp index 9f9322788007..26233eea2ff7 100644 --- a/dom/base/nsFrameLoader.cpp +++ b/dom/base/nsFrameLoader.cpp @@ -395,8 +395,6 @@ nsFrameLoader::FireWillChangeProcessEvent() return nullptr; } JSContext* cx = jsapi.cx(); - GlobalObject global(cx, mOwnerContent->GetOwnerGlobal()->GetGlobalJSObject()); - MOZ_ASSERT(!global.Failed()); // Set our mBrowserChangingProcessBlockers property to refer to the blockers // list. We will synchronously dispatch a DOM event to collect this list of @@ -418,7 +416,7 @@ nsFrameLoader::FireWillChangeProcessEvent() mBrowserChangingProcessBlockers = nullptr; ErrorResult rv; - RefPtr allPromise = Promise::All(global, blockers, rv); + RefPtr allPromise = Promise::All(cx, blockers, rv); return allPromise.forget(); } diff --git a/dom/cache/Cache.cpp b/dom/cache/Cache.cpp index 06df0f2a7cb9..3fb8b6c36040 100644 --- a/dom/cache/Cache.cpp +++ b/dom/cache/Cache.cpp @@ -630,7 +630,7 @@ Cache::AddAll(const GlobalObject& aGlobal, new FetchHandler(mActor->GetWorkerHolder(), this, Move(aRequestList), promise); - RefPtr fetchPromise = Promise::All(aGlobal, fetchList, aRv); + RefPtr fetchPromise = Promise::All(aGlobal.Context(), fetchList, aRv); if (NS_WARN_IF(aRv.Failed())) { return nullptr; } diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index f31085c13894..67b1b6525ed7 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -135,37 +135,40 @@ Promise::Reject(nsIGlobalObject* aGlobal, JSContext* aCx, // static already_AddRefed -Promise::All(const GlobalObject& aGlobal, +Promise::All(JSContext* aCx, const nsTArray>& aPromiseList, ErrorResult& aRv) { - nsCOMPtr global; - global = do_QueryInterface(aGlobal.GetAsSupports()); + JS::Rooted globalObj(aCx, JS::CurrentGlobalOrNull(aCx)); + if (!globalObj) { + aRv.Throw(NS_ERROR_UNEXPECTED); + return nullptr; + } + + nsCOMPtr global = xpc::NativeGlobal(globalObj); if (!global) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } - JSContext* cx = aGlobal.Context(); - - JS::AutoObjectVector promises(cx); + JS::AutoObjectVector promises(aCx); if (!promises.reserve(aPromiseList.Length())) { - aRv.NoteJSContextException(cx); + aRv.NoteJSContextException(aCx); return nullptr; } for (auto& promise : aPromiseList) { - JS::Rooted promiseObj(cx, promise->PromiseObj()); + JS::Rooted promiseObj(aCx, promise->PromiseObj()); // Just in case, make sure these are all in the context compartment. - if (!JS_WrapObject(cx, &promiseObj)) { - aRv.NoteJSContextException(cx); + if (!JS_WrapObject(aCx, &promiseObj)) { + aRv.NoteJSContextException(aCx); return nullptr; } promises.infallibleAppend(promiseObj); } - JS::Rooted result(cx, JS::GetWaitForAllPromise(cx, promises)); + JS::Rooted result(aCx, JS::GetWaitForAllPromise(aCx, promises)); if (!result) { - aRv.NoteJSContextException(cx); + aRv.NoteJSContextException(aCx); return nullptr; } diff --git a/dom/promise/Promise.h b/dom/promise/Promise.h index 447a2a195152..2c13d0146c40 100644 --- a/dom/promise/Promise.h +++ b/dom/promise/Promise.h @@ -111,23 +111,26 @@ public: return mGlobal; } - // Do the equivalent of Promise.resolve in the current compartment of aCx. - // Errorrs are reported on the ErrorResult; if aRv comes back !Failed(), this - // function MUST return a non-null value. + // Do the equivalent of Promise.resolve in the compartment of aGlobal. The + // compartment of aCx is ignored. Errors are reported on the ErrorResult; if + // aRv comes back !Failed(), this function MUST return a non-null value. static already_AddRefed Resolve(nsIGlobalObject* aGlobal, JSContext* aCx, JS::Handle aValue, ErrorResult& aRv); - // Do the equivalent of Promise.reject in the current compartment of aCx. - // Errorrs are reported on the ErrorResult; if aRv comes back !Failed(), this - // function MUST return a non-null value. + // Do the equivalent of Promise.reject in the compartment of aGlobal. The + // compartment of aCx is ignored. Errors are reported on the ErrorResult; if + // aRv comes back !Failed(), this function MUST return a non-null value. static already_AddRefed Reject(nsIGlobalObject* aGlobal, JSContext* aCx, JS::Handle aValue, ErrorResult& aRv); + // Do the equivalent of Promise.all in the current compartment of aCx. Errors + // are reported on the ErrorResult; if aRv comes back !Failed(), this function + // MUST return a non-null value. static already_AddRefed - All(const GlobalObject& aGlobal, - const nsTArray>& aPromiseList, ErrorResult& aRv); + All(JSContext* aCx, const nsTArray>& aPromiseList, + ErrorResult& aRv); void Then(JSContext* aCx, diff --git a/layout/style/FontFaceSet.cpp b/layout/style/FontFaceSet.cpp index 130127a94cf9..ccd683a27a17 100644 --- a/layout/style/FontFaceSet.cpp +++ b/layout/style/FontFaceSet.cpp @@ -346,17 +346,7 @@ FontFaceSet::Load(JSContext* aCx, } } - nsIGlobalObject* globalObject = GetParentObject(); - if (!globalObject) { - aRv.Throw(NS_ERROR_FAILURE); - return nullptr; - } - - JS::Rooted jsGlobal(aCx, globalObject->GetGlobalJSObject()); - GlobalObject global(aCx, jsGlobal); - - RefPtr result = Promise::All(global, promises, aRv); - return result.forget(); + return Promise::All(aCx, promises, aRv); } bool