Bug 1170760 part 7. Add subclassing support to Promise::Race. r=baku,efaust

Note that the web platform tests don't actually have quite the behavior they're
expected to per the spec yet.  They will get adjusted later on as we add
subclassing support to Promise.resolve and Promise.prototype.then.

dom/bindings/Errors.msg

@@ -87,5 +87,6 @@ MSG_DEF(MSG_ILLEGAL_PROMISE_CONSTRUCTOR, 0, JSEXN_TYPEERR, "Non-constructor valu
 MSG_DEF(MSG_PROMISE_CAPABILITY_HAS_SOMETHING_ALREADY, 0, JSEXN_TYPEERR, "GetCapabilitiesExecutor function already invoked with non-undefined values.")
 MSG_DEF(MSG_PROMISE_RESOLVE_FUNCTION_NOT_CALLABLE, 0, JSEXN_TYPEERR, "A Promise subclass passed a non-callable value as the resolve function.")
 MSG_DEF(MSG_PROMISE_REJECT_FUNCTION_NOT_CALLABLE, 0, JSEXN_TYPEERR, "A Promise subclass passed a non-callable value as the reject function.")
+MSG_DEF(MSG_PROMISE_ARG_NOT_ITERABLE, 1, JSEXN_TYPEERR, "{0} is not iterable")
 MSG_DEF(MSG_SW_INSTALL_ERROR, 2, JSEXN_TYPEERR, "ServiceWorker script at {0} for scope {1} encountered an error during installation.")
 MSG_DEF(MSG_SW_SCRIPT_THREW, 2, JSEXN_TYPEERR, "ServiceWorker script at {0} for scope {1} threw an exception during script evaluation.")

dom/promise/Promise.cpp

@@ -1325,47 +1325,106 @@ Promise::All(const GlobalObject& aGlobal,
   return promise.forget();
 }
 
-/* static */ already_AddRefed<Promise>
+/* static */ void
 Promise::Race(const GlobalObject& aGlobal, JS::Handle<JS::Value> aThisv,
-              const Sequence<JS::Value>& aIterable, ErrorResult& aRv)
+              JS::Handle<JS::Value> aIterable, JS::MutableHandle<JS::Value> aRetval,
+              ErrorResult& aRv)
 {
+  // Implements http://www.ecma-international.org/ecma-262/6.0/#sec-promise.race
   nsCOMPtr<nsIGlobalObject> global =
     do_QueryInterface(aGlobal.GetAsSupports());
   if (!global) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
-    return nullptr;
+    return;
   }
 
   JSContext* cx = aGlobal.Context();
 
-  JS::Rooted<JSObject*> obj(cx, JS::CurrentGlobalOrNull(cx));
-  if (!obj) {
-    aRv.Throw(NS_ERROR_UNEXPECTED);
-    return nullptr;
-  }
+  // Steps 1-5: nothing to do.  Note that the @@species bits got removed in
+  // https://github.com/tc39/ecma262/pull/211
+  PromiseCapability capability(cx);
 
-  RefPtr<Promise> promise = Create(global, aRv);
+  // Step 6.
+  NewPromiseCapability(cx, global, aThisv, true, capability, aRv);
+  // Step 7.
   if (aRv.Failed()) {
-    return nullptr;
+    return;
   }
 
-  RefPtr<PromiseCallback> resolveCb =
-    new ResolvePromiseCallback(promise, obj);
+  MOZ_ASSERT(aThisv.isObject(), "How did NewPromiseCapability succeed?");
+  JS::Rooted<JSObject*> constructorObj(cx, &aThisv.toObject());
 
