Bug 875289 - Remove .done() and allow undefined to be passed to .then() and .catch(). r=mounir, sr=bz
☠☠ backed out by 2e39187b8199 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 19 Jun 2013 20:57:38 -0400
changeset 135746 8df19e23b3aea82edc656aaf3f31104d658e3b8f
parent 135745 c1596bee956c700875d972f4d20bb462603cfb76
child 135747 6a4bbbbe19bbd77e7228a6aba31eb10d69d8f073
push id29800
push userryanvm@gmail.com
push dateThu, 20 Jun 2013 00:57:52 +0000
treeherdermozilla-inbound@6a4bbbbe19bb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmounir, bz
bugs875289
milestone24.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 875289 - Remove .done() and allow undefined to be passed to .then() and .catch(). r=mounir, sr=bz
dom/promise/Promise.cpp
dom/promise/Promise.h
dom/promise/tests/test_promise.html
dom/promise/tests/test_resolve.html
dom/webidl/Promise.webidl
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -169,59 +169,45 @@ Promise::Reject(const GlobalObject& aGlo
   nsRefPtr<Promise> promise = new Promise(window);
 
   Optional<JS::Handle<JS::Value> > value(aCx, aValue);
   promise->mResolver->Reject(aCx, value);
   return promise.forget();
 }
 
 already_AddRefed<Promise>
-Promise::Then(AnyCallback* aResolveCallback, AnyCallback* aRejectCallback)
+Promise::Then(const Optional<OwningNonNull<AnyCallback> >& aResolveCallback,
+              const Optional<OwningNonNull<AnyCallback> >& aRejectCallback)
 {
   nsRefPtr<Promise> promise = new Promise(GetParentObject());
 
   nsRefPtr<PromiseCallback> resolveCb =
     PromiseCallback::Factory(promise->mResolver,
-                             aResolveCallback,
+                             aResolveCallback.WasPassed()
+                               ? aResolveCallback.Value().get()
+                               : nullptr,
                              PromiseCallback::Resolve);
 
   nsRefPtr<PromiseCallback> rejectCb =
     PromiseCallback::Factory(promise->mResolver,
-                             aRejectCallback,
+                             aRejectCallback.WasPassed()
+                               ? aRejectCallback.Value().get()
+                               : nullptr,
                              PromiseCallback::Reject);
 
   AppendCallbacks(resolveCb, rejectCb);
 
   return promise.forget();
 }
 
 already_AddRefed<Promise>
