Bug 1539694 - Part 1: Modify shared Promise.all/race helper to allow passing in a different reject function. r=jorendorff
☠☠ backed out by 7befb6bf6b3b ☠ ☠
authorAndré Bargull <andre.bargull@gmail.com>
Thu, 11 Apr 2019 12:22:16 +0000
changeset 469062 75b6666c5095daaa79ffbd94b54be8f7990f5eb1
parent 469061 27abf0e843c3122eb5318774f1a069c8606665b7
child 469063 2e36a4f4d99662dfe8bf771d0fb95a80a5928c36
push id35856
push usercsabou@mozilla.com
push dateFri, 12 Apr 2019 03:19:48 +0000
treeherdermozilla-central@940684cd1065 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1539694
milestone68.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 1539694 - Part 1: Modify shared Promise.all/race helper to allow passing in a different reject function. r=jorendorff And a shared helper function for Promise.all, Promise.race, and soon Promise.allSettled to avoid code repetition. Differential Revision: https://phabricator.services.mozilla.com/D25208
js/src/builtin/Promise.cpp
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -2267,27 +2267,45 @@ class MOZ_STACK_CLASS PromiseForOfIterat
     return index != NOT_ARRAY && IsPackedArray(iterator);
   }
 };
 
 static MOZ_MUST_USE bool PerformPromiseAll(
     JSContext* cx, PromiseForOfIterator& iterator, HandleObject C,
     Handle<PromiseCapability> resultCapability, bool* done);
 
-// ES2016, 25.4.4.1.
-static bool Promise_static_all(JSContext* cx, unsigned argc, Value* vp) {
-  CallArgs args = CallArgsFromVp(argc, vp);
+static MOZ_MUST_USE bool PerformPromiseRace(
+    JSContext* cx, PromiseForOfIterator& iterator, HandleObject C,
+    Handle<PromiseCapability> resultCapability, bool* done);
+
+enum class IterationMode { All, Race };
+
+// ES2020 draft rev a09fc232c137800dbf51b6204f37fdede4ba1646
+//
+// Unified implementation of
+// 25.6.4.1 Promise.all ( iterable )
+// 25.6.4.3 Promise.race ( iterable )
+static MOZ_MUST_USE bool CommonStaticAllRace(JSContext* cx, CallArgs& args,
+                                             IterationMode mode) {
   HandleValue iterable = args.get(0);
 
   // Step 2 (reordered).
   HandleValue CVal = args.thisv();
   if (!CVal.isObject()) {
+    const char* message;
+    switch (mode) {
+      case IterationMode::All:
+        message = "Receiver of Promise.all call";
+        break;
+      case IterationMode::Race:
+        message = "Receiver of Promise.race call";
+        break;
+    }
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_NOT_NONNULL_OBJECT,
-                              "Receiver of Promise.all call");
+                              JSMSG_NOT_NONNULL_OBJECT, message);
     return false;
   }
 
   // Step 1.
   RootedObject C(cx, &CVal.toObject());
 
   // Step 3.
   Rooted<PromiseCapability> promiseCapability(cx);
@@ -2297,26 +2315,42 @@ static bool Promise_static_all(JSContext
 
   // Steps 4-5.
   PromiseForOfIterator iter(cx);
   if (!iter.init(iterable, JS::ForOfIterator::AllowNonIterable)) {
     return AbruptRejectPromise(cx, args, promiseCapability);
   }
 
   if (!iter.valueIsIterable()) {
+    const char* message;
+    switch (mode) {
+      case IterationMode::All:
+        message = "Argument of Promise.all";
+        break;
+      case IterationMode::Race:
+        message = "Argument of Promise.race";
+        break;
+    }
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_NOT_ITERABLE,
-                              "Argument of Promise.all");
+                              message);
     return AbruptRejectPromise(cx, args, promiseCapability);
   }
 
   // Step 6 (implicit).
 
   // Step 7.
