Bug 1377246 - Always filter Sync metadata from profiles. r=markh,lchang
authorKit Cambridge <kit@yakshaving.ninja>
Tue, 11 Jul 2017 16:26:19 -0700
changeset 418648 1051372286caa087190b3f77f6246e50c4513ea5
parent 418647 d9fcc95f6c426f7de89c8cde847581492c9229e5
child 418649 b6a93a94e7d012ed281a84addcf30f7a02ca3b28
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmarkh, lchang
bugs1377246
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1377246 - Always filter Sync metadata from profiles. r=markh,lchang We don't want to expose Sync metadata via `{rawData: true}` for two reasons. First, Sync's `createRecord` method passes this option to filter computed fields. We don't want to include computed fields in the record, but we also don't want to upload the Sync metadata. Second, `_clone` uses a shallow copy, so the `_sync` object can still be mutated. The only callers that rely on `{rawData: true}` returning `_sync` are tests, so we can add a test-only helper instead of exposing the object. MozReview-Commit-ID: DudYwSiEgJE
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/head.js
browser/extensions/formautofill/test/unit/test_addressRecords.js
browser/extensions/formautofill/test/unit/test_storage_syncfields.js
browser/extensions/formautofill/test/unit/test_sync.js
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -459,17 +459,17 @@ class AutofillRecords {
   get(guid, {rawData = false} = {}) {
     this.log.debug("get:", guid, rawData);
     let recordFound = this._findByGUID(guid);
     if (!recordFound) {
       return null;
     }
 
     // The record is cloned to avoid accidental modifications from outside.
-    let clonedRecord = this._clone(recordFound, {rawData});
+    let clonedRecord = this._clone(recordFound);
     if (rawData) {
       this._stripComputedFields(clonedRecord);
     } else {
       this._recordReadProcessor(clonedRecord);
     }
     return clonedRecord;
   }
 
@@ -483,17 +483,17 @@ class AutofillRecords {
    * @returns {Array.<Object>}
    *          An array containing clones of all records.
    */
   getAll({rawData = false, includeDeleted = false} = {}) {
     this.log.debug("getAll", rawData, includeDeleted);
 
     let records = this._store.data[this._collectionName].filter(r => !r.deleted || includeDeleted);
     // Records are cloned to avoid accidental modifications from outside.
-    let clonedRecords = records.map(r => this._clone(r, {rawData}));
+    let clonedRecords = records.map(r => this._clone(r));
     clonedRecords.forEach(record => {
       if (rawData) {
         this._stripComputedFields(record);
       } else {
         this._recordReadProcessor(record);
       }
     });
     return clonedRecords;
@@ -695,24 +695,21 @@ class AutofillRecords {
     }
     return record._sync;
   }
 
   /**
    * Internal helper functions.
    */
 
-  _clone(record, {rawData = false} = {}) {
-    let result = Object.assign({}, record);
-    if (rawData) {
-      return result;
-    }
-    for (let key of Object.keys(result)) {
-      if (key.startsWith("_")) {
-        delete result[key];
+  _clone(record) {
+    let result = {};
+    for (let key in record) {
+      if (!key.startsWith("_")) {
+        result[key] = record[key];
       }
     }
     return result;
   }
 
   _findByGUID(guid, {includeDeleted = false} = {}) {
     let found = this._findIndexByGUID(guid, {includeDeleted});
     return found < 0 ? undefined : this._store.data[this._collectionName][found];
--- a/browser/extensions/formautofill/test/unit/head.js
+++ b/browser/extensions/formautofill/test/unit/head.js
@@ -1,14 +1,14 @@
 /**
  * Provides infrastructure for automated formautofill components tests.
  */
 
 /* exported getTempFile, loadFormAutofillContent, runHeuristicsTest, sinon,
- *          initProfileStorage
+ *          initProfileStorage, getSyncChangeCounter
  */
 
 "use strict";
 
 var {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
@@ -146,16 +146,41 @@ function runHeuristicsTest(patterns, fix
           expectedField.elementWeakRef = field.elementWeakRef;
           Assert.deepEqual(field, expectedField);
         });
       });
     });
   });
 }
 
