Bug 1413488 - [Form Autofill] Rename the "data" property of "AutofillRecords" to "_data". r=seanlee
authorLuke Chang <lchang@mozilla.com>
Thu, 16 Nov 2017 13:25:43 +0800
changeset 446678 130f10cfd1b5a4409a1da256478008dc07902ad3
parent 446677 37707fcd8a55a2dcb3a0fee5c032f87ec81d921c
child 446679 e0bd3937d98a6d7d3f871e06fbacf4c45cf83657
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseanlee
bugs1413488
milestone59.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 1413488 - [Form Autofill] Rename the "data" property of "AutofillRecords" to "_data". r=seanlee MozReview-Commit-ID: CmvqUW9PrrW
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/moz.build
browser/extensions/formautofill/test/unit/test_addressRecords.js
browser/extensions/formautofill/test/unit/test_creditCardRecords.js
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -260,17 +260,17 @@ class AutofillRecords {
     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.data.reduce(hasChanges, false)) {
+    if (this._data.reduce(hasChanges, false)) {
       this._store.saveSoon();
     }
   }
 
   /**
    * Gets the schema version number.
    *
    * @returns {number}
@@ -281,17 +281,17 @@ class AutofillRecords {
   }
 
   /**
    * Gets the data of this collection.
    *
    * @returns {array}
    *          The data object.
    */
