Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(704)

Unified Diff: src/js/promise.js

Issue 1488783002: Clean up promises and fix an edge case bug (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Minor code cleanup; disable some soon-to-be-obsolete mjsunit tests Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/mjsunit/es6/promises.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/js/promise.js
diff --git a/src/js/promise.js b/src/js/promise.js
index 1016cbba29be3fbf6afa3cdd8e159a5517a7d96f..f59809f0bd6034cd7b6e6f3beb977e89fd0c4995 100644
--- a/src/js/promise.js
+++ b/src/js/promise.js
@@ -40,9 +40,6 @@ function CreateResolvingFunctions(promise) {
var resolve = function(value) {
if (alreadyResolved === true) return;
alreadyResolved = true;
- if (value === promise) {
- return PromiseReject(promise, MakeTypeError(kPromiseCyclic, value));
- }
PromiseResolve(promise, value);
};
@@ -116,37 +113,11 @@ function PromiseDone(promise, status, value, promiseQueue) {
}
}
-function PromiseCoerce(constructor, x) {
- if (!IsPromise(x) && IS_SPEC_OBJECT(x)) {
- var then;
- try {
- then = x.then;
- } catch(r) {
- return %_Call(PromiseRejected, constructor, r);
- }
- if (IS_CALLABLE(then)) {
- var deferred = NewPromiseCapability(constructor);
- try {
- %_Call(then, x, deferred.resolve, deferred.reject);
- } catch(r) {
- deferred.reject(r);
- }
- return deferred.promise;
- }
- }
- return x;
-}
-
function PromiseHandle(value, handler, deferred) {
try {
%DebugPushPromise(deferred.promise, PromiseHandle);
var result = handler(value);
- if (result === deferred.promise)
- throw MakeTypeError(kPromiseCyclic, result);
- else if (IsPromise(result))
- %_Call(PromiseChain, result, deferred.resolve, deferred.reject);
- else
- deferred.resolve(result);
+ deferred.resolve(result);
} catch (exception) {
try { deferred.reject(exception); } catch (e) { }
} finally {
@@ -193,28 +164,29 @@ function PromiseCreate() {
}
function PromiseResolve(promise, x) {
- if (GET_PRIVATE(promise, promiseStatusSymbol) === 0) {
- if (IS_SPEC_OBJECT(x)) {
- // 25.4.1.3.2 steps 8-12
- try {
- var then = x.then;
- } catch (e) {
- return PromiseReject(promise, e);
- }
- if (IS_CALLABLE(then)) {
- // PromiseResolveThenableJob
- return %EnqueueMicrotask(function() {
- try {
- var callbacks = CreateResolvingFunctions(promise);
- %_Call(then, x, callbacks.resolve, callbacks.reject);
- } catch (e) {
- PromiseReject(promise, e);
- }
- });
- }
+ if (x === promise) {
+ return PromiseReject(promise, MakeTypeError(kPromiseCyclic, x));
+ }
+ if (IS_SPEC_OBJECT(x)) {
+ // 25.4.1.3.2 steps 8-12
+ try {
+ var then = x.then;
+ } catch (e) {
+ return PromiseReject(promise, e);
+ }
+ if (IS_CALLABLE(then)) {
+ // PromiseResolveThenableJob
+ return %EnqueueMicrotask(function() {
+ try {
+ var callbacks = CreateResolvingFunctions(promise);
+ %_Call(then, x, callbacks.resolve, callbacks.reject);
+ } catch (e) {
+ PromiseReject(promise, e);
+ }
+ });
}
- PromiseDone(promise, +1, x, promiseOnResolveSymbol);
}
+ PromiseDone(promise, +1, x, promiseOnResolveSymbol);
}
function PromiseReject(promise, r) {
@@ -245,6 +217,7 @@ function NewPromiseCapability(C) {
} else {
var result = {promise: UNDEFINED, resolve: UNDEFINED, reject: UNDEFINED };
result.promise = new C(function(resolve, reject) {
+ // TODO(littledan): Check for resolve and reject being not undefined
result.resolve = resolve;
result.reject = reject;
});
@@ -257,15 +230,13 @@ function PromiseDeferred() {
}
function PromiseResolved(x) {
- if (this === GlobalPromise) {
- // Optimized case, avoid extra closure.
- return PromiseCreateAndSet(+1, x);
- } else {
- return new this(function(resolve, reject) { resolve(x) });
- }
+ return %_Call(PromiseCast, this, x);
}
function PromiseRejected(r) {
+ if (!IS_SPEC_OBJECT(this)) {
+ throw MakeTypeError(kCalledOnNonObject, PromiseRejected);
+ }
var promise;
if (this === GlobalPromise) {
// Optimized case, avoid extra closure.
@@ -279,15 +250,18 @@ function PromiseRejected(r) {
return promise;
}
-// Simple chaining.
+// Multi-unwrapped chaining with thenable coercion.
-// PromiseChain a.k.a. flatMap
-function PromiseChainInternal(constructor, onResolve, onReject) {
- onResolve = IS_UNDEFINED(onResolve) ? PromiseIdResolveHandler : onResolve;
- onReject = IS_UNDEFINED(onReject) ? PromiseIdRejectHandler : onReject;
+function PromiseThen(onResolve, onReject) {
+ var constructor = this.constructor;
+ onResolve = IS_CALLABLE(onResolve) ? onResolve : PromiseIdResolveHandler;
+ onReject = IS_CALLABLE(onReject) ? onReject : PromiseIdRejectHandler;
var deferred = NewPromiseCapability(constructor);
switch (GET_PRIVATE(this, promiseStatusSymbol)) {
case UNDEFINED:
+ // TODO(littledan): The type check should be called before
+ // constructing NewPromiseCapability; this is observable when
+ // erroneously copying this method to other classes.
throw MakeTypeError(kNotAPromise, this);
case 0: // Pending
GET_PRIVATE(this, promiseOnResolveSymbol).push(onResolve, deferred);
@@ -317,47 +291,25 @@ function PromiseChainInternal(constructor, onResolve, onReject) {
return deferred.promise;
}
+// Chain is left around for now as an alias for then
function PromiseChain(onResolve, onReject) {
- return %_Call(PromiseChainInternal, this, this.constructor,
- onResolve, onReject);
+ return %_Call(PromiseThen, this, onResolve, onReject);
}
function PromiseCatch(onReject) {
return this.then(UNDEFINED, onReject);
}
-// Multi-unwrapped chaining with thenable coercion.
-
-function PromiseThen(onResolve, onReject) {
- onResolve = IS_CALLABLE(onResolve) ? onResolve : PromiseIdResolveHandler;
- onReject = IS_CALLABLE(onReject) ? onReject : PromiseIdRejectHandler;
- var that = this;
- var constructor = this.constructor;
- return %_Call(
- PromiseChainInternal,
- this,
- constructor,
- function(x) {
- x = PromiseCoerce(constructor, x);
- if (x === that) {
- return onReject(MakeTypeError(kPromiseCyclic, x));
- } else if (IsPromise(x)) {
- return x.then(onResolve, onReject);
- } else {
- return onResolve(x);
- }
- },
- onReject
- );
-}
-
// Combinators.
function PromiseCast(x) {
+ if (!IS_SPEC_OBJECT(this)) {
+ throw MakeTypeError(kCalledOnNonObject, PromiseCast);
+ }
if (IsPromise(x) && x.constructor === this) {
return x;
} else {
- return new this(function(resolve) { resolve(x) });
+ return new this(function(resolve, reject) { resolve(x) });
}
}
« no previous file with comments | « no previous file | test/mjsunit/es6/promises.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698