+/**
+ * Returns the Sync change counter for a profile storage record. Synced records
+ * store additional metadata for tracking changes and resolving merge conflicts.
+ * Deleting a synced record replaces the record with a tombstone.
+ *
+ * @param   {AutofillRecords} records
+ *          The `AutofillRecords` instance to query.
+ * @param   {string} guid
+ *          The GUID of the record or tombstone.
+ * @returns {number}
+ *          The change counter, or -1 if the record doesn't exist or hasn't
+ *          been synced yet.
+ */
+function getSyncChangeCounter(records, guid) {
+  let record = records._findByGUID(guid, {includeDeleted: true});
+  if (!record) {
+    return -1;
+  }
+  let sync = records._getSyncMetaData(record);
+  if (!sync) {
+    return -1;
+  }
+  return sync.changeCounter;
+}
+
 add_task(async function head_initialize() {
   Services.prefs.setBoolPref("extensions.formautofill.experimental", true);
   Services.prefs.setBoolPref("extensions.formautofill.heuristics.enabled", true);
   Services.prefs.setBoolPref("dom.forms.autocomplete.experimental", true);
 
   // Clean up after every test.
   do_register_cleanup(function head_cleanup() {
     Services.prefs.clearUserPref("extensions.formautofill.experimental");
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js
@@ -149,23 +149,16 @@ add_task(async function test_getAll() {
   addresses = profileStorage.addresses.getAll({rawData: true});
   do_check_eq(addresses[0].name, undefined);
   do_check_eq(addresses[0]["address-line1"], undefined);
   do_check_eq(addresses[0]["address-line2"], undefined);
 
   // Modifying output shouldn't affect the storage.
   addresses[0].organization = "test";
   do_check_record_matches(profileStorage.addresses.getAll()[0], TEST_ADDRESS_1);
-
-  // Test with rawData.
-  profileStorage.addresses.pullSyncChanges(); // force sync metadata, which is what we are checking.
-  addresses = profileStorage.addresses.getAll({rawData: true});
-  Assert.ok(addresses[0]._sync && addresses[1]._sync);
-  Assert.equal(addresses[0]._sync.changeCounter, 1);
-  Assert.equal(addresses[1]._sync.changeCounter, 1);
 });
 
 add_task(async function test_get() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1, TEST_ADDRESS_2]);
 
   let addresses = profileStorage.addresses.getAll();
   let guid = addresses[0].guid;
@@ -179,21 +172,16 @@ add_task(async function test_get() {
   do_check_eq(address["address-line1"], undefined);
   do_check_eq(address["address-line2"], undefined);
 
   // Modifying output shouldn't affect the storage.
   address.organization = "test";
   do_check_record_matches(profileStorage.addresses.get(guid), TEST_ADDRESS_1);
 
   do_check_eq(profileStorage.addresses.get("INVALID_GUID"), null);
-
-  // rawData should include _sync, which should have a value of 1
-  profileStorage.addresses.pullSyncChanges(); // force sync metadata, which is what we are checking.
-  let {_sync} = profileStorage.addresses.get(guid, {rawData: true});
-  do_check_eq(_sync.changeCounter, 1);
 });
 
 add_task(async function test_getByFilter() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1, TEST_ADDRESS_2]);
 
   let filter = {info: {fieldName: "street-address"}, searchString: "Some"};
   let addresses = profileStorage.addresses.getByFilter(filter);
@@ -266,17 +254,17 @@ add_task(async function test_update() {
 
   profileStorage.addresses.pullSyncChanges(); // force sync metadata, which we check below.
 
   let address = profileStorage.addresses.get(guid, {rawData: true});
 
   do_check_eq(address.country, undefined);
   do_check_neq(address.timeLastModified, timeLastModified);
   do_check_record_matches(address, TEST_ADDRESS_3);
-  do_check_eq(address._sync.changeCounter, 1);
+  do_check_eq(getSyncChangeCounter(profileStorage.addresses, guid), 1);
 
   Assert.throws(
     () => profileStorage.addresses.update("INVALID_GUID", TEST_ADDRESS_3),
     /No matching record\./
   );
 
   Assert.throws(
     () => profileStorage.addresses.update(guid, TEST_ADDRESS_WITH_INVALID_FIELD),
--- a/browser/extensions/formautofill/test/unit/test_storage_syncfields.js
+++ b/browser/extensions/formautofill/test/unit/test_storage_syncfields.js
@@ -40,70 +40,71 @@ function findGUID(storage, guid, options
   equal(records.length, 1);
   return records[0];
 }
 
 add_task(async function test_changeCounter() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1]);
 
-  let address = profileStorage.addresses.getAll({rawData: true})[0];
+  let [address] = profileStorage.addresses.getAll();
   // new records don't get the sync metadata.
-  ok(!address._sync);
+  equal(getSyncChangeCounter(profileStorage.addresses, address.guid), -1);
   // But we can force one.
   profileStorage.addresses.pullSyncChanges();
-  address = profileStorage.addresses.getAll({rawData: true})[0];
-  equal(address._sync.changeCounter, 1);
+  equal(getSyncChangeCounter(profileStorage.addresses, address.guid), 1);
 });
 
 add_task(async function test_pushChanges() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1, TEST_ADDRESS_2]);
 
   profileStorage.addresses.pullSyncChanges(); // force sync metadata for all items
 
-  let [, address] = profileStorage.addresses.getAll({rawData: true});
+  let [, address] = profileStorage.addresses.getAll();
   let guid = address.guid;
+  let changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
 
   // Pretend we're doing a sync now, and an update occured mid-sync.
   let changes = {
     [guid]: {
       profile: address,
-      counter: address._sync.changeCounter,
+      counter: changeCounter,
       modified: address.timeLastModified,
       synced: true,
     },
   };
 
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == "update");
   profileStorage.addresses.update(guid, TEST_ADDRESS_3);
   await onChanged;
 
