Promises: some adaptations to spec

src/promise.js

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 44 matching lines @@
 var promiseOnResolve = NEW_PRIVATE("Promise#onResolve");
 var promiseOnReject = NEW_PRIVATE("Promise#onReject");
 var promiseRaw = NEW_PRIVATE("Promise#raw");
 
 function IsPromise(x) {
   return IS_SPEC_OBJECT(x) && %HasLocalProperty(x, promiseStatus);
 }
 
 function Promise(resolver) {
   if (resolver === promiseRaw) return;
+  if (!%_IsConstructCall()) throw MakeTypeError('not_a_promise', [this]);

[arv (Not doing code reviews), 2014/01/09 15:47:15]

Doesn't this break sub classing?

function SubPromise(resolver) {
  Promise.call(this, resolver);
}
SubPromise.prototype = {
  __proto__: Promise.prototype,
  constructor: SubPromise
};

[rossberg, 2014/01/09 16:09:56]

I agree, but it's what the spec says.

[arv (Not doing code reviews), 2014/01/09 19:13:27]

I think this is the right call for now. Once we support @@create we should be
able to fix this part to check the internal slots as required by the spec.

+  if (typeof resolver !== 'function')
+    throw MakeTypeError('resolver_not_a_function', [resolver]);
   var promise = PromiseInit(this);
-  resolver(function(x) { PromiseResolve(promise, x) },
-           function(r) { PromiseReject(promise, r) });
-  // TODO(rossberg): current draft makes exception from this call asynchronous,
-  // but that's probably a mistake.
+  try {
+    resolver(function(x) { PromiseResolve(promise, x) },
+             function(r) { PromiseReject(promise, r) });
+  } catch (e) {
+    PromiseReject(promise, e);
+  }
 }
 
 function PromiseSet(promise, status, value, onResolve, onReject) {
   SET_PRIVATE(promise, promiseStatus, status);
   SET_PRIVATE(promise, promiseValue, value);
   SET_PRIVATE(promise, promiseOnResolve, onResolve);
   SET_PRIVATE(promise, promiseOnReject, onReject);
   return promise;
 }
 
 function PromiseInit(promise) {
   return PromiseSet(promise, 0, UNDEFINED, new InternalArray, new InternalArray)
 }
 
 function PromiseDone(promise, status, value, promiseQueue) {
-  if (GET_PRIVATE(promise, promiseStatus) !== 0) return;
-  PromiseEnqueue(value, GET_PRIVATE(promise, promiseQueue));
-  PromiseSet(promise, status, value);
+  if (GET_PRIVATE(promise, promiseStatus) === 0) {
+    PromiseEnqueue(value, GET_PRIVATE(promise, promiseQueue));
+    PromiseSet(promise, status, value);
+  }
 }
 
 function PromiseResolve(promise, x) {
   PromiseDone(promise, +1, x, promiseOnResolve)
 }
 
 function PromiseReject(promise, r) {
   PromiseDone(promise, -1, r, promiseOnReject)
 }
 