-  get data() {
+  get _data() {
     return this._store.data[this._collectionName];
   }
 
   // Ensures that we don't try to apply synced records with newer schema
   // versions. This is a temporary measure to ensure we don't accidentally
   // bump the schema version without a syncing strategy in place (bug 1377204).
   _ensureMatchingVersion(record) {
     if (record.version != this.version) {
@@ -317,19 +317,19 @@ class AutofillRecords {
 
     if (sourceSync) {
       // Remove tombstones for incoming items that were changed on another
       // device. Local deletions always lose to avoid data loss.
       let index = this._findIndexByGUID(recordToSave.guid, {
         includeDeleted: true,
       });
       if (index > -1) {
-        let existing = this.data[index];
+        let existing = this._data[index];
         if (existing.deleted) {
-          this.data.splice(index, 1);
+          this._data.splice(index, 1);
         } else {
           throw new Error(`Record ${recordToSave.guid} already exists`);
         }
       }
     } else if (!recordToSave.deleted) {
       this._normalizeRecord(recordToSave);
 
       recordToSave.guid = this._generateGUID();
@@ -367,17 +367,17 @@ class AutofillRecords {
       this._computeFields(recordToSave);
     }
 
     if (sourceSync) {
       let sync = this._getSyncMetaData(recordToSave, true);
       sync.changeCounter = 0;
     }
 
-    this.data.push(recordToSave);
+    this._data.push(recordToSave);
 
     this._store.saveSoon();
 
     Services.obs.notifyObservers({wrappedJSObject: {sourceSync}}, "formautofill-storage-changed", "add");
     return recordToSave.guid;
   }
 
   _generateGUID() {
@@ -403,17 +403,17 @@ class AutofillRecords {
     this.log.debug("update:", guid, record);
 
     let recordFoundIndex = this._findIndexByGUID(guid);
     if (recordFoundIndex == -1) {
       throw new Error("No matching record.");
     }
 
     // Clone the record before modifying it to avoid exposing incomplete changes.
-    let recordFound = this._clone(this.data[recordFoundIndex]);
+    let recordFound = this._clone(this._data[recordFoundIndex]);
     this._stripComputedFields(recordFound);
 
     let recordToUpdate = this._clone(record);
     this._normalizeRecord(recordToUpdate, true);
 
     let hasValidField = false;
     for (let field of this.VALID_FIELDS) {
       let oldValue = recordFound[field];
@@ -440,17 +440,17 @@ class AutofillRecords {
 
     recordFound.timeLastModified = Date.now();
     let syncMetadata = this._getSyncMetaData(recordFound);
     if (syncMetadata) {
       syncMetadata.changeCounter += 1;
     }
 
     this._computeFields(recordFound);
-    this.data[recordFoundIndex] = recordFound;
+    this._data[recordFoundIndex] = recordFound;
 
     this._store.saveSoon();
 
     let str = Cc["@mozilla.org/supports-string;1"].createInstance(Ci.nsISupportsString);
     str.data = guid;
     Services.obs.notifyObservers(str, "formautofill-storage-changed", "update");
   }
 
@@ -491,35 +491,35 @@ class AutofillRecords {
     if (sourceSync) {
       this._removeSyncedRecord(guid);
     } else {
       let index = this._findIndexByGUID(guid, {includeDeleted: false});
       if (index == -1) {
         this.log.warn("attempting to remove non-existing entry", guid);
         return;
       }
-      let existing = this.data[index];
+      let existing = this._data[index];
       if (existing.deleted) {
         return; // already a tombstone - don't touch it.
       }
       let existingSync = this._getSyncMetaData(existing);
       if (existingSync) {
         // existing sync metadata means it has been synced. This means we must
         // leave a tombstone behind.
-        this.data[index] = {
+        this._data[index] = {
           guid,
           timeLastModified: Date.now(),
           deleted: true,
           _sync: existingSync,
         };
         existingSync.changeCounter++;
       } else {
         // If there's no sync meta-data, this record has never been synced, so
         // we can delete it.
-        this.data.splice(index, 1);
+        this._data.splice(index, 1);
       }
     }
 
     this._store.saveSoon();
     Services.obs.notifyObservers({wrappedJSObject: {sourceSync}}, "formautofill-storage-changed", "remove");
   }
 
   /**
@@ -559,17 +559,17 @@ class AutofillRecords {
    * @param   {boolean} [options.includeDeleted = false]
    *          Also return any tombstone records.
    * @returns {Array.<Object>}
    *          An array containing clones of all records.
    */
   getAll({rawData = false, includeDeleted = false} = {}) {
     this.log.debug("getAll", rawData, includeDeleted);
 
-    let records = this.data.filter(r => !r.deleted || includeDeleted);
+    let records = this._data.filter(r => !r.deleted || includeDeleted);
     // Records are cloned to avoid accidental modifications from outside.
     let clonedRecords = records.map(r => this._cloneAndCleanUp(r));
     clonedRecords.forEach(record => {
       if (rawData) {
         this._stripComputedFields(record);
       } else {
         this._recordReadProcessor(record);
       }
@@ -707,22 +707,22 @@ class AutofillRecords {
    *          Should we copy Sync metadata? This is true if `remoteRecord` is a
    *          merged record with local changes that we need to upload. Passing
    *          `keepSyncMetadata` retains the record's change counter and
    *          last synced fields, so that we don't clobber the local change if
    *          the sync is interrupted after the record is merged, but before
    *          it's uploaded.
    */
   _replaceRecordAt(index, remoteRecord, {keepSyncMetadata = false} = {}) {
-    let localRecord = this.data[index];
+    let localRecord = this._data[index];
     let newRecord = this._clone(remoteRecord);
 
     this._stripComputedFields(newRecord);
 
-    this.data[index] = newRecord;
+    this._data[index] = newRecord;
 
     if (keepSyncMetadata) {
       // It's safe to move the Sync metadata from the old record to the new
       // record, since we always clone records when we return them, and we
       // never hand out references to the metadata object via public methods.
       newRecord._sync = localRecord._sync;
     } else {
       // As a side effect, `_getSyncMetaData` marks the record as syncing if the
@@ -767,17 +767,17 @@ class AutofillRecords {
 
     // Give the record fresh Sync metadata and bump its change counter as a
     // side effect. This also excludes the forked record from de-duping on the
     // next sync, if the current sync is interrupted before the record can be
     // uploaded.
     this._getSyncMetaData(forkedLocalRecord, true);
 
     this._computeFields(forkedLocalRecord);
-    this.data.push(forkedLocalRecord);
+    this._data.push(forkedLocalRecord);
 
     return forkedLocalRecord;
   }
 
   /**
    * Reconciles an incoming remote record into the matching local record. This
    * method is only used by Sync; other callers should use `merge`.
    *
@@ -798,17 +798,17 @@ class AutofillRecords {
       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`);
     }
 
-    let localRecord = this.data[localIndex];
+    let localRecord = this._data[localIndex];
     let sync = this._getSyncMetaData(localRecord, true);
 
     let forkedGUID = null;
 
     if (sync.changeCounter === 0) {
       // Local not modified. Replace local with remote.
       this._replaceRecordAt(localIndex, remoteRecord, {
         keepSyncMetadata: false,
@@ -852,38 +852,38 @@ class AutofillRecords {
       let tombstone = {
         guid,
         timeLastModified: Date.now(),
         deleted: true,
       };
 
       let sync = this._getSyncMetaData(tombstone, true);
       sync.changeCounter = 0;
-      this.data.push(tombstone);
+      this._data.push(tombstone);
       return;
     }
 
-    let existing = this.data[index];
+    let existing = this._data[index];
     let sync = this._getSyncMetaData(existing, true);
     if (sync.changeCounter > 0) {
       // Deleting a record with unsynced local changes. To avoid potential
       // data loss, we ignore the deletion in favor of the changed record.
       this.log.info("Ignoring deletion for record with local changes",
                     existing);
       return;
     }
 
     if (existing.deleted) {
       this.log.info("Ignoring deletion for tombstone", existing);
       return;
     }
 
     // Removing a record that's not changed locally, and that's not already
     // deleted. Replace the record with a synced tombstone.
-    this.data[index] = {
+    this._data[index] = {
       guid,
       timeLastModified: Date.now(),
       deleted: true,
       _sync: sync,
     };
   }
 
   /**
@@ -895,17 +895,17 @@ class AutofillRecords {
    * the object to pushSyncChanges, which will apply the changes to the store.
    *
    * @returns {object}
    *          An object describing the changes to sync.
    */
   pullSyncChanges() {
     let changes = {};
 
-    let profiles = this.data;
+    let profiles = this._data;
     for (let profile of profiles) {
       let sync = this._getSyncMetaData(profile, true);
       if (sync.changeCounter < 1) {
         if (sync.changeCounter != 0) {
           this.log.error("negative change counter", profile);
         }
         continue;
       }
@@ -952,17 +952,17 @@ class AutofillRecords {
 
   /**
    * Reset all sync metadata for all items.
    *
    * This is called when Sync is disconnected from this device. All sync
    * metadata for all items is removed.
    */
   resetSync() {
-    for (let record of this.data) {
+    for (let record of this._data) {
       delete record._sync;
     }
     // XXX - we should probably also delete all tombstones?
     this.log.info("All sync metadata was reset");
   }
 
   /**
    * Changes the GUID of an item. This should be called only by Sync. There
@@ -982,17 +982,17 @@ class AutofillRecords {
     if (oldID == newID) {
       throw new Error("changeGUID: old and new IDs are the same");
     }
     if (this._findIndexByGUID(newID) >= 0) {
       throw new Error("changeGUID: record with destination id exists already");
     }
 
     let index = this._findIndexByGUID(oldID);
-    let profile = this.data[index];
+    let profile = this._data[index];
     if (!profile) {
       throw new Error("changeGUID: no source record");
     }
     if (this._getSyncMetaData(profile)) {
       throw new Error("changeGUID: existing record has already been synced");
     }
 
     profile.guid = newID;
@@ -1032,17 +1032,17 @@ class AutofillRecords {
       throw new Error("Record missing GUID");
     }
     this._ensureMatchingVersion(remoteRecord);
     if (remoteRecord.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");
     }
-    let localRecords = this.data;
+    let localRecords = this._data;
     for (let localRecord of localRecords) {
       if (localRecord.deleted) {
         continue;
       }
       if (localRecord.guid == remoteRecord.guid) {
         throw new Error(`Record ${remoteRecord.guid} already exists`);
       }
       if (this._getSyncMetaData(localRecord)) {
@@ -1105,21 +1105,21 @@ class AutofillRecords {
         result[key] = record[key];
       }
     }
     return result;
   }
 
   _findByGUID(guid, {includeDeleted = false} = {}) {
     let found = this._findIndexByGUID(guid, {includeDeleted});
-    return found < 0 ? undefined : this.data[found];
+    return found < 0 ? undefined : this._data[found];
   }
 
   _findIndexByGUID(guid, {includeDeleted = false} = {}) {
-    return this.data.findIndex(record => {
+    return this._data.findIndex(record => {
       return record.guid == guid && (!record.deleted || includeDeleted);
     });
   }
 
   _migrateRecord(record) {
     let hasChanges = false;
 
     if (record.deleted) {
@@ -1173,17 +1173,17 @@ class AutofillRecords {
    * @param {boolean} [strict = false]
    *        In strict merge mode, we'll treat the subset record with empty field
    *        as unable to be merged, but mergeable if in non-strict mode.
    * @returns {Array.<string>}
    *          Return an array of the merged GUID string.
    */
   mergeToStorage(targetRecord, strict = false) {
     let mergedGUIDs = [];
-    for (let record of this.data) {
+    for (let record of this._data) {
       if (!record.deleted && this.mergeIfPossible(record.guid, targetRecord, strict)) {
         mergedGUIDs.push(record.guid);
       }
     }
     this.log.debug("Existing records matching and merging count is", mergedGUIDs.length);
     return mergedGUIDs;
   }
 
@@ -1660,17 +1660,17 @@ class CreditCards extends AutofillRecord
    * @param {Object} targetCreditCard
    *        The credit card for duplication checking.
    * @returns {string|null}
    *          Return the first guid if storage has the same credit card and null otherwise.
    */
   getDuplicateGuid(targetCreditCard) {
     let clonedTargetCreditCard = this._clone(targetCreditCard);
     this._normalizeRecord(clonedTargetCreditCard);
-    for (let creditCard of this.data) {
+    for (let creditCard of this._data) {
       let isDuplicate = this.VALID_FIELDS.every(field => {
         if (!clonedTargetCreditCard[field]) {
           return !creditCard[field];
         }
         if (field == "cc-number") {
           return this._getMaskedCCNumber(clonedTargetCreditCard[field]) == creditCard[field];
         }
         return clonedTargetCreditCard[field] == creditCard[field];
--- a/browser/extensions/formautofill/moz.build
+++ b/browser/extensions/formautofill/moz.build
@@ -21,9 +21,9 @@ BROWSER_CHROME_MANIFESTS += ['test/brows
 
 XPCSHELL_TESTS_MANIFESTS += ['test/unit/xpcshell.ini']
 
 MOCHITEST_MANIFESTS += ['test/mochitest/mochitest.ini']
 
 JAR_MANIFESTS += ['jar.mn']
 
 with Files('**'):
-    BUG_COMPONENT = ('Toolkit', 'Form Manager')
+    BUG_COMPONENT = ('Toolkit', 'Form Autofill')
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js
@@ -328,17 +328,17 @@ add_task(async function test_add() {
   do_check_eq(addresses[0].version, 1);
   do_check_neq(addresses[0].timeCreated, undefined);
   do_check_eq(addresses[0].timeLastModified, addresses[0].timeCreated);
   do_check_eq(addresses[0].timeLastUsed, 0);
   do_check_eq(addresses[0].timesUsed, 0);
 
   // Empty string should be deleted before saving.
   profileStorage.addresses.add(TEST_ADDRESS_WITH_EMPTY_FIELD);
-  let address = profileStorage.addresses.data[2];
+  let address = profileStorage.addresses._data[2];
   do_check_eq(address.name, TEST_ADDRESS_WITH_EMPTY_FIELD.name);
   do_check_eq(address["street-address"], undefined);
 
   Assert.throws(() => profileStorage.addresses.add(TEST_ADDRESS_WITH_INVALID_FIELD),
     /"invalidField" is not a valid field\./);
 
   Assert.throws(() => profileStorage.addresses.add({}),
     /Record contains no valid field\./);
@@ -388,18 +388,18 @@ add_task(async function test_update() {
   do_check_eq(address["given-name"], "Tim");
   do_check_eq(address["family-name"], "Berners");
   do_check_eq(address["street-address"], undefined);
   do_check_eq(address["postal-code"], "12345");
   do_check_neq(address.timeLastModified, timeLastModified);
   do_check_eq(getSyncChangeCounter(profileStorage.addresses, guid), 2);
 
   // Empty string should be deleted while updating.
-  profileStorage.addresses.update(profileStorage.addresses.data[0].guid, TEST_ADDRESS_WITH_EMPTY_FIELD);
-  address = profileStorage.addresses.data[0];
+  profileStorage.addresses.update(profileStorage.addresses._data[0].guid, TEST_ADDRESS_WITH_EMPTY_FIELD);
+  address = profileStorage.addresses._data[0];
   do_check_eq(address.name, TEST_ADDRESS_WITH_EMPTY_FIELD.name);
   do_check_eq(address["street-address"], undefined);
 
   Assert.throws(
     () => profileStorage.addresses.update("INVALID_GUID", TEST_ADDRESS_3),
     /No matching record\./
   );
 
--- a/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_creditCardRecords.js
@@ -251,17 +251,17 @@ add_task(async function test_add() {
   do_check_eq(creditCards[0].version, 1);
   do_check_neq(creditCards[0].timeCreated, undefined);
   do_check_eq(creditCards[0].timeLastModified, creditCards[0].timeCreated);
   do_check_eq(creditCards[0].timeLastUsed, 0);
   do_check_eq(creditCards[0].timesUsed, 0);
 
   // Empty string should be deleted before saving.
   profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_EMPTY_FIELD);
-  let creditCard = profileStorage.creditCards.data[2];
+  let creditCard = profileStorage.creditCards._data[2];
   do_check_eq(creditCard["cc-exp-month"], TEST_CREDIT_CARD_WITH_EMPTY_FIELD["cc-exp-month"]);
   do_check_eq(creditCard["cc-name"], undefined);
 
   Assert.throws(() => profileStorage.creditCards.add(TEST_CREDIT_CARD_WITH_INVALID_FIELD),
     /"invalidField" is not a valid field\./);
 
   Assert.throws(() => profileStorage.creditCards.add({}),
     /Record contains no valid field\./);
@@ -294,18 +294,18 @@ add_task(async function test_update() {
 
   let creditCard = profileStorage.creditCards.get(guid);
 
   do_check_eq(creditCard["cc-name"], undefined);
   do_check_neq(creditCard.timeLastModified, timeLastModified);
   do_check_credit_card_matches(creditCard, TEST_CREDIT_CARD_3);
 
   // Empty string should be deleted while updating.
-  profileStorage.creditCards.update(profileStorage.creditCards.data[0].guid, TEST_CREDIT_CARD_WITH_EMPTY_FIELD);
-  creditCard = profileStorage.creditCards.data[0];
+  profileStorage.creditCards.update(profileStorage.creditCards._data[0].guid, TEST_CREDIT_CARD_WITH_EMPTY_FIELD);
+  creditCard = profileStorage.creditCards._data[0];
   do_check_eq(creditCard["cc-exp-month"], TEST_CREDIT_CARD_WITH_EMPTY_FIELD["cc-exp-month"]);
   do_check_eq(creditCard["cc-name"], undefined);
 
   Assert.throws(
     () => profileStorage.creditCards.update("INVALID_GUID", TEST_CREDIT_CARD_3),
     /No matching record\./
   );