2014568 - Prevent duplicate Setting objects from being created

Status: ASSIGNED

(Firefox :: Settings UI, defect, P3)

Description

[Mark Striemer [:mstriemer]]

In development we encountered a memory leak due to a Setting being registered multiple times (duplicate id). The Preferences.addSetting() method takes a SettingConfig and creates a Setting for it, but it does not perform any checks that this Setting hasn't been created already

Generally this doesn't cause any problems so it went unnoticed (likely the DOM node is connected to the first Setting and the additional Settings that get created just sit around without being accessed). However if the Setting has a setup() function, then we only call the teardown() for the last created Setting with that id. This causes a window leak if the setup() registers an observer.

It looks like we have a few places where we've accidentally registered the same id for multiple Settings. They all look benign and are likely consuming a little more memory than necessary, but they're simple wrappers for the same Preference object.

Comment 1

[Mark Striemer [:mstriemer]]

Attachment: WIP: Bug 2014568 - Prevent duplicate Setting objects from being created r?#settings-reviewers

Comment 2

[Stuart Colville [:muffinresearch]]

:scunnane - hi as a heads-up this bug has been moved to be post-MVP and since the blocker is fixed, we're working on the basis this isn't a blocker any more but shout if that's not the case.

Comment 3

[Stephanie Cunnane [:scunnane]]

Thanks for the head's up, Stuart. Not a blocker for any of the search settings work anymore - I landed a patch a while back that prevented creating extra Settings objects.

Comment 4

[Mark Striemer [:mstriemer]]

Attachment: Bug 2014568 - Prevent duplicate Setting objects from being created r?#settings-reviewers!

Comment 5

[BugBot [:suhaib / :marco]]

There is an r+ patch which didn't land and no activity in this bug for 1 week.
:mstriemer, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.