1505941 - Prefs created very early in startup cannot be deleted with Services.prefs.deleteBranch

Status: NEW

(Core :: Preferences: Backend, defect, P3)

Description

[Michael Cooper [:mythmon]]

This was discovered as the root cause in bug 1502410, and is a regression introduced by bug 1471025. The problem in Normandy reported in 1502410 has been worked around by not using deleteBranch, but the underlying problem still exists, and may affect other users.

I don't fully understand the causes of this problem. My theory is that it is triggered when a preference is created by code before content processes have been created. In this case, calling deleteBranch on that preference will reset it to whatever it was set to during early startup, instead of removing it.

# STR

Setting preferences in the relevant time period is tricky. Here we use a combination of abusing a mechanism Normandy uses, and some code in the browser console to illustrate the problem, but there are other ways as well. See the STR in bug 1502410 for a more realistic way (though keep in mind that it won't work once bug 1502410's fix lands).

1. Set "app.normandy.startupExperimentPrefs.foo.bar.baz" to any value on the user branch. This will trigger Normandy to set the preference "foo.bar.baz" on the default branch to have the value set. "foo.bar.baz" can be replaced with any valid preference name that does't already exist.
2. Restart Firefox.
3. Observe that "foo.bar.baz" has been set by Normandy to the value given in step 1.
4. In a browser console, run the following code:

   ChromeUtils.import("resource://gre/modules/Services.jsm");
   Services.prefs.getDefaultBranch("").deleteBranch("foo.bar.baz");

Expected results:

Prior to bug 1471025, the preference foo.bar.baz is deleted. It does not show up in about:config, and calling Services.prefs.getPrefType("foo.bar.baz") returns Services.prefs.PREF_INVALID.

Actual results:

After bug 1471025, the preference is not affected. It remains in place, and continues to have the type and value assigned to it in step 3 above. This is counter to the documentation of deleteBranch.

Comment 1

[Kris Maglione [:kmag]]

I think we probably just want to get rid of resetBranch (which is unimplemented) and deleteBranch at this point. They're really designed with the idea of using preferences as an arbitrary key-value store, which we generally want to discourage at this point.

It *may* make sense to add a method to clear a single preference default value, but I'd rather not, for the same reason. That encourages use as an arbitrary key-value store in itself. We really want the set of managed preferences to be as close to static as possible.

Comment 2

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Updating tracking (as was set in bug 1502410)

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Kris, as it is marked blocking for 63, I am not sure that the severity should be minor. Could you explain why you reverted my change?

Comment 4

[Kris Maglione [:kmag]]

This shouldn't block 63. This only matters if the API has consumers, which, after the inbound Normandy changes, it shouldn'tn

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Given that Normandy was the only feature hitting this issue and we since worked around the problem within it, I don't think this issue needs to be tracked even though it still seems like a footgun we don't want to forget about.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.