Patching Services.jsm from the outside is just bad form. Let's move those to Services.jsm proper. (Note also that bug 732982 is going to define the DOMRequestService there.) Also, while we're making that modification, let's not use two-letter abbreviations for new service definitions. It is entirely not obvious what "Services.rs" means (same goes for Services.tm, Services.ww, etc... I should really file a bug to fix those.)

Comment on attachment 612744 [details] [diff] [review] v1 r=me on the Services.jsm change (but also add the new entries to the browser_Services.js test).

Comment on attachment 612744 [details] [diff] [review] v1 Review of attachment 612744 [details] [diff] [review]: ----------------------------------------------------------------- We must also update https://developer.mozilla.org/en/JavaScript/Code_modules/Services.jsm

Try run for 53d6f1da2815 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=53d6f1da2815 Results (out of 296 total builds): exception: 2 success: 208 warnings: 86 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/pweitershausen@mozilla.com-53d6f1da2815

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/e46fd6a8cff5 I can't view the try results, and I figured the 2 exceptions were just random oranges. Apparently not. For posterity, here's the inbound push that turned orange: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d7a4dfac3acf

So the problems are leaks. I suspect that we're now somehow leaking Services.ppmm and Services.cpmm (at shutdown, I bet). Am I missing some sort of clean up here? These can't be a special case... Or maybe they are?

I split the patch up and -- for now -- left out the part where I updated all uses of cpmm/ppmm to use Services.{cpmm|ppmm}, suspecting those changes introduced the leaks. I'll deal with those in a follow-up, I just want to get this bug out the door. Here's a try build for just (a) not patching the DOMRequestService into Services.jsm and (b) defining cpmm/ppmm in Services.jsm: https://tbpl.mozilla.org/?tree=Try&rev=162558c57a6b

We probably shouldn't add cpmm/ppmm to Services.jsm if any use of them introduces leaks.

(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #9) > We probably shouldn't add cpmm/ppmm to Services.jsm if any use of them > introduces leaks. That's assuming that there isn't a bug somewhere that causes the leak. But you make a good point, let's be cautious. I'll just commit the "don't patch DOMRequest into Service.jsm" part.

Try run for 162558c57a6b is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=162558c57a6b Results (out of 290 total builds): exception: 3 success: 254 warnings: 32 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/pweitershausen@mozilla.com-162558c57a6b