We should be able to use the entry global here instead, I would think, and check whether it's one of our inners. That would be equivalent to what we do now, I think.

Note that the change in the condition is ok because we have the invariant that aCalledNoScript != bool(aJSCallerContext), though we never actually clearly assert that in this method (we instead assert that at least one has to be false, but it turns out they can't _both_ be false either.

(In reply to Boris Zbarsky [:bz] from comment #2) > Created attachment 8757132 [details] [diff] [review] > part 2. Remove the now-unused aJSCallerContext argument of > nsGlobalWindow::OpenInternal Patch is empty.

Comment on attachment 8757131 [details] [diff] [review] part 1. Stop using GetDefaultScopeFromJSContext in nsGlobalWindow::OpenInternal. Instead, use the entry global's outer window (if the entry global is a window), which should correspond to the stack-top JSContext in all web-visible cases right now Yeah, looks like aJSCallerContext == !aCalledNoScript

Comment on attachment 8757131 [details] [diff] [review] part 1. Stop using GetDefaultScopeFromJSContext in nsGlobalWindow::OpenInternal. Instead, use the entry global's outer window (if the entry global is a window), which should correspond to the stack-top JSContext in all web-visible cases right now hmm, or, I couldn't convince this is the same for OpenDialogOuter case, but that is chrome only, so checkForPopup is false and this code doesn't run at all, so fine.

> hmm, or, I couldn't convince this is the same for OpenDialogOuter case In the OpenDialogOuter, we pass false for aCalledNoScript. So the only question is whether aCx can be null there. The only caller of OpenDialogOuter is OpenDialog, the version with a JSContext argument. The only caller of that is binding code, so we're definitely going to have a JSContext there.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6adda4788ea8 https://hg.mozilla.org/integration/mozilla-inbound/rev/ff41a86fc3b3

(In reply to Boris Zbarsky [:bz] from comment #7) > In the OpenDialogOuter, we pass false for aCalledNoScript. So the only > question is whether aCx can be null there. The only caller of > OpenDialogOuter is OpenDialog, the version with a JSContext argument. The > only caller of that is binding code, so we're definitely going to have a > JSContext there. I was wondering whether mContext == GetScriptContextFromJSContext(aJSCallerContext) comparison is the same as entryWindow->GetOuterWindow() == this->AsOuter(), not whether cx is passed or anything.

Ah. That should be the case as long as people aren't messing up AutoEntryScript vs AutoJSAPI...

https://hg.mozilla.org/mozilla-central/rev/6adda4788ea8 https://hg.mozilla.org/mozilla-central/rev/ff41a86fc3b3