I found an instance of test code relying on per-compartment XPCWNs. In XPCShell tests, for various reasons tests want to shim in their own implementation of nsIXULAppInfo (and a few similar interfaces). This is done by calling updateAppInfo in AppInfo.jsm, which registers a factory for "@mozilla.org/xre/app-info;1". This is accessed, for instance, via a lazy getter in Services.jsm. However, whenever you call Components.classes["@mozilla.org/xre/app-info;1"] I believe it looks up what the CID is mapped to at that very moment, and stores that on the XPCWN object for the compartment (in nsXPCComponents_Classes::Resolve()). If you then call updateAppInfo, then call Cc again later, you'll still get the old app info. You can trigger this same effect even without JSM global sharing by adding this line to the top of Services.jsm: let whatever = Cc["@mozilla.org/xre/app-info;1"]; This locks in the value for the Services.jsm compartment before the test has a chance to register its own app info. With global sharing, this problem is triggered even without that change, because somebody else in another jsm (which is now in the same compartment) has done the same Cc lookup. Ironically, one such place is in AppInfo.jsm itself. One test that breaks with this is: security/manager/ssl/tests/unit/test_cert_blocklist.js but I think there are many more.

One way to fix this is to avoid the call to Cc["@mozilla.org/xre/app-info;1"] at the top of AppInfo.jsm. This is only used to look up platformBuildID. This may be too fragile to be a real solution: if one test uses Services.appinfo without updateAppInfo, and runs before another which does use updateAppInfo, then the latter will break.

Nathan, do you have any ideas here? I'm not sure if there already exist better ways to shim XPCOM components. Maybe we could always shim app-info in XPCShell tests, which seems to be the only place this is used. Of course, this issue may come up again with other components.

The object we add to the Cc object is a nsJSCID. Another approach might be to explicitly allow it to recompute what CID it maps to. Right now, that's done in nsJSCID::NewID(), but I can't think of any particular reason to not just allow you to call it again later. IIRC, using the shimmed app info breaks some other code, but maybe that needs to be fixed anyways.

There's actually an Initialize method on nsJSID that does what I need. However, it was removed from access to script as part of removing same compartment security wrappers. I think I can just add a method that calls into it on Cc (because Cc is only available to higher privileged code anyways) and not cause problems.

(In reply to Andrew McCreight [:mccr8] from comment #1) > One way to fix this is to avoid the call to > Cc["@mozilla.org/xre/app-info;1"] at the top of AppInfo.jsm. This is only > used to look up platformBuildID. This may be too fragile to be a real > solution: if one test uses Services.appinfo without updateAppInfo, and runs > before another which does use updateAppInfo, then the latter will break. There are some places I know of where we currently access the app-info service directly to avoid locking in the Services.appinfo value before AppInfo.jsm gets a chance to shim it. So I don't think we have to worry about breaking code that access Services.appinfo, but I don't think we'll be able to get away with trying to avoid touching Cc[appInfo] anywhere before it can be shimmed.

These patches add this to AppInfo.jsm: Cc.initialize(Cc[cid], cid); This recomputes the value on the CID value that Cc maps cid to. We still end up using the shim AppInfo.jsm, which doesn't define many of the same values. Oddly enough that doesn't seem to cause any test failures, just a fair amount of console spam. I'm a little alarmed by that. I have a patch to add more of the standard AppInfo properties to the shim (by just iterating over the properties on the original) which may be a good idea.

Comment on attachment 8899621 [details] Bug 1366896, part 1 - Factor out initialize code and make initialize work with CIDs. https://reviewboard.mozilla.org/r/170924/#review176216 ::: js/xpconnect/src/XPCJSID.cpp:614 (Diff revision 1) > NS_ERROR("no string"); > return nullptr; > } > > RefPtr<nsJSCID> idObj = new nsJSCID(); > - if (str[0] == '{') { > + if (NS_FAILED(idObj->Initialize(str))) You can use NS_ENSURE_SUCCESS here if you want to get rid of the |if|.

Comment on attachment 8899622 [details] Bug 1366896, part 2 - Add and use Cc.Initialize method. https://reviewboard.mozilla.org/r/170926/#review176218

Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/08aa23aacd83 part 1 - Factor out initialize code and make initialize work with CIDs. r=krizsa https://hg.mozilla.org/integration/autoland/rev/ff26fa1ed97d part 2 - Add and use Cc.Initialize method. r=krizsa

https://hg.mozilla.org/mozilla-central/rev/08aa23aacd83 https://hg.mozilla.org/mozilla-central/rev/ff26fa1ed97d