1539029 - Overridden prefs values don't show in RDD process

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Michael Froman [:mjf]]

Only the default value seems to work. Specifically, :achronop found this when turning on dav1d decoding on RDD. I've put in a request to :njn for some guidance here, but so far everything I've tried only gets the default values.

I tried using an observer for pref changes like ContentParent/ContentChild, but this still misses the overridden values at startup although does see pref changes by the user later.

Comment 1

[Michael Froman [:mjf]]

From Nicholas Nethercote (via email):
Some useful background on libpref: https://docs.google.com/document/d/1V5Wc9bXwfgMG2JOsswvDPVwZl_xiaSBxwJXf3fiEaU8/.

Note particularly the reference to https://bugzilla.mozilla.org/show_bug.cgi?id=1471025#c95, which describes aspects of the IPC mechanisms.

To make all this work I suspect you'll need to use SharedPreferenceSerializer as is done in these locations:
https://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#2065
https://searchfox.org/mozilla-central/source/netwerk/ipc/SocketProcessHost.cpp#110

and then do similar things on the child side with SharedPreferenceDeserializer.

Comment 2

[Michael Froman [:mjf]]

Attachment: intial attempt at using SharedPreference{Serializer|Deserializer}

This is my initial, but failing attempt at adding SharedPreferenceSerializer/SharedPreferenceDeserializer to RDDProcessHost and RDDProcessImpl.

Comment 3

[Michael Froman [:mjf]]

This is the assertion that is failing with the patch above:
Assertion failure: result.isOk(), at /home/mfroman/mozilla/moz-central/modules/libpref/SharedPrefMap.cpp:28

#0 0x00007ff2026aa026 in mozilla::SharedPrefMap::SharedPrefMap(mozilla::ipc::FileDescriptor const&, unsigned long) (this=0x7ff2180ae1c0, aMapFile=..., aMapSize=180513)
at /home/mfroman/mozilla/moz-central/modules/libpref/SharedPrefMap.cpp:28
#1 0x00007ff20269ea3b in mozilla::Preferences::InitSnapshot(mozilla::ipc::FileDescriptor const&, unsigned long) (aHandle=..., aSize=180513)
at /home/mfroman/mozilla/moz-central/modules/libpref/Preferences.cpp:3670
#2 0x00007ff20316aaee in mozilla::ipc::SharedPreferenceDeserializer::DeserializeFromSharedMemory(char*, char*, char*, char*) (this=0x7ffe88a8e440, aPrefsHandleStr=0x0, aPrefMapHandleStr=0x0, aPrefsLenStr=0x7ffe88a90d43 "", aPrefMapSizeStr=0x7ffe88a90d57 "") at /home/mfroman/mozilla/moz-central/ipc/glue/ProcessUtils_common.cpp:139
#3 0x00007ff206ba717d in mozilla::RDDProcessImpl::Init(int, char**) (this=0x7ff21802b2e0, aArgc=10, aArgv=0x7ffe88a8eaa8)
at /home/mfroman/mozilla/moz-central/dom/media/ipc/RDDProcessImpl.cpp:74
#4 0x00007ff20a552507 in XRE_InitChildProcess(int, char**, XREChildData const*) (aArgc=10, aArgv=0x7ffe88a8eaa8, aChildData=0x7ffe88a8e950)
at /home/mfroman/mozilla/moz-central/toolkit/xre/nsEmbedFunctions.cpp:741
#5 0x00007ff20a55d1e7 in mozilla::BootstrapImpl::XRE_InitChildProcess(int, char**, XREChildData const*) (this=0x7ff21802a6b0, argc=12, argv=0x7ffe88a8eaa8, aChildData=0x7ffe88a8e950) at /home/mfroman/mozilla/moz-central/toolkit/xre/Bootstrap.cpp:67
#6 0x000056350068a18a in content_process_main(mozilla::Bootstrap*, int, char**) (bootstrap=0x7ff21802a6b0, argc=12, argv=0x7ffe88a8eaa8)
at /home/mfroman/mozilla/moz-central/browser/app/../../ipc/contentproc/plugin-container.cpp:56
#7 0x000056350068a28c in main(int, char**, char**) (argc=13, argv=0x7ffe88a8eaa8, envp=0x7ffe88a8eb18)
at /home/mfroman/mozilla/moz-central/browser/app/nsBrowserApp.cpp:263

Comment 4

[Michael Froman [:mjf]]

I asked Kris Maglione about the failure, and his responses are:

I think the problem is that you're using GeckoChildProcessHost::AsyncLaunch with file descriptors which will be automatically closed when your SharedPreferenceSerializer instance goes out of scope. AsyncLaunch just posts a task, and then immediately returns, which means that by the time the task runs, the file descriptors will be invalid.

You can use AsyncLaunch as long as you keep the SharedPreferenceSerializer alive until the process launch is complete.

Comment 5

[Michael Froman [:mjf]]

Attachment: updated shared pref serializer

Builds and executes without the prior assert, but still not seeing non-default pref values.

Comment 6

[Michael Froman [:mjf]]