@@ skipping 114 matching lines @@
       return x === that ? onReject(MakeTypeError('promise_cyclic', [x])) :
              IsPromise(x) ? x.then(onResolve, onReject) : onResolve(x);
     },
     onReject
   );
 }
 
 PromiseCoerce.table = new $WeakMap;
 
 function PromiseCoerce(constructor, x) {
-  var then;
-  if (IsPromise(x)) {
-    return x;
-  } else if (!IS_NULL_OR_UNDEFINED(x) && %IsCallable(then = x.then)) {
-    if (PromiseCoerce.table.has(x)) {
-      return PromiseCoerce.table.get(x);
-    } else {
-      var deferred = constructor.deferred();
-      PromiseCoerce.table.set(x, deferred.promise);
-      try {
-        %_CallFunction(x, deferred.resolve, deferred.reject, then);
-      } catch(e) {
-        deferred.reject(e);
+  if (!(IsPromise(x) || IS_NULL_OR_UNDEFINED(x))) {
+    var then = x.then;
+    if (typeof then === 'function') {
+      if (PromiseCoerce.table.has(x)) {
+        return PromiseCoerce.table.get(x);
+      } else {
+        var deferred = %_CallFunction(constructor, PromiseDeferred);
+        PromiseCoerce.table.set(x, deferred.promise);
+        try {
+          %_CallFunction(x, deferred.resolve, deferred.reject, then);
+        } catch(e) {
+          deferred.reject(e);
+        }
+        return deferred.promise;
       }
-      return deferred.promise;
     }
-  } else {
-    return x;
   }
+  return x;
 }
 
 
 // Combinators.
 
 function PromiseCast(x) {
   // TODO(rossberg): cannot do better until we support @@create.
-  return IsPromise(x) ? x : this.resolved(x);
+  return IsPromise(x) ? x : this.resolve(x);
 }
 
 function PromiseAll(values) {
-  var deferred = this.deferred();
+  var deferred = %_CallFunction(this, PromiseDeferred);
   var resolutions = [];
-  var count = values.length;
-  if (count === 0) {
-    deferred.resolve(resolutions);
-  } else {
-    for (var i = 0; i < values.length; ++i) {
-      this.cast(values[i]).chain(
-        function(i, x) {
-          resolutions[i] = x;
-          if (--count === 0) deferred.resolve(resolutions);
-        }.bind(UNDEFINED, i),  // TODO(rossberg): use let loop once available
-        function(r) {
-          if (count > 0) { count = 0; deferred.reject(r) }
-        }
-      );
+  try {
+    var count = values.length;
+    if (count === 0) {
+      deferred.resolve(resolutions);
+    } else {
+      for (var i = 0; i < values.length; ++i) {
+        this.cast(values[i]).chain(
+          function(i, x) {
+            resolutions[i] = x;
+            if (--count === 0) deferred.resolve(resolutions);
+          }.bind(UNDEFINED, i),  // TODO(rossberg): use let loop once available
+          function(r) { deferred.reject(r) }
+        );
+      }
     }
+  } catch (e) {
+    deferred.reject(e)
   }
   return deferred.promise;
 }
 
-function PromiseOne(values) {  // a.k.a. race
-  var deferred = this.deferred();
-  var done = false;
-  for (var i = 0; i < values.length; ++i) {
-    this.cast(values[i]).chain(
-      function(x) { if (!done) { done = true; deferred.resolve(x) } },
-      function(r) { if (!done) { done = true; deferred.reject(r) } }
-    );
+function PromiseOne(values) {

[arv (Not doing code reviews), 2014/01/09 15:47:15]

Rename this to PromiseRace to match?

Why isn't the done check needed any more?

[rossberg, 2014/01/09 16:09:56]

Because the resolve/reject functions at some point changed to sloppily ignore
that case, so it became redundant.

+  var deferred = %_CallFunction(this, PromiseDeferred);
+  try {
+    for (var i = 0; i < values.length; ++i) {
+      this.cast(values[i]).chain(
+        function(x) { deferred.resolve(x) },
+        function(r) { deferred.reject(r) }
+      );
+    }
+  } catch (e) {
+    deferred.reject(e)
   }
   return deferred.promise;
 }
 
 //-------------------------------------------------------------------
 
 function SetUpPromise() {
   %CheckIsBootstrapping()
   global.Promise = $Promise;
   InstallFunctions($Promise, DONT_ENUM, [
-    "deferred", PromiseDeferred,
-    "resolved", PromiseResolved,
-    "rejected", PromiseRejected,
+    "defer", PromiseDeferred,
+    "resolve", PromiseResolved,
+    "reject", PromiseRejected,
     "all", PromiseAll,
-    "one", PromiseOne,
+    "race", PromiseOne,
     "cast", PromiseCast
   ]);
   InstallFunctions($Promise.prototype, DONT_ENUM, [
     "chain", PromiseChain,

[arv (Not doing code reviews), 2014/01/09 15:47:15]

Any plan to upstream chain into the spec?

[rossberg, 2014/01/09 16:09:56]

Yes, but current stakeholders refuse to consider it, and it fell off the agenda
of the last meeting. :(

     "then", PromiseThen,
     "catch", PromiseCatch
   ]);
 }
 
 SetUpPromise();