Bug 875289 - Remove .done() and allow undefined to be passed to .then() and .catch(), r=mounir
☠☠ backed out by 2266ccdba6d7 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Thu, 20 Jun 2013 10:49:47 +0200
changeset 147302 80732a2b842203580374a0f8aba841c4b366ca2b
parent 147301 96036434bb78b0d8418d1fd4b01605862a8fe9c3
child 147303 46e6080dcdd4c661780cb7f10f8e3f0448eeba6f
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmounir
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
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()
+                               : nullptr,
                              PromiseCallback::Resolve);
 
   nsRefPtr<PromiseCallback> rejectCb =
     PromiseCallback::Factory(promise->mResolver,
-                             aRejectCallback,
+                             aRejectCallback.WasPassed()
+                               ? &aRejectCallback.Value()
+                               : 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);
 };