This is sort of obvious, but I'm not sure it's something we directly considered. IIRC, one of the blockers for pulling object principals directly off the compartment was that some consumers asked for the principal when the really wanted the URI. CPG mostly solved this. But inner window reuse can still cause us to alter the principal of a window to something different-but-same-origin without altering the compartment. We could add an API to update the compartment principal, or we could just declare that we don't care and fix any consumers that get broken by it. bz, what do you think?

I would slightly prefer the former, I think... ccing some usual culprits.

I think I'd prefer the former here too.

So, there are actually two issues here: 1 - Reusing a chrome window for content via the semantics of WouldReuseInnerWindow. 2 - Switching between same-origin principals with document.open. We could theoretically take care of both in nsGlobalWindow::SetNewDocument, but unfortunately in nsHTMLDocument::Open we call SetNewDocument _before_ calling Reset. Rather than sticking my hand in that pile of rattlesnakes, bz and I decided it would be best to just add an explicit notification method to nsGlobalWindow that we can call when we alter the document principal.

Here's the JSAPI part. I'm hoping luke can verify that there's nowhere in the js engine that depends on principals not changing.

And here are the gecko bits. Flagging bz.

Comment on attachment 633165 [details] [diff] [review] Part 2 - Update compartment principals on inner window reuse and on document.write. v1 >+++ b/content/base/src/nsDocument.cpp >+ nsCOMPtr<nsPIDOMWindow> win = GetWindow(); How about: nsPIDOMWindow* win = GetInnerWindow(); ? >+++ b/dom/base/nsGlobalWindow.cpp >+ JS_SetCompartmentPrincipals(js::GetObjectCompartment(currentInner->mJSObject), >+ nsJSPrincipals::get(aDocument->NodePrincipal())); currentInner->RefreshCompartmentPrincipal(), unless there's a strong reason not to. r=me with those.

(In reply to Boris Zbarsky (:bz) from comment #6) > currentInner->RefreshCompartmentPrincipal(), unless there's a strong reason > not to. Doesn't work, because mDoc hasn't been switched over yet. :-( I considered adding an optional parameter to allow the method to be shared, but given that the method was a one-liner (and that the method needed to be declared in two places) it didn't seem worth it.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=84ab2afa0b32

> Doesn't work, because mDoc hasn't been switched over yet. :-( Even on the curentInner? If so, please clearly document this so someone doesn't "fix" it...

(In reply to Boris Zbarsky (:bz) from comment #9) > Even on the curentInner? Yeah. AFAICT that doesn't happen until here: http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#2057 > If so, please clearly document this so someone doesn't "fix" it... ok.

Landed to m-i: http://hg.mozilla.org/integration/mozilla-inbound/rev/2bcf136fc64c http://hg.mozilla.org/integration/mozilla-inbound/rev/4176490cd0d2

https://hg.mozilla.org/mozilla-central/rev/4176490cd0d2 https://hg.mozilla.org/mozilla-central/rev/2bcf136fc64c

Comment on attachment 633165 [details] [diff] [review] Part 2 - Update compartment principals on inner window reuse and on document.write. v1 This fixes the crash over in bug 760745, which is being tracked for aurora. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 739825 User impact if declined: Potential security bugs Testing completed (on m-c, etc.): On m-c for ~1 week, no problems. Risk to taking this patch (and alternatives if risky): Not very risky. String or UUID changes made by this patch: None

Beta appears as if it's affected as well. Should we consider uplift?

(In reply to Alex Keybl [:akeybl] from comment #14) > Beta appears as if it's affected as well. Should we consider uplift? I don't think so. The fix here doesn't make any sense pre-CPG.

Landed to m-a: http://hg.mozilla.org/releases/mozilla-aurora/rev/34347ff5cd76 http://hg.mozilla.org/releases/mozilla-aurora/rev/9f9876174f1b