Right now CheckedUnwrap is a static check: it just examines the wrapper being unwrapped. In a multiple-globals-per-compartment world, that doesn't work, because whether a wrapper can be unwrapped starts to depend on which global in the compartment asking, at least for the wrappers used for Window and Location objects.

Even without multiple-globals-per-compartment CheckedUnwrap stops being a static check once wrapper recomputation on document.domain set is removed.

The proposal is to add a JSContext argument to CheckedUnwrap, with the current Realm of the JSContext representing the global to use for the dynamic check. And then we need to go through all the callsites and fix them.

I'll add the basic infrastructure and do some conversions, but we may want to parallelize the overall work some.

Does this also affect CheckedUnwraps where it's totally fine to return nullptr for Window/Location wrappers? Consider JS_IsTypedArrayObject for instance... I think we have a lot of those in the JS engine. Keeping an API (with a different name maybe) for that use case might be nice?

That's a good question. I was trying to figure that out for some other cases too, actually.

One simple possibility I've considered is that such callers could pass nullptr for the JSContext and we would use that to mean "do the strict check where we fail unwrap if there's any chance that something in our compartment might not have access to the thing". As in, no JSContext means we just skip the dynamic check and never unwrap the WindowProxy/Location wrappers.

I would also be fine with having an explicit differently-named API for that case. Not quite sure how to name it.

In the short term, I'd like to not keep around a CheckedUnwrap API that does not take a JSContext, because that will make it hard to determine which callsites still need auditing. And in the long term, I think the subtle distinction between the two APIs would make it too easy to reach for the wrong one if they have the same name...

Maybe CheckedUnwrapStatic vs CheckedUnwrapDynamic? Then we could document that CheckedUnwrapDynamic is only necessary when you care about unwrapping special wrappers like WindowProxy and Location. In the JS engine most unwraps are "can we safely get a FooObject out of this" and CheckedUnwrapStatic would be what people should use for that.

I'm going to do all the non-js/src bits in bug 1521907. This bug is going to track the js/src bits.

The main cases where we can definitely use CheckedUnwrapStatic are:

(1) Code behaves the same when unwrapping or a Class check fails (and the Class
is not a WindowProxy/Location Class). Example:
obj = CheckedUnwrapStatic(obj); return obj && obj->is<FooObject>();

There's no observable difference between unwrapping or not unwrapping
WindowProxy/Location here.

(2) Code knows we have a wrapped FooObject. Example:
obj = CheckedUnwrapStatic(obj); if (!obj) { .. } return obj->as<FooObject>().bar();

https://hg.mozilla.org/mozilla-central/rev/e7dc5234c656

This uses CheckedUnwrapStatic in places where we don't expect WindowProxy/Location
objects to show up or where we will throw an exception in that case anyway. I also
used this in the more complicated cases (the compartment situation in PromiseObject
for example is very complicated).

https://hg.mozilla.org/mozilla-central/rev/17d8de567ea4

The CacheIR code only sees transparent CCWs so it's fine to do a static unwrap.

DebuggerObject::unwrap is more complicated. We're in the debugger's compartment
there; I went with UnwrapOneCheckedStatic as it seems safest and simplest for
now.

Depends on D21353

https://hg.mozilla.org/mozilla-central/rev/82df15131f15

https://hg.mozilla.org/mozilla-central/rev/4325cfacfc49