The drawback is that now if you try to do "foo.QueryInterface(SpecialPowers.Ci.nsIFoo)" where "foo" is not a SpecialPowers-wrapped object, that will fail. That seems ok to me, though. Consumers can either do "foo.QueryInterace(Ci.nsIFoo)" if they're in chrome or "SpecialPowers.wrap(foo).QueryInterface(SpecialPowers.Ci.nsIFoo)" if they're not. In practice, the only non-chrome cases involve XBL, and I'm going to end up forcing those to use SpecialPowers.wrap() by making QueryInterface chromeonly (see bug 1448397). The benefit is that this will allow me to fix bug 1389585 and bug 1389581.

Comment on attachment 8961843 [details] [diff] [review] Stop returning unwrapped Components.interfaces from SpecialPowers.Ci Review of attachment 8961843 [details] [diff] [review]: ----------------------------------------------------------------- s/Components.utils/Components.interfaces/ in attachment and bug summary please :) I was pretty confused until I read through the patch. ::: testing/specialpowers/content/specialpowersAPI.js @@ +237,5 @@ > + // target. We only do this when the target doesn't have its own > + // Symbol.hasInstance already. Once we get rid of JS engine class > + // instance hooks (bug 1448218) and always use Symbol.hasInstance, we can > + // remove this bit (bug 1448400). > + return wrapIfUnwrapped(specialPowersHasInstance); Nit: Can just be wrapPrivileged(specialPowersHasInstance) @@ +278,5 @@ > + // our target. We only do this when the target doesn't have its own > + // Symbol.hasInstance already. Once we get rid of JS engine class > + // instance hooks (bug 1448218) and always use Symbol.hasInstance, we > + // can remove this bit (bug 1448400). > + return { value: wrapIfUnwrapped(specialPowersHasInstance), writeable: true, wrapPrivileged here, too. ::: toolkit/components/url-classifier/tests/mochitest/test_donottrack.html @@ -14,5 @@ > > <script class="testbody" type="text/javascript"> > > -var Cc = SpecialPowers.Cc; > -var Ci = SpecialPowers.Ci; I wonder if we should have an ESLint rule to stop people from doing things like this in chrome/browser tests...

> s/Components.utils/Components.interfaces/ in attachment and bug summary please Yikes, nice catch. > Nit: Can just be wrapPrivileged(specialPowersHasInstance) Will do. > I wonder if we should have an ESLint rule to stop people from doing things like this in chrome/browser tests... Might be nice. On the other hand, it will fail in pretty obvious ways with this patch, so may not be worth it.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9945898d46e1 Stop returning unwrapped Components.interfaces from SpecialPowers.Ci. r=kmag

This is a chrome test, so it doesn't need to play SpecialPowers games.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/282b7f33195e followup to fix Windows-only test. r=kmag pending

https://hg.mozilla.org/mozilla-central/rev/9945898d46e1 https://hg.mozilla.org/mozilla-central/rev/282b7f33195e