1379023 - Clean up system principal handling in xpconnect/loader/

Status: RESOLVED FIXED

(Core :: XPConnect, enhancement)

Description

[Andrew McCreight [:mccr8]]

This function saves the target object, prior to unwrapping, and also the principal of the target object. At least the latter used to be needed, but now we only need to know if the unwrapped principal is the system or not to decide if we're using the cache. Limiting what is done makes it easier to understand what is happening, I think.

Also, I'm renaming result_obj to resultObj.

Comment 1

[Andrew McCreight [:mccr8]]

There's a bunch of old gunk related to the system principal in js/xpconnect/loader that can be cleaned up.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1379023, part 1 - Directly compute if we're compiling in a content compartment in DoLoadSubScriptWithOptions.

The existing code uses various intermediate objects, but the only
thing they are used for now is to figure out if the compartment we're
in has the system principal or not, so just compute that directly.

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

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Bug 1379023, part 2 - Remove unused systemPrincipal argument to ReadCachedScript.

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

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Bug 1379023, part 3 - Make WriteCachedScript assert about the system principal not require the argument.

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

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: Bug 1379023, part 4 - Remove mSystemPrincipal fields from mozJSSubScriptLoader and mozJSComponentLoader.

The lone remaining use is trivial.

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

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 8884391 [details]
Bug 1379023, part 1 - Directly compute if we're compiling in a content compartment in DoLoadSubScriptWithOptions.

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

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 8884392 [details]
Bug 1379023, part 2 - Remove unused systemPrincipal argument to ReadCachedScript.

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

Comment 8

[Andrew McCreight [:mccr8]]

Comment on attachment 8884393 [details]
Bug 1379023, part 3 - Make WriteCachedScript assert about the system principal not require the argument.

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

Comment 9

[Andrew McCreight [:mccr8]]

Comment on attachment 8884394 [details]
Bug 1379023, part 4 - Remove mSystemPrincipal fields from mozJSSubScriptLoader and mozJSComponentLoader.

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

Comment 10

[Andrew McCreight [:mccr8]]

I removed a little bit more dead code from part 3 and 4.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8884391 [details]
Bug 1379023, part 1 - Directly compute if we're compiling in a content compartment in DoLoadSubScriptWithOptions.

https://reviewboard.mozilla.org/r/155314/#review160446

Nice cleanups!

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8884392 [details]
Bug 1379023, part 2 - Remove unused systemPrincipal argument to ReadCachedScript.

https://reviewboard.mozilla.org/r/155316/#review160448

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8884393 [details]
Bug 1379023, part 3 - Make WriteCachedScript assert about the system principal not require the argument.

https://reviewboard.mozilla.org/r/155318/#review160450

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8884394 [details]
Bug 1379023, part 4 - Remove mSystemPrincipal fields from mozJSSubScriptLoader and mozJSComponentLoader.

https://reviewboard.mozilla.org/r/155320/#review160452

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41e05eeb1e45
part 1 - Directly compute if we're compiling in a content compartment in DoLoadSubScriptWithOptions. r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/03939ed47d9b
part 2 - Remove unused systemPrincipal argument to ReadCachedScript. r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/b1193f5966e1
part 3 - Make WriteCachedScript assert about the system principal not require the argument. r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/98172f51813a
part 4 - Remove mSystemPrincipal fields from mozJSSubScriptLoader and mozJSComponentLoader. r=mrbkap

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/41e05eeb1e45
https://hg.mozilla.org/mozilla-central/rev/03939ed47d9b
https://hg.mozilla.org/mozilla-central/rev/b1193f5966e1
https://hg.mozilla.org/mozilla-central/rev/98172f51813a