Closed Bug 1127475 Opened 5 years ago Closed 5 years ago
Remove unnecessary parent arguments
Most of the time parent can be null, in which case we automatically use cx->global(). See also bug 1127443 comment 7.
OK, so the two cases: 1) nsWindowSH::GlobalResolve. This is called from one place only: nsGlobalWindow::DoResolve. The "obj" passed in is the aObj argument to DoResolve. DoResolve is called from bindings code only and passes either the "obj" of the resolve hook or the "obj" or "wrapper" passed to ResolveOwnPropertyViaResolve. So in all cases it's either the JSObject for the (inner) window or the JSObject for the Xray. Importantly, cx is in the compartment of obj. Since the JS_NewObject call here is conditioned on !xpc::IsXrayWrapper(obj), we know that "obj" is in fact the window and hence obj == cx->global(), so we can drop it: that will be the default parent anyway. We can add an assert, I guess. 2) xpc::NewOutObject. This passes the return value of JS_GetGlobalForObject(cx, scope). JS_GetGlobalForObject asserts cx and obj are same-compartment, so JS_GetGlobalForObject(cx, scope) is the same thing as cx->global(). So again, we can elide the parent arg (and drop the "scope" arg to NewOutObject, for that matter). Even if we're worried that maybe we only hit this codepath in an opt build when cx and scope are not same-compartment, xpc::NewOutObject only has two callers. One caller is in WrapperAnswer.cpp and looks like this: 420 JSCompartment *compartment = js::GetContextCompartment(cx); 421 RootedObject global(cx, JS_GetGlobalForCompartmentOrNull(cx, compartment)); 422 RootedObject obj(cx, xpc::NewOutObject(cx, global)); OK, so that one is clearly cx->global(). We should be able to just kill off lines 420/421! The second caller is in XPCWrappedJSClass.cpp and is: 1122 RootedObject out_obj(cx, NewOutObject(cx, obj)); This is in the scope of a "JSAutoCompartment ac(cx, obj)" and no one has messed with obj or other compartment stuff, so cx and obj are certainly same-compartment.
We already assert that proto and parent are the same compartment as cx, so this probably trivial as a custom parent on a normal object should not matter.
The point is that comment 1 shows that the parent being passed at both these callsites is the compartment global.
What would be the consequence if the parent wasn't a global? I think I have definitely seen some instances of that.
> What would be the consequence if the parent wasn't a global? Then it would need to be passed in explicitly, no? The whole point is that if no parent is passed the compartment global is used. So if that's what you want, you don't have to pass anything.
Comment on attachment 8563588 [details] [diff] [review] remove-unnecessary-parent r=me
Attachment #8563588 - Flags: review?(bzbarsky) → review+
You need to log in before you can comment on or make changes to this bug.