Bug 1368008 - [Form Autofill] Cache the computed fields in profileStorage. r=MattN, steveck, seanlee draft
authorLuke Chang <lchang@mozilla.com>
Fri, 26 May 2017 18:05:48 +0800
changeset 608720 023bcae335df2d081b8cc83dd5946578ddb8ed87
parent 608658 67cd1ee26f2661fa5efe3d952485ab3c89af4271
child 637401 ba04ef4f6563d5f0a8882a4db096db1b5f685b27
push id68386
push userbmo:lchang@mozilla.com
push dateFri, 14 Jul 2017 03:07:10 +0000
reviewersMattN, steveck, seanlee
bugs1368008
milestone56.0a1
Bug 1368008 - [Form Autofill] Cache the computed fields in profileStorage. r=MattN, steveck, seanlee MozReview-Commit-ID: 5bloPW93DEr
browser/extensions/formautofill/FormAutofillNameUtils.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/test_addressRecords.js
browser/extensions/formautofill/test/unit/test_creditCardRecords.js
browser/extensions/formautofill/test/unit/test_migrateRecords.js
browser/extensions/formautofill/test/unit/test_storage_tombstones.js
browser/extensions/formautofill/test/unit/test_transformFields.js
browser/extensions/formautofill/test/unit/xpcshell.ini
--- a/browser/extensions/formautofill/FormAutofillNameUtils.jsm
+++ b/browser/extensions/formautofill/FormAutofillNameUtils.jsm
@@ -209,23 +209,27 @@ var FormAutofillNameUtils = {
     let sandbox = FormAutofillUtils.loadDataFromScript(NAME_REFERENCES);
     Object.assign(this, sandbox.nameReferences);
     this._dataLoaded = true;
 
     this.reCJK = new RegExp("[" + this.CJK_RANGE.join("") + "]", "u");
   },
 
   splitName(name) {
-    let nameTokens = name.trim().split(/[ ,\u3000\u30FB\u00B7]+/);
     let nameParts = {
       given: "",
       middle: "",
       family: "",
     };
 
+    if (!name) {
+      return nameParts;
+    }
+
+    let nameTokens = name.trim().split(/[ ,\u3000\u30FB\u00B7]+/);
     nameTokens = this._stripPrefixes(nameTokens);
 
     if (this._isCJKName(name)) {
       let parts = this._splitCJKName(nameTokens);
       if (parts) {
         return parts;
       }
     }
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -107,38 +107,55 @@ XPCOMUtils.defineLazyGetter(this, "REGIO
 });
 
 const PROFILE_JSON_FILE_NAME = "autofill-profiles.json";
 
 const STORAGE_SCHEMA_VERSION = 1;
 const ADDRESS_SCHEMA_VERSION = 1;
 const CREDIT_CARD_SCHEMA_VERSION = 1;
 
