@bz: I tried to find a way to test this, but, because I know little about how to create tests that would be able to invoke the JSAPI functions *and* create Xray'd objects, I wasn't able to. Maybe you can give me a hint for how to go about this?

Which JSAPI functions?

Comment on attachment 8785353 [details] Bug 1298414 - Properly handle resolve/reject callbacks on xray'd promises. ,f?bz https://reviewboard.mozilla.org/r/74588/#review73826 Looks sane to me.

(In reply to Boris Zbarsky [:bz] from comment #3) > Which JSAPI functions? Sorry, that was pretty unclear. I mean JS::ResolvePromise and JS::RejectPromise, which call PromiseObject::resolve/reject.

Ah, ok. I think your best bet is to add something to TestingFunctions that will do the ResolvePromise/RejectPromise call. Then in a chrome mochitest where you can get access to Promise Xrays (e.g. dom/promise/tests/test_promise_xrays.html is a perfectly reasonable place for this) you can do: var obj = Components.utils.getJSTestingFunctions(); obj.whateverYourNewFunctionIs(whateverArgs);

Testing this turned out to be useful: rejecting xray'd promises didn't actually worked. I failed to figure out how to re-request a review in mozreview, so setting needinfo instead.

Comment on attachment 8785353 [details] Bug 1298414 - Properly handle resolve/reject callbacks on xray'd promises. ,f?bz https://reviewboard.mozilla.org/r/74588/#review74592 ::: dom/promise/tests/test_promise_xrays.html:237 (Diff revision 2) > +function testResolve4() { > + var p = new win.Promise((res, rej) => {}); > + Components.utils.getJSTestingFunctions().resolvePromise(p, 42); > + p.then( > + function(arg) { > + is(arg, 42, "TestingFunctions resolvePromise with chrome Promise should work"); There is no chrome Promise here. For testResolve3 the Promise used as the resolution value came from chrome, which is why the strings were what they were. Over here, it's more like "Resolving an Xray to a promise with TestingFunctions resolvePromise should work" and similar for the "should not fail" case. And similar in testReject2. ::: js/src/builtin/Promise.cpp:964 (Diff revision 2) > { > if (state() != JS::PromiseState::Pending) > return true; > > RootedValue funVal(cx, this->getReservedSlot(PROMISE_RESOLVE_FUNCTION_SLOT)); > - // TODO: ensure that this holds for xray'd promises. (It probably doesn't) > + MOZ_ASSERT(IsCallable(funVal)); So funVal can't end up being a dead object wrapper? r=me

I don't know how to re-review either, but r=me on the update. Just adding ni? so this makes more noise in bugmail ;)

Pushed by tschneidereit@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2cf0ccb1a85c Properly handle resolve/reject callbacks on xray'd promises. r=bz,efaust