Bug 1395122 - [Form Autofill] Part 2: Make "reconcile" and "findDuplicateGUID" async. r=markh,kitcambridge,seanlee draft
authorLuke Chang <lchang@mozilla.com>
Tue, 05 Sep 2017 12:30:41 +0800
changeset 660546 a607019239ae68f0097449d9328ff75d7f1314b0
parent 660545 72e308ecb96b24f2fe53c3a3eed500e1d447cd55
child 660547 51e2df8fd667b9d2e8fab44fff3e0e0b51d0412e
push id78427
push userbmo:lchang@mozilla.com
push dateThu, 07 Sep 2017 03:59:04 +0000
reviewersmarkh, kitcambridge, seanlee
bugs1395122
milestone57.0a1
Bug 1395122 - [Form Autofill] Part 2: Make "reconcile" and "findDuplicateGUID" async. r=markh,kitcambridge,seanlee MozReview-Commit-ID: FrgVm4WniKu
browser/extensions/formautofill/FormAutofillSync.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/test_reconcile.js
browser/extensions/formautofill/test/unit/test_storage_syncfields.js
--- a/browser/extensions/formautofill/FormAutofillSync.jsm
+++ b/browser/extensions/formautofill/FormAutofillSync.jsm
@@ -118,17 +118,17 @@ FormAutofillStore.prototype = {
 
     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.toEntry());
+    let localDupeID = await this.storage.findDuplicateGUID(remoteRecord.toEntry());
     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;
     }
