Promises: some adaptations to spec

src/promise.js

Index: src/promise.js
diff --git a/src/promise.js b/src/promise.js
index 30f4f07b4b7b21c1c83b0c6e590eeb8f4c2803d6..e0c6a0b8afa8d6ee66c3ce7ec802b7708e638d4d 100644
--- a/src/promise.js
+++ b/src/promise.js
@@ -62,11 +62,14 @@ function IsPromise(x) {
 
 function Promise(resolver) {
   if (resolver === promiseRaw) return;
+  if (!%_IsConstructCall()) throw MakeTypeError('not_a_promise', [this]);
   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 +85,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) {
@@ -222,11 +226,12 @@ function PromiseCoerce(constructor, x) {
   var then;
   if (IsPromise(x)) {
     return x;
-  } else if (!IS_NULL_OR_UNDEFINED(x) && %IsCallable(then = x.then)) {
+  } else if (!IS_NULL_OR_UNDEFINED(x) &&
+             typeof (then = x.then) === 'function') {

[Dmitry Lomov (no reviews), 2013/12/04 09:24:43]

Can we not have assignments in conditions? Pretty please? Replace with
  if (IS_NULL_OR_UNDEFINED(x)) return x;
  var then = x.then;
  if (typeof(then) !== 'function') return x;

[rossberg, 2013/12/06 11:43:29]

Done.

     if (PromiseCoerce.table.has(x)) {
       return PromiseCoerce.table.get(x);
     } else {
-      var deferred = constructor.deferred();
+      var deferred = %_CallFunction(constructor, PromiseDeferred);
       PromiseCoerce.table.set(x, deferred.promise);
       try {
         %_CallFunction(x, deferred.resolve, deferred.reject, then);
@@ -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);

[Dmitry Lomov (no reviews), 2013/12/04 09:24:43]

Instead of using %_CallFunction here and above, let's have utility method
PromiseDeferredImpl, replace these calls with PromiseDeferredImpl(this), and
   Promise.defer = function(this) { PromiseDeferredImpl(this); }
Even better, I suggest not shipping defer (see below) :)

[rossberg, 2013/12/06 11:43:29]

Why? Avoiding extra hoops like that is exactly why we have %_CallFunction (which
really just is a safe and efficient version of f.call).

   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) }

[Dmitry Lomov (no reviews), 2013/12/04 09:24:43]

Why 'count > 0' condition disappeared here? What if more than one of values
fail?

[rossberg, 2013/12/06 11:43:29]

Reject used to throw on non-pending promises in an earlier version. For legacy
compatibility, I already changed that to just ignore the attempt, so the extra
check here became redundant (as in PromiseOne below).

+        );
+      }
     }
+  } 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) } }
-    );
+  var deferred = %_CallFunction(this, PromiseDeferred);

[Dmitry Lomov (no reviews), 2013/12/04 09:24:43]

Ditto re: %_CallFunction

+  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,9 +298,9 @@ function SetUpPromise() {
   %CheckIsBootstrapping()
   global.Promise = $Promise;
   InstallFunctions($Promise, DONT_ENUM, [
-    "deferred", PromiseDeferred,
-    "resolved", PromiseResolved,
-    "rejected", PromiseRejected,
+    "defer", PromiseDeferred,

[Dmitry Lomov (no reviews), 2013/12/04 09:24:43]

So, my understanding is that 'defer' is non-standard as of now? Let's not ship
non-standard API maybe? (maybe a separate CL)

[rossberg, 2013/12/06 11:43:29]

As per off-line discussion, I leave it in for the time being.

+    "resolve", PromiseResolved,
+    "reject", PromiseRejected,
     "all", PromiseAll,
     "one", PromiseOne,
     "cast", PromiseCast