Can we make XPConnect ignore QueryInterface in content JS objects, as suggested by roc in bug 503699 comment 4? This would be a general fix for bugs where QI is weird (bug 503644, bug 503697) or has side effects (bug 503699). Programmers expect QI to have no side-effects apart from perhaps creating a tearoff. It's hard to reason about code where QI can have arbitrary side effects.

instanceof will still work, I assume, which flattens interfaces. Does it rely on QI?

Nochum, that's unrelated, this is QI *implemented by* content script.

So, in that case content objects would trivially support all requested interfaces? Or trivially *not* support any interfaces? Either way, it's easy to do this. I do note that if we allow them to implement all interfaces, then any code touching a maybe-content-JS-C++-thingi will still have to be careful for misbehavior...

Ideally, XPConnect would behave as if content hadn't mucked with QI. For example, document.body should be able to QI to nsIDOMNode but not nsIDOMSVGAnimationElement. Is that significantly harder than claiming to support no interfaces? Claiming to support all interfaces sounds scary, I don't know why we'd want to do that.

For XPCWrappedNatives, I'd be perfectly happy if QI did absolutely nothing magic. The issue is vanilla JSObjects (XPCWrappedJS); those need to be able to QI to the interface they were created for (esp. nsIDOMEventListener). I think content objects should claim support for as few interfaces as we can get away with...

Before I check this in, I need to go through and make sure that we actually make all components' global objects STOBJ_IS_SYSTEM. Otherwise, this should be good to go.

Hmm. Could this cause issues for cases when people load chrome documents in content docshells? In those cases the global is not system, right? Not that they should be doing it, but.... I'm looking at about:config here, say. :(

Note that I'm all in favor of causing problems for such (ab)users. Maybe it'll finally make the tabbrowser folks get off their butts and fix things...

Comment on attachment 389035 [details] [diff] [review] Proposed fix In that case, I'll call GetObjectPrincipal before bailing. Hopefully, that'll let chrome code run without any sort of bad perf hit and still let the case bz points out in comment 7 still work.

The chrome mochitests test both the STOBJ_IS_SYSTEM codepath and the codepath bz mentioned in comment 7.

Comment on attachment 390127 [details] [diff] [review] Updated to comments r=me, but I'd rather the test explicitly opened both its windows, so we don't depend on this "chrome tests run in a content docshell" thing that I think is a bug...

Note also that UniversalXPConnect code will still fail here; do we want to be testing for that instead of system principal? Might not be worth worrying about.

(In reply to comment #11) > r=me, but I'd rather the test explicitly opened both its windows, so we don't Would you mind if the first test is an iframe instead of a window? (In reply to comment #12) > Note also that UniversalXPConnect code will still fail here; do we want to be > testing for that instead of system principal? Might not be worth worrying > about. There's no way to do that test given our current setup. I'm also not sure what it would mean: |new function() { enableprivilege("UniversalXPConnect") }|? |enableprivielege("UniversalXPConnect"); new Object()|? Either way, by the time this code runs, it's likely that the privilege isn't enabled any more.

> Would you mind if the first test is an iframe instead of a window? With type="content"? Not at all; sounds great.

http://hg.mozilla.org/mozilla-central/rev/94fc37d58985 ... using the same file for both the dialog and the frame was a little ugly, but not too bad.

This is needed for 1.9.1 to fix blocker bug 504021.

Comment on attachment 390127 [details] [diff] [review] Updated to comments This applies cleanly to 1.9.1 and passes mochitest locally.

Comment on attachment 390127 [details] [diff] [review] Updated to comments Approved for 1.9.1.7, a=dveditz for release-drivers