Promises: some adaptations to spec

src/promise.js

Index: src/promise.js
diff --git a/src/promise.js b/src/promise.js
index 30f4f07b4b7b21c1c83b0c6e590eeb8f4c2803d6..2a3f4fd5cfd93f1a396bc362d3bd2a42b09d25cd 100644
--- a/src/promise.js
+++ b/src/promise.js
@@ -62,11 +62,16 @@ function IsPromise(x) {
 
 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) {
@@ -82,9 +87,10 @@ function PromiseInit(promise) {
 }
 
 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) {
@@ -219,25 +225,24 @@ function PromiseThen(onResolve, 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;
 }
 
 
@@ -245,39 +250,44 @@ function PromiseCoerce(constructor, x) {
 
 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;
 }
@@ -288,11 +298,11 @@ 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, [