-  address = profileStorage.addresses.get(guid, {rawData: true});
-  do_check_eq(address._sync.changeCounter, 2);
+  changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
+  do_check_eq(changeCounter, 2);
 
   profileStorage.addresses.pushSyncChanges(changes);
-  address = profileStorage.addresses.get(guid, {rawData: true});
+  address = profileStorage.addresses.get(guid);
+  changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
 
   // Counter should still be 1, since our sync didn't record the mid-sync change
-  do_check_eq(address._sync.changeCounter, 1, "Counter shouldn't be zero because it didn't record update");
+  do_check_eq(changeCounter, 1, "Counter shouldn't be zero because it didn't record update");
 
   // now, push a new set of changes, which should make the changeCounter 0
   profileStorage.addresses.pushSyncChanges({
     [guid]: {
       profile: address,
-      counter: address._sync.changeCounter,
+      counter: changeCounter,
       modified: address.timeLastModified,
       synced: true,
     },
   });
-  address = profileStorage.addresses.get(guid, {rawData: true});
 
-  do_check_eq(address._sync.changeCounter, 0);
+  changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
+  do_check_eq(changeCounter, 0);
 });
 
 async function checkingSyncChange(action, callback) {
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == action);
   await callback();
   let [subject] = await onChanged;
   ok(subject.wrappedJSObject.sourceSync, "change notification should have source sync");
@@ -114,46 +115,46 @@ add_task(async function test_add_sourceS
 
   // Hardcode a guid so that we don't need to generate a dynamic regex
   let guid = "aaaaaaaaaaaa";
   let testAddr = Object.assign({guid}, TEST_ADDRESS_1);
 
   await checkingSyncChange("add", () =>
     profileStorage.addresses.add(testAddr, {sourceSync: true}));
 
-  let added = profileStorage.addresses.get(guid, {rawData: true});
-  equal(added._sync.changeCounter, 0);
+  let changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
+  equal(changeCounter, 0);
 
   Assert.throws(() =>
     profileStorage.addresses.add({guid, deleted: true}, {sourceSync: true}),
     /Record aaaaaaaaaaaa already exists/
   );
 });
 
 add_task(async function test_add_tombstone_sourceSync() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, []);
 
   let guid = profileStorage.addresses._generateGUID();
   let testAddr = {guid, deleted: true};
   await checkingSyncChange("add", () =>
     profileStorage.addresses.add(testAddr, {sourceSync: true}));
 
   let added = findGUID(profileStorage.addresses, guid,
-    {rawData: true, includeDeleted: true});
+    {includeDeleted: true});
   ok(added);
