Bug 1512008 - Fix assertion failure with ReadableStream and Promise[Symbol.species]. r=arai
authorJason Orendorff <jorendorff@mozilla.com>
Sun, 09 Dec 2018 22:24:22 +0000
changeset 508916 c0c4ff2b0485101a3f587d0a799841c0039c2b18
parent 508915 3d9e1dd87d4539957dbcfa5ff93ef5c2b3036e8f
child 508917 1b641c848a60987464e438095f426baa66702086
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1512008
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1512008 - Fix assertion failure with ReadableStream and Promise[Symbol.species]. r=arai Differential Revision: https://phabricator.services.mozilla.com/D13945
js/src/builtin/Promise.cpp
js/src/jit-test/tests/stream/bug-1512008.js
js/src/jsapi.cpp
js/src/jsapi.h
js/src/vm/Compartment-inl.h
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -3339,18 +3339,27 @@ static bool PromiseThenNewPromiseCapabil
   return true;
 }
 
 // ES2016, 25.4.5.3., steps 3-5.
 MOZ_MUST_USE bool js::OriginalPromiseThen(
     JSContext* cx, HandleObject promiseObj, HandleValue onFulfilled,
     HandleValue onRejected, MutableHandleObject dependent,
     CreateDependentPromise createDependent) {
+  RootedValue promiseVal(cx, ObjectValue(*promiseObj));
   Rooted<PromiseObject*> promise(
-      cx, &CheckedUnwrap(promiseObj)->as<PromiseObject>());
+      cx,
+      UnwrapAndTypeCheckValue<PromiseObject>(cx, promiseVal, [cx, promiseObj] {
+        JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr,
+                                   JSMSG_INCOMPATIBLE_PROTO, "Promise", "then",
+                                   promiseObj->getClass()->name);
+      }));
+  if (!promise) {
+    return false;
+  }
 
   // Steps 3-4.
   Rooted<PromiseCapability> resultCapability(cx);
   if (!PromiseThenNewPromiseCapability(cx, promiseObj, createDependent,
                                        &resultCapability)) {
     return false;
   }
 
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/stream/bug-1512008.js
@@ -0,0 +1,6 @@
+Object.defineProperty(Promise, Symbol.species, {
+  value: function(g) {
+    g(function() {}, function() {})
+  }
+});
+new ReadableStream().tee();
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4029,36 +4029,28 @@ JS_PUBLIC_API bool JS::ResolvePromise(JS
   return ResolveOrRejectPromise(cx, promiseObj, resolutionValue, false);
 }
 
 JS_PUBLIC_API bool JS::RejectPromise(JSContext* cx, JS::HandleObject promiseObj,
                                      JS::HandleValue rejectionValue) {
   return ResolveOrRejectPromise(cx, promiseObj, rejectionValue, true);
 }
 
