Bug 1413488 - [Form Autofill] Rename the "data" property of "AutofillRecords" to "_data". r=seanlee draft
authorLuke Chang <lchang@mozilla.com>
Thu, 16 Nov 2017 13:25:43 +0800
changeset 699455 dbc9eed7ee949a9fa6b3078d710f68c5473aaa03
parent 699096 a3f183201f7f183c263d554bfb15fbf0b0ed2ea4
child 740633 06d5ad3d04760e4156208186a00e2b635705a998
push id89576
push userbmo:lchang@mozilla.com
push dateFri, 17 Nov 2017 06:56:44 +0000
reviewersseanlee
bugs1413488
milestone59.0a1
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\./
   );