-  equal(added._sync.changeCounter, 0);
+  equal(getSyncChangeCounter(profileStorage.addresses, guid), 0);
   ok(added.deleted);
 
   // Adding same record again shouldn't throw (or change anything)
   await checkingSyncChange("add", () =>
     profileStorage.addresses.add(testAddr, {sourceSync: true}));
 
   added = findGUID(profileStorage.addresses, guid,
-    {rawData: true, includeDeleted: true});
-  equal(added._sync.changeCounter, 0);
+    {includeDeleted: true});
+  equal(getSyncChangeCounter(profileStorage.addresses, guid), 0);
   ok(added.deleted);
 });
 
 add_task(async function test_add_resurrects_tombstones() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, []);
 
   let guid = profileStorage.addresses._generateGUID();
 
@@ -172,79 +173,75 @@ add_task(async function test_add_resurre
   let got = profileStorage.addresses.get(guid);
   equal(got["given-name"], TEST_ADDRESS_1["given-name"]);
 });
 
 add_task(async function test_remove_sourceSync_localChanges() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, [TEST_ADDRESS_1]);
   profileStorage.addresses.pullSyncChanges(); // force sync metadata
 
-  let [{guid, _sync}] = profileStorage.addresses.getAll({rawData: true});
+  let [{guid}] = profileStorage.addresses.getAll();
 
-  equal(_sync.changeCounter, 1);
+  equal(getSyncChangeCounter(profileStorage.addresses, guid), 1);
   // try and remove a record stored locally with local changes
   await checkingSyncChange("remove", () =>
     profileStorage.addresses.remove(guid, {sourceSync: true}));
 
-  let record = profileStorage.addresses.get(guid, {
-    rawData: true,
-  });
+  let record = profileStorage.addresses.get(guid);
   ok(record);
-  equal(record._sync.changeCounter, 1);
+  equal(getSyncChangeCounter(profileStorage.addresses, guid), 1);
 });
 
 add_task(async function test_remove_sourceSync_unknown() {
   // remove a record not stored locally
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, []);
 
   let guid = profileStorage.addresses._generateGUID();
   await checkingSyncChange("remove", () =>
     profileStorage.addresses.remove(guid, {sourceSync: true}));
 
   let tombstone = findGUID(profileStorage.addresses, guid, {
-    rawData: true,
     includeDeleted: true,
   });
   ok(tombstone.deleted);
-  equal(tombstone._sync.changeCounter, 0);
+  equal(getSyncChangeCounter(profileStorage.addresses, guid), 0);
 });
 
 add_task(async function test_remove_sourceSync_unchanged() {
   // Remove a local record without a change counter.
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, []);
 
   let guid = profileStorage.addresses._generateGUID();
   let addr = Object.assign({guid}, TEST_ADDRESS_1);
   // add a record with sourceSync to guarantee changeCounter == 0
   await checkingSyncChange("add", () =>
     profileStorage.addresses.add(addr, {sourceSync: true}));
 
-  let added = profileStorage.addresses.get(guid, {rawData: true});
-  equal(added._sync.changeCounter, 0);
+  equal(getSyncChangeCounter(profileStorage.addresses, guid), 0);
 
   await checkingSyncChange("remove", () =>
     profileStorage.addresses.remove(guid, {sourceSync: true}));
 
   let tombstone = findGUID(profileStorage.addresses, guid, {
-    rawData: true,
     includeDeleted: true,
   });
   ok(tombstone.deleted);
-  equal(tombstone._sync.changeCounter, 0);
+  equal(getSyncChangeCounter(profileStorage.addresses, guid), 0);
 });
 
 add_task(async function test_pullSyncChanges() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1, TEST_ADDRESS_2]);
 
   let startAddresses = profileStorage.addresses.getAll();
   equal(startAddresses.length, 2);
   // All should start without sync metadata
