Skip to content

V
V8
Commit 4e7990d3 authored by machenbach's avatar machenbach Committed by Commit bot
Browse files
Options

Revert of Revert of Revert of Promise assimilation fix. (patchset #1 id:1 of... 

Revert of Revert of Revert of Promise assimilation fix. (patchset #1 id:1 of https://codereview.chromium.org/1181533006/)

Reason for revert:
[Sheriff] Changes/breaks layout tests. Please land upstream needsmanualrebaseline requests first or fix the tests.

E.g.
http://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Win/builds/400
One test fails, one times out. See e.g. expectations changes:
https://storage.googleapis.com/chromium-layout-test-archives/V8-Blink_Win/400/layout-test-results/inspector/sources/debugger-async/async-callstack-promises-diff.txt

On linux debug theses tests crash:
http://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064%20%28dbg%29/builds/346
https://storage.googleapis.com/chromium-layout-test-archives/V8-Blink_Linux_64__dbg_/346/layout-test-results/results.html

Original issue's description:
> Revert of Revert of Promise assimilation fix. (patchset #1 id:1 of https://codereview.chromium.org/1176163004/)
>
> Reason for revert:
> Test failures are bogus. Snapshot blob and natives blob are out of sync due to build being weird.
>
> Original issue's description:
> > Revert of Promise assimilation fix. (patchset #8 id:160001 of https://codereview.chromium.org/1098663002/)
> >
> > Reason for revert:
> > Test failures: https://chromegw.corp.google.com/i/client.v8/builders/V8%20Linux64/builds/3829
> >
> > Original issue's description:
> > > Promise assimilation fix.
> > >
> > > Let x be a fulfilled promise and y be another promise. |x.then(() => y)|
> > > should call |y.then|, but the current implementation calls PromiseChain.
> > > We can see the difference when we set a custom function to |y.then|.
> > >
> > > This CL fixes the spec violation, but as a result |then| is no longer
> > > a wrapper of |chain| and in some cases it does not work well with
> > > |accept| or |chain|. That is not a problem for ES6 promise users because
> > > ES6 promise doesn't have them.
> > >
> > > LOG=N
> > > BUG=477921
> > >
> > > Committed: https://crrev.com/2f57dff3ea0c45e1a61b334fda962460f89d71bc
> > > Cr-Commit-Position: refs/heads/master@{#28926}
> >
> > TBR=arv@chromium.org,caitpotter88@gmail.com,rossberg@chromium.org,yhirano@chromium.org
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=477921
> >
> > Committed: https://crrev.com/5bb75f514027f79303396dba823c2d78c6add83b
> > Cr-Commit-Position: refs/heads/master@{#28927}
>
> TBR=arv@chromium.org,caitpotter88@gmail.com,rossberg@chromium.org,yhirano@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=477921
>
> Committed: https://crrev.com/6f214bdd8bcdc76d48bd85c3bd897f0e2427ff95
> Cr-Commit-Position: refs/heads/master@{#28928}

TBR=arv@chromium.org,caitpotter88@gmail.com,rossberg@chromium.org,yhirano@chromium.org,yangguo@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=477921

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

Cr-Commit-Position: refs/heads/master@{#28931}
parent 20e16231
Hide whitespace changes
Inline Side-by-side
Showing with 40 additions and 231 deletions
src/promise.js
View file @ 4e7990d3
...@@ -105,25 +105,17 @@ function PromiseCoerce(constructor, x) { ...@@ -105,25 +105,17 @@ function PromiseCoerce(constructor, x) {
return x; return x;
} }
function PromiseHandle(value, handler, deferred, thenable) { function PromiseHandle(value, handler, deferred) {
try { try {
%DebugPushPromise(deferred.promise, PromiseHandle); %DebugPushPromise(deferred.promise, PromiseHandle);
DEBUG_PREPARE_STEP_IN_IF_STEPPING(handler); DEBUG_PREPARE_STEP_IN_IF_STEPPING(handler);
var result = handler(value); var result = handler(value);
if (result === deferred.promise) { if (result === deferred.promise)
throw MakeTypeError(kPromiseCyclic, result); throw MakeTypeError(kPromiseCyclic, result);
} else if (IsPromise(result) && thenable) { else if (IsPromise(result))
var then = result.then;
if (IS_SPEC_FUNCTION(then)) {
PromiseCallThenInSeparateTask(result, deferred, then);
} else {
deferred.resolve(result);
}
} else if (IsPromise(result)) {
%_CallFunction(result, deferred.resolve, deferred.reject, PromiseChain); %_CallFunction(result, deferred.resolve, deferred.reject, PromiseChain);
} else { else
deferred.resolve(result); deferred.resolve(result);
}
} catch (exception) { } catch (exception) {
try { deferred.reject(exception); } catch (e) { } try { deferred.reject(exception); } catch (e) { }
} finally { } finally {
...@@ -131,38 +123,14 @@ function PromiseHandle(value, handler, deferred, thenable) { ...@@ -131,38 +123,14 @@ function PromiseHandle(value, handler, deferred, thenable) {
} }
} }
function PromiseCallThenInSeparateTask(result, deferred, then) {
var id, name, instrumenting = DEBUG_IS_ACTIVE;
%EnqueueMicrotask(function() {
if (instrumenting) {
%DebugAsyncTaskEvent({ type: "willCall", id: id, name: name });
}
try {
%_CallFunction(result, deferred.resolve, deferred.reject, then);
} catch(exception) {
try { deferred.reject(exception); } catch (e) { }
}
if (instrumenting) {
%DebugAsyncTaskEvent({ type: "didCall", id: id, name: name });
}
});
if (instrumenting) {
id = ++lastMicrotaskId;
name = "Promise.prototype.then";
%DebugAsyncTaskEvent({ type: "enqueue", id: id, name: name });
}
}
function PromiseEnqueue(value, tasks, status) { function PromiseEnqueue(value, tasks, status) {
var id, name, instrumenting = DEBUG_IS_ACTIVE; var id, name, instrumenting = DEBUG_IS_ACTIVE;
%EnqueueMicrotask(function() { %EnqueueMicrotask(function() {
if (instrumenting) { if (instrumenting) {
%DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name }); %DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name });
} }
for (var i = 0; i < tasks.length; i += 3) { for (var i = 0; i < tasks.length; i += 2) {
// tasks[i: i + 2] consists of the reaction handler, the associated PromiseHandle(value, tasks[i], tasks[i + 1])
// deferred object and whether the thenable handling is required.
PromiseHandle(value, tasks[i], tasks[i + 1], tasks[i + 2])
} }
if (instrumenting) { if (instrumenting) {
%DebugAsyncTaskEvent({ type: "didHandle", id: id, name: name }); %DebugAsyncTaskEvent({ type: "didHandle", id: id, name: name });
...@@ -175,41 +143,6 @@ function PromiseEnqueue(value, tasks, status) { ...@@ -175,41 +143,6 @@ function PromiseEnqueue(value, tasks, status) {
} }
} }
function PromiseChainInternal(onResolve, onReject, thenable) {
onResolve = IS_UNDEFINED(onResolve) ? PromiseIdResolveHandler : onResolve;
onReject = IS_UNDEFINED(onReject) ? PromiseIdRejectHandler : onReject;
var deferred = %_CallFunction(this.constructor, PromiseDeferred);
switch (GET_PRIVATE(this, promiseStatus)) {
case UNDEFINED:
throw MakeTypeError(kNotAPromise, this);
case 0: // Pending
GET_PRIVATE(this, promiseOnResolve).push(onResolve, deferred, thenable);
GET_PRIVATE(this, promiseOnReject).push(onReject, deferred, thenable);
break;
case +1: // Resolved
PromiseEnqueue(GET_PRIVATE(this, promiseValue),
[onResolve, deferred, thenable],
+1);
break;
case -1: // Rejected
if (!HAS_DEFINED_PRIVATE(this, promiseHasHandler)) {
// Promise has already been rejected, but had no handler.
// Revoke previously triggered reject event.
%PromiseRevokeReject(this);
}
PromiseEnqueue(GET_PRIVATE(this, promiseValue),
[onReject, deferred, thenable],
-1);
break;
}
// Mark this promise as having handler.
SET_PRIVATE(this, promiseHasHandler, true);
if (DEBUG_IS_ACTIVE) {
%DebugPromiseEvent({ promise: deferred.promise, parentPromise: this });
}
return deferred.promise;
}
function PromiseIdResolveHandler(x) { return x } function PromiseIdResolveHandler(x) { return x }
function PromiseIdRejectHandler(r) { throw r } function PromiseIdRejectHandler(r) { throw r }
...@@ -291,8 +224,38 @@ function PromiseRejected(r) { ...@@ -291,8 +224,38 @@ function PromiseRejected(r) {
// Simple chaining. // Simple chaining.
function PromiseChain(onResolve, onReject) { // a.k.a. flatMap function PromiseChain(onResolve, onReject) { // a.k.a. flatMap
return %_CallFunction(this, onResolve, onReject, false, onResolve = IS_UNDEFINED(onResolve) ? PromiseIdResolveHandler : onResolve;
PromiseChainInternal); onReject = IS_UNDEFINED(onReject) ? PromiseIdRejectHandler : onReject;
var deferred = %_CallFunction(this.constructor, PromiseDeferred);
switch (GET_PRIVATE(this, promiseStatus)) {
case UNDEFINED:
throw MakeTypeError(kNotAPromise, this);
case 0: // Pending
GET_PRIVATE(this, promiseOnResolve).push(onResolve, deferred);
GET_PRIVATE(this, promiseOnReject).push(onReject, deferred);
break;
case +1: // Resolved
PromiseEnqueue(GET_PRIVATE(this, promiseValue),
[onResolve, deferred],
+1);
break;
case -1: // Rejected
if (!HAS_DEFINED_PRIVATE(this, promiseHasHandler)) {
// Promise has already been rejected, but had no handler.
// Revoke previously triggered reject event.
%PromiseRevokeReject(this);
}
PromiseEnqueue(GET_PRIVATE(this, promiseValue),
[onReject, deferred],
-1);
break;
}
// Mark this promise as having handler.
SET_PRIVATE(this, promiseHasHandler, true);
if (DEBUG_IS_ACTIVE) {
%DebugPromiseEvent({ promise: deferred.promise, parentPromise: this });
}
return deferred.promise;
} }
function PromiseCatch(onReject) { function PromiseCatch(onReject) {
...@@ -323,8 +286,7 @@ function PromiseThen(onResolve, onReject) { ...@@ -323,8 +286,7 @@ function PromiseThen(onResolve, onReject) {
} }
}, },
onReject, onReject,
true, PromiseChain
PromiseChainInternal
); );
} }
...@@ -386,7 +348,7 @@ function PromiseRace(iterable) { ...@@ -386,7 +348,7 @@ function PromiseRace(iterable) {
function PromiseHasUserDefinedRejectHandlerRecursive(promise) { function PromiseHasUserDefinedRejectHandlerRecursive(promise) {
var queue = GET_PRIVATE(promise, promiseOnReject); var queue = GET_PRIVATE(promise, promiseOnReject);
if (IS_UNDEFINED(queue)) return false; if (IS_UNDEFINED(queue)) return false;
for (var i = 0; i < queue.length; i += 3) { for (var i = 0; i < queue.length; i += 2) {
if (queue[i] != PromiseIdRejectHandler) return true; if (queue[i] != PromiseIdRejectHandler) return true;
if (PromiseHasUserDefinedRejectHandlerRecursive(queue[i + 1].promise)) { if (PromiseHasUserDefinedRejectHandlerRecursive(queue[i + 1].promise)) {
return true; return true;
......
test/mjsunit/es6/promises.js
View file @ 4e7990d3
...@@ -390,59 +390,6 @@ function assertAsyncDone(iteration) { ...@@ -390,59 +390,6 @@ function assertAsyncDone(iteration) {
assertAsyncRan() assertAsyncRan()
})(); })();
(function() {
var p1 = Promise.accept(5)
var p2 = Promise.accept(p1)
var called = false
p2.then = function(onResolve, onReject) {
called = true;
return call(Promise.prototype.then, p2, onResolve, onReject)
}
var p3 = Promise.accept(p2)
p3.chain(
function(x) {
assertAsync(x === p2 && !called, "resolved/thenable-promise/chain")
},
assertUnreachable)
assertAsyncRan()
})();
(function() {
var p1 = Promise.accept(5)
var p2 = Promise.accept(p1)
var called = false
p2.then = function(onResolve, onReject) {
called = true
return call(Promise.prototype.then, p2, onResolve, onReject)
}
var p3 = Promise.accept(p2)
p3.then(
function(x) {
assertAsync(x === 5 && called, "resolved/thenable-promise/then")
},
assertUnreachable)
assertAsyncRan()
})();
(function() {
var p1 = Promise.accept(5)
var called = false
var p3 = p1.then(function(x) {
var p2 = Promise.accept(5)
p2.then = function(onResolve, onReject) {
called = true
throw 25
}
return p2
});
p3.then(
assertUnreachable,
function(x) {
assertAsync(called && x === 25, "thenable-promise/then-call-throw")
})
assertAsyncRan()
})();
(function() { (function() {
var deferred = Promise.defer() var deferred = Promise.defer()
var p1 = deferred.promise var p1 = deferred.promise
...@@ -571,106 +518,6 @@ function assertAsyncDone(iteration) { ...@@ -571,106 +518,6 @@ function assertAsyncDone(iteration) {
assertAsyncRan() assertAsyncRan()
})(); })();
(function() {
var deferred = Promise.defer()
var deferred2 = Promise.defer()
var deferred3 = Promise.defer()
var p1 = deferred.promise
var p2 = deferred2.promise
var p3 = deferred3.promise
var called = false
p2.then = function(onResolve, onReject) {
called = true
return call(Promise.prototype.then, p2, onResolve, onReject)
}
p3.chain(
function(x) { assertAsync(x === p2 && !called,
"chain/resolve/thenable-promise") },
assertUnreachable
)
deferred3.resolve(p2)
deferred2.resolve(p1)
deferred.resolve(5)
assertAsyncRan()
})();
(function() {
var deferred = Promise.defer()
var deferred2 = Promise.defer()
var deferred3 = Promise.defer()
var p1 = deferred.promise
var p2 = deferred2.promise
var p3 = deferred3.promise
var called = false
p2.then = function(onResolve, onReject) {
called = true
return call(Promise.prototype.then, p2, onResolve, onReject)
}
p3.then(
function(x) { assertAsync(x === 5 && called,
"then/resolve/thenable-promise") },
assertUnreachable
)
deferred3.resolve(p2)
deferred2.resolve(p1)
deferred.resolve(5)
assertAsyncRan()
})();
(function() {
var deferred = Promise.defer()
var deferred2 = Promise.defer()
var deferred3 = Promise.defer()
var p1 = deferred.promise
var p2 = deferred2.promise
var p3 = deferred3.promise
var called = false
p2.then = function(onResolve, onReject) {
called = true
return call(Promise.prototype.then, p2, onResolve, onReject)
}
p3.chain(
function(x) { assertAsync(x === p2 && !called,
"chain/reject/thenable-promise") },
assertUnreachable
)
deferred3.resolve(p2)
deferred2.resolve(p1)
deferred.reject(5)
assertAsyncRan()
})();
(function() {
var deferred = Promise.defer()
var deferred2 = Promise.defer()
var deferred3 = Promise.defer()
var p1 = deferred.promise
var p2 = deferred2.promise
var p3 = deferred3.promise
var called = false
p2.then = function(onResolve, onReject) {
called = true
return call(Promise.prototype.then, p2, onResolve, onReject)
}
p3.then(
assertUnreachable,
function(x) { assertAsync(x === 5 && called,
"then/reject/thenable-promise") }
)
deferred3.resolve(p2)
deferred2.resolve(p1)
deferred.reject(5)
assertAsyncRan()
})();
(function() { (function() {
var p1 = Promise.accept(5) var p1 = Promise.accept(5)
var p2 = Promise.accept(p1) var p2 = Promise.accept(p1)
......
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