As a first step for bug 872980, we should deprecate any attempt to store more than 16kb of preferences in a field (or possibly in a non-root branch) and print a warning whenever code attempts to do this.
It might be possible to do this here: http://dxr.mozilla.org/mozilla-central/modules/libpref/src/prefapi.cpp#l704
Is this about the length of a single string type preference, or about the total size of preferences used by an extension? HTTPS-everywhere stores all its website rules as preferences - or at least it used to, I haven't used it in a while. I finally removed them from my prefs.js file manually a few days ago, making it significantly smaller.
If there is a simple way to determine the total size of preferences used by an extension, that's even better. If not, measuring single string type preference would be a good first step.
As a note, the initial patch to do a limit at all (which was set to 1MB for now) was done in bug 836263. And anyone storing a larger amount of data should probably switch to indexedDB, just like Crossrider did (see the bug I mentioned).
(In reply to David Rajchenbach Teller [:Yoric] from comment #3) > If there is a simple way to determine the total size of preferences used by > an extension, that's even better. If not, measuring single string type > preference would be a good first step. The bug that inspired this approach is caused by single preferences having too much data, so I think that's what we should be aiming for, rather than setting a quota per-extension. Also, this quote would not be very easy to implement, and very easy to circumvent.
This looks easy enough to do by following the example of https://bug836263.bugzilla.mozilla.org/attachment.cgi?id=743345 Let's make this a mentored bug.
Hopefully it is fixed now. Please verify it.
Aritra, it's a good start, although not exactly what I want. However, could you please post the fix as a patch? See https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch for more details.
Ah, well, I have nothing better to do while waiting for reviews. I'll try and produce a patch.
Created attachment 754763 [details] [diff] [review] Display a warning when setting a preference larger than 16kb. Here we go. Somewhere along the way, I have also fixed the patch to bug 836263 to ensure that it works also with setCharPref.
There's a typo in the warning message: shoudl.
Comment on attachment 754763 [details] [diff] [review] Display a warning when setting a preference larger than 16kb. 16k still seems excessive: are there normal prefs that exceed 4k or can we make the warning cutoff 4k? >diff --git a/modules/libpref/test/unit/test_warnings.js b/modules/libpref/test/unit/test_warnings.js >\ No newline at end of file pls fix
That's hard to tell. Some add-ons store lists of URLs in preferences and those can grow pretty quickly. I think we should aim to minimize add-on breakage. As far as I understand, this limit is well below what would cause stability problems, so I think it's a reasonable middle point.
This was a pretty arbitrary value, I am willing to lower it to 4kb. We can also lower it progressively, if we want to. @jorgev The issue here is not only stability but also performance. Writing anything non-trivial to pref slows down both start-up and further pref writes, so we would like to encourage add-on developers to migrate away from preferences for this kind of uses. So, any consensus on the cutoff length?
Let's go with 4k for now and we can back it off if we start seeing it too much. This is only a warning, so it isn't going to break anything yet!
Created attachment 757524 [details] [diff] [review] Display a warning when setting a preference larger than 4kb. Try: https://tbpl.mozilla.org/?tree=Try&rev=31f074a73030
Just a warning: removing addon-compat.
We still want to track this for developer outreach.
Created attachment 757602 [details] [diff] [review] Display a warning when setting a preference larger than 4kb, v2 Same one, with a null pointer check. Try: https://tbpl.mozilla.org/?tree=Try&rev=3486d64f6da9
(In reply to Jorge Villalobos [:jorgev] from comment #21) > We still want to track this for developer outreach. Ah, my bad, I had misinterpreted the use of the keyword.