VOOZH about

URL: https://bugzilla.mozilla.org/show_bug.cgi?id=1573067

⇱ 1573067 - Automatically detect unused prefs


Open Bug 1573067 Opened 6 years ago Updated 3 years ago

Automatically detect unused prefs

Automatically detect unused prefs
Core
Preferences: Backend
unspecified
Unspecified
Unspecified
task
Points:
---
NEW
---
Iteration:
---
a11y-review
Accessibility Severity
Performance Impact
Size Estimate
Webcompat Priority
Webcompat Score
Tracking Status
relnote-firefox
thunderbird_esr115
thunderbird_esr140
firefox-esr115
firefox-esr140
firefox-esr153
firefox152
firefox153
firefox154
---
QA Whiteboard:
---
Has STR:
---
Change Request:
---
Bug Flags:
Signature:
None
This bug is publicly visible.

 

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.

  1. 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.
  2. If it appears in StaticPrefList.yaml, we should look for the above, and also for occurrences of StaticPrefs::foo_bar_baz() in C++ code and pref!("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.

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);

Plus all the cases where the caller gets a branch and asks for the pref relative to the branch.

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.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.