Automatically detect unused prefs
| Reporter | |
Description•6 years ago
|
We have so many prefs that it's not that hard to find dead ones. (E.g. bug 1572633 removes three.) It would be good to have a script that can detect dead ones.
Consider a pref named foo.bar-baz.
- If it only appears in the standard pref data files (
all.js,firefox.js,mobile.js,geckoview-prefs.js, plus a few other small ones), the script should look for occurrences of"foo.bar-baz"within C++ code and JS code. - If it appears in
StaticPrefList.yaml, we should look for the above, and also for occurrences ofStaticPrefs::foo_bar_baz()in C++ code andpref!("foo.bar-baz")in Rust code.
If there are no hits, there's a good chance the pref can be removed. For case 1 some care is needed; it's possible that the searching may miss some weird cases, and erroneous removal of a pref could change behaviour. For case 2 things are easier; erroneous removal of a pref should cause a compile error.
It's also possible for a pref in StaticPrefList.yaml to be marked with a mirror value of always or once when it should be never, because it's not accessed via a C++ or Rust getter. The script could potentially also detect these cases.
This bug is related to bug 1566315, which is about detecting duplicate definitions of prefs.
Comment 1•6 years ago
|
A few variations (just on the C++ side) on weird situations we'd need to handle for case 1:
// variations on using #defines
bool useBaselineInterp = Preferences::GetBool(JS_OPTIONS_DOT_STR "blinterp");
if (NS_SUCCEEDED(Preferences::GetBool(TRR_PREF("wait-for-portal"), &tmp))) {
if (Preferences::GetBool(GFX_MISSING_FONTS_NOTIFY_PREF)) {
// constants
mFileHandleDisabled = !Preferences::GetBool(kPrefFileHandleEnabled);
bool cipherEnabled =
Preferences::GetBool(cp[i].pref, cp[i].enabledByDefault);
// variations on building up a nsCString
nsAutoCString provider(aProvider);
nsPrintfCString reportEnablePref(
"browser.safebrowsing.provider.%s.dataSharing.enabled", provider.get());
if (!Preferences::GetBool(reportEnablePref.get(), false)) {
nsCString prefName(kPrefLoadInParentPrefix);
prefName += aMimeType;
return Preferences::GetBool(prefName.get(), false);
MAKE_FONT_PREF_KEY(pref, "font.default.", langGroup);
Preferences::GetCString(pref.get(), value);
// pass-throughs
bool nsGlobalWindowOuter::CanSetProperty(const char* aPrefName) {
// Chrome can set any property.
if (nsContentUtils::LegacyIsCallerChromeOrNativeCode()) {
return true;
}
// If the pref is set to true, we can not set the property
// and vice versa.
return !Preferences::GetBool(aPrefName, true);
}
// Weird pref name builders
Preferences::GetString(NamePref(generic, langGroup).get(), fontlistValue);
Comment 2•6 years ago
|
Plus all the cases where the caller gets a branch and asks for the pref relative to the branch.
| Reporter | |
Comment 3•6 years ago
|
These are all good examples of tricky cases. Because pref removal can be tricky, reviews for pref removal are best handled by peers of the relevant submodule, rather than peers of libpref.
Updated•3 years ago
|
