Bug 744626 - ensure default prefs are synced such that they remain as default on other devices. r=rnewman

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;

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

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

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]