1316480 - Get rid of IsCallerChrome and friends

Status: NEW

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

Description

[Boris Zbarsky [:bzbarsky]]

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...

Comment 1

[Bobby Holley (:bholley)]

(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?

Comment 2

[Boris Zbarsky [:bzbarsky]]

> 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...

Comment 3

[Boris Zbarsky [:bzbarsky]]

> I'm suggesting we add [CallerIsSystem] which passes a boolean

Or better [NeedsCallerIsSystem].

Comment 4

[Bobby Holley (:bholley)]

(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".

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK.  I guess I'll poke the JS folks to see what API they're willing to take here.  Filed bug 1316616.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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.

Comment 7

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.