-  RefPtr<PromiseCallback> rejectCb = new RejectPromiseCallback(promise, obj);
-
-  for (uint32_t i = 0; i < aIterable.Length(); ++i) {
-    JS::Rooted<JS::Value> value(cx, aIterable.ElementAt(i));
-    RefPtr<Promise> nextPromise = Promise::Resolve(aGlobal, aThisv, value, aRv);
-    // According to spec, Resolve can throw, but our implementation never does.
-    // Well it does when window isn't passed on the main thread, but that is an
-    // implementation detail which should never be reached since we are checking
-    // for window above. Remove this when subclassing is supported.
-    MOZ_ASSERT(!aRv.Failed());
-    nextPromise->AppendCallbacks(resolveCb, rejectCb);
+  // After this point we have a useful promise value in "capability", so just go
+  // ahead and put it in our retval now.  Every single return path below would
+  // want to do that anyway.
+  aRetval.set(capability.PromiseValue());
+  if (!MaybeWrapValue(cx, aRetval)) {
+    aRv.NoteJSContextException();
+    return;
   }
 
-  return promise.forget();
+  // The arguments we're going to be passing to "then" on each loop iteration.
+  JS::AutoValueArray<2> callbackFunctions(cx);
+  callbackFunctions[0].set(capability.mResolve);
+  callbackFunctions[1].set(capability.mReject);
+
+  // Steps 8 and 9.
+  JS::ForOfIterator iter(cx);
+  if (!iter.init(aIterable, JS::ForOfIterator::AllowNonIterable)) {
+    capability.RejectWithException(cx, aRv);
+    return;
+  }
+
+  if (!iter.valueIsIterable()) {
+    ThrowErrorMessage(cx, MSG_PROMISE_ARG_NOT_ITERABLE,
+                      "Argument of Promise.race");
+    capability.RejectWithException(cx, aRv);
+    return;
+  }
+
+  // Step 10 doesn't need to be done, because ForOfIterator handles it
+  // for us.
+
+  // Now we jump over to
+  // http://www.ecma-international.org/ecma-262/6.0/#sec-performpromiserace
+  // and do its steps.
+  JS::Rooted<JS::Value> nextValue(cx);
+  while (true) {
+    bool done;
+    // Steps a, b, c, e, f, g.
+    if (!iter.next(&nextValue, &done)) {
+      capability.RejectWithException(cx, aRv);
+      return;
+    }
+
+    // Step d.
+    if (done) {
+      // We're all set!
+      return;
+    }
+
+    // Step h.  Sadly, we can't take a shortcut here even if
+    // capability.mNativePromise exists, because someone could have overridden
+    // "resolve" on the canonical Promise constructor.
+    JS::Rooted<JS::Value> nextPromise(cx);
+    if (!JS_CallFunctionName(cx, constructorObj, "resolve",
+                             JS::HandleValueArray(nextValue), &nextPromise)) {
+      // Step i.
+      capability.RejectWithException(cx, aRv);
+      return;
+    }
+
+    // Step j.  And now we don't know whether nextPromise has an overridden
+    // "then" method, so no shortcuts here either.
+    JS::Rooted<JSObject*> nextPromiseObj(cx);
+    JS::Rooted<JS::Value> ignored(cx);
+    if (!JS_ValueToObject(cx, nextPromise, &nextPromiseObj) ||
+        !JS_CallFunctionName(cx, nextPromiseObj, "then", callbackFunctions,
+                             &ignored)) {
+      // Step k.
+      capability.RejectWithException(cx, aRv);
+    }
+  }
 }
 
 /* static */

dom/promise/Promise.h

@@ -194,9 +194,10 @@ public:
   All(const GlobalObject& aGlobal,
       const nsTArray<RefPtr<Promise>>& aPromiseList, ErrorResult& aRv);
 
-  static already_AddRefed<Promise>
+  static void
   Race(const GlobalObject& aGlobal, JS::Handle<JS::Value> aThisv,
-       const Sequence<JS::Value>& aIterable, ErrorResult& aRv);
+       JS::Handle<JS::Value> aIterable, JS::MutableHandle<JS::Value> aRetval,
+       ErrorResult& aRv);
 
   static bool
   PromiseSpecies(JSContext* aCx, unsigned aArgc, JS::Value* aVp);

dom/promise/tests/chrome.ini

@@ -5,3 +5,4 @@ skip-if = buildapp == 'b2g'
 [test_on_new_promise.html]
 [test_on_promise_settled.html]
 [test_on_promise_settled_duplicates.html]
+[test_promise_xrays.html]

dom/promise/tests/file_promise_xrays.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+  <script>
+    function vendGetter(name) {
+      return function() { throw "Getting " + String(name) };
+    }
+    function vendSetter(name) {
+      return function() { throw "Setting " + String(name) };
+    }
+    var setupThrew = false;
+    try {
+      // Neuter everything we can think of on Promise.
+      for (var obj of [Promise, Promise.prototype]) {
+        propNames = Object.getOwnPropertyNames(obj);
+        propNames = propNames.concat(Object.getOwnPropertySymbols(obj));
+        for (var propName of propNames) {
+          if (propName == "prototype" && obj == Promise) {
+            // It's not configurable
+            continue;
+          }
+          Object.defineProperty(obj, propName,
+            { get: vendGetter(propName), set: vendSetter(propName) });
+        }
+      }
+    } catch (e) {
+      // Something went wrong.  Save that info so the test can check for it.
+      setupThrew = e;
+    }
+  </script>
+</html>