-static bool CallOriginalPromiseThenImpl(
-    JSContext* cx, JS::HandleObject promiseObj, JS::HandleObject onResolvedObj_,
-    JS::HandleObject onRejectedObj_, JS::MutableHandleObject resultObj,
+static MOZ_MUST_USE bool CallOriginalPromiseThenImpl(
+    JSContext* cx, JS::HandleObject promiseObj, JS::HandleObject onFulfilledObj,
+    JS::HandleObject onRejectedObj, JS::MutableHandleObject resultObj,
     CreateDependentPromise createDependent) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  cx->check(promiseObj, onResolvedObj_, onRejectedObj_);
-
-  MOZ_ASSERT_IF(onResolvedObj_, IsCallable(onResolvedObj_));
-  MOZ_ASSERT_IF(onRejectedObj_, IsCallable(onRejectedObj_));
-  RootedObject onResolvedObj(cx, onResolvedObj_);
-  RootedObject onRejectedObj(cx, onRejectedObj_);
-
-  if (IsWrapper(promiseObj) && !CheckedUnwrap(promiseObj)) {
-    ReportAccessDenied(cx);
-    return false;
-  }
-  MOZ_ASSERT(CheckedUnwrap(promiseObj)->is<PromiseObject>());
-
-  RootedValue onFulfilled(cx, ObjectOrNullValue(onResolvedObj));
+  cx->check(promiseObj, onFulfilledObj, onRejectedObj);
+
+  MOZ_ASSERT_IF(onFulfilledObj, IsCallable(onFulfilledObj));
+  MOZ_ASSERT_IF(onRejectedObj, IsCallable(onRejectedObj));
+
+  RootedValue onFulfilled(cx, ObjectOrNullValue(onFulfilledObj));
   RootedValue onRejected(cx, ObjectOrNullValue(onRejectedObj));
   return OriginalPromiseThen(cx, promiseObj, onFulfilled, onRejected, resultObj,
                              createDependent);
 }
 
 JS_PUBLIC_API JSObject* JS::CallOriginalPromiseThen(
     JSContext* cx, JS::HandleObject promiseObj, JS::HandleObject onResolvedObj,
     JS::HandleObject onRejectedObj) {
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3451,39 +3451,43 @@ extern JS_PUBLIC_API bool ResolvePromise
 extern JS_PUBLIC_API bool RejectPromise(JSContext* cx,
                                         JS::HandleObject promiseObj,
                                         JS::HandleValue rejectionValue);
 
 /**
  * Calls the current compartment's original Promise.prototype.then on the
  * given `promise`, with `onResolve` and `onReject` passed as arguments.
  *
- * Asserts if the passed-in `promise` object isn't an unwrapped instance of
- * `Promise` or a subclass or `onResolve` and `onReject` aren't both either
- * `nullptr` or callable objects.
+ * Throws a TypeError if `promise` isn't a Promise (or possibly a different
+ * error if it's a security wrapper or dead object proxy).
+ *
+ * Asserts that `onFulfilled` and `onRejected` are each either callable or
+ * null.
  */
 extern JS_PUBLIC_API JSObject* CallOriginalPromiseThen(
-    JSContext* cx, JS::HandleObject promise, JS::HandleObject onResolve,
-    JS::HandleObject onReject);
+    JSContext* cx, JS::HandleObject promise, JS::HandleObject onFulfilled,
+    JS::HandleObject onRejected);
 
 /**
  * Unforgeable, optimized version of the JS builtin Promise.prototype.then.
  *
  * Takes a Promise instance and `onResolve`, `onReject` callables to enqueue
  * as reactions for that promise. In difference to Promise.prototype.then,
  * this doesn't create and return a new Promise instance.
  *
- * Asserts if the passed-in `promise` object isn't an unwrapped instance of
- * `Promise` or a subclass or `onResolve` and `onReject` aren't both callable
- * objects.
+ * Throws a TypeError if `promise` isn't a Promise (or possibly a different
+ * error if it's a security wrapper or dead object proxy).
+ *
+ * Asserts that `onFulfilled` and `onRejected` are each either callable or
+ * null.
  */
 extern JS_PUBLIC_API bool AddPromiseReactions(JSContext* cx,
                                               JS::HandleObject promise,
-                                              JS::HandleObject onResolve,
-                                              JS::HandleObject onReject);
+                                              JS::HandleObject onFulfilled,
+                                              JS::HandleObject onRejected);
 
 // This enum specifies whether a promise is expected to keep track of
 // information that is useful for embedders to implement user activation
 // behavior handling as specified in the HTML spec:
 // https://html.spec.whatwg.org/multipage/interaction.html#triggered-by-user-activation
 // By default, promises created by SpiderMonkey do not make any attempt to keep
 // track of information about whether an activation behavior was being processed
 // when the original promise in a promise chain was created.  If the embedder
