Bug 1565930, try to make Promise less error prone to compartment mismatches, r=bzbarsky a=RyanVM
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Mon, 29 Jul 2019 14:43:54 +0000
changeset 544847 13b13662c19a39e5a9660cbd5e54a473b1392cfe
parent 544846 cc88bee2c7a9f8fe9254371b8bd03afd8d6ace59
child 544848 9d64723b543f4dfac783ed782829b661422024f2
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky, RyanVM
bugs1565930
milestone69.0
Bug 1565930, try to make Promise less error prone to compartment mismatches, r=bzbarsky a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D38762
dom/base/BodyConsumer.cpp
dom/base/Document.cpp
dom/ipc/JSWindowActor.cpp
dom/promise/Promise.cpp
dom/promise/Promise.h
dom/push/PushManager.cpp
js/xpconnect/loader/ChromeScriptLoader.cpp
tools/profiler/gecko/nsProfiler.cpp
xpcom/base/nsSystemInfo.cpp
--- a/dom/base/BodyConsumer.cpp
+++ b/dom/base/BodyConsumer.cpp
@@ -686,17 +686,17 @@ void BodyConsumer::ContinueConsumeBody(n
       JS::Rooted<JSObject*> arrayBuffer(cx);
       BodyUtil::ConsumeArrayBuffer(cx, &arrayBuffer, aResultLength, aResult,
                                    error);
 
       if (!error.Failed()) {
         JS::Rooted<JS::Value> val(cx);
         val.setObjectOrNull(arrayBuffer);
 
-        localPromise->MaybeResolve(cx, val);
+        localPromise->MaybeResolve(val);
         // ArrayBuffer takes over ownership.
         aResult = nullptr;
       }
       break;
     }
     case CONSUME_BLOB: {
       MOZ_CRASH("This should not happen.");
       break;
@@ -720,17 +720,17 @@ void BodyConsumer::ContinueConsumeBody(n
       if (NS_SUCCEEDED(
               BodyUtil::ConsumeText(aResultLength, aResult, decoded))) {
         if (mConsumeType == CONSUME_TEXT) {
           localPromise->MaybeResolve(decoded);
         } else {
           JS::Rooted<JS::Value> json(cx);
           BodyUtil::ConsumeJson(cx, &json, decoded, error);
           if (!error.Failed()) {
-            localPromise->MaybeResolve(cx, json);
+            localPromise->MaybeResolve(json);
           }
         }
       };
       break;
     }
     default:
       MOZ_ASSERT_UNREACHABLE("Unexpected consume body type");
   }
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -12464,23 +12464,23 @@ class UnblockParsingPromiseHandler final
       mDocument->BlockOnload();
       mDocument->BlockDOMContentLoaded();
     }
   }
 
   void ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
     MaybeUnblockParser();
 
-    mPromise->MaybeResolve(aCx, aValue);
+    mPromise->MaybeResolve(aValue);
   }
 
   void RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
     MaybeUnblockParser();
 
-    mPromise->MaybeReject(aCx, aValue);
+    mPromise->MaybeReject(aValue);
   }
 
  protected:
   virtual ~UnblockParsingPromiseHandler() {
     // If we're being cleaned up by the cycle collector, our mDocument reference
     // may have been unlinked while our mParser weak reference is still alive.
     if (mDocument) {
       MaybeUnblockParser();
--- a/dom/ipc/JSWindowActor.cpp
+++ b/dom/ipc/JSWindowActor.cpp
@@ -210,17 +210,17 @@ void JSWindowActor::ReceiveMessageOrQuer
   messageListener->ReceiveMessage(argument, &retval, aRv,
                                   "JSWindowActor receive message");
 
   // If we have a promise, resolve or reject it respectively.
   if (promise) {
     if (aRv.Failed()) {
       promise->MaybeReject(aRv);
     } else {
-      promise->MaybeResolve(aCx, retval);
+      promise->MaybeResolve(retval);
     }
   }
 }
 
 void JSWindowActor::ReceiveQueryReply(JSContext* aCx,
                                       const JSWindowActorMessageMeta& aMetadata,
                                       JS::Handle<JS::Value> aData,
                                       ErrorResult& aRv) {
@@ -239,17 +239,17 @@ void JSWindowActor::ReceiveQueryReply(JS
   JSAutoRealm ar(aCx, promise->PromiseObj());
   JS::RootedValue data(aCx, aData);
   if (NS_WARN_IF(!JS_WrapValue(aCx, &data))) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
   if (aMetadata.kind() == JSWindowActorMessageKind::QueryResolve) {
-    promise->MaybeResolve(aCx, data);
+    promise->MaybeResolve(data);
   } else {
     promise->MaybeReject(NS_ERROR_DOM_OPERATION_ERR);
   }
 }
 
 // Native handler for our generated promise which is used to handle Queries and
 // send the reply when their promises have been resolved.
 JSWindowActor::QueryHandler::QueryHandler(
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -233,23 +233,23 @@ void Promise::Then(JSContext* aCx,
 }
 
 void PromiseNativeThenHandlerBase::ResolvedCallback(
     JSContext* aCx, JS::Handle<JS::Value> aValue) {
   RefPtr<Promise> promise = CallResolveCallback(aCx, aValue);
   if (promise) {
     mPromise->MaybeResolve(promise);
   } else {
-    mPromise->MaybeResolve(JS::UndefinedHandleValue);
+    mPromise->MaybeResolveWithUndefined();
   }
 }
 
 void PromiseNativeThenHandlerBase::RejectedCallback(
     JSContext* aCx, JS::Handle<JS::Value> aValue) {
-  mPromise->MaybeReject(aCx, aValue);
+  mPromise->MaybeReject(aValue);
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(PromiseNativeThenHandlerBase)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(PromiseNativeThenHandlerBase)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPromise)
   tmp->Traverse(cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
@@ -465,17 +465,17 @@ void Promise::AppendNativeHandler(Promis
     return;
   }
 }
 
 void Promise::HandleException(JSContext* aCx) {
   JS::Rooted<JS::Value> exn(aCx);
   if (JS_GetPendingException(aCx, &exn)) {
     JS_ClearPendingException(aCx);
-    // This is only called from MaybeSomething, so it's OK to MaybeReject here.
+    // Always reject even if this was called in *Resolve.
     MaybeReject(aCx, exn);
   }
 }
 
 // static
 already_AddRefed<Promise> Promise::CreateFromExisting(
     nsIGlobalObject* aGlobal, JS::Handle<JSObject*> aPromiseObj,
     PropagateUserInteraction aPropagateUserInteraction) {
@@ -551,30 +551,36 @@ void Promise::MaybeResolveWithClone(JSCo
                                     JS::Handle<JS::Value> aValue) {
   JS::Rooted<JSObject*> sourceScope(aCx, JS::CurrentGlobalOrNull(aCx));
   AutoEntryScript aes(GetParentObject(), "Promise resolution");
   JSContext* cx = aes.cx();
   JS::Rooted<JS::Value> value(cx, aValue);
 
   xpc::StackScopedCloneOptions options;
   options.wrapReflectors = true;
-  StackScopedClone(cx, options, sourceScope, &value);
+  if (!StackScopedClone(cx, options, sourceScope, &value)) {
+    HandleException(cx);
+    return;
+  }
   MaybeResolve(aCx, value);
 }
 
 void Promise::MaybeRejectWithClone(JSContext* aCx,
                                    JS::Handle<JS::Value> aValue) {
   JS::Rooted<JSObject*> sourceScope(aCx, JS::CurrentGlobalOrNull(aCx));
   AutoEntryScript aes(GetParentObject(), "Promise rejection");
   JSContext* cx = aes.cx();
   JS::Rooted<JS::Value> value(cx, aValue);
 
   xpc::StackScopedCloneOptions options;
   options.wrapReflectors = true;
-  StackScopedClone(cx, options, sourceScope, &value);
+  if (!StackScopedClone(cx, options, sourceScope, &value)) {
+    HandleException(cx);
+    return;
+  }
   MaybeReject(aCx, value);
 }
 
 // A WorkerRunnable to resolve/reject the Promise on the worker thread.
 // Calling thread MUST hold PromiseWorkerProxy's mutex before creating this.
 class PromiseWorkerProxyRunnable : public WorkerRunnable {
  public:
   PromiseWorkerProxyRunnable(PromiseWorkerProxy* aPromiseWorkerProxy,
--- a/dom/promise/Promise.h
+++ b/dom/promise/Promise.h
@@ -70,30 +70,30 @@ class Promise : public nsISupports, publ
           eDontPropagateUserInteraction);
 
   // Reports a rejected Promise by sending an error report.
   static void ReportRejectedPromise(JSContext* aCx, JS::HandleObject aPromise);
 
   typedef void (Promise::*MaybeFunc)(JSContext* aCx,
                                      JS::Handle<JS::Value> aValue);
 
-  void MaybeResolve(JSContext* aCx, JS::Handle<JS::Value> aValue);
-  void MaybeReject(JSContext* aCx, JS::Handle<JS::Value> aValue);
-
   // Helpers for using Promise from C++.
   // Most DOM objects are handled already.  To add a new type T, add a
   // ToJSValue overload in ToJSValue.h.
   // aArg is a const reference so we can pass rvalues like integer constants
   template <typename T>
   void MaybeResolve(T&& aArg) {
     MaybeSomething(std::forward<T>(aArg), &Promise::MaybeResolve);
   }
 
   void MaybeResolveWithUndefined();
 
+  void MaybeReject(JS::Handle<JS::Value> aValue) {
+    MaybeSomething(aValue, &Promise::MaybeReject);
+  }
   inline void MaybeReject(nsresult aArg) {
     MOZ_ASSERT(NS_FAILED(aArg));
     MaybeSomething(aArg, &Promise::MaybeReject);
   }
 
   inline void MaybeReject(ErrorResult& aArg) {
     MOZ_ASSERT(aArg.Failed());
     MaybeSomething(aArg, &Promise::MaybeReject);
@@ -231,16 +231,19 @@ class Promise : public nsISupports, publ
   // Pass ePropagateUserInteraction for aPropagateUserInteraction if you want
   // the promise resolve handler to be called as if we were handling user
   // input events in case we are currently handling user input events.
   void CreateWrapper(JS::Handle<JSObject*> aDesiredProto, ErrorResult& aRv,
                      PropagateUserInteraction aPropagateUserInteraction =
                          eDontPropagateUserInteraction);
 
  private:
+  void MaybeResolve(JSContext* aCx, JS::Handle<JS::Value> aValue);
+  void MaybeReject(JSContext* aCx, JS::Handle<JS::Value> aValue);
+
   template <typename T>
   void MaybeSomething(T&& aArgument, MaybeFunc aFunc) {
     MOZ_ASSERT(PromiseObj());  // It was preserved!
 
     AutoEntryScript aes(mGlobal, "Promise resolution or rejection");
     JSContext* cx = aes.cx();
 
     JS::Rooted<JS::Value> val(cx);
--- a/dom/push/PushManager.cpp
+++ b/dom/push/PushManager.cpp
@@ -294,17 +294,17 @@ class PermissionResultRunnable final : p
   bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override {
     MOZ_ASSERT(aWorkerPrivate);
     aWorkerPrivate->AssertIsOnWorkerThread();
 
     RefPtr<Promise> promise = mProxy->WorkerPromise();
     if (NS_SUCCEEDED(mStatus)) {
       promise->MaybeResolve(mState);
     } else {
-      promise->MaybeReject(aCx, JS::UndefinedHandleValue);
+      promise->MaybeRejectWithUndefined();
     }
 
     mProxy->CleanUp();
 
     return true;
   }
 
  private:
@@ -432,17 +432,17 @@ already_AddRefed<Promise> PushManager::P
   nsCOMPtr<nsIGlobalObject> global = worker->GlobalScope();
   RefPtr<Promise> p = Promise::Create(global, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   RefPtr<PromiseWorkerProxy> proxy = PromiseWorkerProxy::Create(worker, p);
   if (!proxy) {
-    p->MaybeReject(worker->GetJSContext(), JS::UndefinedHandleValue);
+    p->MaybeRejectWithUndefined();
     return p.forget();
   }
 
   RefPtr<PermissionStateRunnable> r = new PermissionStateRunnable(proxy);
   NS_DispatchToMainThread(r);
 
   return p.forget();
 }
--- a/js/xpconnect/loader/ChromeScriptLoader.cpp
+++ b/js/xpconnect/loader/ChromeScriptLoader.cpp
@@ -186,17 +186,17 @@ void AsyncScriptCompiler::Finish(JSConte
   mPromise->MaybeResolve(result);
 }
 
 void AsyncScriptCompiler::Reject(JSContext* aCx) {
   RootedValue value(aCx, JS::UndefinedValue());
   if (JS_GetPendingException(aCx, &value)) {
     JS_ClearPendingException(aCx);
   }
-  mPromise->MaybeReject(aCx, value);
+  mPromise->MaybeReject(value);
 }
 
 void AsyncScriptCompiler::Reject(JSContext* aCx, const char* aMsg) {
   nsAutoString msg;
   msg.AppendASCII(aMsg);
   msg.AppendLiteral(": ");
   AppendUTF8toUTF16(mURL, msg);
 
--- a/tools/profiler/gecko/nsProfiler.cpp
+++ b/tools/profiler/gecko/nsProfiler.cpp
@@ -303,17 +303,17 @@ nsProfiler::GetProfileDataAsync(double a
                 if (!jsapi.HasException()) {
                   promise->MaybeReject(NS_ERROR_DOM_UNKNOWN_ERR);
                 } else {
                   JS::RootedValue exn(cx);
                   DebugOnly<bool> gotException = jsapi.StealException(&exn);
                   MOZ_ASSERT(gotException);
 
                   jsapi.ClearException();
-                  promise->MaybeReject(cx, exn);
+                  promise->MaybeReject(exn);
                 }
               } else {
                 promise->MaybeResolve(val);
               }
             }
           },
           [promise](nsresult aRv) { promise->MaybeReject(aRv); });
 
--- a/xpcom/base/nsSystemInfo.cpp
+++ b/xpcom/base/nsSystemInfo.cpp
@@ -1155,17 +1155,17 @@ nsSystemInfo::GetDiskInfo(JSContext* aCx
         // The above can fail due to OOM
         if (!succeededSettingAllObjects) {
           JS_ClearPendingException(cx);
           capturedPromise->MaybeReject(NS_ERROR_FAILURE);
           return;
         }
 
         JS::Rooted<JS::Value> val(cx, JS::ObjectValue(*jsInfo));
-        capturedPromise->MaybeResolve(cx, val);
+        capturedPromise->MaybeResolve(val);
       },
       [capturedPromise](const nsresult rv) {
         capturedPromise->MaybeReject(rv);
       });
 
   promise.forget(aResult);
 #endif
   return NS_OK;