I don't actually know if this patch is worth checking in. It's nice to know that it works (as far as I can tell, it means that we never actually use the outer window's global object before SetNewDocument) but it leaves the context half-initialized from the time between nsGlobalWindow::EnsureScriptEnvironment and nsGlobalWindow::SetNewDocument, and I'm not sure if that's the right way to go. One other thing I noticed: we call nsJSContext::GetGlobal twice before we give it a global: both times pushing the context onto the context stack. Because we never actually try to run code on the global object, I think it's OK to return null there. The one thing that this patch does that I actually really like (and if we decide we don't want the full patch, I'll write this up separately) is to split nsJSContext::InitContext into separate functions depending on what situation we're in (first creating it, initializing the outer window, or setting up the outer window with its inner window).

Drive-by comment: >+ /** >+ * Prepares this context for use with the current inner window for the >+ * context's global object. This must be called after InitOuterWindow. >+ */ >+ virtual nsresult InitOuterWindow() = 0; The last word of the comment is puzzling. This looks uncontroversial, and bug 563106 now depends on it. Can we get this reviewed and in?

Comment on attachment 454974 [details] [diff] [review] patch Looks good to me.

Blake, what's the status of this patch? Is it ready for review?

This patch is more conservative than attachment 454974 [details] [diff] [review] in that it still creates the outer object way before the inner one. I'll have to address that in the patch that makes things so that actually matters. I just kicked this off to the try server, but I'm expecting a green result, so asking for review.

http://hg.mozilla.org/mozilla-central/rev/8fdc06de731f 2.1 --- a/content/xul/document/src/nsXULPrototypeDocument.cpp 2.2 +++ b/content/xul/document/src/nsXULPrototypeDocument.cpp 2.3 @@ -684,7 +684,7 @@ nsXULPDGlobalObject::SetScriptContext(PR 2.4 aScriptContext->WillInitializeContext(); 2.5 // NOTE: We init this context with a NULL global - this is subtly 2.6 // different than nsGlobalWindow which passes 'this' 2.7 - rv = aScriptContext->InitContext(nsnull); Would have been nice if the comment had been fixed.