Commit 7459d8ce authored by caitpotter88's avatar caitpotter88 Committed by Commit bot

[promise] Make Promise.all match spec, and always respect [[AlreadyResolved]]

Testing the promise status is not enough to ensure that resolve functions are
called only once.

This change adds a similar version of the [[AlreadyResolved]] slot to the
Promise.all resolve element function, and also ensures that [[AlreadyResolved]]
is respected in the Promise executor, and when resolving thenables. This means
replacing PromiseReject() shortcuts with promiseCapability.reject(), which has
an [[AlreadyResolved]] record in a context slot.

Also ensures that changes to the list accumulator in Promise.all() is not observable
via accessors installed in the Array prototype chain, using the same mechanism used
in several Array methods.

Fixes the following Test262 tests:
- built-ins/Promise/all/call-resolve-element-items.js
- built-ins/Promise/all/call-resolve-element.js
- built-ins/Promise/all/call-resolve-element-after-return.js
- built-ins/Promise/all/same-reject-function.js
- built-ins/Promise/all/resolve-from-same-thenable.js
- built-ins/Promise/all/resolve-before-loop-exit.js
- built-ins/Promise/all/resolve-before-loop-exit-from-same.js
- built-ins/Promise/exception-after-resolve-in-executor.js
- built-ins/Promise/exception-after-resolve-in-thenable-job.js
- built-ins/Promise/all/does-not-invoke-array-setters.js

