Afaik, it's dead code. We only have two callers, once bug 1136292 is fixed: 1) nsGlobalWindow::SetNewDocument is doing: JS_SetParent(cx, outerObject, newInnerGlobal); but parents are same-compartment, outerObject is a proxy, and proxies are always parented to globals once bug 1136925 is fixed. So afaict the parent of outerObject here should already be newInnerGlobal, no? 2) JSObject2JSObjectMap::Reparent calls it like so: if (inner == aNewInner && inner != parent) JS_SetParent(aCx, wrapper, aNewInner); where inner and aNewInner are guaranteed to be inner windows and "parent" is the JS_GetParent of whatever is in mWaiverWrapperMap. But, again, given that parents are same-compartment, how can we have inner != parent? Or am I missing something?

Bobby, _am_ I missing something?

(In reply to Not doing reviews right now from comment #0) > Afaik, it's dead code. > > We only have two callers, once bug 1136292 is fixed: > > 1) nsGlobalWindow::SetNewDocument is doing: > > JS_SetParent(cx, outerObject, newInnerGlobal); > > but parents are same-compartment, outerObject is a proxy, and proxies are > always parented to globals once bug 1136925 is fixed. So afaict the parent > of outerObject here should already be newInnerGlobal, no? Well, the key line here is: outerObject = xpc::TransplantObject(cx, obj, outerObject); Given how brain transplanting works, this means that outerObject is not necessarily its original value - it may have been gut-swapped with cross-compartment wrapper in the target scope, which then gets returned by TransplantObject. But as long as cross-compartment wrappers are always parented to globals, I agree that we should be guaranteed to end up with an outer parented to its current inner. Please assert this. > 2) JSObject2JSObjectMap::Reparent calls it like so: > > if (inner == aNewInner && inner != parent) > JS_SetParent(aCx, wrapper, aNewInner); > > where inner and aNewInner are guaranteed to be inner windows and "parent" is > the JS_GetParent of whatever is in mWaiverWrapperMap. But, again, given > that parents are same-compartment, how can we have inner != parent? Hm. The caller does XPCWrappedNativeScope* scope = xpc::ObjectScope(outerObject); after the transplant, which will put us in the scope of newInnerGlobal, and then it passes newInnerGlobal to Reparent. That inner is now current, so this whole setup isn't going to do anything at all. So yes, please just remove the whole Reparent machinery.

> But as long as cross-compartment wrappers are always parented to globals After bug 1136925 all proxies are always parented to globals. I'll add an assert, but that will end up dying when GetObjectParent/JS_GetParent die, of course. > So yes, please just remove the whole Reparent machinery. Sweet!

https://hg.mozilla.org/integration/mozilla-inbound/rev/cec80d0c3a02 https://hg.mozilla.org/integration/mozilla-inbound/rev/d4516fac0a6f

https://hg.mozilla.org/mozilla-central/rev/cec80d0c3a02 https://hg.mozilla.org/mozilla-central/rev/d4516fac0a6f