Bug 744626 - ensure default prefs are synced such that they remain as default on other devices. r=rnewman
authorMark Hammond <mhammond@skippinet.com.au>
Tue, 15 Mar 2016 16:24:15 +1100
changeset 288570 eb2c4646b5a6a3d8b2c5f86d47d0d32fc2be771c
parent 288569 947c1a6bee31e13736c8a6a6739ccdca3c641397
child 288571 d0c962286d136bb4a78599661582a1a4bd135b79
push id18171
push usermhammond@skippinet.com.au
push dateTue, 15 Mar 2016 05:39:16 +0000
treeherderfx-team@eb2c4646b5a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs744626
milestone48.0a1
Bug 744626 - ensure default prefs are synced such that they remain as default on other devices. r=rnewman
services/sync/modules/engines/prefs.js
services/sync/tests/unit/prefs_test_prefs_store.js
services/sync/tests/unit/test_prefs_store.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/modules/engines/prefs.js
+++ b/services/sync/modules/engines/prefs.js
@@ -98,18 +98,18 @@ PrefStore.prototype = {
     return pref.startsWith(PREF_SYNC_PREFS_PREFIX) ||
            this._prefs.get(PREF_SYNC_PREFS_PREFIX + pref, false);
   },
 
   _getAllPrefs: function () {
     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;
   },
 
   _setAllPrefs: function (values) {
     let selectedThemeIDPref = "lightweightThemes.selectedThemeID";
     let selectedThemeIDBefore = this._prefs.get(selectedThemeIDPref, null);
new file mode 100644
--- /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");
--- a/services/sync/tests/unit/test_prefs_store.js
+++ b/services/sync/tests/unit/test_prefs_store.js
@@ -18,35 +18,32 @@ function makePersona(id) {
   return {
     id: id || Math.random().toString(),
     name: Math.random().toString(),
     headerURL: "http://localhost:1234/a"
   };
 }
 
 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);
     do_check_eq(ids.length, 1);
     do_check_eq(ids[0], PREFS_GUID);
     do_check_true(allIDs[PREFS_GUID], true);
 
     do_check_true(store.itemExists(PREFS_GUID));
@@ -56,27 +53,32 @@ function run_test() {
     let record = store.createRecord("random-gibberish", "prefs");
     do_check_true(record.deleted);
 
     _("Prefs record contains only prefs that should be synced.");
     record = store.createRecord(PREFS_GUID, "prefs");
     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!");
     record = new PrefRec("prefs", PREFS_GUID);
     record.value = {
       "testing.int": 42,
       "testing.string": "im in ur prefs",
       "testing.bool": false,
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -164,16 +164,17 @@ skip-if = debug
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_places_guid_downgrade.js]
 [test_password_store.js]
 [test_password_tracker.js]
 # 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]
 [test_tab_tracker.js]
 
 [test_warn_on_truncated_response.js]
 
 # FxA migration