For the JS code below: Promise.resolve(333) .then(val => { log(val); }) .then(val => { log(`resolved: ${val}`); }) The second then will print "resolved: undefined" instead of "resolved: 333". This is inconsistent with our MozPromise where the 2nd then is resolved with 333 when the 1st then doesn't return a MozPromise. Since there is no easy way to pass an 'undefined' value in C++, we should just assert again it. Note this bug is a prerequisite for bug 1362910 where we should not pass the resolved value to more than one Then values.

Comment on attachment 8866202 [details] Bug 1362912. P1 - disallow promise chaining when any of the Then callbacks doesn't return a promise. https://reviewboard.mozilla.org/r/137850/#review140962 ::: xpcom/threads/MozPromise.h:441 (Diff revision 1) > > // Invoke the resolve or reject method. > RefPtr<MozPromise> result = DoResolveOrRejectInternal(aValue); > > - // If there's a completion promise, resolve it appropriately with the > - // result of the method. > + MOZ_DIAGNOSTIC_ASSERT(!mCompletionPromise || result, > + "Can't do promising chaining for a non-promise-returning method."); promising -> promise ::: xpcom/threads/MozPromise.h:765 (Diff revision 1) > + template <typename...> > operator RefPtr<MozPromise>() > { > + static_assert(SupportChaining, > + "The resolve/reject callback needs to return a RefPtr<MozPromise> " > + "in order to do promising chaining."); promising -> promise

Comment on attachment 8866203 [details] Bug 1362912. P2 - fix the callers. https://reviewboard.mozilla.org/r/137852/#review140964

Thanks for the review!

Pushed by jwwang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/54b325c535a7 P1 - disallow promise chaining when any of the Then callbacks doesn't return a promise. r=gerald https://hg.mozilla.org/integration/autoland/rev/86297713a790 P2 - fix the callers. r=gerald

https://hg.mozilla.org/mozilla-central/rev/54b325c535a7 https://hg.mozilla.org/mozilla-central/rev/86297713a790