From 3b4a81565780b724841c2950513416f14ab95ff3 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 12 Apr 2018 23:26:44 -0400 Subject: Bug 1453339 - Make it harder to mess up Promise::All. r=peterv, a=RyanVM MozReview-Commit-ID: UO4wssYHj7 --- dom/cache/Cache.cpp | 2 +- dom/promise/Promise.cpp | 27 +++++++++++++++------------ dom/promise/Promise.h | 19 +++++++++++-------- dom/workers/ServiceWorkerEvents.cpp | 4 +--- layout/style/FontFaceSet.cpp | 12 +----------- 5 files changed, 29 insertions(+), 35 deletions(-) diff --git a/dom/cache/Cache.cpp b/dom/cache/Cache.cpp index 0d5815edb..b183bf387 100644 --- a/dom/cache/Cache.cpp +++ b/dom/cache/Cache.cpp @@ -618,7 +618,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 00b78143e..557f3a1f9 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -561,37 +561,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()); - if (!global) { + JS::Rooted globalObj(aCx, JS::CurrentGlobalOrNull(aCx)); + if (!globalObj) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } - JSContext* cx = aGlobal.Context(); + nsCOMPtr global = xpc::NativeGlobal(globalObj); + if (!global) { + aRv.Throw(NS_ERROR_UNEXPECTED); + return nullptr; + } - 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 f2ad3bd6c..642603a11 100644 --- a/dom/promise/Promise.h +++ b/dom/promise/Promise.h @@ -188,23 +188,26 @@ public: WrapObject(JSContext* aCx, JS::Handle aGivenProto, JS::MutableHandle aWrapper); - // 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/dom/workers/ServiceWorkerEvents.cpp b/dom/workers/ServiceWorkerEvents.cpp index 09b09a24b..780b2f5f8 100644 --- a/dom/workers/ServiceWorkerEvents.cpp +++ b/dom/workers/ServiceWorkerEvents.cpp @@ -948,10 +948,8 @@ ExtendableEvent::GetPromise() } JSContext* cx = jsapi.cx(); - GlobalObject global(cx, globalObj->GetGlobalJSObject()); - ErrorResult result; - RefPtr p = Promise::All(global, Move(mPromises), result); + RefPtr p = Promise::All(cx, Move(mPromises), result); if (NS_WARN_IF(result.MaybeSetPendingException(cx))) { return nullptr; } diff --git a/layout/style/FontFaceSet.cpp b/layout/style/FontFaceSet.cpp index 59626fba4..550a7d71a 100644 --- a/layout/style/FontFaceSet.cpp +++ b/layout/style/FontFaceSet.cpp @@ -343,17 +343,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 -- cgit v1.2.3