1514153 - Loader global check in Cu.permitCPOWsInScope is broken

Status: RESOLVED WORKSFORME

(Core :: XPConnect, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

We assert !mozJSComponentLoader::Get()->IsLoaderGlobal(scopeObj), but callers typically pass |this| and that's the JSM environment or something in the shared JSM global.

We have a few modules like Task.jsm that call Cu.permitCPOWsInScope(this). Comments suggest we can probably remove at least some of these calls now, I'll see how far I get with that.

Comment 1

[Jan de Mooij [:jandem]]

Also maybe we can make this use xpc::CrashIfNotInAutomation().

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #1)
> Also maybe we can make this use xpc::CrashIfNotInAutomation().

Oh evilpie already did something similar in the actual CPOW code in bug 1465911, by making it throw, so it seems the risk of this bug causing issues currently is pretty small.

Comment 3

[Jan de Mooij [:jandem]]

See also bug 1512655 comment 6.

Comment 4

[Kris Maglione [:kmag]]

(In reply to Jan de Mooij [:jandem] from comment #0)
> We assert !mozJSComponentLoader::Get()->IsLoaderGlobal(scopeObj), but
> callers typically pass |this| and that's the JSM environment or something in
> the shared JSM global.

The JSMs that call this should all be whitelisted to run in their own globals:

https://searchfox.org/mozilla-central/rev/49e78df13e7a505827a3a86daae9efdf827133c6/js/xpconnect/loader/mozJSComponentLoader.cpp#626-632

So they shouldn't be running in the shared JSM global.

> We have a few modules like Task.jsm that call Cu.permitCPOWsInScope(this).
> Comments suggest we can probably remove at least some of these calls now,
> I'll see how far I get with that.

Task.jsm is basically dead at this point, except for some odd references from test code and Android.

Comment 5

[Kris Maglione [:kmag]]

(To be clear, I agree this check is broken, and that we need to remove calls in places like Task.jsm and Promise-backend.js.)

Comment 6

[Andrew McCreight [:mccr8]]

Gijs removed this method in bug 1620226.