@@ -156,17 +156,17 @@ FormAutofillStore.prototype = {
     }
     return record;
   },
 
   async _doUpdateRecord(record) {
     this._log.trace("Updating record", record);
 
     let entry = record.toEntry();
-    let {forkedGUID} = this.storage.reconcile(entry);
+    let {forkedGUID} = await this.storage.reconcile(entry);
     if (this._log.level <= Log.Level.Debug) {
       let forkedRecord = forkedGUID ? this.storage.get(forkedGUID) : null;
       let reconciledRecord = this.storage.get(record.id);
       this._log.debug("Updated local record", {
         forked: sanitizeStorageObject(forkedRecord),
         updated: sanitizeStorageObject(reconciledRecord),
       });
     }
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -755,17 +755,17 @@ class AutofillRecords {
    *          `add` to insert remote records that don't exist locally, and
    *          `remove` to apply remote tombstones.
    * @returns {Object}
    *          A `{forkedGUID}` tuple. `forkedGUID` is `null` if the merge
    *          succeeded without conflicts, or a new GUID referencing the
    *          existing locally modified record if the conflicts could not be
    *          resolved.
    */
-  reconcile(remoteRecord) {
+  async reconcile(remoteRecord) {
     this._ensureMatchingVersion(remoteRecord);
     if (remoteRecord.deleted) {
       throw new Error(`Can't reconcile tombstone ${remoteRecord.guid}`);
     }
 
     let localIndex = this._findIndexByGUID(remoteRecord.guid);
     if (localIndex < 0) {
       throw new Error(`Record ${remoteRecord.guid} not found`);
@@ -987,17 +987,17 @@ class AutofillRecords {
    * duplicate profiles with the same information.
    *
    * @param   {Object} record
    *          The remote record.
    * @returns {string|null}
    *          The GUID of the matching local record, or `null` if no records
    *          match.
    */
-  findDuplicateGUID(record) {
+  async findDuplicateGUID(record) {
     if (!record.guid) {
       throw new Error("Record missing GUID");
     }
     this._ensureMatchingVersion(record);
     if (record.deleted) {
       // Tombstones don't carry enough info to de-dupe, and we should have
       // handled them separately when applying the record.
       throw new Error("Tombstones can't have duplicates");
--- a/browser/extensions/formautofill/test/unit/test_reconcile.js
+++ b/browser/extensions/formautofill/test/unit/test_reconcile.js
@@ -463,24 +463,22 @@ const RECONCILE_TESTCASES = [
     },
   },
 ];
 
 add_task(async function test_reconcile_unknown_version() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME);
 
   // Cross-version reconciliation isn't supported yet. See bug 1377204.
-  throws(() => {
-    profileStorage.addresses.reconcile({
-      "guid": "31d83d2725ec",
-      "version": 2,
-      "given-name": "Mark",
-      "family-name": "Hammond",
-    });
-  }, /Got unknown record version/);
+  await Assert.rejects(profileStorage.addresses.reconcile({
+    "guid": "31d83d2725ec",
+    "version": 2,
+    "given-name": "Mark",
+    "family-name": "Hammond",
+  }), /Got unknown record version/);
 });
 
 add_task(async function test_reconcile_idempotent() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME);
 
   let guid = "de1ba7b094fe";
   profileStorage.addresses.add({
     guid,
@@ -498,33 +496,33 @@ add_task(async function test_reconcile_i
     guid,
     "version": 1,
     "given-name": "Mark",
     "family-name": "Hammond",
     "tel": "123456",
   };
 
   {
-    let {forkedGUID} = profileStorage.addresses.reconcile(remote);
+    let {forkedGUID} = await profileStorage.addresses.reconcile(remote);
     let updatedRecord = profileStorage.addresses.get(guid, {
       rawData: true,
     });
 
     ok(!forkedGUID, "First merge should not fork record");
     ok(objectMatches(updatedRecord, {
       "guid": "de1ba7b094fe",
       "given-name": "Skip",
       "family-name": "Hammond",
       "organization": "Mozilla",
       "tel": "123456",
     }), "First merge should merge local and remote changes");
   }
 
   {
-    let {forkedGUID} = profileStorage.addresses.reconcile(remote);
+    let {forkedGUID} = await profileStorage.addresses.reconcile(remote);
     let updatedRecord = profileStorage.addresses.get(guid, {
       rawData: true,
     });
 
     ok(!forkedGUID, "Second merge should not fork record");
     ok(objectMatches(updatedRecord, {
       "guid": "de1ba7b094fe",
       "given-name": "Skip",
@@ -546,17 +544,17 @@ add_task(async function test_reconcile_t
     for (let updatedRecord of test.local) {
       profileStorage.addresses.update(test.parent.guid, updatedRecord);
     }
 
     let localRecord = profileStorage.addresses.get(test.parent.guid, {
       rawData: true,
     });
 
-    let {forkedGUID} = profileStorage.addresses.reconcile(test.remote);
+    let {forkedGUID} = await profileStorage.addresses.reconcile(test.remote);
     let reconciledRecord = profileStorage.addresses.get(test.parent.guid, {
       rawData: true,
     });
     if (forkedGUID) {
       let forkedRecord = profileStorage.addresses.get(forkedGUID, {
         rawData: true,
       });
 
--- a/browser/extensions/formautofill/test/unit/test_storage_syncfields.js
+++ b/browser/extensions/formautofill/test/unit/test_storage_syncfields.js
@@ -362,33 +362,33 @@ add_task(async function test_changeGUID(
   ok(!profileStorage.addresses.get(guid_u1), "old guid no longer exists.");
 });
 
 add_task(async function test_findDuplicateGUID() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1]);
 
   let [record] = profileStorage.addresses.getAll({rawData: true});
-  Assert.throws(() => profileStorage.addresses.findDuplicateGUID(record),
-                /Record \w+ already exists/,
-                "Should throw if the GUID already exists");
+  await Assert.rejects(profileStorage.addresses.findDuplicateGUID(record),
+    /Record \w+ already exists/,
+    "Should throw if the GUID already exists");
 
   // Add a malformed record, passing `sourceSync` to work around the record
   // normalization logic that would prevent this.
   let timeLastModified = Date.now();
   let timeCreated = timeLastModified - 60 * 1000;
 
   profileStorage.addresses.add({
     guid: profileStorage.addresses._generateGUID(),
     version: 1,
     timeCreated,
     timeLastModified,
   }, {sourceSync: true});
 
-  strictEqual(profileStorage.addresses.findDuplicateGUID({
+  strictEqual(await profileStorage.addresses.findDuplicateGUID({
     guid: profileStorage.addresses._generateGUID(),
     version: 1,
     timeCreated,
     timeLastModified,
   }), null, "Should ignore internal fields and malformed records");
 });
 
 add_task(async function test_reset() {