Cu.importGlobalProperties is used like this: Cu.importGlobalProperties(["crypto", "TextEncoder"]); Now if Cu.importGlobalProperties is a cross-realm function, we can end up importing the properties on the wrong global because we switch to that realm when invoking the native function (this is similar to bug 1512417). Our options are: (1) Require passing |this| as argument to importGlobalProperties. Doable but there are hundreds of callers... (2) Do something like this in nsXPCComponents_Utils::ImportGlobalProperties: RootedObject global(cx, JS::GetScriptedCallerGlobal(cx)); MOZ_ASSERT(global); MOZ_ASSERT(js::GetObjectCompartment(global) == js::GetContextCompartment(cx)); JSAutoRealm ar(cx, global); I tried (2) and it fixes the test failure I was debugging.

Maybe mozJSComponentLoader::FindTargetObject should do something similar. We are already using js::GetJSMEnvironmentOfScriptedCaller there so it seems JS::GetScriptedCallerGlobal would fit in nicely.

(2) makes a lot of sense to me.

Hopefully this will fix the remaining oranges I didn't track down yet. Fingers crossed...

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #2) > (2) makes a lot of sense to me. Agreed.

+1. We could also use IncumbentGlobal, but ScriptedCallerGlobal more directly does what we want.

With same-compartment-realms enabled we can call a cross-realm Cu.importGlobalProperties and we ended up defining properties on the wrong global.

We have a few places where C++ calls ChromeUtils::Import directly. I fixed these to pass the target object directly instead of an empty Optional<>. Depends on D14179

Hm part 2 doesn't work because the ChromeUtils.import call here can end up going through a CCW: https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/testing/mochitest/chrome-harness.js#7 Then scriptedCallerGlobal->compartment != cx->compartment and we fail the AssertSameCompartment I added. cx->global is a Window and scriptedCallerGlobal is a Sandbox. Maybe mozJSComponentLoader::ImportInto should emplace a Maybe<JSAutoRealm> after calling FindTargetObject? It feels like I might be opening a can of worms but using the scripted caller's global is the behavior we want I think...

Also, any idea how this happens to work right now? We define things on the wrong global I think, but maybe I'm missing something.

(In reply to Jan de Mooij [:jandem] from comment #9) > Also, any idea how this happens to work right now? We define things on the > wrong global I think, but maybe I'm missing something. The only time that happens is when we load the script into a sandbox whose prototype is the global ChromeUtils.import comes from. In that case, defining the variables on the wrong global will actually still work. But in that case, they also happen to already be defined. We should perhaps just return null in that case rather than asserting, and have that script explicitly pass `this` to ChromeUtils.import.

Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/63993693c624 part 1 - Use the scripted caller's global in Cu.importGlobalProperties. r=kmag https://hg.mozilla.org/integration/autoland/rev/c38131baf660 part 2 - Use the scripted caller's global in mozJSComponentLoader::FindTargetObject. r=kmag

https://hg.mozilla.org/mozilla-central/rev/63993693c624 https://hg.mozilla.org/mozilla-central/rev/c38131baf660