dom/promise/tests/mochitest.ini

@@ -1,4 +1,7 @@
 [DEFAULT]
+# Support files for chrome tests that we want to load over HTTP need
+# to go in here, not chrome.ini, apparently.
+support-files = file_promise_xrays.html
 
 [test_abortable_promise.html]
 [test_bug883683.html]

dom/promise/tests/test_promise_xrays.html

@@ -0,0 +1,125 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1170760
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1170760</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://global/skin"/>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1170760">Mozilla Bug 1170760</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+<iframe id="t" src="http://example.org/tests/dom/promise/tests/file_promise_xrays.html"></iframe>
+</div>
+
+<pre id="test">
+<script type="application/javascript">
+
+var win = $("t").contentWindow;
+
+/** Test for Bug 1170760 **/
+SimpleTest.waitForExplicitFinish();
+
+function testLoadComplete() {
+  is(win.location.href, $("t").src, "Should have loaded the right thing");
+  nextTest();
+}
+
+function testHaveXray() {
+  is(typeof win.Promise.race, "function", "Should see a race() function");
+  var exception;
+  try {
+    win.Promise.wrappedJSObject.race;
+  } catch (e) {
+    exception = e;
+  }
+  is(exception, "Getting race", "Should have thrown the right exception");
+  is(win.wrappedJSObject.setupThrew, false, "Setup should not have thrown");
+  nextTest();
+}
+
+function testRace1() {
+  var p = win.Promise.race(new win.Array(1, 2));
+  p.then(
+    function(arg) {
+      ok(arg == 1 || arg == 2,
+         "Should get the right value when racing content-side array");
+    },
+    function(e) {
+      ok(false, "testRace1 threw exception: " + e);
+    }
+  ).then(nextTest);
+}
+
+function testRace2() {
+  var p = win.Promise.race(
+    new Array(win.Promise.resolve(1), win.Promise.resolve(2)));
+  p.then(
+    function(arg) {
+      ok(arg == 1 || arg == 2,
+         "Should get the right value when racing content-side array of explicit Promises");
+    },
+    function(e) {
+      ok(false, "testRace2 threw exception: " + e);
+    }
+  ).then(nextTest);
+}
+
+function testRace3() {
+  // This works with a chrome-side array because we do the iteration
+  // while still in the Xray compartment.
+  var p = win.Promise.race([1, 2]);
+  p.then(
+    function(arg) {
+      ok(arg == 1 || arg == 2,
+         "Should get the right value when racing chrome-side array");
+    },
+    function(e) {
+      ok(false, "testRace3 threw exception: " + e);
+    }
+  ).then(nextTest);
+}
+
+function testRace4() {
+  // This works with both content-side and chrome-side Promises because we want
+  // it to and go to some lengths to make it work.
+  var p = win.Promise.race([Promise.resolve(1), win.Promise.resolve(2)]);
+  p.then(
+    function(arg) {
+      ok(arg == 1 || arg == 2,
+         "Should get the right value when racing chrome-side promises");
+    },
+    function(e) {
+      ok(false, "testRace4 threw exception: " + e);
+    }
+  ).then(nextTest);
+}
+
+var tests = [
+  testLoadComplete,
+  testHaveXray,
+  testRace1,
+  testRace2,
+  testRace3,
+  testRace4,
+];
+
+function nextTest() {
+  if (tests.length == 0) {
+    SimpleTest.finish();
+    return;
+  }
+  tests.shift()();
+}
+
+addLoadEvent(nextTest);
+
+</script>
+</pre>
+</body>
+</html>

dom/webidl/Promise.webidl

@@ -42,6 +42,11 @@ interface _Promise {
   [NewObject]
   static Promise<any> all(sequence<any> iterable);
 
-  [NewObject]
-  static Promise<any> race(sequence<any> iterable);
+  // Have to use "any" (or "object", but "any" is simpler) as the type to
+  // support the subclassing behavior, since nothing actually requires the
+  // return value of PromiseSubclass.race to be a Promise object.  As a result,
+  // we also have to do our argument conversion manually, because we want to
+  // convert its exceptions into rejections.
+  [NewObject, Throws]
+  static any race(optional any iterable);
 };

testing/web-platform/meta/MANIFEST.json