-const VALID_PROFILE_FIELDS = [
+const VALID_ADDRESS_FIELDS = [
   "given-name",
   "additional-name",
   "family-name",
   "organization",
   "street-address",
   "address-level2",
   "address-level1",
   "postal-code",
   "country",
   "tel",
   "email",
 ];
 
+const STREET_ADDRESS_COMPONENTS = [
+  "address-line1",
+  "address-line2",
+  "address-line3",
+];
+
+const VALID_ADDRESS_COMPUTED_FIELDS = [
+  "name",
+  "country-name",
+].concat(STREET_ADDRESS_COMPONENTS);
+
 const VALID_CREDIT_CARD_FIELDS = [
   "cc-name",
   "cc-number-encrypted",
   "cc-number-masked",
   "cc-exp-month",
   "cc-exp-year",
 ];
 
+const VALID_CREDIT_CARD_COMPUTED_FIELDS = [
+  "cc-given-name",
+  "cc-additional-name",
+  "cc-family-name",
+];
+
 const INTERNAL_FIELDS = [
   "guid",
   "version",
   "timeCreated",
   "timeLastUsed",
   "timeLastModified",
   "timesUsed",
 ];
@@ -155,27 +172,35 @@ class AutofillRecords {
    * Creates an AutofillRecords.
    *
    * @param {JSONFile} store
    *        An instance of JSONFile.
    * @param {string} collectionName
    *        A key of "store.data".
    * @param {Array.<string>} validFields
    *        A list containing non-metadata field names.
+   * @param {Array.<string>} validComputedFields
+   *        A list containing computed field names.
    * @param {number} schemaVersion
    *        The schema version for the new record.
    */
-  constructor(store, collectionName, validFields, schemaVersion) {
+  constructor(store, collectionName, validFields, validComputedFields, schemaVersion) {
     FormAutofillUtils.defineLazyLogGetter(this, "AutofillRecords:" + collectionName);
 
     this.VALID_FIELDS = validFields;
+    this.VALID_COMPUTED_FIELDS = validComputedFields;
 
     this._store = store;
     this._collectionName = collectionName;
     this._schemaVersion = schemaVersion;
+
+    let hasChanges = (result, record) => this._migrateRecord(record) || result;
+    if (this._store.data[this._collectionName].reduce(hasChanges, false)) {
+      this._store.saveSoon();
+    }
   }
 
   /**
    * Gets the schema version number.
    *
    * @returns {number}
    *          The current schema version number.
    */
@@ -221,16 +246,18 @@ class AutofillRecords {
       // Metadata
       let now = Date.now();
       recordToSave.timeCreated = now;
       recordToSave.timeLastModified = now;
       recordToSave.timeLastUsed = 0;
       recordToSave.timesUsed = 0;
     }
 
+    this._computeFields(recordToSave);
+
     this._store.data[this._collectionName].push(recordToSave);
     this._store.saveSoon();
 
     Services.obs.notifyObservers(null, "formautofill-storage-changed", "add");
     return recordToSave.guid;
   }
 
   /**
@@ -256,18 +283,20 @@ class AutofillRecords {
         recordFound[field] = recordToUpdate[field];
       } else {
         delete recordFound[field];
       }
     }
 
     recordFound.timeLastModified = Date.now();
 
+    this._stripComputedFields(recordFound);
+    this._computeFields(recordFound);
+
     this._store.saveSoon();
-
     Services.obs.notifyObservers(null, "formautofill-storage-changed", "update");
   }
 
   /**
    * Notifies the stroage of the use of the specified record, so we can update
    * the metadata accordingly.
    *
    * @param  {string} guid
@@ -313,50 +342,62 @@ class AutofillRecords {
     Services.obs.notifyObservers(null, "formautofill-storage-changed", "remove");
   }
 
   /**
    * Returns the record with the specified GUID.
    *
    * @param   {string} guid
    *          Indicates which record to retrieve.
+   * @param   {boolean} [options.rawData = false]
+   *          Returns a raw record without modifications and the computed fields.
    * @returns {Object}
    *          A clone of the record.
    */
-  get(guid) {
-    this.log.debug("get:", guid);
+  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);
-    this._recordReadProcessor(clonedRecord);
+    if (rawData) {
+      this._stripComputedFields(clonedRecord);
+    } else {
+      this._recordReadProcessor(clonedRecord);
+    }
     return clonedRecord;
   }
 
   /**
    * Returns all records.
    *
-   * @param   {boolean} [options.noComputedFields = false]
-   *          Returns raw record without those computed fields.
+   * @param   {boolean} [options.rawData = false]
+   *          Returns raw records without modifications and the computed fields.
    * @param   {boolean} [options.includeDeleted = false]
    *          Also return any tombstone records.
    * @returns {Array.<Object>}
    *          An array containing clones of all records.
    */
-  getAll({noComputedFields = false, includeDeleted = false} = {}) {
-    this.log.debug("getAll", noComputedFields, includeDeleted);
+  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(this._clone);
-    clonedRecords.forEach(record => this._recordReadProcessor(record, {noComputedFields}));
+    clonedRecords.forEach(record => {
+      if (rawData) {
+        this._stripComputedFields(record);
+      } else {
+        this._recordReadProcessor(record);
+      }
+    });
     return clonedRecords;
   }
 
   /**
    * Returns the filtered records based on input's information and searchString.
    *
    * @returns {Array.<Object>}
    *          An array containing clones of matched record.
@@ -392,151 +433,186 @@ class AutofillRecords {
   }
 
   _findIndexByGUID(guid, {includeDeleted = false} = {}) {
     return this._store.data[this._collectionName].findIndex(record => {
       return record.guid == guid && (!record.deleted || includeDeleted);
     });
   }
 
+  _migrateRecord(record) {
+    let hasChanges = false;
+
+    if (!record.version || isNaN(record.version) || record.version < 1) {
+      this.log.warn("Invalid record version:", record.version);
+
+      // Force to run the migration.
+      record.version = 0;
+    }
+
+    if (record.version < this.version) {
+      hasChanges = true;
+      record.version = this.version;
+
+      // Force to recompute fields if we upgrade the schema.
+      this._stripComputedFields(record);
+    }
+
+    hasChanges |= this._computeFields(record);
+    return hasChanges;
+  }
+
   _normalizeRecord(record) {
-    this._recordWriteProcessor(record);
+    this._normalizeFields(record);
 
     for (let key in record) {
       if (!this.VALID_FIELDS.includes(key)) {
         throw new Error(`"${key}" is not a valid field.`);
       }
       if (typeof record[key] !== "string" &&
           typeof record[key] !== "number") {
         throw new Error(`"${key}" contains invalid data type.`);
       }
     }
   }
 
-  // An interface to be inherited.
-  _recordReadProcessor(record, {noComputedFields = false} = {}) {}
+  _stripComputedFields(record) {
+    this.VALID_COMPUTED_FIELDS.forEach(field => delete record[field]);
+  }
 
   // An interface to be inherited.
-  _recordWriteProcessor(record) {}
+  _recordReadProcessor(record) {}
+
+  // An interface to be inherited.
+  _computeFields(record) {}
+
+  // An interface to be inherited.
+  _normalizeFields(record) {}
 
   // An interface to be inherited.
   mergeIfPossible(guid, record) {}
 
   // An interface to be inherited.
   mergeToStorage(targetRecord) {}
 }
 
 class Addresses extends AutofillRecords {
   constructor(store) {
-    super(store, "addresses", VALID_PROFILE_FIELDS, ADDRESS_SCHEMA_VERSION);
+    super(store, "addresses", VALID_ADDRESS_FIELDS, VALID_ADDRESS_COMPUTED_FIELDS, ADDRESS_SCHEMA_VERSION);
   }
 
-  _recordReadProcessor(profile, {noComputedFields} = {}) {
-    if (noComputedFields) {
-      return;
-    }
-
-    // Compute name
-    let name = FormAutofillNameUtils.joinNameParts({
-      given: profile["given-name"],
-      middle: profile["additional-name"],
-      family: profile["family-name"],
-    });
-    if (name) {
-      profile.name = name;
-    }
-
-    // Compute address
-    if (profile["street-address"]) {
-      let streetAddress = profile["street-address"].split("\n").map(s => s.trim());
-      for (let i = 0; i < 2; i++) {
-        if (streetAddress[i]) {
-          profile["address-line" + (i + 1)] = streetAddress[i];
-        }
-      }
-      if (streetAddress.length > 2) {
-        profile["address-line3"] = FormAutofillUtils.toOneLineAddress(
-          streetAddress.splice(2)
-        );
-      }
-    }
-
-    // Compute country name
-    if (profile.country) {
-      if (profile.country == "US") {
-        let countryName = REGION_NAMES[profile.country];
-        if (countryName) {
-          profile["country-name"] = countryName;
-        }
-      } else {
-        // TODO: We only support US in MVP so hide the field if it's not. We
-        //       are going to support more countries in bug 1370193.
-        delete profile.country;
-      }
+  _recordReadProcessor(address) {
+    // TODO: We only support US in MVP so hide the field if it's not. We
+    //       are going to support more countries in bug 1370193.
+    if (address.country && address.country != "US") {
+      address["country-name"] = "";
+      delete address.country;
     }
   }
 
-  _recordWriteProcessor(profile) {
-    // Normalize name
-    if (profile.name) {
-      let nameParts = FormAutofillNameUtils.splitName(profile.name);
-      if (!profile["given-name"] && nameParts.given) {
-        profile["given-name"] = nameParts.given;
+  _computeFields(address) {
+    // NOTE: Remember to bump the schema version number if any of the existing
+    //       computing algorithm changes. (No need to bump when just adding new
+    //       computed fields)
+
+    let hasNewComputedFields = false;
+
+    // Compute name
+    if (!("name" in address)) {
+      let name = FormAutofillNameUtils.joinNameParts({
+        given: address["given-name"],
+        middle: address["additional-name"],
+        family: address["family-name"],
+      });
+      address.name = name;
+      hasNewComputedFields = true;
+    }
+
+    // Compute address lines
+    if (!("address-line1" in address)) {
+      let streetAddress = [];
+      if (address["street-address"]) {
+        streetAddress = address["street-address"].split("\n").map(s => s.trim());
       }
-      if (!profile["additional-name"] && nameParts.middle) {
-        profile["additional-name"] = nameParts.middle;
+      for (let i = 0; i < 3; i++) {
+        address["address-line" + (i + 1)] = streetAddress[i] || "";
       }
-      if (!profile["family-name"] && nameParts.family) {
-        profile["family-name"] = nameParts.family;
+      if (streetAddress.length > 3) {
+        address["address-line3"] = FormAutofillUtils.toOneLineAddress(
+          streetAddress.splice(2)
+        );
       }
-      delete profile.name;
+      hasNewComputedFields = true;
     }
 
-    // Normalize address
-    if (profile["address-line1"] || profile["address-line2"] ||
-        profile["address-line3"]) {
+    // Compute country name
+    if (!("country-name" in address)) {
+      if (address.country && REGION_NAMES[address.country]) {
+        address["country-name"] = REGION_NAMES[address.country];
+      } else {
+        address["country-name"] = "";
+      }
+      hasNewComputedFields = true;
+    }
+
+    return hasNewComputedFields;
+  }
+
+  _normalizeFields(address) {
+    // Normalize name
+    if (address.name) {
+      let nameParts = FormAutofillNameUtils.splitName(address.name);
+      if (!address["given-name"] && nameParts.given) {
+        address["given-name"] = nameParts.given;
+      }
+      if (!address["additional-name"] && nameParts.middle) {
+        address["additional-name"] = nameParts.middle;
+      }
+      if (!address["family-name"] && nameParts.family) {
+        address["family-name"] = nameParts.family;
+      }
+      delete address.name;
+    }
+
+    // Normalize address lines
+    if (STREET_ADDRESS_COMPONENTS.some(c => address[c])) {
       // Treat "street-address" as "address-line1" if it contains only one line
       // and "address-line1" is omitted.
-      if (!profile["address-line1"] && profile["street-address"] &&
-          !profile["street-address"].includes("\n")) {
-        profile["address-line1"] = profile["street-address"];
-        delete profile["street-address"];
+      if (!address["address-line1"] && address["street-address"] &&
+          !address["street-address"].includes("\n")) {
+        address["address-line1"] = address["street-address"];
+        delete address["street-address"];
       }
 
-      // Remove "address-line*" but keep the values.
-      let addressLines = [1, 2, 3].map(i => {
-        let value = profile["address-line" + i];
-        delete profile["address-line" + i];
-        return value;
-      });
+      // Concatenate "address-line*" if "street-address" is omitted.
+      if (!address["street-address"]) {
+        address["street-address"] = STREET_ADDRESS_COMPONENTS.map(c => address[c]).join("\n");
+      }
 
-      // Concatenate "address-line*" if "street-address" is omitted.
-      if (!profile["street-address"]) {
-        profile["street-address"] = addressLines.join("\n");
-      }
+      STREET_ADDRESS_COMPONENTS.forEach(c => delete address[c]);
     }
 
     // Normalize country
-    if (profile.country) {
-      let country = profile.country.toUpperCase();
+    if (address.country) {
+      let country = address.country.toUpperCase();
       // Only values included in the region list will be saved.
       if (REGION_NAMES[country]) {
-        profile.country = country;
+        address.country = country;
       } else {
-        delete profile.country;
+        delete address.country;
       }
-    } else if (profile["country-name"]) {
+    } else if (address["country-name"]) {
       for (let region in REGION_NAMES) {
-        if (REGION_NAMES[region].toLowerCase() == profile["country-name"].toLowerCase()) {
-          profile.country = region;
+        if (REGION_NAMES[region].toLowerCase() == address["country-name"].toLowerCase()) {
+          address.country = region;
           break;
         }
       }
     }
-    delete profile["country-name"];
+    delete address["country-name"];
   }
 
   /**
    * Merge new address into the specified address if mergeable.
    *
    * @param  {string} guid
    *         Indicates which address to merge.
    * @param  {Object} address
@@ -582,16 +658,20 @@ class Addresses extends AutofillRecords 
 
     for (let field in addressToMerge) {
       if (this.VALID_FIELDS.includes(field)) {
         addressFound[field] = addressToMerge[field];
       }
     }
 
     addressFound.timeLastModified = Date.now();
+
+    this._stripComputedFields(addressFound);
+    this._computeFields(addressFound);
+
     this._store.saveSoon();
     let str = Cc["@mozilla.org/supports-string;1"]
                  .createInstance(Ci.nsISupportsString);
     str.data = guid;
     Services.obs.notifyObservers(str, "formautofill-storage-changed", "merge");
     return true;
   }
 
@@ -611,40 +691,39 @@ class Addresses extends AutofillRecords 
     }
     this.log.debug("Existing records matching and merging count is", mergedGUIDs.length);
     return mergedGUIDs;
   }
 }
 
 class CreditCards extends AutofillRecords {
   constructor(store) {
-    super(store, "creditCards", VALID_CREDIT_CARD_FIELDS, CREDIT_CARD_SCHEMA_VERSION);
+    super(store, "creditCards", VALID_CREDIT_CARD_FIELDS, VALID_CREDIT_CARD_COMPUTED_FIELDS, CREDIT_CARD_SCHEMA_VERSION);
   }
 
-  _recordReadProcessor(creditCard, {noComputedFields} = {}) {
-    if (noComputedFields) {
-      return;
-    }
+  _computeFields(creditCard) {
+    // NOTE: Remember to bump the schema version number if any of the existing
+    //       computing algorithm changes. (No need to bump when just adding new
+    //       computed fields)
+
+    let hasNewComputedFields = false;
 
     // Compute split names
-    if (creditCard["cc-name"]) {
+    if (!("cc-given-name" in creditCard)) {
       let nameParts = FormAutofillNameUtils.splitName(creditCard["cc-name"]);
-      if (nameParts.given) {
-        creditCard["cc-given-name"] = nameParts.given;
-      }
-      if (nameParts.middle) {
-        creditCard["cc-additional-name"] = nameParts.middle;
-      }
-      if (nameParts.family) {
-        creditCard["cc-family-name"] = nameParts.family;
-      }
+      creditCard["cc-given-name"] = nameParts.given;
+      creditCard["cc-additional-name"] = nameParts.middle;
+      creditCard["cc-family-name"] = nameParts.family;
+      hasNewComputedFields = true;
     }
+
+    return hasNewComputedFields;
   }
 
-  _recordWriteProcessor(creditCard) {
+  _normalizeFields(creditCard) {
     // Fields that should not be set by content.
     delete creditCard["cc-number-encrypted"];
     delete creditCard["cc-number-masked"];
 
     // Validate and encrypt credit card numbers, and calculate the masked numbers
     if (creditCard["cc-number"]) {
       let ccNumber = creditCard["cc-number"].replace(/\s/g, "");
       delete creditCard["cc-number"];
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js
@@ -136,18 +136,18 @@ add_task(async function test_getAll() {
   do_check_record_matches(addresses[0], TEST_ADDRESS_1);
   do_check_record_matches(addresses[1], TEST_ADDRESS_2);
 
   // Check computed fields.
   do_check_eq(addresses[0].name, "Timothy John Berners-Lee");
   do_check_eq(addresses[0]["address-line1"], "32 Vassar Street");
   do_check_eq(addresses[0]["address-line2"], "MIT Room 32-G524");
 
-  // Test with noComputedFields set.
-  addresses = profileStorage.addresses.getAll({noComputedFields: true});
+  // Test with rawData set.
+  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);
 });
@@ -157,16 +157,22 @@ add_task(async function test_get() {
                                                 [TEST_ADDRESS_1, TEST_ADDRESS_2]);
 
   let addresses = profileStorage.addresses.getAll();
   let guid = addresses[0].guid;
 
   let address = profileStorage.addresses.get(guid);
   do_check_record_matches(address, TEST_ADDRESS_1);
 
+  // Test with rawData set.
+  address = profileStorage.addresses.get(guid, {rawData: true});
+  do_check_eq(address.name, undefined);
+  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);
 });
 
 add_task(async function test_getByFilter() {
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -118,18 +118,18 @@ add_task(async function test_getAll() {
   do_check_eq(creditCards.length, 2);
   do_check_credit_card_matches(creditCards[0], TEST_CREDIT_CARD_1);
   do_check_credit_card_matches(creditCards[1], TEST_CREDIT_CARD_2);
 
   // Check computed fields.
   do_check_eq(creditCards[0]["cc-given-name"], "John");
   do_check_eq(creditCards[0]["cc-family-name"], "Doe");
 
-  // Test with noComputedFields set.
-  creditCards = profileStorage.creditCards.getAll({noComputedFields: true});
+  // Test with rawData set.
+  creditCards = profileStorage.creditCards.getAll({rawData: true});
   do_check_eq(creditCards[0]["cc-given-name"], undefined);
   do_check_eq(creditCards[0]["cc-family-name"], undefined);
 
   // Modifying output shouldn't affect the storage.
   creditCards[0]["cc-name"] = "test";
   do_check_credit_card_matches(profileStorage.creditCards.getAll()[0], TEST_CREDIT_CARD_1);
 });
 
new file mode 100644
--- /dev/null
+++ b/browser/extensions/formautofill/test/unit/test_migrateRecords.js
@@ -0,0 +1,229 @@
+/**
+ * Tests the migration algorithm in profileStorage.
+ */
+
+"use strict";
+
+const {ProfileStorage} = Cu.import("resource://formautofill/ProfileStorage.jsm", {});
+
+const TEST_STORE_FILE_NAME = "test-profile.json";
+
+const ADDRESS_SCHEMA_VERSION = 1;
+const CREDIT_CARD_SCHEMA_VERSION = 1;
+
+const ADDRESS_TESTCASES = [
+  {
+    description: "The record version is equal to the current version. The migration shouldn't be invoked.",
+    record: {
+      guid: "test-guid",
+      version: ADDRESS_SCHEMA_VERSION,
+      "given-name": "Timothy",
+      name: "John", // The cached name field doesn't align "given-name" but it
+                    // won't be recomputed because the migration isn't invoked.
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: ADDRESS_SCHEMA_VERSION,
+      "given-name": "Timothy",
+      name: "John",
+    },
+  },
+  {
+    description: "The record version is greater than the current version. The migration shouldn't be invoked.",
+    record: {
+      guid: "test-guid",
+      version: 99,
+      "given-name": "Timothy",
+      name: "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: 99,
+      "given-name": "Timothy",
+      name: "John",
+    },
+  },
+  {
+    description: "The record version is less than the current version. The migration should be invoked.",
+    record: {
+      guid: "test-guid",
+      version: 0,
+      "given-name": "Timothy",
+      name: "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: ADDRESS_SCHEMA_VERSION,
+      "given-name": "Timothy",
+      name: "Timothy",
+    },
+  },
+  {
+    description: "The record version is omitted. The migration should be invoked.",
+    record: {
+      guid: "test-guid",
+      "given-name": "Timothy",
+      name: "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: ADDRESS_SCHEMA_VERSION,
+      "given-name": "Timothy",
+      name: "Timothy",
+    },
+  },
+  {
+    description: "The record version is an invalid value. The migration should be invoked.",
+    record: {
+      guid: "test-guid",
+      version: "ABCDE",
+      "given-name": "Timothy",
+      name: "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: ADDRESS_SCHEMA_VERSION,
+      "given-name": "Timothy",
+      name: "Timothy",
+    },
+  },
+  {
+    description: "The omitted computed fields should be always recomputed even the record version is up-to-date.",
+    record: {
+      guid: "test-guid",
+      version: ADDRESS_SCHEMA_VERSION,
+      "given-name": "Timothy",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: ADDRESS_SCHEMA_VERSION,
+      "given-name": "Timothy",
+      name: "Timothy",
+    },
+  },
+];
+
+const CREDIT_CARD_TESTCASES = [
+  {
+    description: "The record version is equal to the current version. The migration shouldn't be invoked.",
+    record: {
+      guid: "test-guid",
+      version: CREDIT_CARD_SCHEMA_VERSION,
+      "cc-name": "Timothy",
+      "cc-given-name": "John", // The cached "cc-given-name" field doesn't align
+                               // "cc-name" but it won't be recomputed because
+                               // the migration isn't invoked.
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: CREDIT_CARD_SCHEMA_VERSION,
+      "cc-name": "Timothy",
+      "cc-given-name": "John",
+    },
+  },
+  {
+    description: "The record version is greater than the current version. The migration shouldn't be invoked.",
+    record: {
+      guid: "test-guid",
+      version: 99,
+      "cc-name": "Timothy",
+      "cc-given-name": "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: 99,
+      "cc-name": "Timothy",
+      "cc-given-name": "John",
+    },
+  },
+  {
+    description: "The record version is less than the current version. The migration should be invoked.",
+    record: {
+      guid: "test-guid",
+      version: 0,
+      "cc-name": "Timothy",
+      "cc-given-name": "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: CREDIT_CARD_SCHEMA_VERSION,
+      "cc-name": "Timothy",
+      "cc-given-name": "Timothy",
+    },
+  },
+  {
+    description: "The record version is omitted. The migration should be invoked.",
+    record: {
+      guid: "test-guid",
+      "cc-name": "Timothy",
+      "cc-given-name": "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: CREDIT_CARD_SCHEMA_VERSION,
+      "cc-name": "Timothy",
+      "cc-given-name": "Timothy",
+    },
+  },
+  {
+    description: "The record version is an invalid value. The migration should be invoked.",
+    record: {
+      guid: "test-guid",
+      version: "ABCDE",
+      "cc-name": "Timothy",
+      "cc-given-name": "John",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: CREDIT_CARD_SCHEMA_VERSION,
+      "cc-name": "Timothy",
+      "cc-given-name": "Timothy",
+    },
+  },
+  {
+    description: "The omitted computed fields should be always recomputed even the record version is up-to-date.",
+    record: {
+      guid: "test-guid",
+      version: CREDIT_CARD_SCHEMA_VERSION,
+      "cc-name": "Timothy",
+    },
+    expectedResult: {
+      guid: "test-guid",
+      version: CREDIT_CARD_SCHEMA_VERSION,
+      "cc-name": "Timothy",
+      "cc-given-name": "Timothy",
+    },
+  },
+];
+
+let do_check_record_matches = (expectedRecord, record) => {
+  for (let key in expectedRecord) {
+    do_check_eq(expectedRecord[key], record[key]);
+  }
+};
+
+add_task(async function test_migrateAddressRecords() {
+  let path = getTempFile(TEST_STORE_FILE_NAME).path;
+
+  let profileStorage = new ProfileStorage(path);
+  await profileStorage.initialize();
+
+  ADDRESS_TESTCASES.forEach(testcase => {
+    do_print(testcase.description);
+    profileStorage.addresses._migrateRecord(testcase.record);
+    do_check_record_matches(testcase.expectedResult, testcase.record);
+  });
+});
+
+add_task(async function test_migrateCreditCardRecords() {
+  let path = getTempFile(TEST_STORE_FILE_NAME).path;
+
+  let profileStorage = new ProfileStorage(path);
+  await profileStorage.initialize();
+
+  CREDIT_CARD_TESTCASES.forEach(testcase => {
+    do_print(testcase.description);
+    profileStorage.creditCards._migrateRecord(testcase.record);
+    do_check_record_matches(testcase.expectedResult, testcase.record);
+  });
+});
--- a/browser/extensions/formautofill/test/unit/test_storage_tombstones.js
+++ b/browser/extensions/formautofill/test/unit/test_storage_tombstones.js
@@ -74,17 +74,17 @@ add_storage_task(async function test_add
   let guid = storage.add({guid: "test-guid-1", deleted: true});
 
   // should be unable to get it normally.
   Assert.equal(storage.get(guid), null);
   // and getAll should also not return it.
   Assert.equal(storage.getAll().length, 0);
 
   // but getAll allows us to access deleted items.
-  let all = storage.getAll({includeDeleted: true});
+  let all = storage.getAll({rawData: true, includeDeleted: true});
   Assert.equal(all.length, 1);
 
   do_check_tombstone_record(all[0]);
 });
 
 add_storage_task(async function test_add_tombstone_without_guid(storage, record) {
   do_print("Should not be able to add a new tombstone without specifying the guid");
   Assert.throws(() => { storage.add({deleted: true}); });
@@ -107,14 +107,14 @@ add_storage_task(async function test_upd
   Assert.throws(() => storage.update(guid, {}), /No matching record./);
 });
 
 add_storage_task(async function test_remove_existing_tombstone(storage, record) {
   do_print("Removing a record that's already a tombstone should be a no-op");
   let guid = storage.add({guid: "test-guid-1", deleted: true, timeLastModified: 1234});
 
   storage.remove(guid);
-  let all = storage.getAll({includeDeleted: true});
+  let all = storage.getAll({rawData: true, includeDeleted: true});
   Assert.equal(all.length, 1);
 
   do_check_tombstone_record(all[0]);
   equal(all[0].timeLastModified, 1234); // should not be updated to now().
 });
--- a/browser/extensions/formautofill/test/unit/test_transformFields.js
+++ b/browser/extensions/formautofill/test/unit/test_transformFields.js
@@ -60,17 +60,17 @@ const ADDRESS_COMPUTE_TESTCASES = [
   {
     description: "\"street-address\" with multiple lines but line2 is omitted",
     address: {
       "street-address": "line1\n\nline3",
     },
     expectedResult: {
       "street-address": "line1\n\nline3",
       "address-line1": "line1",
-      "address-line2": undefined,
+      "address-line2": "",
       "address-line3": "line3",
     },
   },
   {
     description: "\"street-address\" with 4 lines",
     address: {
       "street-address": "line1\nline2\nline3\nline4",
     },
@@ -84,17 +84,17 @@ const ADDRESS_COMPUTE_TESTCASES = [
   {
     description: "\"street-address\" with blank lines",
     address: {
       "street-address": "line1\n \nline3\n \nline5",
     },
     expectedResult: {
       "street-address": "line1\n \nline3\n \nline5",
       "address-line1": "line1",
-      "address-line2": null,
+      "address-line2": "",
       "address-line3": "line3 line5",
     },
   },
 
   // Country
   {
     description: "Has \"country\"",
     address: {
@@ -224,49 +224,59 @@ const ADDRESS_NORMALIZE_TESTCASES = [
   },
   {
     description: "Has \"country-name\"",
     address: {
       "country-name": "united states",
     },
     expectedResult: {
       "country": "US",
-      "country-name": undefined,
+      "country-name": "United States",
     },
   },
   {
     description: "Has unknown \"country-name\"",
     address: {
       "country-name": "unknown country name",
     },
     expectedResult: {
       "country": undefined,
-      "country-name": undefined,
+      "country-name": "",
     },
   },
   {
     description: "Has \"country\" and unknown \"country-name\"",
     address: {
       "country": "us",
       "country-name": "unknown country name",
     },
     expectedResult: {
       "country": "US",
-      "country-name": undefined,
+      "country-name": "United States",
     },
   },
   {
     description: "Has \"country-name\" and unknown \"country\"",
     address: {
       "country": "AA",
       "country-name": "united states",
     },
     expectedResult: {
       "country": undefined,
-      "country-name": undefined,
+      "country-name": "",
+    },
+  },
+  {
+    description: "Has unsupported \"country\"",
+    address: {
+      "country": "CA",
+    },
+    expectedResult: {
+      "country": undefined,
+      "country-name": "",
     },
   },
 ];
 
 const CREDIT_CARD_COMPUTE_TESTCASES = [
   // Empty
   {
     description: "Empty credit card",
--- a/browser/extensions/formautofill/test/unit/xpcshell.ini
+++ b/browser/extensions/formautofill/test/unit/xpcshell.ini
@@ -25,15 +25,16 @@ support-files =
 [test_findLabelElements.js]
 [test_getAdaptedProfiles.js]
 [test_getCategoriesFromFieldNames.js]
 [test_getFormInputDetails.js]
 [test_getInfo.js]
 [test_isCJKName.js]
 [test_isFieldEligibleForAutofill.js]
 [test_markAsAutofillField.js]
+[test_migrateRecords.js]
 [test_nameUtils.js]
 [test_onFormSubmitted.js]
 [test_profileAutocompleteResult.js]
 [test_savedFieldNames.js]
 [test_toOneLineAddress.js]
 [test_storage_tombstones.js]
 [test_transformFields.js]