-  for (let addr of profileStorage.addresses._store.data.addresses) {
-    ok(!addr._sync);
+  for (let {guid} of profileStorage.addresses._store.data.addresses) {
+    let changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
+    equal(changeCounter, -1);
   }
   profileStorage.addresses.pullSyncChanges(); // force sync metadata
 
   let addedDirectGUID = profileStorage.addresses._generateGUID();
   let testAddr = Object.assign({guid: addedDirectGUID}, TEST_ADDRESS_3);
 
   await checkingSyncChange("add", () =>
     profileStorage.addresses.add(testAddr, {sourceSync: true}));
@@ -258,17 +255,16 @@ add_task(async function test_pullSyncCha
   let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
                                           (subject, data) => data == "remove");
 
   profileStorage.addresses.remove(startAddresses[0].guid);
   await onChanged;
 
   let addresses = profileStorage.addresses.getAll({
     includeDeleted: true,
-    rawData: true,
   });
 
   // Should contain changes with a change counter
   let changes = profileStorage.addresses.pullSyncChanges();
   equal(Object.keys(changes).length, 2);
 
   ok(changes[startAddresses[0].guid].profile.deleted);
   equal(changes[startAddresses[0].guid].counter, 2);
@@ -280,52 +276,54 @@ add_task(async function test_pullSyncCha
   ok(!changes[addedDirectGUID], "Missing because it was added with sourceSync");
 
   for (let address of addresses) {
     let change = changes[address.guid];
     if (!change) {
       continue;
     }
     equal(change.profile.guid, address.guid);
-    equal(change.counter, address._sync.changeCounter);
+    let changeCounter = getSyncChangeCounter(profileStorage.addresses,
+      change.profile.guid);
+    equal(change.counter, changeCounter);
     ok(!change.synced);
   }
 });
 
 add_task(async function test_pullPushChanges() {
   // round-trip changes between pull and push
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, []);
   let psa = profileStorage.addresses;
 
   let guid1 = psa.add(TEST_ADDRESS_1);
   let guid2 = psa.add(TEST_ADDRESS_2);
   let guid3 = psa.add(TEST_ADDRESS_3);
 
   let changes = psa.pullSyncChanges();
 
-  equal(psa.get(guid1, {rawData: true})._sync.changeCounter, 1);
-  equal(psa.get(guid2, {rawData: true})._sync.changeCounter, 1);
-  equal(psa.get(guid3, {rawData: true})._sync.changeCounter, 1);
+  equal(getSyncChangeCounter(psa, guid1), 1);
+  equal(getSyncChangeCounter(psa, guid2), 1);
+  equal(getSyncChangeCounter(psa, guid3), 1);
 
   // between the pull and the push we change the second.
   psa.update(guid2, Object.assign({}, TEST_ADDRESS_2, {country: "AU"}));
-  equal(psa.get(guid2, {rawData: true})._sync.changeCounter, 2);
+  equal(getSyncChangeCounter(psa, guid2), 2);
   // and update the changeset to indicated we did update the first 2, but failed
   // to update the 3rd for some reason.
   changes[guid1].synced = true;
   changes[guid2].synced = true;
 
   psa.pushSyncChanges(changes);
 
   // first was synced correctly.
-  equal(psa.get(guid1, {rawData: true})._sync.changeCounter, 0);
+  equal(getSyncChangeCounter(psa, guid1), 0);
   // second was synced correctly, but it had a change while syncing.
-  equal(psa.get(guid2, {rawData: true})._sync.changeCounter, 1);
+  equal(getSyncChangeCounter(psa, guid2), 1);
   // 3rd wasn't marked as having synced.
-  equal(psa.get(guid3, {rawData: true})._sync.changeCounter, 1);
+  equal(getSyncChangeCounter(psa, guid3), 1);
 });
 
 add_task(async function test_changeGUID() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME, []);
 
   let newguid = () => profileStorage.addresses._generateGUID();
 
   let guid_synced = profileStorage.addresses.add(TEST_ADDRESS_1);
