[meta] Convert all VarCache prefs to use StaticPrefs
Categories
(Core :: General, task)
Tracking
()
People
(Reporter: n.nethercote, Unassigned)
References
Details
(Keywords: meta)
Reporter | ||
Updated•7 years ago
|
Reporter | ||
Updated•7 years ago
|
Comment hidden (typo) |
Reporter | ||
Comment 3•5 years ago
|
||
KrisWright: If you are interested in doing some conversions, a good place to start would be TimoutManager::Initialize()
in dom/base/TimeoutManager.cpp
. There are 13 VarCache prefs there that look straightforward.
The following steps should work for most (but not all) VarCache prefs.
- Find an existing VarCache pref to convert, by searching for
Add*VarCache
calls. - Look for the pref's entry in a prefs data file, usually
modules/libpref/init/all.js
. (You won't always find one.) - Add a new entry to
modules/libpref/init/StaticPrefList.yaml
, based on what you found in the above two steps.- Note that sometimes the default value given in those two locations won't agree, in which case you probably should use the
all.js
one.
- Note that sometimes the default value given in those two locations won't agree, in which case you probably should use the
- Remove the entry from
all.js
, if present. - Remove the
Add*VarCache
call, along with the mirror variable it used, and any accompanying stuff such as:- any constant for the default value;
- any static bool being used to prevent
Add*VarCache
from being called more than once.
- Replace uses of the mirror variable with
StaticPrefs::foo_bar_baz()
.- If there are no such uses, give it a
mirror
value ofnever
. - Unless the pref is never used at all, in which case remove it!
- If there are no such uses, give it a
- Also look for any entries in other prefs files, particularly
mobile/android/app/mobile.js
orbrowser/app/profile/firefox.js
. If they are equivalent to theStaticPrefList.yaml
entry, then remove them. (This is surprisingly common.) Or if they can be merged into theStaticPrefList.yaml
entry (e.g. with#ifdef ANDROID
), then do that.
It's usually best to do one pref per commit, because it makes reviewing and bisecting any regressions easier. Sometimes you might have two or three tightly-coupled prefs, in which case doing them in a single commit might be better.
You can look at commits in bugs blocking this bug for examples of previous conversions. They all predate the introduction of StaticPrefList.yaml
, and therefore add the new static pref to StaticPrefList.h
instead, but otherwise they'll match the steps above.
Comment 4•5 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #3)
KrisWright: If you are interested in doing some conversions, a good place to start would be
TimoutManager::Initialize()
indom/base/TimeoutManager.cpp
. There are 13 VarCache prefs there that look straightforward.
Thanks for all of the info! I filed bug 1569004 to get started on this.
Reporter | ||
Comment 5•5 years ago
|
||
The following is a list of prefs for which we call Add*VarCache()
when the pref doesn't even exist yet. (This is just during browser startup; there may be additional cases when running different workloads.) In that case, the VarCache variable takes the default value, and if the pref is created later on then the VarCache variable will be updated. It's a pretty gross pattern, and the kind of thing I want to outlaw, so these ones should be considered higher priority cases to convert to static prefs. I will convert privacy.firstparty.isolate.block_post_message
myself because it's causing problems for me in bug 1569526.
BAD browser.cache.disk.telemetry_report_ID
BAD browser.tabs.remote.force-paint
BAD content.cors.disable
BAD content.cors.no_private_data
BAD content.notify.backoffcount
BAD content.notify.interval
BAD content.notify.ontimer
BAD content.sink.enable_perf_mode
BAD content.sink.event_probe_rate
BAD content.sink.initial_perf_time
BAD content.sink.interactive_deflect_count
BAD content.sink.interactive_parse_time
BAD content.sink.interactive_time
BAD content.sink.perf_deflect_count
BAD content.sink.perf_parse_time
BAD dom.allow_XUL_XBL_for_file
BAD dom.disable_window_print
BAD dom.ipc.processPrelaunch.delayMs
BAD dom.ipc.processPriorityManager.backgroundGracePeriodMS
BAD dom.ipc.processPriorityManager.backgroundPerceivableGracePeriodMS
BAD dom.ipc.processPriorityManager.enabled
BAD dom.ipc.processPriorityManager.testMode
BAD dom.ipc.tabs.disabled
BAD dom.largeAllocation.testing.allHttpLoads
BAD dom.quotaManager.temporaryStorage.chunkSize
BAD dom.quotaManager.temporaryStorage.fixedLimit
BAD dom.securecontext.whitelist_onions
BAD dom.testing.sync-content-blocking-notifications
BAD layout.framevisibility.amountscrollbeforeupdatehorizontal
BAD layout.framevisibility.amountscrollbeforeupdatevertical
BAD layout.reflow.synthMouseMove
BAD media.cloneElementVisually.testing
BAD mozilla.widget.disable-native-theme
BAD network.dns.disablePrefetchFromHTTPS
BAD privacy.firstparty.isolate.block_post_message
BAD privacy.fuzzyfox.clockgrainus
BAD privacy.resistFingerprinting.target_video_res
BAD privacy.resistFingerprinting.video_dropped_ratio
BAD privacy.resistFingerprinting.video_frames_per_sec
BAD security.all_resource_uri_content_accessible
BAD security.turn_off_all_security_so_that_viruses_can_take_over_this_computer
BAD toolkit.telemetry.ipcBatchTimeout
BAD ui.mouse.radius.reposition
BAD vsync.parentProcess.highPriority
Reporter | ||
Updated•5 years ago
|
Comment 6•5 years ago
|
||
I've encountered some crashes related to the changes here that weren't reported because of a bug in the crash reporting machinery (see bug 1566855). The top frames of the stack look like this:
0 libxul.so!mozilla::StaticPrefs::InitStaticPrefsFromShared() [StaticPrefList_dom.h:b0124f06562982dce60b820d95aad23afd5cec90 : 1261 + 0x11]
1 libxul.so!mozilla::ipc::SharedPreferenceDeserializer::DeserializeFromSharedMemory(char*, char*, char*, char*)
2 libxul.so!mozilla::dom::ContentProcess::Init(int, char**) [ContentProcess.cpp:b0124f06562982dce60b820d95aad23afd5cec90 : 174 + 0x17]
3 libxul.so!XRE_InitChildProcess(int, char**, XREChildData const*) [nsEmbedFunctions.cpp:b0124f06562982dce60b820d95aad23afd5cec90 : 724 + 0xf]
The crash is NULL-pointer dereference. The entry that seems to correspond to the crash is dom.webdriver.enabled
.
Updated•4 years ago
|
Comment 8•4 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #7)
Kris, are we officially done here?
Yes, it looks like we have removed all of them! All that's left is deleting VarCache itself. Since there are no more prefs left to convert, I am going to go ahead and mark this bug as fixed.
Comment 9•4 years ago
|
||
Bravo!
Description
•