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

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 (file)
--- 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 (file)
--- 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()