From bfaefeb591266dd325809febac76d76ebe145a12 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Tue, 15 Mar 2016 16:24:15 +1100 Subject: [PATCH] Bug 744626 - ensure default prefs are synced such that they remain as default on other devices. r=rnewman --- services/sync/modules/engines/prefs.js | 4 +- .../sync/tests/unit/prefs_test_prefs_store.js | 25 +++++++++++ services/sync/tests/unit/test_prefs_store.js | 44 ++++++++++--------- services/sync/tests/unit/xpcshell.ini | 1 + 4 files changed, 51 insertions(+), 23 deletions(-) create mode 100644 services/sync/tests/unit/prefs_test_prefs_store.js diff --git a/services/sync/modules/engines/prefs.js b/services/sync/modules/engines/prefs.js index e801f0147dfd..1cf68173f79b 100644 --- a/services/sync/modules/engines/prefs.js +++ b/services/sync/modules/engines/prefs.js @@ -103,8 +103,8 @@ PrefStore.prototype = { let values = {}; for (let pref of this._getSyncPrefs()) { if (this._isSynced(pref)) { - // Missing prefs get the null value. - values[pref] = this._prefs.get(pref, null); + // Missing and default prefs get the null value. + values[pref] = this._prefs.isSet(pref) ? this._prefs.get(pref, null) : null; } } return values; diff --git a/services/sync/tests/unit/prefs_test_prefs_store.js b/services/sync/tests/unit/prefs_test_prefs_store.js new file mode 100644 index 000000000000..109757a35c02 --- /dev/null +++ b/services/sync/tests/unit/prefs_test_prefs_store.js @@ -0,0 +1,25 @@ +// This is a "preferences" file used by test_prefs_store.js + +// The prefs that control what should be synced. +// Most of these are "default" prefs, so the value itself will not sync. +pref("services.sync.prefs.sync.testing.int", true); +pref("services.sync.prefs.sync.testing.string", true); +pref("services.sync.prefs.sync.testing.bool", true); +pref("services.sync.prefs.sync.testing.dont.change", true); +// this one is a user pref, so it *will* sync. +user_pref("services.sync.prefs.sync.testing.turned.off", false); +pref("services.sync.prefs.sync.testing.nonexistent", true); +pref("services.sync.prefs.sync.testing.default", true); + +// The preference values - these are all user_prefs, otherwise their value +// will not be synced. +user_pref("testing.int", 123); +user_pref("testing.string", "ohai"); +user_pref("testing.bool", true); +user_pref("testing.dont.change", "Please don't change me."); +user_pref("testing.turned.off", "I won't get synced."); +user_pref("testing.not.turned.on", "I won't get synced either!"); + +// A pref that exists but still has the default value - will be synced with +// null as the value. +pref("testing.default", "I'm the default value"); diff --git a/services/sync/tests/unit/test_prefs_store.js b/services/sync/tests/unit/test_prefs_store.js index d6fd100db795..9f2aed3d1dc2 100644 --- a/services/sync/tests/unit/test_prefs_store.js +++ b/services/sync/tests/unit/test_prefs_store.js @@ -23,25 +23,22 @@ function makePersona(id) { } function run_test() { + _("Test fixtures."); + // read our custom prefs file before doing anything. + Services.prefs.readUserPrefs(do_get_file("prefs_test_prefs_store.js")); + // Now we've read from this file, any writes the pref service makes will be + // back to this prefs_test_prefs_store.js directly in the obj dir. This + // upsets things in confusing ways :) We avoid this by explicitly telling the + // pref service to use a file in our profile dir. + let prefFile = do_get_profile(); + prefFile.append("prefs.js"); + Services.prefs.savePrefFile(prefFile); + Services.prefs.readUserPrefs(prefFile); + let store = Service.engineManager.get("prefs")._store; let prefs = new Preferences(); try { - _("Test fixtures."); - Svc.Prefs.set("prefs.sync.testing.int", true); - Svc.Prefs.set("prefs.sync.testing.string", true); - Svc.Prefs.set("prefs.sync.testing.bool", true); - Svc.Prefs.set("prefs.sync.testing.dont.change", true); - Svc.Prefs.set("prefs.sync.testing.turned.off", false); - Svc.Prefs.set("prefs.sync.testing.nonexistent", true); - - prefs.set("testing.int", 123); - prefs.set("testing.string", "ohai"); - prefs.set("testing.bool", true); - prefs.set("testing.dont.change", "Please don't change me."); - prefs.set("testing.turned.off", "I won't get synced."); - prefs.set("testing.not.turned.on", "I won't get synced either!"); - _("The GUID corresponds to XUL App ID."); let allIDs = store.getAllIDs(); let ids = Object.keys(allIDs); @@ -61,17 +58,22 @@ function run_test() { do_check_eq(record.value["testing.int"], 123); do_check_eq(record.value["testing.string"], "ohai"); do_check_eq(record.value["testing.bool"], true); + // non-existing prefs get null as the value do_check_eq(record.value["testing.nonexistent"], null); + // as do prefs that have a default value. + do_check_eq(record.value["testing.default"], null); do_check_false("testing.turned.off" in record.value); do_check_false("testing.not.turned.on" in record.value); - _("Prefs record contains pref sync prefs too."); - do_check_eq(record.value["services.sync.prefs.sync.testing.int"], true); - do_check_eq(record.value["services.sync.prefs.sync.testing.string"], true); - do_check_eq(record.value["services.sync.prefs.sync.testing.bool"], true); - do_check_eq(record.value["services.sync.prefs.sync.testing.dont.change"], true); + _("Prefs record contains non-default pref sync prefs too."); + do_check_eq(record.value["services.sync.prefs.sync.testing.int"], null); + do_check_eq(record.value["services.sync.prefs.sync.testing.string"], null); + do_check_eq(record.value["services.sync.prefs.sync.testing.bool"], null); + do_check_eq(record.value["services.sync.prefs.sync.testing.dont.change"], null); + // but this one is a user_pref so *will* be synced. do_check_eq(record.value["services.sync.prefs.sync.testing.turned.off"], false); - do_check_eq(record.value["services.sync.prefs.sync.testing.nonexistent"], true); + do_check_eq(record.value["services.sync.prefs.sync.testing.nonexistent"], null); + do_check_eq(record.value["services.sync.prefs.sync.testing.default"], null); _("Update some prefs, including one that's to be reset/deleted."); Svc.Prefs.set("testing.deleteme", "I'm going to be deleted!"); diff --git a/services/sync/tests/unit/xpcshell.ini b/services/sync/tests/unit/xpcshell.ini index d018f7fd13ab..831e0c367298 100644 --- a/services/sync/tests/unit/xpcshell.ini +++ b/services/sync/tests/unit/xpcshell.ini @@ -169,6 +169,7 @@ skip-if = debug # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479) skip-if = debug [test_prefs_store.js] +support-files = prefs_test_prefs_store.js [test_prefs_tracker.js] [test_tab_engine.js] [test_tab_store.js]