We'll soon be able to add a sameCompartmentAs option (similar to sameZoneAs) to newGlobal in the JS shell, so you can do this to create a same-compartment realm: newGlobal({sameCompartmentAs: this}); (This should probably be a no-op initially with --fuzzing-safe) I think we should also: * Add a testing function that calls JS::GetScriptedCallerGlobal. It's nice to expose that API to the shell anyway and it lets us easily test realm entering for scripted functions. * Add a testing functions that returns the global for a (non-wrapper) JSObject. Then we can use this to check objects are allocated in the correct realm.

ZoneSpecifier => CompartmentSpecifier, make it an enum class, rename the values. Then we can do options.setExistingCompartment(..) in the JS shell for newGlobal({sameCompartmentAs: this}). This option is a no-op for now if --fuzzing-safe is used. I also added a basic.js jit-test in a new jit-test/tests/realms/ directory. I verified with some logging we indeed have a compartment with 3 realms when I run it.

This also adds a simple test we can extend later to test realm switching for same-compartment scripted calls.

objectGlobal(obj) returns null if the object is a wrapper. This is sweet because now we can use this function in the realms/basic.js test and the test will now fail if same-compartment realms are disabled or broken (this happens with --fuzzing-safe, for instance).

Comment on attachment 8983717 [details] [diff] [review] Part 1 - Refactor ZoneSpecifier and add a sameCompartmentAs option to newGlobal in the shell Review of attachment 8983717 [details] [diff] [review]: ----------------------------------------------------------------- Nice and explicit. ::: js/src/gc/GC.cpp @@ +8057,5 @@ > return nullptr; > } > > + if (compHolder) { > + if (!zone->compartments().append(comp)) { If the below gc.zones().append(zone) fails, I think this would leave a dangling JSCompartment* in the zone. Really, the existing rt->gc.zones().append(zone) that this is copying was hazard-prone since simply adding a failure path between it and the release() calls below causes failure. My preference would be to append(Move((comp|zone)Holder)) so there is a transactional transfer of ownership.

Comment on attachment 8983732 [details] [diff] [review] Part 2 - Add a scriptedCallerGlobal() testing function for JS::GetScriptedCallerGlobal Review of attachment 8983732 [details] [diff] [review]: ----------------------------------------------------------------- Nice!

Comment on attachment 8983740 [details] [diff] [review] Part 3 - Add an objectGlobal(obj) testing function Review of attachment 8983740 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/TestingFunctions.cpp @@ +5122,5 @@ > + > + obj = ToWindowProxyIfWindow(&obj->global()); > + > + if (!cx->compartment()->wrap(cx, &obj)) > + return false; While harmless, is this wrap() needed? I think it was necessary for GetScriptedCallerGlobal() b/c the caller can be in a different compartment, but here we're same-compartment with args[0] and we haven't unwrapped anything and I think the WindowProxy of a given GlobalObject is same-compartment as the GlobalObject. Grepping around, other uses of ToWindowProxyIfWindow() (e.g., LexicalEnvironmentObject::thisValue()) don't wrap().

(In reply to Luke Wagner [:luke] from comment #4) > If the below gc.zones().append(zone) fails, I think this would leave a > dangling JSCompartment* in the zone. If we fail to append(zone), nobody will touch the JSCompartment* in the Zone's Vector anyway - the UniquePtrs will just delete both the compartment and zone. > My preference would be to append(Move((comp|zone)Holder)) so there > is a transactional transfer of ownership. Yeah I agree it would be nice to use UniquePtr more for this code. I'll try something. (In reply to Luke Wagner [:luke] from comment #6) > While harmless, is this wrap() needed? Oops, you're right, it's not needed here because we have this explicit wrapper check.

I tried to use UniquePtr for the zone/compartment/realm Vectors, but it becomes pretty messy. I'm also not sure it fixes all potential issues here. The attached patch changes NewRealm to first reserve() space in all the Vectors and then we can mutate them all infallibly. This way there should be no way to get into a weird/inconsistent state.

Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6bbae91a2eaf part 1 - Refactor ZoneSpecifier and add a sameCompartmentAs option to newGlobal in the shell. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/22ea0700931c part 2 - Add a scriptedCallerGlobal() testing function for JS::GetScriptedCallerGlobal. r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/4b6608413e08 part 3 - Add an objectGlobal(obj) testing function. r=luke

https://hg.mozilla.org/mozilla-central/rev/6bbae91a2eaf https://hg.mozilla.org/mozilla-central/rev/22ea0700931c https://hg.mozilla.org/mozilla-central/rev/4b6608413e08

NI myself to update the newGlobal documentation when I have time.

Nvm, this is being fixed in bug 1481858.