Patch in comment 5 produces these logs:
MJF: RDDProcessHost prefsLen: 123
MJF: RDDProcessHost prefMapSize: 181296
MJF: RDDProcessImpl: rdd prefsHandle: (null)
MJF: RDDProcessImpl: rdd prefMapHandle: (null)
MJF: RDDProcessImpl: rdd prefsLen: 123
MJF: RDDProcessImpl: rdd prefMapSize: 181296
MJF: RDDChild StaticPrefs use dav1d: no
[RDD 16451, Main Thread] WARNING: NS_ENSURE_TRUE(InitStaticMembers()) failed: file /home/mfroman/mozilla/moz-central/modules/libpref/Preferences.cpp, line 4942
[RDD 16451, Main Thread] WARNING: NS_ENSURE_TRUE(InitStaticMembers()) failed: file /home/mfroman/mozilla/moz-central/modules/libpref/Preferences.cpp, line 4854
[RDD 16451, Main Thread] WARNING: NS_ENSURE_TRUE(InitStaticMembers()) failed: file /home/mfroman/mozilla/obj/deb/dist/include/mozilla/Preferences.h, line 192
[RDD 16451, Main Thread] WARNING: NS_ENSURE_TRUE(InitStaticMembers()) failed: file /home/mfroman/mozilla/moz-central/modules/libpref/Preferences.cpp, line 4590
[RDD 16451, Main Thread] WARNING: NS_ENSURE_TRUE(InitStaticMembers()) failed: file /home/mfroman/mozilla/moz-central/modules/libpref/Preferences.cpp, line 4625
[RDD 16451, Main Thread] WARNING: NS_ENSURE_TRUE(InitStaticMembers()) failed: file /home/mfroman/mozilla/moz-central/modules/libpref/Preferences.cpp, line 4590
MJF: RDDParent::RecvInit StaticPrefs use dav1d: yes
MJF: RemoteVideoDecoderParent::RemoteVideoDecoderParent: StaticPrefs use dav1d: yes
MJF: RemoteVideoDecoderParent::RemoteVideoDecoderParent: StaticPrefs use dav1d: yes

Comment 7

[Michael Froman [:mjf]]

In the case of RDD, Preferences::InitStaticMembers is getting a null prefService here[1]. Preferences::GetInstanceForService is not called.

[1] https://searchfox.org/mozilla-central/source/modules/libpref/Preferences.cpp#3540

Comment 8

[Michael Froman [:mjf]]

I've used rr and traced through a content process using InitStaticMembers, and RDD process using InitStaticMembers and they both eventually get here[1], where content process succeeds because Module().Active() is true, and RDD process fails because Module().Active() is false.

Kris - it looks like something related to gProcessMatchTable, but I don't know how that is controlled/built. Any thoughts?

[1] https://searchfox.org/mozilla-central/source/__GENERATED__/__linux64__/xpcom/components/StaticComponents.cpp#1578

Comment 9

[Michael Froman [:mjf]]

Looks like our problem is because RDD is not allowed here[1]:
'processes': ProcessSelector.ALLOW_IN_SOCKET_PROCESS,

To test this theory, I changed that to ALLOW_IN_GPU_AND_SOCKET_PROCESS because RDD and GPU are currently sharing a selector here[2]. This stops the warning about InitStaticMembers() failing and also allows us to get the non-default value of the prefs. I have not disabled the SharedPreferenceSerializer/SharedPreferenceDeserializer yet to see if it continues to be necessary.

[1] https://searchfox.org/mozilla-central/source/modules/libpref/components.conf#21
[2] https://searchfox.org/mozilla-central/source/xpcom/components/nsComponentManager.cpp#146

To fix this, I think that means adding ALLOW_IN_RDD_PROCESS and ALLOW_IN_RDD_AND_SOCKET_PROCESS, touching the following files:
modules/libpref/components.conf
xpcom/components/Module.h
xpcom/components/gen_static_components.py
xpcom/components/nsComponentManager.cpp

Will follow up, when I know more.

Comment 10

[Michael Froman [:mjf]]

There are other components.conf files that will require mods, but I am in the process of figuring out exactly which ones.

Comment 11

[Kris Maglione [:kmag]]

Yes, you'll need to use a process selector which will allow the preference service to load in your process type.

Our process selectors are turning into a mess, though. It's getting to the point that I think we really need to start ORing together selectors for process types rather than creating new constants for every possible combination.

Comment 12

[Michael Froman [:mjf]]

Attachment: Bug 1539029 - pt 1 - give RDD its own process selector. r?spohl!

Originally, RDD reused the GPU process selector since they were
using all the same services, and it reduced the number of places
that had to be touched. Now that RDD needs pref handling, it
needs its own process selector to avoid GPU inheriting pref
handling.

Comment 13

[Michael Froman [:mjf]]

Attachment: Bug 1539029 - pt 2 - add RDD process selector to prefs module. r?spohl!

Depends on D26566

Comment 14

[Michael Froman [:mjf]]

Attachment: Bug 1539029 - pt 3 - de-duplicate building cmdline for shared prefs class. r?kmag!

Cmd-line processing for the SharedPreferenceSerializer/Deserializer was
duplicated in ContentParent/ContentProcess and
SocketProcessHost/SocketProcessImpl. Since we'll have a 3rd process (RDD)
using this same code, let's move the repsonsiblity for knowing how to add
and parse these cmdline params into SharedPreferenceSerializer and
SharedPreferenceDeserializer.

Depends on D26567

Comment 15

[Michael Froman [:mjf]]

Attachment: Bug 1539029 - pt 4 - add shared pref serializer/deserializer to RDD process. r?kmag!

Depends on D26568

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f70ba445346
pt 1 - give RDD its own process selector. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/95e2d2baec44
pt 2 - add RDD process selector to prefs module. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/fcab7823cf27
pt 3 - de-duplicate building cmdline for shared prefs class. r=kmag
https://hg.mozilla.org/integration/autoland/rev/dd05584049c6
pt 4 - add shared pref serializer/deserializer to RDD process. r=kmag

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8f70ba445346
https://hg.mozilla.org/mozilla-central/rev/95e2d2baec44
https://hg.mozilla.org/mozilla-central/rev/fcab7823cf27
https://hg.mozilla.org/mozilla-central/rev/dd05584049c6