1579367 - User prefs are loaded after creating the first JSContext in the parent process

Status: RESOLVED FIXED

(Core :: XPConnect, task)

Description

[Jan de Mooij [:jandem]]

The prefs we load here always have their default values in the parent process, because user pref overrides are loaded later on, in Preferences::InitializeUserPrefs(). Bug 1578350 is adding a similar pref affected by this.

The JS engine wants to move to process-wide prefs, for simplicity, and require these to be set early on. However XPConnect is initialized very early on before the user prefs are loaded.

I don't know what to do about this. Loading user prefs much later than all.js and similar seems like a pretty big footgun, so maybe that's the thing to fix somehow.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1579367 - WIP: Initialize XPCJSContext after loading user prefs

This also removes some dead code for cooperative JSContexts (the
JS engine support for this was removed last year in bug 1434211).

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 9090990 [details]
Bug 1579367 - WIP: Initialize XPCJSContext after loading user prefs

How do you feel about this approach? The patch isn't finished yet but the browser starts up and we now get the correct prefs in the parent process too.

Comment 3

[Jan de Mooij [:jandem]]

Hey Kris, after our IRC conversation yesterday I pushed this to Try. The ts_paint and ts_paint_webext numbers don't seem to be affected much. What do you think?

Comment 4

[Kris Maglione [:kmag]]

OK, then let's give it a shot, I guess. We have telemetry for how often we need to wait for a script from the preloader or re-decode it on the main thread, so we should be able to see whether it causes regressions on the wild with pretty high confidence.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1579367 part 1 - Set nsScriptSecurityManager JS engine callbacks with other callbacks instead of doing something different. r?kmag!

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1579367 - Initialize XPCJSContext explicitly, after loading user prefs. r?kmag!

This way we get the correct values for start-up prefs in the parent process.

Depends on D51060

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/955256297d6d
Initialize XPCJSContext explicitly, after loading user prefs. r=kmag

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/955256297d6d

Comment 9

[[:fabrice] Fabrice Desré]

That's causing a startup crash when JS components are used to implement directory providers because they are instantiated by https://hg.mozilla.org/mozilla-central/file/d73cfe3a04e90b5dff64f6d4e3c3a9183aec4ec5/xpcom/build/XPCOMInit.cpp#l459 while NS_InitXPCOM is called with aInitJSContext == false.

I'm not sure what the right solution for that is...

Comment 10

[Dorel Luca [:dluca]]

Backed out changeset 955256297d6d (bug 1579367) for causing a top crash in Bug 1594404. a=backout

As requested by pascalc in #sheriffs.

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

Comment 11

[Jan de Mooij [:jandem]]

(In reply to [:fabrice] Fabrice Desré from comment #9)

That's causing a startup crash when JS components are used to implement directory providers because they are instantiated by https://hg.mozilla.org/mozilla-central/file/d73cfe3a04e90b5dff64f6d4e3c3a9183aec4ec5/xpcom/build/XPCOMInit.cpp#l459 while NS_InitXPCOM is called with aInitJSContext == false.

I'm not sure what the right solution for that is...

How did you find out about this? Is it something we need to support still, or can we do this later in startup? It's been frustrating to have these features we apparently care about but don't have any tests for..

Comment 12

[[:fabrice] Fabrice Desré]

(In reply to Jan de Mooij [:jandem] from comment #11)

How did you find out about this? Is it something we need to support still, or can we do this later in startup? It's been frustrating to have these features we apparently care about but don't have any tests for..

I'm working on a gecko based product that happens to have a such a js component. If the solution is to rewrite it in c++, so be it...

Comment 13

[Jan de Mooij [:jandem]]

Any thoughts on the JS-implemented directory providers, Kris?

Comment 14

[Kris Maglione [:kmag]]

(In reply to Jan de Mooij [:jandem] from comment #13)

Any thoughts on the JS-implemented directory providers, Kris?

I don't see any reason to support that.

Comment 15

[Jan de Mooij [:jandem]]

This seems to fix the Valgrind issue in bug 1582512, so I'll try relanding this soon..

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3fd24f5f0a7
Initialize XPCJSContext explicitly, after loading user prefs. r=kmag

Comment 17

[Arthur Iakab [arthur_iakab]]

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