BUG=v8:4633
LOG=N
R=littledan@chromium.org, cbruni@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#33163}
parent adac5956
...@@ -66,13 +66,13 @@ var GlobalPromise = function Promise(resolver) { ...@@ -66,13 +66,13 @@ var GlobalPromise = function Promise(resolver) {
throw MakeTypeError(kResolverNotAFunction, resolver); throw MakeTypeError(kResolverNotAFunction, resolver);
var promise = PromiseInit(%NewObject(GlobalPromise, new.target)); var promise = PromiseInit(%NewObject(GlobalPromise, new.target));
var callbacks = CreateResolvingFunctions(promise);
try { try {
%DebugPushPromise(promise, Promise); %DebugPushPromise(promise, Promise);
var callbacks = CreateResolvingFunctions(promise);
resolver(callbacks.resolve, callbacks.reject); resolver(callbacks.resolve, callbacks.reject);
} catch (e) { } catch (e) {
PromiseReject(promise, e); %_Call(callbacks.reject, UNDEFINED, e);
} finally { } finally {
%DebugPopPromise(); %DebugPopPromise();
} }
...@@ -181,11 +181,11 @@ function PromiseResolve(promise, x) { ...@@ -181,11 +181,11 @@ function PromiseResolve(promise, x) {
if (instrumenting) { if (instrumenting) {
%DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name }); %DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name });
} }
try {
var callbacks = CreateResolvingFunctions(promise); var callbacks = CreateResolvingFunctions(promise);
try {
%_Call(then, x, callbacks.resolve, callbacks.reject); %_Call(then, x, callbacks.resolve, callbacks.reject);
} catch (e) { } catch (e) {
PromiseReject(promise, e); %_Call(callbacks.reject, UNDEFINED, e);
} }
if (instrumenting) { if (instrumenting) {
%DebugAsyncTaskEvent({ type: "didHandle", id: id, name: name }); %DebugAsyncTaskEvent({ type: "didHandle", id: id, name: name });
...@@ -338,33 +338,50 @@ function PromiseCast(x) { ...@@ -338,33 +338,50 @@ function PromiseCast(x) {
} }
function PromiseAll(iterable) { function PromiseAll(iterable) {
if (!IS_RECEIVER(this)) {
throw MakeTypeError(kCalledOnNonObject, "Promise.all");
}
var deferred = NewPromiseCapability(this); var deferred = NewPromiseCapability(this);
var resolutions = []; var resolutions = new InternalArray();
var count;
function CreateResolveElementFunction(index, values, promiseCapability) {
var alreadyCalled = false;
return function(x) {
if (alreadyCalled === true) return;
alreadyCalled = true;
values[index] = x;
if (--count === 0) {
var valuesArray = [];
%MoveArrayContents(values, valuesArray);
%_Call(promiseCapability.resolve, UNDEFINED, valuesArray);
}
};
}
try { try {
var count = 0;
var i = 0; var i = 0;
count = 1;
for (var value of iterable) { for (var value of iterable) {
var reject = function(r) { deferred.reject(r) }; var nextPromise = this.resolve(value);
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] = x;
if (--count === 0) deferred.resolve(resolutions);
}
})(i), reject);
SET_PRIVATE(reject, promiseCombinedDeferredSymbol, deferred);
++i;
++count; ++count;
nextPromise.then(
CreateResolveElementFunction(i, resolutions, deferred),
deferred.reject);
SET_PRIVATE(deferred.reject, promiseCombinedDeferredSymbol, deferred);
++i;
} }
if (count === 0) { // 6.d
deferred.resolve(resolutions); if (--count === 0) {
var valuesArray = [];
%MoveArrayContents(resolutions, valuesArray);
%_Call(deferred.resolve, UNDEFINED, valuesArray);
} }
} catch (e) { } catch (e) {
deferred.reject(e) %_Call(deferred.reject, UNDEFINED, e);
} }
return deferred.promise; return deferred.promise;
} }
......
...@@ -410,26 +410,16 @@ ...@@ -410,26 +410,16 @@
# https://bugs.chromium.org/p/v8/issues/detail?id=4632 # https://bugs.chromium.org/p/v8/issues/detail?id=4632
'built-ins/Promise/race/same-resolve-function': [FAIL], 'built-ins/Promise/race/same-resolve-function': [FAIL],
'built-ins/Promise/race/same-reject-function': [FAIL], 'built-ins/Promise/race/same-reject-function': [FAIL],
'built-ins/Promise/all/same-resolve-function': [FAIL],
'built-ins/Promise/all/same-reject-function': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=4633 # https://bugs.chromium.org/p/v8/issues/detail?id=4633
'built-ins/Promise/exception-after-resolve-in-executor': [FAIL],
'built-ins/Promise/reject-function-name': [FAIL], 'built-ins/Promise/reject-function-name': [FAIL],
'built-ins/Promise/reject-function-nonconstructor': [FAIL], 'built-ins/Promise/reject-function-nonconstructor': [FAIL],
'built-ins/Promise/resolve-function-name': [FAIL], 'built-ins/Promise/resolve-function-name': [FAIL],
'built-ins/Promise/resolve-function-nonconstructor': [FAIL], 'built-ins/Promise/resolve-function-nonconstructor': [FAIL],
'built-ins/Promise/all/call-resolve-element-after-return': [FAIL],
'built-ins/Promise/all/call-resolve-element': [FAIL],
'built-ins/Promise/all/call-resolve-element-items': [FAIL],
'built-ins/Promise/all/resolve-before-loop-exit-from-same': [FAIL],
'built-ins/Promise/all/resolve-element-function-name': [FAIL], 'built-ins/Promise/all/resolve-element-function-name': [FAIL],
'built-ins/Promise/all/resolve-element-function-nonconstructor': [FAIL], 'built-ins/Promise/all/resolve-element-function-nonconstructor': [FAIL],
'built-ins/Promise/exception-after-resolve-in-thenable-job': [FAIL],
'built-ins/Promise/all/resolve-before-loop-exit': [FAIL],
'built-ins/Promise/executor-function-name': [FAIL], 'built-ins/Promise/executor-function-name': [FAIL],
'built-ins/Promise/executor-function-nonconstructor': [FAIL], 'built-ins/Promise/executor-function-nonconstructor': [FAIL],
'built-ins/Promise/all/resolve-from-same-thenable': [FAIL],
# https://bugs.chromium.org/p/v8/issues/detail?id=4634 # https://bugs.chromium.org/p/v8/issues/detail?id=4634
'built-ins/DataView/prototype/setFloat64/index-check-before-value-conversion': [FAIL], 'built-ins/DataView/prototype/setFloat64/index-check-before-value-conversion': [FAIL],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment