Bug 1374500 - Add a new sync engine for addresses and credit-cards. r?markh,kitcambridge draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Fri, 23 Jun 2017 15:43:37 -0400
changeset 610915 da9339db984ff30e5a7ab57d37ed8792551ec522
parent 610912 9084a464c568b660bee5183ebf626ce21257b206
child 637995 5c0848bdc280734898d59ff0e28c9330026438da
push id69043
push userbmo:tchiovoloni@mozilla.com
push dateTue, 18 Jul 2017 23:02:32 +0000
reviewersmarkh, kitcambridge
bugs1374500
milestone56.0a1
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,412 @@
+/* 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://services-common/async.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) {
+      Async.promiseSpinningly(profileStorage.initialize());
+      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 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"],