1335890 - Factor out getting to the Principal from JSContext

Status: RESOLVED FIXED

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

Description

[Tomislav Jovanovic :zombie]

If I understood Boris's comment in bug 1310318 comment #17, this is something we should do.

Comment 1

[Tomislav Jovanovic :zombie]

I can probably take this next week, provided my understanding above is correct.

Comment 2

[Tomislav Jovanovic :zombie]

Attachment: Bug 1335890 - Factor out nsContentUtils::SubjectPrincipal version that takes JSContext

Review commit: https://reviewboard.mozilla.org/r/109948/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109948/

Comment 3

[Tomislav Jovanovic :zombie]

Comment on attachment 8833810 [details]
Bug 1335890 - Factor out nsContentUtils::SubjectPrincipal version that takes JSContext

https://reviewboard.mozilla.org/r/109948/#review111068

::: dom/base/nsContentUtils.cpp:2818
(Diff revision 1)
>    //
>    // So we use a singleton null principal. To avoid it being accidentally
>    // inherited and becoming a "real" subject or object principal, we do a
>    // release-mode assert during compartment creation against using this
>    // principal on an actual global.
>    if (!compartment) {

This is my best guess at what each version of SubjectPrincipal() should do.  Because of the big comment wall above, this still returns a NullPrincipal without a compartment, even if it marely calls the other version when the JSContext has a compartment (since it requires one).

Comment 4

[Tomislav Jovanovic :zombie]

Hope this is what you had in mind for a followup Boris?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8833810 [details]
Bug 1335890 - Factor out nsContentUtils::SubjectPrincipal version that takes JSContext

https://reviewboard.mozilla.org/r/109948/#review111184

r=me with the one nit below.  Thank you!

::: dom/base/nsContentUtils.cpp:2082
(Diff revision 1)
>  
>  // static
>  bool
>  nsContentUtils::CallerHasPermission(JSContext* aCx, const nsAString& aPerm)
>  {
>    // Chrome gets access by default.

So I think we should just make CallerHasPermission call PrincipalHasPermission(SubjectPrincipal(aCx)), and we should add such a PrincipalHasPermission.

Then we can also call that from nsContentUtils::IsCutCopyAllowed.

Comment 6

[Tomislav Jovanovic :zombie]

Comment on attachment 8833810 [details]
Bug 1335890 - Factor out nsContentUtils::SubjectPrincipal version that takes JSContext

https://reviewboard.mozilla.org/r/109948/#review111198

::: dom/base/nsContentUtils.cpp:2082
(Diff revision 1)
>  
>  // static
>  bool
>  nsContentUtils::CallerHasPermission(JSContext* aCx, const nsAString& aPerm)
>  {
>    // Chrome gets access by default.

I meant to do that in the next bug 1312260, where I'll actually need PrincipalHasPermission().

Comment 7

[Tomislav Jovanovic :zombie]

Comment on attachment 8833810 [details]
Bug 1335890 - Factor out nsContentUtils::SubjectPrincipal version that takes JSContext

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109948/diff/1-2/

Comment 8

[Boris Zbarsky [:bzbarsky]]

> I meant to do that in the next bug 1312260, where I'll actually need PrincipalHasPermission().

Works for me!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec252adeb5c3
Factor out nsContentUtils::SubjectPrincipal version that takes JSContext r=bz

Comment 10

[Carsten Book [:Tomcat]]

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