-  bool done;
-  bool result = PerformPromiseAll(cx, iter, C, promiseCapability, &done);
+  bool done, result;
+  switch (mode) {
+    case IterationMode::All:
+      result = PerformPromiseAll(cx, iter, C, promiseCapability, &done);
+      break;
+    case IterationMode::Race:
+      result = PerformPromiseRace(cx, iter, C, promiseCapability, &done);
+      break;
+  }
 
   // Step 8.
   if (!result) {
     // Step 8.a.
     if (!done) {
       iter.closeThrow();
     }
 
@@ -2324,16 +2358,23 @@ static bool Promise_static_all(JSContext
     return AbruptRejectPromise(cx, args, promiseCapability);
   }
 
   // Step 9.
   args.rval().setObject(*promiseCapability.promise());
   return true;
 }
 
+// ES2020 draft rev a09fc232c137800dbf51b6204f37fdede4ba1646
+// 25.6.4.1 Promise.all ( iterable )
+static bool Promise_static_all(JSContext* cx, unsigned argc, Value* vp) {
+  CallArgs args = CallArgsFromVp(argc, vp);
+  return CommonStaticAllRace(cx, args, IterationMode::All);
+}
+
 static MOZ_MUST_USE bool PerformPromiseThen(
     JSContext* cx, Handle<PromiseObject*> promise, HandleValue onFulfilled_,
     HandleValue onRejected_, Handle<PromiseCapability> resultCapability);
 
 static MOZ_MUST_USE bool PerformPromiseThenWithoutSettleHandlers(
     JSContext* cx, Handle<PromiseObject*> promise,
     Handle<PromiseObject*> promiseToResolve,
     Handle<PromiseCapability> resultCapability);
@@ -2540,18 +2581,18 @@ static MOZ_MUST_USE JSObject* CommonStat
 static bool IsPromiseSpecies(JSContext* cx, JSFunction* species);
 
 // ES2019 draft rev dd269df67d37409a6f2099a842b8f5c75ee6fc24
 // 25.6.4.1.1 Runtime Semantics: PerformPromiseAll, step 6.
 // 25.6.4.3.1 Runtime Semantics: PerformPromiseRace, step 3.
 template <typename T>
 static MOZ_MUST_USE bool CommonPerformPromiseAllRace(
     JSContext* cx, PromiseForOfIterator& iterator, HandleObject C,
-    Handle<PromiseCapability> resultCapability, bool* done,
-    bool resolveReturnsUndefined, T getResolveFun) {
+    HandleObject resultPromise, bool* done, bool resolveReturnsUndefined,
+    T getResolveAndReject) {
   RootedObject promiseCtor(
       cx, GlobalObject::getOrCreatePromiseConstructor(cx, cx->global()));
   if (!promiseCtor) {
     return false;
   }
 
   // Optimized dense array iteration ensures no side-effects take place
   // during the iteration.
@@ -2561,19 +2602,18 @@ static MOZ_MUST_USE bool CommonPerformPr
   // with |C == promiseCtor| because we can only perform this optimization
   // for the builtin Promise constructor.
   bool isDefaultPromiseState = C == promiseCtor;
   bool validatePromiseState = true;
 
   PromiseLookup& promiseLookup = cx->realm()->promiseLookup;
 
   RootedValue CVal(cx, ObjectValue(*C));
-  HandleObject resultPromise = resultCapability.promise();
   RootedValue resolveFunVal(cx);
-  RootedValue rejectFunVal(cx, ObjectValue(*resultCapability.reject()));
+  RootedValue rejectFunVal(cx);
 
   // We're reusing rooted variables in the loop below, so we don't need to
   // declare a gazillion different rooted variables here. Rooted variables
   // which are reused include "Or" in their name.
   RootedValue nextValueOrNextPromise(cx);
   RootedObject nextPromiseObj(cx);
   RootedValue resolveOrThen(cx);
   RootedObject thenSpeciesOrBlockedPromise(cx);
@@ -2652,23 +2692,21 @@ static MOZ_MUST_USE bool CommonPerformPr
         return false;
       }
 
       if (!Call(cx, staticResolve, CVal, nextValue, &nextPromise)) {
         return false;
       }
     }
 
-    // Get the resolve function for this iteration.
+    // Get the resolving functions for this iteration.
     // 25.6.4.1.1, steps 6.j-q.
