Bug 1534392 - Part 2: Fix error handling in %AsyncFromSyncIteratorPrototype% methods. r=anba

This makes %AsyncFromSyncIteratorPrototype%.next/return/throw return a rejected
promise, not throw, when PromiseResolve throws, following the usual convention
for methods that return promises. This follows proposed spec change
<https://github.com/tc39/ecma262/pull/1470>, which I expect will land with
little controversy.

Differential Revision: https://phabricator.services.mozilla.com/D23030

--HG--
extra : moz-landing-system : lando

js/src/builtin/Promise.cpp

@@ -3798,27 +3798,31 @@ bool js::AsyncFromSyncIteratorMethod(JSContext* cx, CallArgs& args,
     return AbruptRejectPromise(cx, args, resultPromise, nullptr);
   }
 
-  // Steps 6-8 (reordered).
-  // Step 6: Let steps be the algorithm steps defined in Async-from-Sync
+  // Step numbers below include the changes in
+  // <https://github.com/tc39/ecma262/pull/1470>, which inserted a new step 6.
+  //
+  // Steps 7-9 (reordered).
+  // Step 7: Let steps be the algorithm steps defined in Async-from-Sync
   //         Iterator Value Unwrap Functions.
-  // Step 7: Let onFulfilled be CreateBuiltinFunction(steps, « [[Done]] »).
-  // Step 8: Set onFulfilled.[[Done]] to done.
+  // Step 8: Let onFulfilled be CreateBuiltinFunction(steps, « [[Done]] »).
+  // Step 9: Set onFulfilled.[[Done]] to done.
   RootedValue onFulfilled(
       cx,
       Int32Value(done ? PromiseHandlerAsyncFromSyncIteratorValueUnwrapDone
                       : PromiseHandlerAsyncFromSyncIteratorValueUnwrapNotDone));
   RootedValue onRejected(cx, Int32Value(PromiseHandlerThrower));
 
-  // These steps are identical to some steps in Await; we have a utility
+  // Steps 5 and 10 are identical to some steps in Await; we have a utility
   // function InternalAwait() that implements the idiom.
   //
-  // Step 5: Let valueWrapper be ? PromiseResolve(%Promise%, « value »).
-  // Step 9: Perform ! PerformPromiseThen(valueWrapper, onFulfilled,
+  // Step 5: Let valueWrapper be PromiseResolve(%Promise%, « value »).
+  // Step 6: IfAbruptRejectPromise(valueWrapper, promiseCapability).
+  // Step 10: Perform ! PerformPromiseThen(valueWrapper, onFulfilled,
   //                                      undefined, promiseCapability).
   auto extra = [](Handle<PromiseReactionRecord*> reaction) {};
   if (!InternalAwait(cx, value, resultPromise, onFulfilled, onRejected,
                      extra)) {
-    return false;
+    return AbruptRejectPromise(cx, args, resultPromise, nullptr);
   }
 
   // Step 11: Return promiseCapability.[[Promise]].

js/src/tests/non262/async-functions/ecma262-issue-1461.js

@@ -0,0 +1,25 @@
+// <https://github.com/tc39/ecma262/pull/1470> changes a detail of
+// error-handling in %AsyncFromSyncIteratorPrototype% methods. This test is
+// based on a comment in the thread where the issue was first reported,
+// <https://github.com/tc39/ecma262/issues/1461#issuecomment-468602852>
+
+let log = [];
+
+{
+  async function f() {
+    var p = Promise.resolve(0);
+    Object.defineProperty(p, "constructor", {get() { throw "hi" }});
+    for await (var x of [p]);
+  }
+  Promise.resolve(0)
+         .then(() => log.push("tick 1"))
+         .then(() => log.push("tick 2"))
+         .then(() => log.push("tick 3"));
+  f().catch(exc => log.push(exc));
+}
+
+drainJobQueue();
+assertEq(log.join(), "tick 1,tick 2,hi,tick 3");
+
+if (typeof reportCompare === 'function')
+  reportCompare(0, 0);