Due to limitations of SpiderMonkey we cannot implement promises there for now (see bug 911216). We should update our implementation in DOM to match https://github.com/domenic/promises-unwrapping as closely as possible and enable it by default now TC39 has declared consensus.

Uh, how is ehsan the right person to make this call?

Also, why do we want to switch to disabling in chrome if the pref is disabled?

Comment on attachment 8341024 [details] [diff] [review] enabled.patch (In reply to :Ms2ger from comment #2) > Uh, how is ehsan the right person to make this call? mmm good question. He just reviewed a lot of my promise/datastore patches, so in automatic I selected him. Can you? :)

What is the latest status of the spec and our implementation's conformance to it?

https://github.com/domenic/promises-unwrapping is stable as far as what we can implement in DOM is concerned (subclassing details are still twiddled). However, we have not made an attempt to do that yet. I believe our implementation is still based on the old DOM promises with only the constructor changed around.

(In reply to Anne (:annevk) from comment #7) > https://github.com/domenic/promises-unwrapping is stable as far as what we > can implement in DOM is concerned (subclassing details are still twiddled). > However, we have not made an attempt to do that yet. I believe our > implementation is still based on the old DOM promises with only the > constructor changed around. That is correct. I'm not sure how we'd support subclassing and the internal methods in a DOM implementation. We are spec compliant in the behaviour of the constructor, Promise.resolve()/reject() and then()/catch(). Bug 939332 implements the rest of the spec. I think that covers 90% of the use cases for Promise. In fact, I don't even understand the reason subclassing and so on is supported. I'd appreciate some pointers to use cases.

> I'm not sure how we'd support subclassing and the internal methods in a DOM > implementation. As in, allowing web pages to subclass our Promise impl? We could in fact support that with some cooperation from the JS engine; it just needs to create the right sort of object. We need to solve this for DOM object in general, imo.

I don't think we need to support subclassing in order to ship promises. Subclassing is an ES6 feature that we do not support overall. We should aim for supporting promises to the extent they can be implemented in ES5. http://domenic.me/aplus-tests-against-the-browser/ has tests of which we fail most at the moment.

Yep, most of those have to do with the ES spec having totally different edge-case argument semantics from the WebIDL version.

(In reply to comment #11) > Yep, most of those have to do with the ES spec having totally different > edge-case argument semantics from the WebIDL version. Hrm, so what's the plan to reconcile those differences?

Presumably moving away from doing any webidl argument stuff for our worker code. That is, declare all arguments as "optional any", and do all the argument processing using manual JSAPI in the C++. I should really fix it so that "any" implies "optional".

Here is a page that allows you to run most of the existing test suite in your browser: http://domenic.me/aplus-tests-against-the-browser/ Firefox Aurora is at 84 tests passing out of 879 total. I am working on more tests beyond just the Promises/A+ ones (which test `then`, `resolve`, and `reject`); see more details at https://github.com/domenic/aplus-tests-against-the-browser/tree/gh-pages#readme

Oh goodness, Anne already posted those and I didn't scroll up, sorry :(

Comment on attachment 8341061 [details] [diff] [review] enabled.patch Review of attachment 8341061 [details] [diff] [review]: ----------------------------------------------------------------- Is this still in my queue?

I think we need to get more up-to-date with the final spec before we can flip this on. Latest word from dherman is that he thinks that we can get rid of the hash table which would make the spec much simpler. Need to get that confirmed with the others working on the spec though. However he also recommended that we implement some of the "static" functions, like .cast(), .all() etc, before flipping it on. Dave, can you let us know when the relevant pieces have been added to the ecma spec?

Dave, see comment 17

> Dave, can you let us know when the relevant pieces have been added to the ecma spec? They are already added (and have been for over two months); the spec is API complete [1]. > Latest word from dherman is that he thinks that we can get rid of the hash table which would make the spec much simpler. Need to get that confirmed with the others working on the spec though. Not sure what you mean by "the hash table" but if it's the thenable coercions weak map, then that was removed a while ago [2]. We want to add something similar, where a promise caches its fulfillment value to prevent against changes [3], but this is proving annoying due to all the extra internal complexity necessary to support the monadic stuff. I will try to do so tonight though. Relatedly, that fulfillment-value caching is the last remaining change before the spec is feature complete [4]. What remains after that are simple non-normative editorial issues. More worryingly, however, is the fact that the promises in Firefox don't implement promise semantics, as can be seen from bug 945766 and comment 10. Chrome's promises, notably, do pass these tests. [1]: https://github.com/domenic/promises-unwrapping/issues/milestones?state=closed [2]: https://github.com/domenic/promises-unwrapping/commit/65a5e6295534abda714dced18fe4d8ab8ff9137a [3]: https://github.com/domenic/promises-unwrapping/issues/79 [4]: https://github.com/domenic/promises-unwrapping/issues/milestones?state=open

Isn't this done already?

(In reply to Mayhem~ from comment #20) > Isn't this done already? Not for web pages. Only for certified apps and chrome workers.

I think we are ready to flip this on and ship. Will upload a rebased patch right now. Mossop, will enabling this affect addons that use core/promise.js from the addon sdk?

Jordan?

The sdk's `core/promise.js` is its own implementation and this would not affect anyone using it

Jonas, is it ok to enable this in workers too? I mean, Promises are a JS thing and so will be present in workers, but is it observable for workers to know whether they are running DOM Promise or JS Promise and is that a problem?

Should we not remove Promise::EnabledForScope altogether? Seems this should always be available, including in workers.

Get rid of dom.promise.enabled.

Comment on attachment 8367493 [details] [diff] [review] Enable DOM Promises. sr=me

Comment on attachment 8367501 [details] [diff] [review] Remove all mention of dom.promise.enabled from other tests. r=me

Yes, go ahead and enable this both in Windows and in Workers! Woot!

Comment on attachment 8367501 [details] [diff] [review] Remove all mention of dom.promise.enabled from other tests. Review of attachment 8367501 [details] [diff] [review]: ----------------------------------------------------------------- lgtm.

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1d7c2025bb56 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6db8b08a3aea

Backed out for B2G test_interfaces.html failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/7e4ed3b79f4d https://tbpl.mozilla.org/php/getParsedLog.php?id=33824940&tree=Mozilla-Inbound

remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fc70e3e4a241 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/50aff7651394 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f33534019590

https://hg.mozilla.org/mozilla-central/rev/fc70e3e4a241 https://hg.mozilla.org/mozilla-central/rev/50aff7651394 https://hg.mozilla.org/mozilla-central/rev/f33534019590

Is there any need of manual QA for this feature?

(In reply to Ioana Budnar, QA [:ioana] from comment #38) > Is there any need of manual QA for this feature? Nope

I updated the compat. table at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise and saw that this was already mentioned on https://developer.mozilla.org/en-US/Firefox/Releases/29