Bug 1374500 - Add a new sync engine for addresses and credit-cards. r=markh,kitcambridge
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Fri, 23 Jun 2017 15:43:37 -0400
changeset 418647 d9fcc95f6c426f7de89c8cde847581492c9229e5
parent 418646 60ef4d9f30239938dc0769cab69c54f3ccbb7d3f
child 418648 1051372286caa087190b3f77f6246e50c4513ea5
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, kitcambridge
bugs1374500
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 1374500 - Add a new sync engine for addresses and credit-cards. r=markh,kitcambridge MozReview-Commit-ID: BcwS86YhfBc
browser/extensions/formautofill/FormAutofillSync.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/head.js
browser/extensions/formautofill/test/unit/test_sync.js
browser/extensions/formautofill/test/unit/xpcshell.ini
tools/lint/eslint/modules.json
new file mode 100644
--- /dev/null
+++ b/browser/extensions/formautofill/FormAutofillSync.jsm
@@ -0,0 +1,418 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+
+this.EXPORTED_SYMBOLS = ["AddressesEngine", "CreditCardsEngine"];
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://services-sync/engines.js");
+Cu.import("resource://services-sync/record.js");
+Cu.import("resource://services-sync/util.js");
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://formautofill/FormAutofillUtils.jsm");
+
+XPCOMUtils.defineLazyModuleGetter(this, "profileStorage",
+                                  "resource://formautofill/ProfileStorage.jsm");
+
+
+// XXX - this will end up in ProfileStorage - here for a POC (and at the top
+// of the file for eslint)
+function findDuplicateGUID(record) {
+  for (let profile of profileStorage.addresses.getAll()) {
+    let keys = new Set(Object.keys(record));
+    for (let key of Object.keys(profile)) {
+      keys.add(key);
+    }
+    let same = true;
+    for (let key of keys) {
+      if (!same) {
+        break;
+      }
+      if (profile.hasOwnProperty(key) && record.hasOwnProperty(key)) {
+        same = profile[key] == record[key];
+      }
+      // else something smarter, possibly using version field?
+    }
+    if (same) {
+      return profile.guid;
+    }
+  }
+  return null;
+}
+
+// A helper to sanitize address and creditcard entries suitable for logging.
+function sanitizeStorageObject(ob) {
+  // ProfileStorage INTERNAL_FIELDS. These are safe to log as is.
+  const whitelist = [
+    "guid",
+    "version",
+    "timeCreated",
+    "timeLastUsed",
+    "timeLastModified",
+    "timesUsed",
+  ];
+  let result = {};
+  for (let key of Object.keys(ob)) {
+    let origVal = ob[key];
+    if (whitelist.includes(key)) {
+      result[key] = origVal;
+    } else if (typeof origVal == "string") {
+      result[key] = "X".repeat(origVal.length);
+    } else {
+      result[key] = typeof(origVal); // *shrug*
+    }
+  }
+  return result;
+}
+
+
+function AutofillRecord(collection, id) {
+  CryptoWrapper.call(this, collection, id);
+}
+
+AutofillRecord.prototype = {
+  __proto__: CryptoWrapper.prototype,
+
+  cleartextToString() {
+    // And a helper so logging a *Sync* record auto sanitizes.
+    let record = this.cleartext;
+    let result = {entry: {}};
+    if (record.entry) {
+      result.entry = sanitizeStorageObject(record.entry);
+    }
+    return JSON.stringify(result);
+  },
+};
+
+// Profile data is stored in the "entry" object of the record.
+Utils.deferGetSet(AutofillRecord, "cleartext", ["entry"]);
+
+function FormAutofillStore(name, engine) {
+  Store.call(this, name, engine);
+}
+
+FormAutofillStore.prototype = {
+  __proto__: Store.prototype,
+
+  _subStorageName: null, // overridden below.
+  _storage: null,
+
+  get storage() {
+    if (!this._storage) {
+      this._storage = profileStorage[this._subStorageName];
+    }
+    return this._storage;
+  },
+
+  async getAllIDs() {
+    let result = {};
+    for (let {guid} of this.storage.getAll({includeDeleted: true})) {
+      result[guid] = true;
+    }
+    return result;
+  },
+
+  async changeItemID(oldID, newID) {
+    this.storage.changeGUID(oldID, newID);
+  },
+
+  // Note: this function intentionally returns false in cases where we only have
+  // a (local) tombstone - and profileStorage.get() filters them for us.
+  async itemExists(id) {
+    return Boolean(this.storage.get(id));
+  },
+
+  async applyIncoming(remoteRecord) {
+    if (remoteRecord.deleted) {
+      this._log.trace("Deleting record", remoteRecord);
+      this.storage.remove(remoteRecord.id, {sourceSync: true});
+      return;
+    }
+
+    if (await this.itemExists(remoteRecord.id)) {
+      // We will never get a tombstone here, so we are updating a real record.
+      await this._doUpdateRecord(remoteRecord);
+      return;
+    }
+
+    // No matching local record. Try to dedupe a NEW local record.
+    let localDupeID = /* this.storage. */findDuplicateGUID(remoteRecord.entry);
+    if (localDupeID) {
+      this._log.trace(`Deduping local record ${localDupeID} to remote`, remoteRecord);
+      // Change the local GUID to match the incoming record, then apply the
+      // incoming record.
+      await this.changeItemID(localDupeID, remoteRecord.id);
+      await this._doUpdateRecord(remoteRecord);
+      return;
+    }
+
+    // We didn't find a dupe, either, so must be a new record (or possibly
+    // a non-deleted version of an item we have a tombstone for, which create()
+    // handles for us.)
+    this._log.trace("Add record", remoteRecord);
+    remoteRecord.entry.guid = remoteRecord.id;
+    this.storage.add(remoteRecord.entry, {sourceSync: true});
+  },
+
+  async createRecord(id, collection) {
+    this._log.trace("Create record", id);
+    let record = new AutofillRecord(collection, id);
+    record.entry = this.storage.get(id, {
+      noComputedFields: true,
+    });
+    if (!record.entry) {
+      // We should consider getting a more authortative indication it's actually deleted.
+      this._log.debug(`Failed to get autofill record with id "${id}", assuming deleted`);
+      record.deleted = true;
+    } else {
+      // The GUID is already stored in record.id, so we nuke it from the entry
+      // itself to save a tiny bit of space. The profileStorage clones profiles,
+      // so nuking in-place is OK.
+      delete record.entry.guid;
+    }
+    return record;
+  },
+
+  async _doUpdateRecord(record) {
+    this._log.trace("Updating record", record);
+
+    // TODO (bug 1363995) - until we get reconcilliation logic, this is
+    // dangerous, it unconditionally updates, which may cause DATA LOSS.
+    this.storage.update(record.id, record.entry);
+  },
+
+  // NOTE: Because we re-implement the incoming/reconcilliation logic we leave
+  // the |create|, |remove| and |update| methods undefined - the base
+  // implementation throws, which is what we want to happen so we can identify
+  // any places they are "accidentally" called.
+};
+
+function FormAutofillTracker(name, engine) {
+  Tracker.call(this, name, engine);
+}
+
+FormAutofillTracker.prototype = {
+  __proto__: Tracker.prototype,
+  observe: function observe(subject, topic, data) {
+    Tracker.prototype.observe.call(this, subject, topic, data);
+    if (topic != "formautofill-storage-changed") {
+      return;
+    }
+    if (subject && subject.wrappedJSObject && subject.wrappedJSObject.sourceSync) {
+      return;
+    }
+    switch (data) {
+      case "add":
+      case "update":
+      case "remove":
+        this.score += SCORE_INCREMENT_XLARGE;
+        break;
+      default:
+        this._log.debug("unrecognized autofill notification", data);
+        break;
+    }
+  },
+
+  // `_ignore` checks the change source for each observer notification, so we
+  // don't want to let the engine ignore all changes during a sync.
+  get ignoreAll() {
+    return false;
+  },
+
+  // Define an empty setter so that the engine doesn't throw a `TypeError`
+  // setting a read-only property.
+  set ignoreAll(value) {},
+
+  startTracking() {
+    Services.obs.addObserver(this, "formautofill-storage-changed");
+  },
+
+  stopTracking() {
+    Services.obs.removeObserver(this, "formautofill-storage-changed");
+  },
+
+  // We never want to persist changed IDs, as the changes are already stored
+  // in ProfileStorage
+  persistChangedIDs: false,
+
+  // Ensure we aren't accidentally using the base persistence.
+  get changedIDs() {
+    throw new Error("changedIDs isn't meaningful for this engine");
+  },
+
+  set changedIDs(obj) {
+    throw new Error("changedIDs isn't meaningful for this engine");
+  },
+
+  addChangedID(id, when) {
+    throw new Error("Don't add IDs to the autofill tracker");
+  },
+
+  removeChangedID(id) {
+    throw new Error("Don't remove IDs from the autofill tracker");
+  },
+
+  // This method is called at various times, so we override with a no-op
+  // instead of throwing.
+  clearChangedIDs() {},
+};
+
+// This uses the same conventions as BookmarkChangeset in
+// services/sync/modules/engines/bookmarks.js. Specifically,
+// - "synced" means the item has already been synced (or we have another reason
+//   to ignore it), and should be ignored in most methods.
+class AutofillChangeset extends Changeset {
+  constructor() {
+    super();
+  }
+
+  getModifiedTimestamp(id) {
+    throw new Error("Don't use timestamps to resolve autofill merge conflicts");
+  }
+
+  has(id) {
+    let change = this.changes[id];
+    if (change) {
+      return !change.synced;
+    }
+    return false;
+  }
+
+  delete(id) {
+    let change = this.changes[id];
+    if (change) {
+      // Mark the change as synced without removing it from the set. We do this
+      // so that we can update ProfileStorage in `trackRemainingChanges`.
+      change.synced = true;
+    }
+  }
+}
+
+function FormAutofillEngine(service, name) {
+  SyncEngine.call(this, name, service);
+}
+
+FormAutofillEngine.prototype = {
+  __proto__: SyncEngine.prototype,
+
+  // the priority for this engine is == addons, so will happen after bookmarks
+  // prefs and tabs, but before forms, history, etc.
+  syncPriority: 5,
+
+  // We don't use SyncEngine.initialize() for this, as we initialize even if
+  // the engine is disabled, and we don't want to be the loader of
+  // ProfileStorage in this case.
+  async _syncStartup() {
+    await profileStorage.initialize();
+    await SyncEngine.prototype._syncStartup.call(this);
+  },
+
+  // We handle reconciliation in the store, not the engine.
+  async _reconcile() {
+    return true;
+  },
+
+  emptyChangeset() {
+    return new AutofillChangeset();
+  },
+
+  async _uploadOutgoing() {
+    this._modified.replace(this._store.storage.pullSyncChanges());
+    await SyncEngine.prototype._uploadOutgoing.call(this);
+  },
+
+  // Typically, engines populate the changeset before downloading records.
+  // However, we handle conflict resolution in the store, so we can wait
+  // to pull changes until we're ready to upload.
+  async pullAllChanges() {
+    return {};
+  },
+
+  async pullNewChanges() {
+    return {};
+  },
+
+  async trackRemainingChanges() {
+    this._store.storage.pushSyncChanges(this._modified.changes);
+  },
+
+  _deleteId(id) {
+    this._noteDeletedId(id);
+  },
+
+  async _resetClient() {
+    this._store.storage.resetSync();
+  },
+};
+
+// The concrete engines
+
+function AddressesRecord(collection, id) {
+  AutofillRecord.call(this, collection, id);
+}
+
+AddressesRecord.prototype = {
+  __proto__: AutofillRecord.prototype,
+  _logName: "Sync.Record.Addresses",
+};
+
+function AddressesStore(name, engine) {
+  FormAutofillStore.call(this, name, engine);
+}
+
+AddressesStore.prototype = {
+  __proto__: FormAutofillStore.prototype,
+  _subStorageName: "addresses",
+};
+
+function AddressesEngine(service) {
+  FormAutofillEngine.call(this, service, "Addresses");
+}
+
+AddressesEngine.prototype = {
+  __proto__: FormAutofillEngine.prototype,
+  _trackerObj: FormAutofillTracker,
+  _storeObj: AddressesStore,
+  _recordObj: AddressesRecord,
+
+  get prefName() {
+    return "addresses";
+  },
+};
+
+function CreditCardsRecord(collection, id) {
+  AutofillRecord.call(this, collection, id);
+}
+
+CreditCardsRecord.prototype = {
+  __proto__: AutofillRecord.prototype,
+  _logName: "Sync.Record.CreditCards",
+};
+
+function CreditCardsStore(name, engine) {
+  FormAutofillStore.call(this, name, engine);
+}
+
+CreditCardsStore.prototype = {
+  __proto__: FormAutofillStore.prototype,
+  _subStorageName: "creditCards",
+};
+
+function CreditCardsEngine(service) {
+  FormAutofillEngine.call(this, service, "CreditCards");
+}
+
+CreditCardsEngine.prototype = {
+  __proto__: FormAutofillEngine.prototype,
+  _trackerObj: FormAutofillTracker,
+  _storeObj: CreditCardsStore,
+  _recordObj: CreditCardsRecord,
+  get prefName() {
+    return "creditcards";
+  },
+};
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -755,16 +755,22 @@ class AutofillRecords {
       }
       if (typeof record[key] !== "string" &&
           typeof record[key] !== "number") {
         throw new Error(`"${key}" contains invalid data type.`);
       }
     }
   }
 