@@ -362,26 +360,29 @@ add_task(async function test_changeGUID(
   ok(profileStorage.addresses.get(targetguid), "target guid exists.");
   ok(!profileStorage.addresses.get(guid_u1), "old guid no longer exists.");
 });
 
 add_task(async function test_reset() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1, TEST_ADDRESS_2]);
 
-  let addresses = profileStorage.addresses.getAll({rawData: true});
+  let addresses = profileStorage.addresses.getAll();
   // All should start without sync metadata
-  for (let addr of addresses) {
-    ok(!addr._sync);
+  for (let {guid} of addresses) {
+    let changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
+    equal(changeCounter, -1);
   }
   // pullSyncChanges should create the metadata.
   profileStorage.addresses.pullSyncChanges();
-  addresses = profileStorage.addresses.getAll({rawData: true});
-  for (let addr of addresses) {
-    ok(addr._sync);
+  addresses = profileStorage.addresses.getAll();
+  for (let {guid} of addresses) {
+    let changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
+    equal(changeCounter, 1);
   }
   // and resetSync should wipe it.
   profileStorage.addresses.resetSync();
-  addresses = profileStorage.addresses.getAll({rawData: true});
-  for (let addr of addresses) {
-    ok(!addr._sync);
+  addresses = profileStorage.addresses.getAll();
+  for (let {guid} of addresses) {
+    let changeCounter = getSyncChangeCounter(profileStorage.addresses, guid);
+    equal(changeCounter, -1);
   }
 });
--- a/browser/extensions/formautofill/test/unit/test_sync.js
+++ b/browser/extensions/formautofill/test/unit/test_sync.js
@@ -154,24 +154,25 @@ add_task(async function test_outgoing() 
 
     Assert.equal(collection.count(), 2);
     Assert.ok(collection.wbo(existingGUID));
     Assert.ok(collection.wbo(deletedGUID));
 
     expectLocalProfiles([
       {
         guid: existingGUID,
-        _sync: {changeCounter: 0},
       },
       {
         guid: deletedGUID,
-        _sync: {changeCounter: 0},
         deleted: true,
       },
     ]);
+
+    strictEqual(getSyncChangeCounter(profileStorage.addresses, existingGUID), 0);
+    strictEqual(getSyncChangeCounter(profileStorage.addresses, deletedGUID), 0);
   } finally {
     await cleanup(server);
   }
 });
 
 add_task(async function test_incoming_new() {
   let {server, engine} = await setup();
   try {
@@ -190,24 +191,25 @@ add_task(async function test_incoming_ne
     // The tracker should start with no score.
     equal(engine._tracker.score, 0);
 
     await engine.sync();
 
     expectLocalProfiles([
       {
         guid: profileID,
-        _sync: {changeCounter: 0},
       }, {
         guid: deletedID,
-        _sync: {changeCounter: 0},
         deleted: true,
       },
     ]);
 
+    strictEqual(getSyncChangeCounter(profileStorage.addresses, profileID), 0);
+    strictEqual(getSyncChangeCounter(profileStorage.addresses, deletedID), 0);
+
     // The sync applied new records - ensure our tracker knew it came from
     // sync and didn't bump the score.
     equal(engine._tracker.score, 0);
   } finally {
     await cleanup(server);
   }
 });
 
@@ -334,17 +336,16 @@ add_task(async function test_dedupe_mult
         "given-name": "Mark",
         "family-name": "Hammond",
         "organization": "Mozilla",
         "country": "AU",
         "tel": "+12345678910",
       },
     ]);
     // Make sure these are both syncing.
-    let addrA = profileStorage.addresses.get(aGuid, {rawData: true});
-    ok(addrA._sync);
-
-    let addrB = profileStorage.addresses.get(bGuid, {rawData: true});
-    ok(addrB._sync);
+    strictEqual(getSyncChangeCounter(profileStorage.addresses, aGuid), 0,
+      "A should be marked as syncing");
+    strictEqual(getSyncChangeCounter(profileStorage.addresses, bGuid), 0,
+      "B should be marked as syncing");
   } finally {
     await cleanup(server);
   }
 });