Promise.all and race should work with iterables

BUG=v8:3705
LOG=N
R=rossberg@chromium.org

Review URL: https://codereview.chromium.org/948843004

Cr-Commit-Position: refs/heads/master@{#26801}

diff --git a/src/promise.js b/src/promise.js
index c096296..6d752d6 100644 (file)
--- a/src/promise.js
+++ b/src/promise.js
@@ -301,51 +301,44 @@ var lastMicrotaskId = 0;
     return IsPromise(x) ? x : new this(function(resolve) { resolve(x) });
   }
 
-  function PromiseAll(values) {
+  function PromiseAll(iterable) {
     var deferred = %_CallFunction(this, PromiseDeferred);
     var resolutions = [];
-    if (!%_IsArray(values)) {
-      deferred.reject(MakeTypeError('invalid_argument'));
-      return deferred.promise;
-    }
     try {
-      var count = values.length;
-      if (count === 0) {
-        deferred.resolve(resolutions);
-      } else {
-        for (var i = 0; i < values.length; ++i) {
-          this.resolve(values[i]).then(
-            (function() {
-              // Nested scope to get closure over current i (and avoid .bind).
-              // TODO(rossberg): Use for-let instead once available.
-              var i_captured = i;
+      var count = 0;
+      var i = 0;
+      for (var value of iterable) {
+        this.resolve(value).then(
+            // Nested scope to get closure over current i.
+            // TODO(arv): Use an inner let binding once available.
+            (function(i) {
               return function(x) {
-                resolutions[i_captured] = x;
+                resolutions[i] = x;
                 if (--count === 0) deferred.resolve(resolutions);
-              };
-            })(),
-            function(r) { deferred.reject(r) }
-          );
-        }
+              }
+            })(i),
+            function(r) { deferred.reject(r); });
+        ++i;
+        ++count;
+      }
+
+      if (count === 0) {
+        deferred.resolve(resolutions);
       }
+
     } catch (e) {
       deferred.reject(e)
     }
     return deferred.promise;
   }
 
-  function PromiseOne(values) {
+  function PromiseRace(iterable) {
     var deferred = %_CallFunction(this, PromiseDeferred);
-    if (!%_IsArray(values)) {
-      deferred.reject(MakeTypeError('invalid_argument'));
-      return deferred.promise;
-    }
     try {
-      for (var i = 0; i < values.length; ++i) {
-        this.resolve(values[i]).then(
-          function(x) { deferred.resolve(x) },
-          function(r) { deferred.reject(r) }
-        );
+      for (var value of iterable) {
+        this.resolve(value).then(
+            function(x) { deferred.resolve(x) },
+            function(r) { deferred.reject(r) });
       }
     } catch (e) {
       deferred.reject(e)
@@ -388,7 +381,7 @@ var lastMicrotaskId = 0;
     "accept", PromiseResolved,
     "reject", PromiseRejected,
     "all", PromiseAll,
-    "race", PromiseOne,
+    "race", PromiseRace,
     "resolve", PromiseCast
   ]);
   InstallFunctions($Promise.prototype, DONT_ENUM, [

diff --git a/test/mjsunit/es6/promises.js b/test/mjsunit/es6/promises.js
index 04059aa..63b6d2f 100644 (file)
--- a/test/mjsunit/es6/promises.js
+++ b/test/mjsunit/es6/promises.js
@@ -32,6 +32,8 @@ var call = Function.prototype.call.call.bind(Function.prototype.call)
 var observe = Object.observe;
 var getOwnPropertyNames = Object.getOwnPropertyNames;
 var defineProperty = Object.defineProperty;
+var numberPrototype = Number.prototype;
+var symbolIterator = Symbol.iterator;
 
 
 (function() {
@@ -637,14 +639,6 @@ function assertAsyncDone(iteration) {
 })();
 
 (function() {
-  Promise.all({}).chain(
-    assertUnreachable,
-    function(r) { assertAsync(r instanceof TypeError, "all/no-array") }
-  )
-  assertAsyncRan()
-})();
-
-(function() {
   Promise.all([]).chain(
     function(x) { assertAsync(x.length === 0, "all/resolve/empty") },
     assertUnreachable
@@ -653,6 +647,45 @@ function assertAsyncDone(iteration) {
 })();
 
 (function() {
+  function testPromiseAllNonIterable(value) {
+    Promise.all(value).chain(
+        assertUnreachable,
+        function(r) {
+          assertAsync(r instanceof TypeError, 'all/non iterable');
+        });
+    assertAsyncRan();
+  }
+  testPromiseAllNonIterable(null);
+  testPromiseAllNonIterable(undefined);
+  testPromiseAllNonIterable({});
+  testPromiseAllNonIterable(42);
+})();
+
+(function() {
+  var deferred = Promise.defer();
+  var p = deferred.promise;
+  function* f() {
+    yield 1;
+    yield p;
+    yield 3;
+  }
+  Promise.all(f()).chain(
+      function(x) {
+        assertAsync(x.length === 3, "all/resolve/iterable");
+        assertAsync(x[0] === 1, "all/resolve/iterable/0");
+        assertAsync(x[1] === 2, "all/resolve/iterable/1");
+        assertAsync(x[2] === 3, "all/resolve/iterable/2");
+      },
+      assertUnreachable);
+  deferred.resolve(2);
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+
+(function() {
   var deferred1 = Promise.defer()
   var p1 = deferred1.promise
   var deferred2 = Promise.defer()
@@ -706,6 +739,52 @@ function assertAsyncDone(iteration) {
   assertAsyncRan()
 })();
 
+
+(function() {
+  'use strict';
+  var getCalls = 0;
+  var funcCalls = 0;
+  var nextCalls = 0;
+  defineProperty(numberPrototype, symbolIterator, {
+    get: function() {
+      assertEquals('number', typeof this);
+      getCalls++;
+      return function() {
+        assertEquals('number', typeof this);
+        funcCalls++;
+        var n = this;
+        var i = 0
+        return {
+          next() {
+            nextCalls++;
+            return {value: i++, done: i > n};
+          }
+        };
+      };
+    },
+    configurable: true
+  });
+
+  Promise.all(3).chain(
+      function(x) {
+        assertAsync(x.length === 3, "all/iterable/number/length");
+        assertAsync(x[0] === 0, "all/iterable/number/0");
+        assertAsync(x[1] === 1, "all/iterable/number/1");
+        assertAsync(x[2] === 2, "all/iterable/number/2");
+      },
+      assertUnreachable);
+  delete numberPrototype[symbolIterator];
+
+  assertEquals(getCalls, 1);
+  assertEquals(funcCalls, 1);
+  assertEquals(nextCalls, 3 + 1);  // + 1 for {done: true}
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+
 (function() {
   Promise.race([]).chain(
     assertUnreachable,
@@ -736,14 +815,6 @@ function assertAsyncDone(iteration) {
 })();
 
 (function() {
-  Promise.race({}).chain(
-    assertUnreachable,
-    function(r) { assertAsync(r instanceof TypeError, "one/no-array") }
-  )
-  assertAsyncRan()
-})();
-
-(function() {
   var deferred1 = Promise.defer()
   var p1 = deferred1.promise
   var deferred2 = Promise.defer()
@@ -804,6 +875,103 @@ function assertAsyncDone(iteration) {
   assertAsyncRan()
 })();
 
+
+(function() {
+  function testPromiseRaceNonIterable(value) {
+    Promise.race(value).chain(
+        assertUnreachable,
+        function(r) {
+          assertAsync(r instanceof TypeError, 'race/non iterable');
+        });
+    assertAsyncRan();
+  }
+  testPromiseRaceNonIterable(null);
+  testPromiseRaceNonIterable(undefined);
+  testPromiseRaceNonIterable({});
+  testPromiseRaceNonIterable(42);
+})();
+
+
+(function() {
+  var deferred1 = Promise.defer()
+  var p1 = deferred1.promise
+  var deferred2 = Promise.defer()
+  var p2 = deferred2.promise
+  var deferred3 = Promise.defer()
+  var p3 = deferred3.promise
+  function* f() {
+    yield p1;
+    yield p2;
+    yield p3;
+  }
+  Promise.race(f()).chain(
+    function(x) { assertAsync(x === 3, "race/iterable/resolve/reject") },
+    assertUnreachable
+  )
+  deferred3.resolve(3)
+  deferred1.reject(1)
+  assertAsyncRan()
+})();
+
+(function() {
+  var deferred1 = Promise.defer()
+  var p1 = deferred1.promise
+  var deferred2 = Promise.defer()
+  var p2 = deferred2.promise
+  var deferred3 = Promise.defer()
+  var p3 = deferred3.promise
+  function* f() {
+    yield p1;
+    yield p2;
+    yield p3;
+  }
+  Promise.race(f()).chain(
+    assertUnreachable,
+    function(x) { assertAsync(x === 3, "race/iterable/reject/resolve") }
+  )
+  deferred3.reject(3)
+  deferred1.resolve(1)
+  assertAsyncRan()
+})();
+
+(function() {
+  'use strict';
+  var getCalls = 0;
+  var funcCalls = 0;
+  var nextCalls = 0;
+  defineProperty(numberPrototype, symbolIterator, {
+    get: function() {
+      assertEquals('number', typeof this);
+      getCalls++;
+      return function() {
+        assertEquals('number', typeof this);
+        funcCalls++;
+        var n = this;
+        var i = 0
+        return {
+          next() {
+            nextCalls++;
+            return {value: i++, done: i > n};
+          }
+        };
+      };
+    },
+    configurable: true
+  });
+
+  Promise.race(3).chain(
+      function(x) {
+        assertAsync(x === 0, "race/iterable/number");
+      },
+      assertUnreachable);
+  delete numberPrototype[symbolIterator];
+
+  assertEquals(getCalls, 1);
+  assertEquals(funcCalls, 1);
+  assertEquals(nextCalls, 3 + 1);  // + 1 for {done: true}
+  assertAsyncRan();
+})();
+
 (function() {
   var log
   function MyPromise(resolver) {