--- a/js/src/vm/Compartment-inl.h
+++ b/js/src/vm/Compartment-inl.h
@@ -113,122 +113,107 @@ namespace detail {
  * Return the name of class T as a static null-terminated ASCII string constant
  * (for error messages).
  */
 template <class T>
 const char* ClassName() {
   return T::class_.name;
 }
 
-template <class T>
-MOZ_MUST_USE T* UnwrapAndTypeCheckThisSlowPath(JSContext* cx, HandleValue val,
-                                               const char* methodName) {
-  if (!val.isObject()) {
-    JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr,
-                               JSMSG_INCOMPATIBLE_PROTO, ClassName<T>(),
-                               methodName, InformalValueTypeName(val));
-    return nullptr;
-  }
-
-  JSObject* obj = &val.toObject();
-  if (IsWrapper(obj)) {
-    obj = CheckedUnwrap(obj);
-    if (!obj) {
-      ReportAccessDenied(cx);
-      return nullptr;
-    }
-  }
-
-  if (!obj->is<T>()) {
-    JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr,
-                               JSMSG_INCOMPATIBLE_PROTO, ClassName<T>(),
-                               methodName, InformalValueTypeName(val));
-    return nullptr;
-  }
-
-  return &obj->as<T>();
-}
-
-template <class T>
-MOZ_MUST_USE T* UnwrapAndTypeCheckArgumentSlowPath(JSContext* cx,
-                                                   CallArgs& args,
-                                                   const char* methodName,
-                                                   int argIndex) {
-  Value val = args.get(argIndex);
+template <class T, class ErrorCallback>
+MOZ_MUST_USE T* UnwrapAndTypeCheckValueSlowPath(JSContext* cx,
+                                                HandleValue value,
+                                                ErrorCallback throwTypeError) {
   JSObject* obj = nullptr;
-  if (val.isObject()) {
-    obj = &val.toObject();
+  if (value.isObject()) {
+    obj = &value.toObject();
     if (IsWrapper(obj)) {
       obj = CheckedUnwrap(obj);
       if (!obj) {
         ReportAccessDenied(cx);
         return nullptr;
       }
     }
   }
 
   if (!obj || !obj->is<T>()) {
-    ToCStringBuf cbuf;
-    if (char* numStr = NumberToCString(cx, &cbuf, argIndex + 1, 10)) {
-      JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr,
-                                 JSMSG_WRONG_TYPE_ARG, numStr, methodName,
-                                 ClassName<T>(), InformalValueTypeName(val));
-    }
+    throwTypeError();
     return nullptr;
   }
 
   return &obj->as<T>();
 }
 
 }  // namespace detail
 
 /**
- * Remove all wrappers from `val` and try to downcast the result to `T`.
+ * Remove all wrappers from `val` and try to downcast the result to class `T`.
+ *
+ * DANGER: The result may not be same-compartment with `cx`.
+ *
+ * This calls `throwTypeError` if the value isn't an object, cannot be
+ * unwrapped, or isn't an instance of the expected type. `throwTypeError` must
+ * in fact throw a TypeError (or OOM trying).
+ */
+template <class T, class ErrorCallback>
+inline MOZ_MUST_USE T* UnwrapAndTypeCheckValue(JSContext* cx, HandleValue value,
+                                               ErrorCallback throwTypeError) {
+  static_assert(!std::is_convertible<T*, Wrapper*>::value,
+                "T can't be a Wrapper type; this function discards wrappers");
+  cx->check(value);
+  if (value.isObject() && value.toObject().is<T>()) {
+    return &value.toObject().as<T>();
+  }
+  return detail::UnwrapAndTypeCheckValueSlowPath<T>(cx, value, throwTypeError);
+}
+
+/**
+ * Remove all wrappers from `args.thisv()` and try to downcast the result to
+ * class `T`.
  *
  * DANGER: The result may not be same-compartment with `cx`.
  *
  * This throws a TypeError if the value isn't an object, cannot be unwrapped,
  * or isn't an instance of the expected type.
  */
 template <class T>
 inline MOZ_MUST_USE T* UnwrapAndTypeCheckThis(JSContext* cx, CallArgs& args,
                                               const char* methodName) {
-  static_assert(!std::is_convertible<T*, Wrapper*>::value,
-                "T can't be a Wrapper type; this function discards wrappers");
-
   HandleValue thisv = args.thisv();
-  cx->check(thisv);
-  if (thisv.isObject() && thisv.toObject().is<T>()) {
-    return &thisv.toObject().as<T>();
-  }
-  return detail::UnwrapAndTypeCheckThisSlowPath<T>(cx, thisv, methodName);
+  return UnwrapAndTypeCheckValue<T>(cx, thisv, [cx, methodName, thisv] {
+    JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr,
+                               JSMSG_INCOMPATIBLE_PROTO, detail::ClassName<T>(),
+                               methodName, InformalValueTypeName(thisv));
+  });
 }
 
 /**
  * Remove all wrappers from `args[argIndex]` and try to downcast the result to
  * class `T`.
  *
  * DANGER: The result may not be same-compartment with `cx`.
  *
  * This throws a TypeError if the specified argument is missing, isn't an
  * object, cannot be unwrapped, or isn't an instance of the expected type.
  */
 template <class T>
 inline MOZ_MUST_USE T* UnwrapAndTypeCheckArgument(JSContext* cx, CallArgs& args,
                                                   const char* methodName,
                                                   int argIndex) {
-  static_assert(!std::is_convertible<T*, Wrapper*>::value,
-                "T can't be a Wrapper type; this function discards wrappers");
-
-  Value val = args.get(argIndex);
-  if (val.isObject() && val.toObject().is<T>()) {
-    return &val.toObject().as<T>();
-  }
-  return detail::UnwrapAndTypeCheckArgumentSlowPath<T>(cx, args, methodName,
-                                                       argIndex);
+  HandleValue val = args.get(argIndex);
+  return UnwrapAndTypeCheckValue<T>(cx, val, [cx, val, methodName, argIndex] {
+    ToCStringBuf cbuf;
+    if (char* numStr = NumberToCString(cx, &cbuf, argIndex + 1, 10)) {
+      JS_ReportErrorNumberLatin1(
+          cx, GetErrorMessage, nullptr, JSMSG_WRONG_TYPE_ARG, numStr,
+          methodName, detail::ClassName<T>(), InformalValueTypeName(val));
+    } else {
+      ReportOutOfMemory(cx);
+    }
+  });
 }
 
 /**
  * Unwrap an object of a known type.
  *
  * If `obj` is an object of class T, this returns a pointer to that object. If
  * `obj` is a wrapper for such an object, this tries to unwrap the object and
  * return a pointer to it. If access is denied, or `obj` was a wrapper but has