Closed Bug 1637727 Opened 4 years ago Closed 4 years ago

Convert all the Varcache prefs in netwerk/*

Categories

(Core :: Preferences: Backend, task)

task

Tracking

()

RESOLVED FIXED

People

(Reporter: KrisWright, Assigned: Gankra)

References

Details

Attachments

(7 files)

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: nobody → a.beingessner
Status: NEW → ASSIGNED

converts:

  • network.ssl_tokens_cache_enabled
  • network.ssl_tokens_cache_capacity

Depends on D77102

converts:

  • security.data_uri.block_toplevel_data_uri_navigations
  • network.offline-mirrors-connectivity

Depends on D77103

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

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

Flags: needinfo?(odvarko)
Flags: needinfo?(odvarko) → needinfo?(honzab.moz)

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.

Flags: needinfo?(honzab.moz) → needinfo?(michal.novotny)

(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.

Flags: needinfo?(michal.novotny)

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

(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.

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.

Keywords: leave-open
Pushed by abeingessner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/84ee5eb2a198 convert network.security.esni.enabled to a StaticPref. r=KrisWright,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/e43b4b5a8305 convert network.ssl_tokens_cache prefs to StaticPrefs. r=KrisWright,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/f95a75cedb9e convert nsIOService prefs to StaticPrefs. r=KrisWright,necko-reviewers https://hg.mozilla.org/integration/autoland/rev/98398b5b5021 convert network.standard-url.punycode-host to a StaticPref. r=necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/2ce5012804f4 convert network.standard-url.max-length to a StaticPref. r=KrisWright,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/ae8ea9f9df5e convert network.http.sanitize-headers-in-logs to a StaticPref. r=KrisWright,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/3a6ad7dde824 convert network.http.rcwn prefs to StaticPrefs. r=KrisWright,necko-reviewers,valentin

Closing this in favour of followup work being tracked in Bug 1572534

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: