Open Bug 1316480 Opened 3 years ago Updated 8 months ago

Get rid of IsCallerChrome and friends

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

Tracking Status
firefox52 --- wontfix

People

(Reporter: bzbarsky, Unassigned)

References

(Depends on 2 open bugs, Blocks 3 open bugs)

Details

This is a tracking bug for removing IsCallerChrome, ThreadsafeIsCallerChrome, LegacyIsCallerChromeOrNativeCode, and whatever else inspects the JS stack to decide whether the caller is "system code".

We should move to either explicit examination of the caller principal or an "is system" boolean passed in.  We may want a binding annotation for the latter, because on workers we don't have principals.

Maybe what we should really do is add such an annotation, and implement it as a boolean on the JSCompartment, akin to the secureContext boolean?  That would avoid the need for branching on NS_IsMainThread in webidl methods that care and the like...
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #0)
> Maybe what we should really do is add such an annotation, and implement it
> as a boolean on the JSCompartment, akin to the secureContext boolean?  That
> would avoid the need for branching on NS_IsMainThread in webidl methods that
> care and the like...

I'm skeptical about ambient state for permissions checks. Why can't we just do an isSystem boolean supplied by the bindings instead of a subject principal?
> Why can't we just do an isSystem boolean supplied by the bindings

That's what I'm talking about, yes; I'm sorry that wasn't clear.  We already have a [NeedsSubjectPrincipal] annotation; I'm suggesting we add [CallerIsSystem] which passes a boolean, because that's something we could implement on workers too.

That leaves the question of where that boolean comes from.  We could do it like we do ThreadsafeIsCallerChrome right now, with an NS_IsMainThread and checking the compartment principal on the main thread vs checking some worker state on the worker thread.  Or we could have a boolean in the compartmentoptions that we just set up correctly at compartment creation time everywhere, and just check that.

I guess we can add the thing on top of ThreadsafeIsCallerChrome for the moment and then optimize later...
> I'm suggesting we add [CallerIsSystem] which passes a boolean

Or better [NeedsCallerIsSystem].
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #2)
> That leaves the question of where that boolean comes from.  We could do it
> like we do ThreadsafeIsCallerChrome right now, with an NS_IsMainThread and
> checking the compartment principal on the main thread vs checking some
> worker state on the worker thread.  Or we could have a boolean in the
> compartmentoptions that we just set up correctly at compartment creation
> time everywhere, and just check that.

Rather than duplicating the information in the compartment principal, I think I would prefer a runtime-wide tri-state that says "everything in this runtime is system", "everything in this runtime is non-system", or "check the compartment principal".
Depends on: 1316616
OK.  I guess I'll poke the JS folks to see what API they're willing to take here.  Filed bug 1316616.
Depends on: 1316619
Depends on: 1316661
Priority: -- → P3
Depends on: 1317591
Depends on: 1317596
Depends on: 1317597
Depends on: 1317599
Depends on: 1317606
We also have at least two ways of doing "is chrome" checks on workers that end up meaning the same thing: IsCurrentThreadRunningChromeWorker() and WorkePrivate::UsesSystemPrincipal().

And we have a bunch of AccessCheck::isChrome(js::GetContextCompartment(cx)) scattered about that should probably go away too.
Depends on: 1317759
Depends on: 1317813
Depends on: 1318117
Depends on: 1318467
Depends on: 1318471
Depends on: 1321879
Depends on: 1324035
Depends on: 1325028
Depends on: 1334953
Depends on: 1334955
Depends on: 1334957
Depends on: 1334865
No longer depends on: 1334865
Depends on: 1335311
Depends on: 1335368
Mass wontfix for bugs affecting firefox 52.
Blocks: 1208164
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.