Promise assimilation fix.

src/promise.js

Index: src/promise.js
diff --git a/src/promise.js b/src/promise.js
index 443a3b8c01a15d2d8cd4de98e4dfb10219fcb511..bae10fbc92bad2bcaec10fcccf4e9ca17d48b71a 100644
--- a/src/promise.js
+++ b/src/promise.js
@@ -100,16 +100,30 @@ var lastMicrotaskId = 0;
     return x;
   }
 
-  function PromiseHandle(value, handler, deferred) {
+  function PromiseHandle(value, handler, deferred, thenable) {
     try {
       %DebugPushPromise(deferred.promise);
       var result = handler(value);
-      if (result === deferred.promise)
+      if (result === deferred.promise) {
         throw MakeTypeError('promise_cyclic', [result]);
-      else if (IsPromise(result))
+      } else if (IsPromise(result) && thenable) {
+        var then = result.then;

[arv (Not doing code reviews), 2015/05/11 13:53:15]

Isn't this extra Get observable?

[yhirano, 2015/05/12 06:09:19]

In PromiseReactionJob[1], promiseCapability.[[Resolve]] a.k.a. promise resolve
function[2] is called. "then" is accessed in the function, so accessing the
property here is correct.


1: https://people.mozilla.org/~jorendorff/es6-draft.html#sec-promisereactionjob
2:
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-promise-resolve-fun...

+        if (IS_SPEC_FUNCTION(then)) {
+          %EnqueueMicrotask(function() {
+            try {
+              %_CallFunction(result, deferred.resolve, deferred.reject, then);
+            } catch(exception) {
+              try { deferred.reject(exception); } catch (e) { }

[caitp (gmail), 2015/05/11 03:02:16]

Is this not already handled by the enclosing try/catch block?

[caitp (gmail), 2015/05/11 03:06:05]

wait, I get it --- I guess it's not

[arv (Not doing code reviews), 2015/05/11 13:53:15]

I think it is :-)

Line 128

[caitp (gmail), 2015/05/11 15:02:34]

Yeah, but this one is in a closure passed to EnqueueMicrotask, so presumably
this doesn't happen right away

+            }
+          });
+        } else {
+          deferred.resolve(result);

[caitp (gmail), 2015/05/11 03:02:16]

This same branch is repeated a few times, maybe it's worth writing it so that
the duplication isn't needed? On the other hand, maybe it's more readable this
way.

[yhirano, 2015/05/12 06:09:19]

Hmm, I don't have a good idea to unify them (partially because L120 and L125 are
in different functions). Do you have one?

+        }
+      } 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 {
@@ -123,8 +137,8 @@ var lastMicrotaskId = 0;
       if (instrumenting) {
         %DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name });
       }
-      for (var i = 0; i < tasks.length; i += 2) {
-        PromiseHandle(value, tasks[i], tasks[i + 1])
+      for (var i = 0; i < tasks.length; i += 3) {

[caitp (gmail), 2015/05/11 03:02:16]

This gets confusing, comments would be helpful

[yhirano, 2015/05/12 06:09:19]

Done.

+        PromiseHandle(value, tasks[i], tasks[i + 1], tasks[i + 2])
       }
       if (instrumenting) {
         %DebugAsyncTaskEvent({ type: "didHandle", id: id, name: name });
@@ -137,6 +151,41 @@ var lastMicrotaskId = 0;
     }
   }
 
+  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('not_a_promise', [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 }
 
@@ -219,38 +268,8 @@ var lastMicrotaskId = 0;
 
   PromiseChain = function PromiseChain(onResolve, onReject) {  // a.k.a.
                                                                // flatMap
-    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('not_a_promise', [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;
+    return %_CallFunction(this, onResolve, onReject, false,
+                          PromiseChainInternal);
   }
 
   PromiseCatch = function PromiseCatch(onReject) {
@@ -271,10 +290,11 @@ var lastMicrotaskId = 0;
       function(x) {
         x = PromiseCoerce(constructor, x);
         return x === that ? onReject(MakeTypeError('promise_cyclic', [x])) :
-               IsPromise(x) ? x.then(onResolve, onReject) : onResolve(x);
+            IsPromise(x) ? x.then(onResolve, onReject) : onResolve(x);
       },
       onReject,
-      PromiseChain
+      true,
+      PromiseChainInternal
     );
   }
 
@@ -343,7 +363,7 @@ var lastMicrotaskId = 0;
   function PromiseHasUserDefinedRejectHandlerRecursive(promise) {
     var queue = GET_PRIVATE(promise, promiseOnReject);
     if (IS_UNDEFINED(queue)) return false;
-    for (var i = 0; i < queue.length; i += 2) {
+    for (var i = 0; i < queue.length; i += 3) {
       if (queue[i] != PromiseIdRejectHandler) return true;
       if (PromiseHasUserDefinedRejectHandlerRecursive(queue[i + 1].promise)) {
         return true;