+  // A test-only helper.
+  _nukeAllRecords() {
+    this._store.data[this._collectionName] = [];
+    // test-only, so there's no good reason to request a save!
+  }
+
   _stripComputedFields(record) {
     this.VALID_COMPUTED_FIELDS.forEach(field => delete record[field]);
   }
 
   // An interface to be inherited.
   _recordReadProcessor(record) {}
 
   // An interface to be inherited.
--- a/browser/extensions/formautofill/test/unit/head.js
+++ b/browser/extensions/formautofill/test/unit/head.js
@@ -3,17 +3,17 @@
  */
 
 /* exported getTempFile, loadFormAutofillContent, runHeuristicsTest, sinon,
  *          initProfileStorage
  */
 
 "use strict";
 
-const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+var {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/NetUtil.jsm");
 Cu.import("resource://gre/modules/FormLikeFactory.jsm");
 Cu.import("resource://testing-common/MockDocument.jsm");
 Cu.import("resource://testing-common/TestUtils.jsm");
 
@@ -23,30 +23,30 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/FileUtils.jsm");
 
 do_get_profile();
 
 // ================================================
 // Load mocking/stubbing library, sinon
 // docs: http://sinonjs.org/releases/v2.3.2/
 Cu.import("resource://gre/modules/Timer.jsm");
-const {Loader} = Cu.import("resource://gre/modules/commonjs/toolkit/loader.js", {});
-const loader = new Loader.Loader({
+var {Loader} = Cu.import("resource://gre/modules/commonjs/toolkit/loader.js", {});
+var loader = new Loader.Loader({
   paths: {
     "": "resource://testing-common/",
   },
   globals: {
     setTimeout,
     setInterval,
     clearTimeout,
     clearInterval,
   },
 });
-const require = Loader.Require(loader, {id: ""});
-const sinon = require("sinon-2.3.2");
+var require = Loader.Require(loader, {id: ""});
+var sinon = require("sinon-2.3.2");
 // ================================================
 
 // Load our bootstrap extension manifest so we can access our chrome/resource URIs.
 const EXTENSION_ID = "formautofill@mozilla.org";
 let extensionDir = Services.dirsvc.get("GreD", Ci.nsIFile);
 extensionDir.append("browser");
 extensionDir.append("features");
 extensionDir.append(EXTENSION_ID);
new file mode 100644
--- /dev/null
+++ b/browser/extensions/formautofill/test/unit/test_sync.js
@@ -0,0 +1,350 @@
+/**
+ * Tests sync functionality.
+ */
+
+/* import-globals-from ../../../../../services/sync/tests/unit/head_appinfo.js */
+/* import-globals-from ../../../../../services/common/tests/unit/head_helpers.js */
+/* import-globals-from ../../../../../services/sync/tests/unit/head_helpers.js */
+/* import-globals-from ../../../../../services/sync/tests/unit/head_http_server.js */
+
+"use strict";
+
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://testing-common/services/sync/utils.js");
+Cu.import("resource://formautofill/ProfileStorage.jsm");
+
+let {sanitizeStorageObject, AutofillRecord, AddressesEngine} =
+  Cu.import("resource://formautofill/FormAutofillSync.jsm", {});
+
+
+Services.prefs.setCharPref("extensions.formautofill.loglevel", "Trace");
+initTestLogging("Trace");
+
+const TEST_PROFILE_1 = {
+  "given-name": "Timothy",
+  "additional-name": "John",
+  "family-name": "Berners-Lee",
+  organization: "World Wide Web Consortium",
+  "street-address": "32 Vassar Street\nMIT Room 32-G524",
+  "address-level2": "Cambridge",
+  "address-level1": "MA",
+  "postal-code": "02139",
+  country: "US",
+  tel: "+16172535702",
+  email: "timbl@w3.org",
+};
+
+function expectLocalProfiles(expected) {
+  let profiles = profileStorage.addresses.getAll({
+    rawData: true,
+    includeDeleted: true,
+  });
+  expected.sort((a, b) => a.guid.localeCompare(b.guid));
+  profiles.sort((a, b) => a.guid.localeCompare(b.guid));
+  let doCheckObject = (a, b) => {
+    for (let key of Object.keys(a)) {
+      if (typeof a[key] == "object") {
+        doCheckObject(a[key], b[key]);
+      } else {
+        equal(a[key], b[key]);
+      }
+    }
+  };
+  try {
+    deepEqual(profiles.map(p => p.guid), expected.map(p => p.guid));
+    for (let i = 0; i < expected.length; i++) {
+      let thisExpected = expected[i];
+      let thisGot = profiles[i];
+      // always check "deleted".
+      equal(thisExpected.deleted, thisGot.deleted);
+      doCheckObject(thisExpected, thisGot);
+    }
+  } catch (ex) {
+    do_print("Comparing expected profiles:");
+    do_print(JSON.stringify(expected, undefined, 2));
+    do_print("against actual profiles:");
+    do_print(JSON.stringify(profiles, undefined, 2));
+    throw ex;
+  }
+}
+
+async function setup() {
+  await profileStorage.initialize();
+  // should always start with no profiles.
+  Assert.equal(profileStorage.addresses.getAll({includeDeleted: true}).length, 0);
+
+  Services.prefs.setCharPref("services.sync.log.logger.engine.addresses", "Trace");
+  let engine = new AddressesEngine(Service);
+  await engine.initialize();
+  // Avoid accidental automatic sync due to our own changes
+  Service.scheduler.syncThreshold = 10000000;
+  let server = serverForUsers({"foo": "password"}, {
+    meta: {global: {engines: {addresses: {version: engine.version, syncID: engine.syncID}}}},
+    addresses: {},
+  });
+
+  Service.engineManager._engines.addresses = engine;
+  engine.enabled = true;
+
+  generateNewKeys(Service.collectionKeys);
+
+  await SyncTestingInfrastructure(server);
+
+  let collection = server.user("foo").collection("addresses");
+
+  return {server, collection, engine};
+}
+
+async function cleanup(server) {
+  let promiseStartOver = promiseOneObserver("weave:service:start-over:finish");
+  await Service.startOver();
+  await promiseStartOver;
+  await promiseStopServer(server);
+  profileStorage.addresses._nukeAllRecords();
+}
+
+add_task(async function test_log_sanitization() {
+  await profileStorage.initialize();
+  let sanitized = sanitizeStorageObject(TEST_PROFILE_1);
+  // all strings have been mangled.
+  for (let key of Object.keys(TEST_PROFILE_1)) {
+    let val = TEST_PROFILE_1[key];
+    if (typeof val == "string") {
+      notEqual(sanitized[key], val);
+    }
+  }
+  // And check that stringifying a sync record is sanitized.
+  let record = new AutofillRecord("collection", "some-id");
+  record.entry = TEST_PROFILE_1;
+  let serialized = record.toString();
+  // None of the string values should appear in the output.
+  for (let key of Object.keys(TEST_PROFILE_1)) {
+    let val = TEST_PROFILE_1[key];
+    if (typeof val == "string") {
+      ok(!serialized.includes(val), `"${val}" shouldn't be in: ${serialized}`);
+    }
+  }
+  profileStorage.addresses._nukeAllRecords();
+});
+
+add_task(async function test_outgoing() {
+  let {server, collection, engine} = await setup();
+  try {
+    equal(engine._tracker.score, 0);
+    let existingGUID = profileStorage.addresses.add(TEST_PROFILE_1);
+    // And a deleted item.
+    let deletedGUID = profileStorage.addresses._generateGUID();
+    profileStorage.addresses.add({guid: deletedGUID, deleted: true});
+
+    expectLocalProfiles([
+      {
+        guid: existingGUID,
+      },
+      {
+        guid: deletedGUID,
+        deleted: true,
+      },
+    ]);
+
+    // The tracker should have a score recorded for the 2 additions we had.
+    equal(engine._tracker.score, SCORE_INCREMENT_XLARGE * 2);
+
+    await engine.sync();
+
+    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,
+      },
+    ]);
+  } finally {
+    await cleanup(server);
+  }
+});
+
+add_task(async function test_incoming_new() {
+  let {server, engine} = await setup();
+  try {
+    let profileID = Utils.makeGUID();
+    let deletedID = Utils.makeGUID();
+
+    server.insertWBO("foo", "addresses", new ServerWBO(profileID, encryptPayload({
+      id: profileID,
+      entry: TEST_PROFILE_1,
+    }), Date.now() / 1000));
+    server.insertWBO("foo", "addresses", new ServerWBO(deletedID, encryptPayload({
+      id: deletedID,
+      deleted: true,
+    }), Date.now() / 1000));
+
+    // 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,
+      },
+    ]);
+
+    // 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);
+  }
+});
+
+add_task(async function test_tombstones() {
+  let {server, collection, engine} = await setup();
+  try {
+    let existingGUID = profileStorage.addresses.add(TEST_PROFILE_1);
+
+    await engine.sync();
+
+    Assert.equal(collection.count(), 1);
+    let payload = collection.payloads()[0];
+    equal(payload.id, existingGUID);
+    equal(payload.deleted, undefined);
+
+    profileStorage.addresses.remove(existingGUID);
+    await engine.sync();
+
+    // should still exist, but now be a tombstone.
+    Assert.equal(collection.count(), 1);
+    payload = collection.payloads()[0];
+    equal(payload.id, existingGUID);
+    equal(payload.deleted, true);
+  } finally {
+    await cleanup(server);
+  }
+});
+
+// Unlike most sync engines, we want "both modified" to inspect the records,
+// and if materially different, create a duplicate.
+add_task(async function test_reconcile_both_modified_identical() {
+  let {server, engine} = await setup();
+  try {
+    // create a record locally.
+    let guid = profileStorage.addresses.add(TEST_PROFILE_1);
+
+    // and an identical record on the server.
+    server.insertWBO("foo", "addresses", new ServerWBO(guid, encryptPayload({
+      id: guid,
+      entry: TEST_PROFILE_1,
+    }), Date.now() / 1000));
+
+    await engine.sync();
+
+    expectLocalProfiles([{guid}]);
+  } finally {
+    await cleanup(server);
+  }
+});
+
+add_task(async function test_dedupe_identical() {
+  let {server, engine} = await setup();
+  try {
+    // create a record locally.
+    let localGuid = profileStorage.addresses.add(TEST_PROFILE_1);
+
+    // and an identical record on the server but different GUID.
+    let remoteGuid = Utils.makeGUID();
+    notEqual(localGuid, remoteGuid);
+    server.insertWBO("foo", "addresses", new ServerWBO(remoteGuid, encryptPayload({
+      id: remoteGuid,
+      entry: TEST_PROFILE_1,
+    }), Date.now() / 1000));
+
+    await engine.sync();
+
+    // Should no longer have the local guid
+    expectLocalProfiles([
+      {
+        guid: remoteGuid,
+      },
+    ]);
+  } finally {
+    await cleanup(server);
+  }
+});
+
+add_task(async function test_dedupe_multiple_candidates() {
+  let {server, engine} = await setup();
+  try {
+    // It's possible to have duplicate local profiles, with the same fields but
+    // different GUIDs. After a node reassignment, or after disconnecting and
+    // reconnecting to Sync, we might dedupe a local record A to a remote record
+    // B, if we see B before we download and apply A. Since A and B are dupes,
+    // that's OK. We'll write a tombstone for A when we dedupe A to B, and
+    // overwrite that tombstone when we see A.
+
+    let aRecord = {
+      "given-name": "Mark",
+      "family-name": "Hammond",
+      "organization": "Mozilla",
+      "country": "AU",
+      "tel": "+12345678910",
+    };
+    // We don't pass `sourceSync` so that the records are marked as NEW.
+    let aGuid = profileStorage.addresses.add(aRecord);
+
+    let bRecord = Cu.cloneInto(aRecord, {});
+    let bGuid = profileStorage.addresses.add(bRecord);
+
+    // Insert B before A.
+    server.insertWBO("foo", "addresses", new ServerWBO(bGuid, encryptPayload({
+      id: bGuid,
+      entry: bRecord,
+    }), Date.now() / 1000));
+    server.insertWBO("foo", "addresses", new ServerWBO(aGuid, encryptPayload({
+      id: aGuid,
+      entry: aRecord,
+    }), Date.now() / 1000));
+
+    await engine.sync();
+
+    expectLocalProfiles([
+      {
+        "guid": aGuid,
+        "given-name": "Mark",
+        "family-name": "Hammond",
+        "organization": "Mozilla",
+        "country": "AU",
+        "tel": "+12345678910",
+      },
+      {
+        "guid": bGuid,
+        "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);
+  } finally {
+    await cleanup(server);
+  }
+});
--- a/browser/extensions/formautofill/test/unit/xpcshell.ini
+++ b/browser/extensions/formautofill/test/unit/xpcshell.ini
@@ -35,8 +35,10 @@ support-files =
 [test_onFormSubmitted.js]
 [test_profileAutocompleteResult.js]
 [test_phoneNumber.js]
 [test_savedFieldNames.js]
 [test_toOneLineAddress.js]
 [test_storage_tombstones.js]
 [test_storage_syncfields.js]
 [test_transformFields.js]
+[test_sync.js]
+head = head.js ../../../../../services/sync/tests/unit/head_appinfo.js ../../../../../services/common/tests/unit/head_helpers.js ../../../../../services/sync/tests/unit/head_helpers.js ../../../../../services/sync/tests/unit/head_http_server.js
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -72,16 +72,17 @@
   "fakeservices.js": ["FakeCryptoService", "FakeFilesystemService", "FakeGUIDService", "fakeSHA256HMAC"],
   "file_expandosharing.jsm": ["checkFromJSM"],
   "file_stringencoding.jsm": ["checkFromJSM"],
   "file_url.jsm": ["checkFromJSM"],
   "file_worker_url.jsm": ["checkFromJSM"],
   "Finder.jsm": ["Finder", "GetClipboardSearchString"],
   "forms.js": ["FormEngine", "FormRec", "FormValidator"],
   "forms.jsm": ["FormData"],
+  "FormAutofillSync.jsm": ["AddressesEngine", "CreditCardsEngine"],
   "frame.js": ["Collector", "Runner", "events", "runTestFile", "log", "timers", "persisted", "shutdownApplication"],
   "FrameScriptManager.jsm": ["getNewLoaderID"],
   "fxa_utils.js": ["initializeIdentityWithTokenServerResponse"],
   "fxaccounts.jsm": ["Authentication"],
   "FxAccounts.jsm": ["fxAccounts", "FxAccounts"],
   "FxAccountsCommon.js": ["log", "logPII", "FXACCOUNTS_PERMISSION", "DATA_FORMAT_VERSION", "DEFAULT_STORAGE_FILENAME", "ASSERTION_LIFETIME", "ASSERTION_USE_PERIOD", "CERT_LIFETIME", "KEY_LIFETIME", "POLL_SESSION", "ONLOGIN_NOTIFICATION", "ONVERIFIED_NOTIFICATION", "ONLOGOUT_NOTIFICATION", "ON_FXA_UPDATE_NOTIFICATION", "ON_DEVICE_CONNECTED_NOTIFICATION", "ON_DEVICE_DISCONNECTED_NOTIFICATION", "ON_PROFILE_UPDATED_NOTIFICATION", "ON_PASSWORD_CHANGED_NOTIFICATION", "ON_PASSWORD_RESET_NOTIFICATION", "ON_VERIFY_LOGIN_NOTIFICATION", "ON_ACCOUNT_DESTROYED_NOTIFICATION", "ON_COLLECTION_CHANGED_NOTIFICATION", "FXA_PUSH_SCOPE_ACCOUNT_UPDATE", "ON_PROFILE_CHANGE_NOTIFICATION", "ON_ACCOUNT_STATE_CHANGE_NOTIFICATION", "UI_REQUEST_SIGN_IN_FLOW", "UI_REQUEST_REFRESH_AUTH", "FX_OAUTH_CLIENT_ID", "WEBCHANNEL_ID", "ERRNO_ACCOUNT_ALREADY_EXISTS", "ERRNO_ACCOUNT_DOES_NOT_EXIST", "ERRNO_INCORRECT_PASSWORD", "ERRNO_UNVERIFIED_ACCOUNT", "ERRNO_INVALID_VERIFICATION_CODE", "ERRNO_NOT_VALID_JSON_BODY", "ERRNO_INVALID_BODY_PARAMETERS", "ERRNO_MISSING_BODY_PARAMETERS", "ERRNO_INVALID_REQUEST_SIGNATURE", "ERRNO_INVALID_AUTH_TOKEN", "ERRNO_INVALID_AUTH_TIMESTAMP", "ERRNO_MISSING_CONTENT_LENGTH", "ERRNO_REQUEST_BODY_TOO_LARGE", "ERRNO_TOO_MANY_CLIENT_REQUESTS", "ERRNO_INVALID_AUTH_NONCE", "ERRNO_ENDPOINT_NO_LONGER_SUPPORTED", "ERRNO_INCORRECT_LOGIN_METHOD", "ERRNO_INCORRECT_KEY_RETRIEVAL_METHOD", "ERRNO_INCORRECT_API_VERSION", "ERRNO_INCORRECT_EMAIL_CASE", "ERRNO_ACCOUNT_LOCKED", "ERRNO_ACCOUNT_UNLOCKED", "ERRNO_UNKNOWN_DEVICE", "ERRNO_DEVICE_SESSION_CONFLICT", "ERRNO_SERVICE_TEMP_UNAVAILABLE", "ERRNO_PARSE", "ERRNO_NETWORK", "ERRNO_UNKNOWN_ERROR", "OAUTH_SERVER_ERRNO_OFFSET", "ERRNO_UNKNOWN_CLIENT_ID", "ERRNO_INCORRECT_CLIENT_SECRET", "ERRNO_INCORRECT_REDIRECT_URI", "ERRNO_INVALID_FXA_ASSERTION", "ERRNO_UNKNOWN_CODE", "ERRNO_INCORRECT_CODE", "ERRNO_EXPIRED_CODE", "ERRNO_OAUTH_INVALID_TOKEN", "ERRNO_INVALID_REQUEST_PARAM", "ERRNO_INVALID_RESPONSE_TYPE", "ERRNO_UNAUTHORIZED", "ERRNO_FORBIDDEN", "ERRNO_INVALID_CONTENT_TYPE", "ERROR_ACCOUNT_ALREADY_EXISTS", "ERROR_ACCOUNT_DOES_NOT_EXIST", "ERROR_ACCOUNT_LOCKED", "ERROR_ACCOUNT_UNLOCKED", "ERROR_ALREADY_SIGNED_IN_USER", "ERROR_DEVICE_SESSION_CONFLICT", "ERROR_ENDPOINT_NO_LONGER_SUPPORTED", "ERROR_INCORRECT_API_VERSION", "ERROR_INCORRECT_EMAIL_CASE", "ERROR_INCORRECT_KEY_RETRIEVAL_METHOD", "ERROR_INCORRECT_LOGIN_METHOD", "ERROR_INVALID_EMAIL", "ERROR_INVALID_AUDIENCE", "ERROR_INVALID_AUTH_TOKEN", "ERROR_INVALID_AUTH_TIMESTAMP", "ERROR_INVALID_AUTH_NONCE", "ERROR_INVALID_BODY_PARAMETERS", "ERROR_INVALID_PASSWORD", "ERROR_INVALID_VERIFICATION_CODE", "ERROR_INVALID_REFRESH_AUTH_VALUE", "ERROR_INVALID_REQUEST_SIGNATURE", "ERROR_INTERNAL_INVALID_USER", "ERROR_MISSING_BODY_PARAMETERS", "ERROR_MISSING_CONTENT_LENGTH", "ERROR_NO_TOKEN_SESSION", "ERROR_NO_SILENT_REFRESH_AUTH", "ERROR_NOT_VALID_JSON_BODY", "ERROR_OFFLINE", "ERROR_PERMISSION_DENIED", "ERROR_REQUEST_BODY_TOO_LARGE", "ERROR_SERVER_ERROR", "ERROR_SYNC_DISABLED", "ERROR_TOO_MANY_CLIENT_REQUESTS", "ERROR_SERVICE_TEMP_UNAVAILABLE", "ERROR_UI_ERROR", "ERROR_UI_REQUEST", "ERROR_PARSE", "ERROR_NETWORK", "ERROR_UNKNOWN", "ERROR_UNKNOWN_DEVICE", "ERROR_UNVERIFIED_ACCOUNT", "ERROR_UNKNOWN_CLIENT_ID", "ERROR_INCORRECT_CLIENT_SECRET", "ERROR_INCORRECT_REDIRECT_URI", "ERROR_INVALID_FXA_ASSERTION", "ERROR_UNKNOWN_CODE", "ERROR_INCORRECT_CODE", "ERROR_EXPIRED_CODE", "ERROR_OAUTH_INVALID_TOKEN", "ERROR_INVALID_REQUEST_PARAM", "ERROR_INVALID_RESPONSE_TYPE", "ERROR_UNAUTHORIZED", "ERROR_FORBIDDEN", "ERROR_INVALID_CONTENT_TYPE", "ERROR_NO_ACCOUNT", "ERROR_AUTH_ERROR", "ERROR_INVALID_PARAMETER", "ERROR_CODE_METHOD_NOT_ALLOWED", "ERROR_MSG_METHOD_NOT_ALLOWED", "FXA_PWDMGR_PLAINTEXT_FIELDS", "FXA_PWDMGR_SECURE_FIELDS", "FXA_PWDMGR_MEMORY_FIELDS", "FXA_PWDMGR_REAUTH_WHITELIST", "FXA_PWDMGR_HOST", "FXA_PWDMGR_REALM", "SERVER_ERRNO_TO_ERROR", "ERROR_TO_GENERAL_ERROR_CLASS"],
   "FxAccountsOAuthGrantClient.jsm": ["FxAccountsOAuthGrantClient", "FxAccountsOAuthGrantClientError"],
   "FxAccountsProfileClient.jsm": ["FxAccountsProfileClient", "FxAccountsProfileClientError"],