@@ -29919,7 +29919,16 @@
   },
   "local_changes": {
     "deleted": [],
-    "items": {},
+    "items": {
+      "testharness": {
+        "js/builtins/Promise-subclassing.html": [
+          {
+            "path": "js/builtins/Promise-subclassing.html",
+            "url": "/js/builtins/Promise-subclassing.html"
+          }
+        ]
+      }
+    },
     "reftest_nodes": {}
   },
   "reftest_nodes": {

testing/web-platform/tests/js/builtins/Promise-subclassing.html

@@ -0,0 +1,153 @@
+<!doctype html>
+<meta charset=utf-8>
+<title></title>
+<script src=/resources/testharness.js></script>
+<script src=/resources/testharnessreport.js></script>
+<script>
+
+var theLog = [];
+var speciesGets = 0;
+var speciesCalls = 0;
+var constructorCalls = 0;
+var resolveCalls = 0;
+var rejectCalls = 0;
+var thenCalls = 0;
+var catchCalls = 0;
+var allCalls = 0;
+var raceCalls = 0;
+var nextCalls = 0;
+
+function takeLog() {
+  var oldLog = theLog;
+  theLog = [];
+  speciesGets = speciesCalls = constructorCalls = resolveCalls =
+    rejectCalls = thenCalls = catchCalls = allCalls = raceCalls =
+    nextCalls = 0;
+  return oldLog;
+}
+
+function clearLog() {
+  takeLog();
+}
+
+function log(str) {
+  theLog.push(str);
+}
+
+class LoggingPromise extends Promise {
+  constructor(func) {
+    super(func);
+    ++constructorCalls;
+    log(`Constructor ${constructorCalls}`);
+  }
+
+  static get [Symbol.species]() {
+    ++speciesGets;
+    log(`Species get ${speciesGets}`);
+    return LoggingSpecies;
+  }
+
+  static resolve(val) {
+    ++resolveCalls;
+    log(`Resolve ${resolveCalls}`);
+    return super.resolve(val);
+  }
+
+  static reject(val) {
+    ++rejectCalls;
+    log(`Reject ${rejectCalls}`);
+    return super.reject(val);
+  }
+
+  then(resolve, reject) {
+    ++thenCalls;
+    log(`Then ${thenCalls}`);
+    return super.then(resolve, reject);
+  }
+
+  catch(handler) {
+    ++catchCalls;
+    log(`Catch ${catchCalls}`);
+    return super.catch(handler);
+  }
+
+  static all(val) {
+    ++allCalls;
+    log(`All ${allCalls}`);
+    return super.all(val);
+  }
+
+  static race(val) {
+    ++raceCalls;
+    log(`Race ${raceCalls}`);
+    return super.race(val);
+  }
+}
+
+class LoggingIterable {
+  constructor(array) {
+    this.iter = array[Symbol.iterator]();
+  }
+
+  get [Symbol.iterator]() { return () => this }
+
+  next() {
+    ++nextCalls;
+    log(`Next ${nextCalls}`);
+    return this.iter.next();
+  }
+}
+
+class LoggingSpecies extends LoggingPromise {
+  constructor(func) {
+    ++speciesCalls;
+    log(`Species call ${speciesCalls}`);
+    super(func)
+  }
+}
+
+class SpeciesLessPromise extends LoggingPromise {
+  static get [Symbol.species]() {
+    return undefined;
+  }
+}
+
+promise_test(function testBasicConstructor() {
+  var p = new LoggingPromise((resolve) => resolve(5));
+  var log = takeLog();
+  assert_array_equals(log, ["Constructor 1"]);
+  assert_true(p instanceof LoggingPromise);
+  return p.then(function(arg) {
+    assert_equals(arg, 5);
+  });
+}, "Basic constructor behavior");
+
+promise_test(function testPromiseRace() {
+  clearLog();
+  var p = LoggingPromise.race(new LoggingIterable([1, 2]));
+  var log = takeLog();
+  assert_array_equals(log, ["Race 1", "Constructor 1",
+                            "Next 1", "Resolve 1",
+                            "Next 2", "Resolve 2",
+                            "Next 3"]);
+  assert_true(p instanceof LoggingPromise);
+  return p.then(function(arg) {
+    assert_true(arg == 1 || arg == 2);
+  });
+}, "Promise.race behavior");
+
+promise_test(function testPromiseRaceNoSpecies() {
+  clearLog();
+  var p = SpeciesLessPromise.race(new LoggingIterable([1, 2]));
+  var log = takeLog();
+  assert_array_equals(log, ["Race 1", "Constructor 1",
+                            "Next 1", "Resolve 1",
+                            "Next 2", "Resolve 2",
+                            "Next 3"]);
+  assert_true(p instanceof SpeciesLessPromise);
+  return p.then(function(arg) {
+    assert_true(arg == 1 || arg == 2);
+  });
+}, "Promise.race without species behavior");
+
+</script>