539586 - changes to settings not propagated across SettingsStore instances

Status: RESOLVED FIXED

(Mozilla Labs :: Jetpack Prototype, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: fixes problem

If a feature makes a change to a setting through an instance of SettingsStore, and another instance of SettingsStore exists for the feature, the change doesn't propagate to the other instance.

We could fix this by writing code to do the propagation, either by sending a signal to reload the settings from disk (after syncing the change to disk) or by sending a signal with the property name and new value.

Another option is to cache and reuse instances, so there is only one instance per feature.  This seems preferable, because it is simpler, cheaper (both to implement and in terms of memory consumption) and because there is no utility to having multiple instances of the object for a given feature.

Here's a patch that implements the latter approach.  It uses a JavaScript module for cache persistence across JavaScript contexts.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: suppresses bogus memory leak error; removes store from cache

This version suppresses a bogus memory leak error that the test harness generates and removes the SettingsStore instance from the cache when the feature gets unloaded.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: one additional assertion

Here's patch v2 with one additional assertion that Drew asked for over IRC to make sure that the values in the two stores are equal after both set operations.

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 421558 [details] [diff] [review]
patch v3: one additional assertion

r=adw

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Fixed by changeset http://hg.mozilla.org/labs/jetpack/rev/165d6068bee1.