I've ran into a problem with bug 1035206 where none of the builtin APIs that are normally available to workers, such as timeouts, (native) promises, or XHR, work properly in the presence of nested event loops. To work around this, we've decided on a solution where we are going to make timers behave properly in the presence of nested event loops in workers (see bug 757133), and then make Promise.jsm use those timers instead of the thread manager in workers (recall that the Services object isn't available in workers).

Comment on attachment 8472426 [details] [diff] [review] Patch eh?

Comment on attachment 8472426 [details] [diff] [review] Patch Sorry about that, hg blame claimed ryanvm wrote most of Promise-backend.js, so I assigned it to you without thinking :-D Assigning to past since I can't think of a better candidate. This cannot land anyway until the platform stuff lands, so there's no big rush.

Comment on attachment 8472426 [details] [diff] [review] Patch Review of attachment 8472426 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, but Paolo is a better reviewer for the promise changes and Mossop should probably look at the Services change. ::: toolkit/modules/Promise-backend.js @@ +8,5 @@ > > /** > * This implementation file is imported by the Promise.jsm module, and as a > + * special case by the worker loader. Since we do not have access to Components > + * in workers, we cannot use of Cu.import. We therefore require this file Typo: "we cannot use Cu.import"

Comment on attachment 8472426 [details] [diff] [review] Patch Review of attachment 8472426 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Eddy Bruel [:ejpbruel] from comment #3) > This cannot land anyway until the platform stuff lands, so there's no big rush. Cool. I'd appreciate if you could rebase this patch on top bug 1033406, which I expect will land within a few days. ::: toolkit/modules/Promise-backend.js @@ +29,5 @@ > > +// If we are being required as an SDK module by the worker loader, isWorker is > +// defined as true. Otherwise, isWorker is undefined. > +if (!this.isWorker) { > + this.isWorker = false; // Make sure isWorker is defined Maybe you can just set this.isWorker to false in Promise.jsm? Then you can just check "isWorker" instead of "this.isWorker" here, like you do in other cases. ::: toolkit/modules/Services.jsm @@ +78,5 @@ > ["telemetry", "@mozilla.org/base/telemetry;1", "nsITelemetry"], > ["tm", "@mozilla.org/thread-manager;1", "nsIThreadManager"], > ["urlFormatter", "@mozilla.org/toolkit/URLFormatterService;1", "nsIURLFormatter"], > ["vc", "@mozilla.org/xpcom/version-comparator;1", "nsIVersionComparator"], > + ["witness", "@mozilla.org/toolkit/finalizationwitness;1", "nsIFinalizationWitnessService"], I'd keep the lazy getter in Promise.jsm, it doesn't seem to be used elsewhere. You can place the lazy getter within an isWorker check, or make your own implementation if you don't want to import XPCOMUtils. I get you have tests elsewhere that indirectly cover the usage of the Promise backend as an SDK module, right?

I think this somehow landed as part of another patch/bug, but I don't actually know which one :-S In any case, this seems to work now, since worker debugging just landed.