-    JSObject* resolveFun = getResolveFun();
-    if (!resolveFun) {
+    if (!getResolveAndReject(&resolveFunVal, &rejectFunVal)) {
       return false;
     }
-    resolveFunVal.setObject(*resolveFun);
 
     // Call |nextPromise.then| with the provided hooks and add
     // |resultPromise| to the list of dependent promises.
     //
     // If |nextPromise.then| is the original |Promise.prototype.then|
     // function and the call to |nextPromise.then| would use the original
     // |Promise| constructor to create the resulting promise, we skip the
     // call to |nextPromise.then| and thus creating a new promise that
@@ -2815,17 +2853,18 @@ static MOZ_MUST_USE bool CommonPerformPr
         if (!AddDummyPromiseReactionForDebugger(cx, promise, blockedPromise)) {
           return false;
         }
       }
     }
   }
 }
 
-// ES2016, 25.4.4.1.1.
+// ES2020 draft rev a09fc232c137800dbf51b6204f37fdede4ba1646
+// 25.6.4.1.1 PerformPromiseAll (iteratorRecord, constructor, resultCapability)
 static MOZ_MUST_USE bool PerformPromiseAll(
     JSContext* cx, PromiseForOfIterator& iterator, HandleObject C,
     Handle<PromiseCapability> resultCapability, bool* done) {
   *done = false;
 
   // Step 1.
   MOZ_ASSERT(C->isConstructor());
 
@@ -2890,34 +2929,36 @@ static MOZ_MUST_USE bool PerformPromiseA
                               resultCapability.resolve());
   if (!dataHolder) {
     return false;
   }
 
   // Step 5.
   uint32_t index = 0;
 
-  auto getResolve = [cx, &valuesArray, &dataHolder, &index]() -> JSObject* {
+  auto getResolveAndReject = [cx, &resultCapability, &valuesArray, &dataHolder,
+                              &index](MutableHandleValue resolveFunVal,
+                                      MutableHandleValue rejectFunVal) {
     // Step 6.h.
     {  // Scope for the AutoRealm we need to work with valuesArray.  We
       // mostly do this for performance; we could go ahead and do the define via
       // a cross-compartment proxy instead...
       AutoRealm ar(cx, valuesArray);
 
       if (!NewbornArrayPush(cx, valuesArray, UndefinedValue())) {
-        return nullptr;
+        return false;
       }
     }
 
     // Steps 6.j-k.
     JSFunction* resolveFunc =
         NewNativeFunction(cx, PromiseAllResolveElementFunction, 1, nullptr,
                           gc::AllocKind::FUNCTION_EXTENDED, GenericObject);
     if (!resolveFunc) {
-      return nullptr;
+      return false;
     }
 
     // Steps 6.l, 6.n-p.
     resolveFunc->setExtendedSlot(PromiseAllResolveElementFunctionSlot_Data,
                                  ObjectValue(*dataHolder));
 
     // Step 6.m.
     resolveFunc->setExtendedSlot(
@@ -2925,22 +2966,24 @@ static MOZ_MUST_USE bool PerformPromiseA
 
     // Step 6.q.
     dataHolder->increaseRemainingCount();
 
     // Step 6.s.
     index++;
     MOZ_ASSERT(index > 0);
 
-    return resolveFunc;
+    resolveFunVal.setObject(*resolveFunc);
+    rejectFunVal.setObject(*resultCapability.reject());
+    return true;
   };
 
   // Step 6.
-  if (!CommonPerformPromiseAllRace(cx, iterator, C, resultCapability, done,
-                                   true, getResolve)) {
+  if (!CommonPerformPromiseAllRace(cx, iterator, C, resultCapability.promise(),
+                                   done, true, getResolveAndReject)) {
     return false;
   }
 
   // Step 6.d.ii.
   int32_t remainingCount = dataHolder->decreaseRemainingCount();
 
   // Steps 6.d.iii-iv.
   if (remainingCount == 0) {
@@ -3030,101 +3073,53 @@ static bool PromiseAllResolveElementFunc
     }
   }
 
   // Step 11.
   args.rval().setUndefined();
   return true;
 }
 
-static MOZ_MUST_USE bool PerformPromiseRace(
-    JSContext* cx, PromiseForOfIterator& iterator, HandleObject C,
-    Handle<PromiseCapability> resultCapability, bool* done);
-
-// ES2016, 25.4.4.3.
+// ES2020 draft rev a09fc232c137800dbf51b6204f37fdede4ba1646
+// 25.6.4.3 Promise.race ( iterable )
 static bool Promise_static_race(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-  HandleValue iterable = args.get(0);
-
-  // Step 2 (reordered).
-  HandleValue CVal = args.thisv();
-  if (!CVal.isObject()) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_NOT_NONNULL_OBJECT,
-                              "Receiver of Promise.race call");
-    return false;
-  }
-
-  // Step 1.
-  RootedObject C(cx, &CVal.toObject());
-
-  // Step 3.
-  Rooted<PromiseCapability> promiseCapability(cx);
-  if (!NewPromiseCapability(cx, C, &promiseCapability, false)) {
-    return false;
-  }
-
-  // Steps 4-5.
-  PromiseForOfIterator iter(cx);
-  if (!iter.init(iterable, JS::ForOfIterator::AllowNonIterable)) {
-    return AbruptRejectPromise(cx, args, promiseCapability);
-  }
-
-  if (!iter.valueIsIterable()) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_NOT_ITERABLE,
-                              "Argument of Promise.race");
-    return AbruptRejectPromise(cx, args, promiseCapability);
-  }
-
-  // Step 6 (implicit).
-
-  // Step 7.
-  bool done;
-  bool result = PerformPromiseRace(cx, iter, C, promiseCapability, &done);
-
-  // Step 8.
-  if (!result) {
-    // Step 8.a.
-    if (!done) {
-      iter.closeThrow();
-    }
-
-    // Step 8.b.
-    return AbruptRejectPromise(cx, args, promiseCapability);
-  }
-
-  // Step 9.
-  args.rval().setObject(*promiseCapability.promise());
-  return true;
+  return CommonStaticAllRace(cx, args, IterationMode::Race);
 }
 
