In a single-threaded world, the only thing we actually _need_ these things for should be for method calls on reflectors (XPCWrappedNative::CallMethod, and XPCWrappedJSClass::CallMethod). The rest of it is an artifact that all the function signatures expect one of these things, and so we have to pass it everywhere. Because of the performance overhead, we create these XPCLazyCallContexts, which do placement-new to instantiate an XPCCallContext when they need one. But this non-LIFOness is a total nightmare for rooting. It may be a Sisyphusian task, but I'm going to take a crack at seeing how hard it might be to just remove most of this.

I'm all for it. In most places it's really only used for stupid things, like getting the run-time from it. But there are a few bits that needs to get right. I'd suggest you to start in xpconvert... the XPCWrappedNative::GetNewOrUsed should be the hardest part I guess, but I didn't have the time back then to figure it out when I wanted to remove all these nonsense XPCCallContext arguments.

This turned out to be more tractable than I expected. My primary strategy was just to ditch the param entirely (rather than converting ccx -> cx) and use AutoJSContexts whenever we need them. I started in XPCWrappedNative and worked outwards. I managed to get things compiling after removing about half of the ccx instances in XPConnect, so I'm going to push this to try now to nip any regressions in the bud: https://tbpl.mozilla.org/?tree=Try&rev=7625c5ad5472

Nice, that looks green. I'll keep going.

Focusing the bug title on the end goal we have here for rooting.

Starting with the above, this is the smallest unit change that will compile.

Some of these callers seem to be passing a ccx when they don't need to, but let's just remove the param all together for consistency.

There are a number of places where quickstubs does a scary-looking call to lccx->SetWrapper. However, the lccx never gets morphed into a ccx, nor does it escape in any other way. And unlike ccxes, declaring an lccx on the stack doesn't have any observable side-effects. So this should actually be safe.

\o/

Now that we don't have the separate path for initialization from an XPCLazyCallContext, this stuff can be simplified. We get rid of Init entirely in the next patch.

The large block is a simple move.

Comment on attachment 757994 [details] [diff] [review] Part 1 - Stop using XPCCallContext for most stuff in XPCWrappedNative.cpp. v1 Review of attachment 757994 [details] [diff] [review]: ----------------------------------------------------------------- It looks good. https://twitter.com/girayozil/status/306836785739210752 ::: js/xpconnect/src/XPCWrappedNative.cpp @@ +516,5 @@ > > NS_ASSERTION(!xpc::WrapperFactory::IsXrayWrapper(parent), > "Xray wrapper being used to parent XPCWrappedNative?"); > > + ac.construct((JSContext*)cx, parent); I'd prefer static_cast or adding a .get(). @@ +1988,3 @@ > clazz) { > + RootedObject answer(cx, > + clazz->CallQueryInterfaceOnJSObject(cx,jso, *iid)); Missing space

Comment on attachment 757995 [details] [diff] [review] Part 2 - Stop using XPCCallContext for XPCConvert. v1 Review of attachment 757995 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCConvert.cpp @@ +1391,5 @@ > > #define POPULATE(_t) \ > PR_BEGIN_MACRO \ > for (i = 0; i < count; i++) { \ > + if (!NativeData2JS(current.address(), ((_t*)*s)+i, type, iid, pErr) || \ Keep the \s aligned

Comment on attachment 757996 [details] [diff] [review] Part 3 - Stop taking a cx in XPCWrappedJS::GetNewOrUsed. v1 Review of attachment 757996 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/Codegen.py @@ +2389,5 @@ > "${target} = ${source};" > ).substitute(self.substitution) > > return checkObjectType + string.Template( > """nsresult rv; Move the declaration down

Comment on attachment 757999 [details] [diff] [review] Part 5 - Remove a bunch of now-unnecessary ccx declarations from nsXPConnect. v1 Review of attachment 757999 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/nsXPConnect.cpp @@ +1195,5 @@ > nsIVariant ** aResult) > { > NS_PRECONDITION(aJSVal, "bad param"); > NS_PRECONDITION(aResult, "bad param"); > *aResult = nullptr; Can drop this

Looks fine fine, I guess.

https://hg.mozilla.org/mozilla-central/rev/5a678ff4ce22 https://hg.mozilla.org/mozilla-central/rev/9ab08f6d2258 https://hg.mozilla.org/mozilla-central/rev/29351d714861 https://hg.mozilla.org/mozilla-central/rev/6e56f8dae182 https://hg.mozilla.org/mozilla-central/rev/4cb6b2983e9d https://hg.mozilla.org/mozilla-central/rev/c7f87d78d5b1 https://hg.mozilla.org/mozilla-central/rev/16f8376c7b34 https://hg.mozilla.org/mozilla-central/rev/4bf9a018b8f2 https://hg.mozilla.org/mozilla-central/rev/b5dc59eec4c6 https://hg.mozilla.org/mozilla-central/rev/576842d446e3 https://hg.mozilla.org/mozilla-central/rev/902172f7b451