With c-p-g we can unambiguously add JSCompartment::globalObject. This has many uses. The initial patch should add the field and assert commutativity invariants: obj->compartment()->globalObject() == obj->global() fp->compartment()->globalObject() == fp->global() Later, we can redefine the rhs in terms of the lhs.

Luke: is this on your radar? If not, how hard is it?

It's not a hardness thing, it's a waiting-to-be-really-sure-that-cpg-is-100%-fully-stuck-so-we-don't-make-backout-harder thing. (My fears are almost totally gone (bholley has continued to rock it), but, for the moment, I'm watching bug 757639 and bug 758415 and also waiting to see what happens on Aurora.)

This patch just removes JS_NewGlobalObject. This forces newGlobal to always create a new compartment which required some minor fixes in debugger test-cases. On actual compartment-transparency bug exposed was bug 764307 (see comment in Debugger.cpp).

(Later we can remove JSObject::getParent altogether.) With this last patch, we should always be able to know the global of any GC-thing via thing->compartment()->global(). Let the reaping begin. Green on try

Comment on attachment 634125 [details] [diff] [review] rm JS_NewGlobalObject Review of attachment 634125 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the Debugger.cpp change reverted (and other minor comments). ::: js/src/jsapi-tests/testDebugger.cpp @@ +211,5 @@ > "dbg.onNewScript = function (s) {\n" > " hits += Number(s instanceof Debugger.Script);\n" > "};\n"); > > + // Since g is a debuggee, g1.eval should trigger newScript, regardless of g.eval @@ +217,4 @@ > // > // (Not all scripts are permanently associated with specific global > // objects, but eval scripts are, so we deliver them only to debuggers that > // are watching that particular global.) The disclaimer-y part of the comment is no longer true. It can just say: // Scripts are associated with the global where they're compiled, // so we deliver them only to debuggers that are watching that // particular global. @@ +224,1 @@ > return ok; Just return testIndirectEval(g, "Math.abs(0)", 1); or inline it; anyway expectedHits doesn't need to be a parameter. ::: js/src/vm/Debugger.cpp @@ +3626,5 @@ > + Value protov; > + if (!refobj->isWrapper()) > + protov = ObjectOrNullValue(refobj->getProto()); > + else > + protov = ObjectOrNullValue(UnwrapObject(refobj)->getProto()); This change seems unrelated to everything else. Separate bug for sure. I don't think the change is right as-is; unwrapping can cross compartment boundaries, so it seems like at least this would want to unwrap, getProto, then rewrap for refobj's compartment -- before applying wrapDebuggeeValue for the debugger. This would make more sense as part of a broader fix for bug 764307, though, with the unwrap-getProto-rewrap part factored out into common code used by this, __proto__'s getter, and Object.getPrototypeOf. ::: js/xpconnect/shell/xpcshell.cpp @@ +1201,5 @@ > + JSAutoEnterCompartment ac; > + if (!ac.enter(cx, gobj)) > + return false; > + > + if (!JS_SplicePrototype(cx, gobj, obj)) Is this going to work? obj needs to be wrapped, at least, right?

Comment on attachment 634130 [details] [diff] [review] add JSCompartment::global() and assert that it commutes with the parent chain Review of attachment 634130 [details] [diff] [review]: ----------------------------------------------------------------- I am so happy about this. ::: js/src/jscntxtinlines.h @@ +228,5 @@ > + : context(cx), compartment(cx->compartment) > + { > + if (cx->compartment) { > + GlobalObject *global = GetGlobalForScopeChain(cx); > + JS_ASSERT(cx->compartment->global() == *global); /me sees operator==(const JSObject &, const JSObject &) Huh. Well OK. ::: js/src/jscompartment.h @@ +129,2 @@ > JSRuntime *rt; > JSPrincipals *principals; Micro-nit: Keep rt and principals at the top?

Agreed on all counts (removed the whole -P case in xpcshell, changed the gc-05.js debug test to not use proto). https://hg.mozilla.org/integration/mozilla-inbound/rev/68c396f305f4 https://hg.mozilla.org/integration/mozilla-inbound/rev/6966d9832704 https://hg.mozilla.org/integration/mozilla-inbound/rev/80b8680bda1c

https://hg.mozilla.org/mozilla-central/rev/68c396f305f4 https://hg.mozilla.org/mozilla-central/rev/6966d9832704 https://hg.mozilla.org/mozilla-central/rev/80b8680bda1c