From: machenbach Date: Thu, 11 Jun 2015 09:09:03 +0000 (-0700) Subject: Revert of Revert of Revert of Promise assimilation fix. (patchset #1 id:1 of https... X-Git-Tag: upstream/4.7.83~2109 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4e7990d394b3297d7ecd47746e21989131dac11a;p=platform%2Fupstream%2Fv8.git 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} --- diff --git a/src/promise.js b/src/promise.js index 453af02..2634fde 100644 --- a/src/promise.js +++ b/src/promise.js @@ -105,25 +105,17 @@ function PromiseCoerce(constructor, x) { return x; } -function PromiseHandle(value, handler, deferred, thenable) { +function PromiseHandle(value, handler, deferred) { try { %DebugPushPromise(deferred.promise, PromiseHandle); DEBUG_PREPARE_STEP_IN_IF_STEPPING(handler); var result = handler(value); - if (result === deferred.promise) { + if (result === deferred.promise) throw MakeTypeError(kPromiseCyclic, result); - } else if (IsPromise(result) && thenable) { - var then = result.then; - if (IS_SPEC_FUNCTION(then)) { - PromiseCallThenInSeparateTask(result, deferred, then); - } else { - deferred.resolve(result); - } - } else if (IsPromise(result)) { + else if (IsPromise(result)) %_CallFunction(result, deferred.resolve, deferred.reject, PromiseChain); - } else { + else deferred.resolve(result); - } } catch (exception) { try { deferred.reject(exception); } catch (e) { } } finally { @@ -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) { var id, name, instrumenting = DEBUG_IS_ACTIVE; %EnqueueMicrotask(function() { if (instrumenting) { %DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name }); } - for (var i = 0; i < tasks.length; i += 3) { - // tasks[i: i + 2] consists of the reaction handler, the associated - // deferred object and whether the thenable handling is required. - PromiseHandle(value, tasks[i], tasks[i + 1], tasks[i + 2]) + for (var i = 0; i < tasks.length; i += 2) { + PromiseHandle(value, tasks[i], tasks[i + 1]) } if (instrumenting) { %DebugAsyncTaskEvent({ type: "didHandle", id: id, name: name }); @@ -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 PromiseIdRejectHandler(r) { throw r } @@ -291,8 +224,38 @@ function PromiseRejected(r) { // Simple chaining. function PromiseChain(onResolve, onReject) { // a.k.a. flatMap - return %_CallFunction(this, onResolve, onReject, false, - PromiseChainInternal); + 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); + 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) { @@ -323,8 +286,7 @@ function PromiseThen(onResolve, onReject) { } }, onReject, - true, - PromiseChainInternal + PromiseChain ); } @@ -386,7 +348,7 @@ function PromiseRace(iterable) { function PromiseHasUserDefinedRejectHandlerRecursive(promise) { var queue = GET_PRIVATE(promise, promiseOnReject); 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 (PromiseHasUserDefinedRejectHandlerRecursive(queue[i + 1].promise)) { return true; diff --git a/test/mjsunit/es6/promises.js b/test/mjsunit/es6/promises.js index 14425e8..63b6d2f 100644 --- a/test/mjsunit/es6/promises.js +++ b/test/mjsunit/es6/promises.js @@ -391,59 +391,6 @@ function assertAsyncDone(iteration) { })(); (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() { var deferred = Promise.defer() var p1 = deferred.promise var p2 = Promise.accept(p1) @@ -572,106 +519,6 @@ function assertAsyncDone(iteration) { })(); (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() { var p1 = Promise.accept(5) var p2 = Promise.accept(p1) var deferred = Promise.defer()