1482423 - Assert compartments don't contain both system/non-system realms

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jan de Mooij [:jandem]]

We can release-assert this in NewRealm and do some minor JS::SetRealmPrincipals refactoring so we can remove Realm::setIsSystem.

Then we can rely on this in js::IsSystemCompartment (and un-deprecate that) and use IsSystemCompartment in xpc::AccessCheck::isChrome to eliminate more JS_GetCompartmentPrincipals calls.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Assert compartments don't contain both system/non-system realms

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Use js::IsSystemCompartemnt in xpc::AccessCheck::isChrome

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 8999214 [details] [diff] [review]
Part 1 - Assert compartments don't contain both system/non-system realms

Review of attachment 8999214 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsfriendapi.cpp
@@ +178,5 @@
>      if (principals == realm->principals())
>          return;
>  
> +    // We'd like to assert that our new principals is always same-origin
> +    // with the old one, but JSPrincipals doesn't give us a way to do that.

One way to do that would be to check that they mutually subsume each other.  That's a pretty strong degree of equivalence.

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Luke Wagner [:luke] from comment #3)
> One way to do that would be to check that they mutually subsume each other. 
> That's a pretty strong degree of equivalence.

Yes, I was thinking about that too :) However this is a pre-existing comment/code and I'm just interested in the isSystem bit not changing. Thinking about it more, maybe doing the stronger check is a good idea for same-compartment-realms; I may file a follow-up next week.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f73af1f3b2c
part 1 - Assert compartments don't contain both system/non-system realms. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6b953e50fbf
part 2 - Use js::IsSystemCompartment in xpc::AccessCheck::isChrome. r=mrbkap

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/9f73af1f3b2c
https://hg.mozilla.org/mozilla-central/rev/e6b953e50fbf