Promise assimilation fix.

src/promise.js

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 "use strict";
 
 // This file relies on the fact that the following declaration has been made
 // in runtime.js:
 // var $Object = global.Object
 // var $WeakMap = global.WeakMap
@@ skipping 82 matching lines @@
           %_CallFunction(x, deferred.resolve, deferred.reject, then);
         } catch(r) {
           deferred.reject(r);
         }
         return deferred.promise;
       }
     }
     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 {
       %DebugPopPromise();
     }
   }
 
   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 += 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 });
       }
     });
     if (instrumenting) {
       id = ++lastMicrotaskId;
       name = status > 0 ? "Promise.resolve" : "Promise.reject";
       %DebugAsyncTaskEvent({ type: "enqueue", id: id, name: name });
     }
   }
 
+  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 }
 
   function PromiseNopResolver() {}
 
   // -------------------------------------------------------------------
   // Define exported functions.
 
   // For bootstrapper.
 
@@ skipping 62 matching lines @@
     } else {
       promise = new this(function(resolve, reject) { reject(r) });
     }
     return promise;
   }
 
   // Simple chaining.
 
   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) {
     return this.then(UNDEFINED, onReject);
   }
 
   // Multi-unwrapped chaining with thenable coercion.
 
   PromiseThen = function PromiseThen(onResolve, onReject) {
     onResolve = IS_SPEC_FUNCTION(onResolve) ? onResolve
                                             : PromiseIdResolveHandler;
     onReject = IS_SPEC_FUNCTION(onReject) ? onReject
                                           : PromiseIdRejectHandler;
     var that = this;
     var constructor = this.constructor;
     return %_CallFunction(
       this,
       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
     );
   }
 
   // Combinators.
 
   function PromiseCast(x) {
     // TODO(rossberg): cannot do better until we support @@create.
     return IsPromise(x) ? x : new this(function(resolve) { resolve(x) });
   }
 
@@ skipping 48 matching lines @@
     }
     return deferred.promise;
   }
 
 
   // Utility for debugger
 
   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;
       }
     }
     return false;
   }
 
   // Return whether the promise will be handled by a user-defined reject
   // handler somewhere down the promise chain. For this, we do a depth-first
@@ skipping 18 matching lines @@
     "race", PromiseOne,
     "resolve", PromiseCast
   ]);
   InstallFunctions($Promise.prototype, DONT_ENUM, [
     "chain", PromiseChain,
     "then", PromiseThen,
     "catch", PromiseCatch
   ]);
 
 })();