Bug 1339740 - Trigger notifyUsed API during form submission. r=lchang draft
authorsteveck-chung <schung@mozilla.com>
Mon, 19 Jun 2017 18:38:35 +0800
changeset 597843 0882c108bb6f52fe6fd5dc4e56af773eccac0f1d
parent 596219 95543bdc59bd038a3d5d084b85a4fec493c349ee
child 598837 2530f20ead0974891b5c2ed5099dc6b6091700d1
push id65049
push userbmo:schung@mozilla.com
push dateWed, 21 Jun 2017 02:24:16 +0000
reviewerslchang
bugs1339740
milestone56.0a1
Bug 1339740 - Trigger notifyUsed API during form submission. r=lchang MozReview-Commit-ID: 1wiwK74oeeB
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/mochitest/formautofill_parent_utils.js
browser/extensions/formautofill/test/mochitest/test_on_address_submission.html
browser/extensions/formautofill/test/unit/test_addressRecords.js
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -281,20 +281,23 @@ FormAutofillParent.prototype = {
     if (address.guid) {
       if (!this.profileStorage.addresses.mergeIfPossible(address.guid, address.record)) {
         // TODO: Show update doorhanger(bug 1303513) and set probe(bug 990200)
         return;
       }
       this.profileStorage.addresses.notifyUsed(address.guid);
     } else {
       if (!Services.prefs.getBoolPref("extensions.formautofill.firstTimeUse")) {
-        if (!this.profileStorage.addresses.mergeToStorage(address.record)) {
-          this.profileStorage.addresses.add(address.record);
+        let changedGUIDs = this.profileStorage.addresses.mergeToStorage(address.record);
+        if (!changedGUIDs.length) {
+          changedGUIDs.push(this.profileStorage.addresses.add(address.record));
         }
+        changedGUIDs.forEach(guid => this.profileStorage.addresses.notifyUsed(guid));
         return;
       }
 
-      this.profileStorage.addresses.add(address.record);
+      let guid = this.profileStorage.addresses.add(address.record);
+      this.profileStorage.addresses.notifyUsed(guid);
       Services.prefs.setBoolPref("extensions.formautofill.firstTimeUse", false);
       FormAutofillDoorhanger.show(target, "firstTimeUse");
     }
   },
 };
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -521,28 +521,28 @@ class Addresses extends AutofillRecords 
     Services.obs.notifyObservers(str, "formautofill-storage-changed", "merge");
     return true;
   }
 
   /**
    * Merge the address if storage has multiple mergeable records.
    * @param {Object} targetAddress
    *        The address for merge.
-   * @returns {boolean}
-   *          Return true if the target address is mergeable or false if not.
+   * @returns {Array.<string>}
+   *          Return an array of the merged GUID string.
    */
   mergeToStorage(targetAddress) {
-    let merged = false;
+    let mergedGUIDs = [];
     for (let address of this._store.data[this._collectionName]) {
       if (this.mergeIfPossible(address.guid, targetAddress)) {
-        merged = true;
+        mergedGUIDs.push(address.guid);
       }
     }
-    this.log.debug("Existing records matching and merging is", merged);
-    return merged;
+    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);
   }
 
--- a/browser/extensions/formautofill/test/mochitest/formautofill_parent_utils.js
+++ b/browser/extensions/formautofill/test/mochitest/formautofill_parent_utils.js
@@ -45,16 +45,22 @@ var ParentUtils = {
   },
 
   areAddressesMatching(addressA, addressB) {
     for (let field of profileStorage.addresses.VALID_FIELDS) {
       if (addressA[field] !== addressB[field]) {
         return false;
       }
     }
+    // Check the internal field if both addresses have valid value.
+    for (let field of profileStorage.INTERNAL_FIELDS) {
+      if (field in addressA && field in addressB && (addressA[field] !== addressB[field])) {
+        return false;
+      }
+    }
     return true;
   },
 
   checkAddresses({expectedAddresses}) {
     Services.cpmm.addMessageListener("FormAutofill:Addresses", function getResult(result) {
       Services.cpmm.removeMessageListener("FormAutofill:Addresses", getResult);
       let addresses = result.data;
       if (addresses.length !== expectedAddresses.length) {
--- a/browser/extensions/formautofill/test/mochitest/test_on_address_submission.html
+++ b/browser/extensions/formautofill/test/mochitest/test_on_address_submission.html
@@ -39,16 +39,18 @@ add_task(async function check_storage_af
   for (let key in TEST_ADDRESSES[0]) {
     await setInput("#" + key, TEST_ADDRESSES[0][key]);
   }
 
   clickOnElement("input[type=submit]");
 
   let expectedAddresses = TEST_ADDRESSES.slice(0, 1);
   await onAddressChanged("add");
+  // Check if timesUsed is set correctly
+  expectedAddresses[0].timesUsed = 1;
   let matching = await checkAddresses(expectedAddresses);
   ok(matching, "Address saved as expected");
 });
 
 </script>
 
 <div>
 
--- a/browser/extensions/formautofill/test/unit/test_addressRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_addressRecords.js
@@ -354,12 +354,12 @@ add_task(async function test_merge_unabl
 
 add_task(async function test_mergeToStorage() {
   let profileStorage = await initProfileStorage(TEST_STORE_FILE_NAME,
                                                 [TEST_ADDRESS_1, TEST_ADDRESS_2]);
   // Merge an address to storage
   let anotherAddress = profileStorage.addresses._clone(TEST_ADDRESS_2);
   profileStorage.addresses.add(anotherAddress);
   anotherAddress.email = "timbl@w3.org";
-  do_check_eq(profileStorage.addresses.mergeToStorage(anotherAddress), true);
+  do_check_eq(profileStorage.addresses.mergeToStorage(anotherAddress).length, 2);
   do_check_eq(profileStorage.addresses.getAll()[1].email, anotherAddress.email);
   do_check_eq(profileStorage.addresses.getAll()[2].email, anotherAddress.email);
 });