-// ES2016, 25.4.4.3.1.
+// ES2020 draft rev a09fc232c137800dbf51b6204f37fdede4ba1646
+// 25.6.4.3.1 PerformPromiseRace (iteratorRecord, constructor, resultCapability)
 static MOZ_MUST_USE bool PerformPromiseRace(
     JSContext* cx, PromiseForOfIterator& iterator, HandleObject C,
     Handle<PromiseCapability> resultCapability, bool* done) {
   *done = false;
 
   // Step 1.
   MOZ_ASSERT(C->isConstructor());
 
   // Step 2 (omitted).
 
   // BlockOnPromise fast path requires the passed onFulfilled function
   // doesn't return an object value, because otherwise the skipped promise
   // creation is detectable due to missing property lookups.
   bool isDefaultResolveFn =
       IsNativeFunction(resultCapability.resolve(), ResolvePromiseFunction);
 
-  auto getResolve = [&resultCapability]() -> JSObject* {
-    return resultCapability.resolve();
+  auto getResolveAndReject = [&resultCapability](
+                                 MutableHandleValue resolveFunVal,
+                                 MutableHandleValue rejectFunVal) {
+    resolveFunVal.setObject(*resultCapability.resolve());
+    rejectFunVal.setObject(*resultCapability.reject());
+    return true;
   };
 
   // Step 3.
-  return CommonPerformPromiseAllRace(cx, iterator, C, resultCapability, done,
-                                     isDefaultResolveFn, getResolve);
+  return CommonPerformPromiseAllRace(cx, iterator, C,
+                                     resultCapability.promise(), done,
+                                     isDefaultResolveFn, getResolveAndReject);
 }
 
 // https://tc39.github.io/ecma262/#sec-promise.reject
 //
 // Unified implementation of
 // 25.6.4.4 Promise.reject ( r )
 // 25.6.4.5 Promise.resolve ( x )
 // 25.6.4.5.1 PromiseResolve ( C, x )