We should warn, at least for one release, that the Components object is going away.

I think telemetry would be useful here too.

A lot of this stuff can be simplified now, and we can stop using the deprecated APIs.

We want this right now so that we can avoid the scary warning when content Components access happens in XBL (which we're allowing going forward). This patch would be overkill just for that, but I also have plans to introduce a SOW-like protection of the Components wrapper filtering policy. I can't just do the filename hack for that though, because real- world XBL filenames might be all over the place. So let's just be safe here.

Comment on attachment 665943 [details] [diff] [review] Part 1 - Clean up isSystemOnlyAccessPermitted. v1 This is unspeakably nicer.

Comment on attachment 665947 [details] [diff] [review] Part 3 - Warn about content access to |Components|. v1 Review of attachment 665947 [details] [diff] [review]: ----------------------------------------------------------------- Wouldn't it be better to do this when we access the components property directly instead of when we get something off of the components object?

(In reply to Blake Kaplan (:mrbkap) from comment #7) > Wouldn't it be better to do this when we access the components property > directly instead of when we get something off of the components object? But how would we do that? That would require the Components object to become accessible via some kind of getter, right? I'm just kind of afraid of breaking something by defining it as a getter rather than a straight property...

Would it be possible to use one of SpiderMonkey's value getters (i.e. a getter without JSPROP_GETTER)? They are invisible from JS but run code on the C++ side. We'd only do it for content Components, clearly, reducing the possible damage.

Comment on attachment 666496 [details] [diff] [review] Part 3 - Warn about content access to |Components|. v2 Review of attachment 666496 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCComponents.cpp @@ +4778,5 @@ > + do_QueryInterface(nsJSUtils::GetStaticScriptGlobal(cx, obj)); > + if (win) { > + nsCOMPtr<nsIDocument> doc = > + do_QueryInterface(win->GetExtantDocument()); > + if (doc) { Nit: no braces around single-line if in XPConnect.

Thanks Blake. Pushing to try: https://tbpl.mozilla.org/?tree=Try&rev=6f6b441f1cbb

My assertion that obj was its global was too loose, since |obj| might be an outer window. Relaxed the check ever-so-slightly and repushed the borked runs: https://tbpl.mozilla.org/?tree=Try&rev=f189c463ea57

Green. remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/00d03da9049a remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fb8bb9277152 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/43de19945cb1 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/aeda4978c97c remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/223f29a9e730

Comment on attachment 666496 [details] [diff] [review] Part 3 - Warn about content access to |Components|. v2 Review of attachment 666496 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCComponents.cpp @@ +4777,5 @@ > + nsCOMPtr<nsPIDOMWindow> win = > + do_QueryInterface(nsJSUtils::GetStaticScriptGlobal(cx, obj)); > + if (win) { > + nsCOMPtr<nsIDocument> doc = > + do_QueryInterface(win->GetExtantDocument()); For future reference, this could use GetExtantDoc()

https://hg.mozilla.org/mozilla-central/rev/00d03da9049a https://hg.mozilla.org/mozilla-central/rev/fb8bb9277152 https://hg.mozilla.org/mozilla-central/rev/43de19945cb1 https://hg.mozilla.org/mozilla-central/rev/aeda4978c97c https://hg.mozilla.org/mozilla-central/rev/223f29a9e730

Comment on attachment 665945 [details] [diff] [review] Part 2 - Introduce an explicit mechanism for determining if a script is from XBL. v1 This is needed for bug 810082.