Convert all the Varcache prefs in netwerk/*
Categories
(Core :: Preferences: Backend, task)
Tracking
()
People
(Reporter: KrisWright, Assigned: Gankra)
References
Details
Attachments
(7 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
This bug converts the following prefs:
network.security.esni.enabled
network.ssl_tokens_cache_enabled
network.ssl_tokens_cache_capacity
security.data_uri.block_toplevel_data_uri_navigations
network.offline-mirrors-connectivity
network.standard-url.punycode-host
network.standard-url.max-length
browser.cache.disk.capacity
browser.cache.disk.amount_written
network.http.sanitize-headers-in-logs
network.http.rcwn.enabled
network.http.rcwn.cache_queue_normal_threshold
network.http.rcwn.cache_queue_priority_threshold
network.http.rcwn.small_resource_size_kb
network.http.rcwn.min_wait_before_racing_ms
network.http.rcwn.max_wait_before_racing_ms
Assignee | ||
Comment 1•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
converts:
- network.ssl_tokens_cache_enabled
- network.ssl_tokens_cache_capacity
Depends on D77102
Assignee | ||
Comment 3•4 years ago
|
||
converts:
- security.data_uri.block_toplevel_data_uri_navigations
- network.offline-mirrors-connectivity
Depends on D77103
Assignee | ||
Comment 4•4 years ago
|
||
Depends on D77104
Assignee | ||
Comment 5•4 years ago
|
||
Depends on D77105
Assignee | ||
Comment 6•4 years ago
|
||
Depends on D77106
Assignee | ||
Comment 7•4 years ago
|
||
converts:
- network.http.rcwn.enabled
- network.http.rcwn.cache_queue_normal_threshold
- network.http.rcwn.cache_queue_priority_threshold
- network.http.rcwn.small_resource_size_kb
- network.http.rcwn.min_wait_before_racing_ms
- network.http.rcwn.max_wait_before_racing_ms
Depends on D77107
Assignee | ||
Comment 8•4 years ago
|
||
These patches convert everything listed except for
browser.cache.disk.capacity
browser.cache.disk.amount_written
which require additional investigation. Specifically these two prefs are programmatically written to with mozilla::Preferences::SetInt, and it's unclear what the intended purpose of that is. (Also amount_written is clearly not intended to be touched by users; it's not even in all.js)
:honza, any thoughts/insight on what's up with these prefs?
try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=543955ec1c1a19d0e4651a15949149cdd7930d9f
Updated•4 years ago
|
Comment 9•4 years ago
|
||
browser.cache.disk.capacity
browser.cache.disk.amount_written
I looked at these a while back. I can't find the bug now, but I concluded that they are sufficient weird that they should be converted to use libpref's general callback mechanism (Preferences::RegisterCallback(), etc.) rather than trying to shoehorn them into static prefs.
Updated•4 years ago
|
Comment 10•4 years ago
|
||
(In reply to Alexis Beingessner [:Gankra] from comment #8)
These patches convert everything listed except for
browser.cache.disk.capacity
This pref is overwritten when smart sizing is enabled. We could probably change the code so that we would keep the calculated cache capacity only in memory.
browser.cache.disk.amount_written
which require additional investigation. Specifically these two prefs are programmatically written to with mozilla::Preferences::SetInt, and it's unclear what the intended purpose of that is. (Also amount_written is clearly not intended to be touched by users; it's not even in all.js)
Right, this pref is not supposed to be changed by user. We should probably use some different storage to keep number of bytes written to the cache persistent across restart.
Assignee | ||
Comment 11•4 years ago
|
||
try push for current patches -- nervous about all the bc (browser chrome) reftests that seem super flaky on all my trys: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1479edad5628aaf11fa83a8084e152234c1f4236&selectedTaskRun=RRStGvmDTpK5cac192sXMA-0
Reporter | ||
Comment 12•4 years ago
|
||
(In reply to Michal Novotny [:michal] from comment #10)
(In reply to Alexis Beingessner [:Gankra] from comment #8)
Right, this pref is not supposed to be changed by user. We should probably use some different storage to keep number of bytes written to the cache persistent across restart.
If we want to use a pref for this but not one that the user can change, there is a sticky
attribute that you can use. Just add the pref to all.js
as well as the static pref list. This will return a linting failure, so you also want to add it to IGNORE_PREFS as well.
Comment 13•4 years ago
|
||
If we want to use a pref for this but not one that the user can change, there is a
sticky
attribute that you can use.
Hmm, not quite... Normally if a pref's user value is set to be the same as the default value, then the user value is removed, i.e. the pref is effectively reset. However, if a pref is sticky, that won't happen, and it will keep the user value. (This means that if the default value later changes, the user value will persist.) It's a very niche behaviour.
You might be thinking of locked
prefs, which users can't change. But I'd prefer not to use that either. Both of these prefs really go against the spirit of how prefs are supposed to work. I like Michal's suggestions for both of them, which avoid prefs being written by Gecko code. So if a netwerk person could make those changes soon, that would be great. But if it gets to the point where these two prefs are blocking the removal of the VarCache code, then I suggest using the general callback machinery.
Assignee | ||
Updated•4 years ago
|
Comment 14•4 years ago
|
||
Comment 15•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/84ee5eb2a198
https://hg.mozilla.org/mozilla-central/rev/e43b4b5a8305
https://hg.mozilla.org/mozilla-central/rev/f95a75cedb9e
https://hg.mozilla.org/mozilla-central/rev/98398b5b5021
https://hg.mozilla.org/mozilla-central/rev/2ce5012804f4
https://hg.mozilla.org/mozilla-central/rev/ae8ea9f9df5e
https://hg.mozilla.org/mozilla-central/rev/3a6ad7dde824
Assignee | ||
Comment 16•4 years ago
|
||
Closing this in favour of followup work being tracked in Bug 1572534
Updated•4 years ago
|
Description
•