-Promise::Catch(AnyCallback* aRejectCallback)
-{
-  return Then(nullptr, aRejectCallback);
-}
-
-void
-Promise::Done(AnyCallback* aResolveCallback, AnyCallback* aRejectCallback)
+Promise::Catch(const Optional<OwningNonNull<AnyCallback> >& aRejectCallback)
 {
-  if (!aResolveCallback && !aRejectCallback) {
-    return;
-  }
-
-  nsRefPtr<PromiseCallback> resolveCb;
-  if (aResolveCallback) {
-    resolveCb = new SimpleWrapperPromiseCallback(this, aResolveCallback);
-  }
-
-  nsRefPtr<PromiseCallback> rejectCb;
-  if (aRejectCallback) {
-    rejectCb = new SimpleWrapperPromiseCallback(this, aRejectCallback);
-  }
-
-  AppendCallbacks(resolveCb, rejectCb);
+  Optional<OwningNonNull<AnyCallback> > resolveCb;
+  return Then(resolveCb, aRejectCallback);
 }
 
 void
 Promise::AppendCallbacks(PromiseCallback* aResolveCallback,
                          PromiseCallback* aRejectCallback)
 {
   if (aResolveCallback) {
     mResolveCallbacks.AppendElement(aResolveCallback);
--- a/dom/promise/Promise.h
+++ b/dom/promise/Promise.h
@@ -60,22 +60,22 @@ public:
   Resolve(const GlobalObject& aGlobal, JSContext* aCx,
           JS::Handle<JS::Value> aValue, ErrorResult& aRv);
 
   static already_AddRefed<Promise>
   Reject(const GlobalObject& aGlobal, JSContext* aCx,
          JS::Handle<JS::Value> aValue, ErrorResult& aRv);
 
   already_AddRefed<Promise>
-  Then(AnyCallback* aResolveCallback, AnyCallback* aRejectCallback);
+  Then(const Optional<OwningNonNull<AnyCallback> >& aResolveCallback,
+       const Optional<OwningNonNull<AnyCallback> >& aRejectCallback);
+
 
   already_AddRefed<Promise>
-  Catch(AnyCallback* aRejectCallback);
-
-  void Done(AnyCallback* aResolveCallback, AnyCallback* aRejectCallback);
+  Catch(const Optional<OwningNonNull<AnyCallback> >& aRejectCallback);
 
 private:
   enum PromiseState {
     Pending,
     Resolved,
     Rejected
   };
 
--- a/dom/promise/tests/test_promise.html
+++ b/dom/promise/tests/test_promise.html
@@ -20,58 +20,58 @@ function promiseResolve() {
   ok(Promise, "Promise object should exist");
 
   var promise = new Promise(function(resolver) {
     ok(resolver, "PromiseResolver exists");
     ok("reject" in resolver, "PromiseResolver.reject exists");
     ok("resolve" in resolver, "PromiseResolver.resolve exists");
 
     resolver.resolve(42);
-  }).done(function(what) {
-    ok(true, "Done - resolveCb has been called");
+  }).then(function(what) {
+    ok(true, "Then - resolveCb has been called");
     is(what, 42, "ResolveCb received 42");
     runTest();
   }, function() {
-    ok(false, "Done - rejectCb has been called");
+    ok(false, "Then - rejectCb has been called");
     runTest();
   });
 }
 
 function promiseReject() {
   var promise = new Promise(function(resolver) {
     resolver.reject(42);
-  }).done(function(what) {
-    ok(false, "Done - resolveCb has been called");
+  }).then(function(what) {
+    ok(false, "Then - resolveCb has been called");
     runTest();
   }, function(what) {
-    ok(true, "Done - rejectCb has been called");
+    ok(true, "Then - rejectCb has been called");
     is(what, 42, "RejectCb received 42");
     runTest();
   });
 }
 
 function promiseException() {
   var promise = new Promise(function(resolver) {
     throw 42;
-  }).done(function(what) {
-    ok(false, "Done - resolveCb has been called");
+  }).then(function(what) {
+    ok(false, "Then - resolveCb has been called");
     runTest();
   }, function(what) {
-    ok(true, "Done - rejectCb has been called");
+    ok(true, "Then - rejectCb has been called");
     is(what, 42, "RejectCb received 42");
     runTest();
   });
 }
 
 function promiseGC() {
   var resolver;
   var promise = new Promise(function(r) {
     resolver = r;
-  }).done(function(what) {
-    ok(true, "Done - promise is still alive");
+  }).then(function(what) {
+    ok(true, "Then - promise is still alive");
     runTest();
   });
 
   promise = null;
 
   SpecialPowers.gc();
   SpecialPowers.forceGC();
   SpecialPowers.forceCC();
@@ -84,65 +84,61 @@ function promiseAsync() {
   var f = new Promise(function(r) {
     is(global, "foo", "Global should be foo");
     r.resolve(42);
     is(global, "foo", "Global should still be foo");
     setTimeout(function() {
       is(global, "bar", "Global should still be bar!");
       runTest();
     }, 0);
-  }).done(function() {
+  }).then(function() {
     global = "bar";
   });
   is(global, "foo", "Global should still be foo (2)");
 }
 
-function promiseDoubleDone() {
+function promiseDoubleThen() {
   var steps = 0;
   var promise = new Promise(function(resolver) {
     resolver.resolve(42);
   });
 
-  promise.done(function(what) {
-    ok(true, "Done.resolve has been called");
+  promise.then(function(what) {
+    ok(true, "Then.resolve has been called");
     is(what, 42, "Value == 42");
     steps++;
   }, function(what) {
-    ok(false, "Done.reject has been called");
+    ok(false, "Then.reject has been called");
   });
 
-  promise.done(function(what) {
-    ok(true, "Done.resolve has been called");
-    is(steps, 1, "Done.resolve - step == 1");
+  promise.then(function(what) {
+    ok(true, "Then.resolve has been called");
+    is(steps, 1, "Then.resolve - step == 1");
     is(what, 42, "Value == 42");
     runTest();
   }, function(what) {
-    ok(false, "Done.reject has been called");
+    ok(false, "Then.reject has been called");
   });
 }
 
-function promiseDoneException() {
+function promiseThenException() {
   var promise = new Promise(function(resolver) {
     resolver.resolve(42);
   });
 
-  onErrorCb = window.onerror;
-  window.onerror = function(e) {
+  promise.then(function(what) {
+    ok(true, "Then.resolve has been called");
+    throw "booh";
+  }).catch(function(e) {
     ok(true, "window.onerror has been called!");
-    window.onerror = onErrorCb;
     runTest();
-  };
-
-  promise.done(function(what) {
-    ok(true, "Done.resolve has been called");
-    throw "booh";
   });
 }
 
-function promiseThenCatchDone() {
+function promiseThenCatchThen() {
   var promise = new Promise(function(resolver) {
     resolver.resolve(42);
   });
 
   var promise2 = promise.then(function(what) {
     ok(true, "Then.resolve has been called");
     is(what, 42, "Value == 42");
     return what + 1;
@@ -155,26 +151,26 @@ function promiseThenCatchDone() {
   promise2.then(function(what) {
     ok(true, "Then.resolve has been called");
     is(what, 43, "Value == 43");
     return what + 1;
   }, function(what) {
     ok(false, "Then.reject has been called");
   }).catch(function() {
     ok(false, "Catch has been called");
-  }).done(function(what) {
-    ok(true, "Done.resolve has been called");
+  }).then(function(what) {
+    ok(true, "Then.resolve has been called");
     is(what, 44, "Value == 44");
     runTest();
   }, function(what) {
-    ok(false, "Done.reject has been called");
+    ok(false, "Then.reject has been called");
   });
 }
 
-function promiseRejectThenCatchDone() {
+function promiseRejectThenCatchThen() {
   var promise = new Promise(function(resolver) {
     resolver.reject(42);
   });
 
   var promise2 = promise.then(function(what) {
     ok(false, "Then.resolve has been called");
   }, function(what) {
     ok(true, "Then.reject has been called");
@@ -185,102 +181,102 @@ function promiseRejectThenCatchDone() {
   isnot(promise, promise2, "These 2 promise objs are different");
 
   promise2.then(function(what) {
     ok(true, "Then.resolve has been called");
     is(what, 43, "Value == 43");
     return what+1;
   }).catch(function(what) {
     ok(false, "Catch has been called");
-  }).done(function(what) {
+  }).then(function(what) {
     ok(true, "Then.resolve has been called");
     is(what, 44, "Value == 44");
     runTest();
   });
 }
 
-function promiseRejectThenCatchDone2() {
+function promiseRejectThenCatchThen2() {
   var promise = new Promise(function(resolver) {
     resolver.reject(42);
   });
 
   promise.then(function(what) {
     ok(true, "Then.resolve has been called");
     is(what, 42, "Value == 42");
     return what+1;
   }).catch(function(what) {
     is(what, 42, "Value == 42");
     ok(true, "Catch has been called");
     return what+1;
-  }).done(function(what) {
+  }).then(function(what) {
     ok(true, "Then.resolve has been called");
     is(what, 43, "Value == 43");
     runTest();
   });
 }
 
-function promiseRejectThenCatchExceptionDone() {
+function promiseRejectThenCatchExceptionThen() {
   var promise = new Promise(function(resolver) {
     resolver.reject(42);
   });
 
   promise.then(function(what) {
     ok(false, "Then.resolve has been called");
   }, function(what) {
     ok(true, "Then.reject has been called");
     is(what, 42, "Value == 42");
     throw(what + 1);
   }).catch(function(what) {
     ok(true, "Catch has been called");
     is(what, 43, "Value == 43");
     return what + 1;
-  }).done(function(what) {
+  }).then(function(what) {
     ok(true, "Then.resolve has been called");
     is(what, 44, "Value == 44");
     runTest();
   });
 }
 
 function promiseThenCatchOrderingResolve() {
   var global = 0;
   var f = new Promise(function(r) {
     r.resolve(42);
   });
 
-  f.done(function() {
+  f.then(function() {
     f.then(function() {
       global++;
     });
     f.catch(function() {
       global++;
     });
-    f.done(function() {
+    f.then(function() {
       global++;
     });
     setTimeout(function() {
       is(global, 2, "Many steps... should return 2");
       runTest();
     }, 0);
   });
 }
 
 function promiseThenCatchOrderingReject() {
   var global = 0;
   var f = new Promise(function(r) {
     r.reject(42);
   })
 
-  f.done(function() {}, function() {
+  f.then(function() {}, function() {
     f.then(function() {
       global++;
     });
     f.catch(function() {
       global++;
     });
-    f.done(function() {}, function() {
+    f.then(function() {}, function() {
       global++;
     });
     setTimeout(function() {
       is(global, 2, "Many steps... should return 2");
       runTest();
     }, 0);
   });
 }
@@ -337,53 +333,53 @@ function promiseLoop() {
     is(value, 42, "Nested nested promise is executed and then == 42");
     runTest();
   }, function(value) {
      ok(false, "This is wrong");
   });
 }
 
 function promiseReject() {
-  var promise = Promise.reject(42).done(function(what) {
+  var promise = Promise.reject(42).then(function(what) {
     ok(false, "This should not be called");
   }, function(what) {
     is(what, 42, "Value == 42");
     runTest();
   });
 }
 
 function promiseResolve() {
-  var promise = Promise.resolve(42).done(function(what) {
+  var promise = Promise.resolve(42).then(function(what) {
     is(what, 42, "Value == 42");
     runTest();
   }, function() {
     ok(false, "This should not be called");
   });
 }
 
 function promiseResolveNestedPromise() {
   var promise = Promise.resolve(new Promise(function(r) {
     ok(true, "Nested promise is executed");
     r.resolve(42);
   }, function() {
     ok(false, "This should not be called");
-  })).done(function(what) {
+  })).then(function(what) {
     is(what, 42, "Value == 42");
     runTest();
   }, function() {
     ok(false, "This should not be called");
   });
 }
 
 var tests = [ promiseResolve, promiseReject,
               promiseException, promiseGC, promiseAsync,
-              promiseDoubleDone, promiseDoneException,
-              promiseThenCatchDone, promiseRejectThenCatchDone,
-              promiseRejectThenCatchDone2,
-              promiseRejectThenCatchExceptionDone,
+              promiseDoubleThen, promiseThenException,
+              promiseThenCatchThen, promiseRejectThenCatchThen,
+              promiseRejectThenCatchThen2,
+              promiseRejectThenCatchExceptionThen,
               promiseThenCatchOrderingResolve,
               promiseThenCatchOrderingReject,
               promiseNestedPromise, promiseNestedNestedPromise,
               promiseWrongNestedPromise, promiseLoop,
               promiseReject, promiseResolve,
               promiseResolveNestedPromise,
             ];
 
--- a/dom/promise/tests/test_resolve.html
+++ b/dom/promise/tests/test_resolve.html
@@ -43,22 +43,22 @@ function runTest() {
   }
 
   var test = tests.pop();
 
   new Promise(function(resolver) {
     resolver.resolve(test);
   }).then(function(what) {
     ok(test === what, "What is: " + what);
-  }, cbError).done(function() {
+  }, cbError).then(function() {
     new Promise(function(resolver) {
       resolver.reject(test)
     }).then(cbError, function(what) {
       ok(test === what, "What is: " + what);
-    }).done(runTest, cbError);
+    }).then(runTest, cbError);
   });
 }
 
 SimpleTest.waitForExplicitFinish();
 SpecialPowers.pushPrefEnv({"set": [["dom.promise.enabled", true]]}, runTest);
 // -->
 </script>
 </pre>
--- a/dom/webidl/Promise.webidl
+++ b/dom/webidl/Promise.webidl
@@ -3,34 +3,31 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/.
  *
  * The origin of this IDL file is
  * http://dom.spec.whatwg.org/#promises
  */
 
 interface PromiseResolver {
+  // TODO bug 875289 - void fulfill(optional any value);
   void resolve(optional any value);
   void reject(optional any value);
 };
 
 callback PromiseInit = void (PromiseResolver resolver);
 callback AnyCallback = any (optional any value);
 
 [PrefControlled, Constructor(PromiseInit init)]
 interface Promise {
-  // TODO: update this interface - bug 875289
-
+  // TODO bug 875289 - static Promise fulfill(any value);
   [Creator, Throws]
   static Promise resolve(any value); // same as any(value)
   [Creator, Throws]
   static Promise reject(any value);
 
   [Creator]
-  Promise then(optional AnyCallback? resolveCallback = null,
-               optional AnyCallback? rejectCallback = null);
+  Promise then([TreatUndefinedAs=Missing] optional AnyCallback fulfillCallback,
+               [TreatUndefinedAs=Missing] optional AnyCallback rejectCallback);
 
   [Creator]
-  Promise catch(optional AnyCallback? rejectCallback = null);
-
-  void done(optional AnyCallback? resolveCallback = null,
-            optional AnyCallback? rejectCallback = null);
+  Promise catch([TreatUndefinedAs=Missing] optional AnyCallback rejectCallback);
 };