Bug 1494551 - Sort the formautofill data by the last used time. r=MattN
authorJared Wein <jwein@mozilla.com>
Fri, 19 Oct 2018 17:00:31 +0000
changeset 500684 388806780facbc345aa5ce0e7cc6b6bc108b29e8
parent 500683 0e7248074acf71bb6553a68c53299f2fd1b35329
child 500685 140a56e90594382a0c24298b08a5a853e95c0b32
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1494551
milestone64.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 1494551 - Sort the formautofill data by the last used time. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D9174
browser/components/payments/content/paymentDialogWrapper.js
browser/components/payments/res/containers/payment-dialog.js
browser/components/payments/res/debugging.js
browser/components/payments/res/paymentRequest.js
browser/components/payments/test/browser/browser_change_shipping.js
browser/components/payments/test/mochitest/test_address_picker.html
browser/components/payments/test/mochitest/test_payment_method_picker.html
browser/extensions/formautofill/content/manageDialog.js
--- a/browser/components/payments/content/paymentDialogWrapper.js
+++ b/browser/components/payments/content/paymentDialogWrapper.js
@@ -66,17 +66,18 @@ class TempCollection {
   async update(guid, record, preserveOldProperties) {
     let recordToSave = Object.assign(preserveOldProperties ? this._data[guid] : {}, record);
     await this._formAutofillCollection.computeFields(recordToSave);
     return (this._data[guid] = recordToSave);
   }
 
   async add(record) {
     let guid = "temp-" + Math.abs(Math.random() * 0xffffffff|0);
-    let recordToSave = Object.assign({guid}, record);
+    let timeLastModified = Date.now();
+    let recordToSave = Object.assign({guid, timeLastModified}, record);
     await this._formAutofillCollection.computeFields(recordToSave);
     this._data[guid] = recordToSave;
     return guid;
   }
 
   getAll() {
     return this._data;
   }
@@ -497,32 +498,49 @@ var paymentDialogWrapper = {
     paymentSrv.respondPayment(showResponse);
     window.close();
   },
 
   async onPay({
     selectedPayerAddressGUID: payerGUID,
     selectedPaymentCardGUID: paymentCardGUID,
     selectedPaymentCardSecurityCode: cardSecurityCode,
+    selectedShippingAddressGUID: shippingGUID,
   }) {
     let methodData = await this._convertProfileBasicCardToPaymentMethodData(paymentCardGUID,
                                                                             cardSecurityCode);
 
     if (!methodData) {
       // TODO (Bug 1429265/Bug 1429205): Handle when a user hits cancel on the
       // Master Password dialog.
       Cu.reportError("Bug 1429265/Bug 1429205: User canceled master password entry");
       return;
     }
 
-    let {
-      payerName,
-      payerEmail,
-      payerPhone,
-    } = await this._convertProfileAddressToPayerData(payerGUID);
+    let payerName = "";
+    let payerEmail = "";
+    let payerPhone = "";
+    if (payerGUID) {
+      let payerData = await this._convertProfileAddressToPayerData(payerGUID);
+      payerName = payerData.payerName;
+      payerEmail = payerData.payerEmail;
+      payerPhone = payerData.payerPhone;
+    }
+
+    // Update the lastUsedTime for the payerAddress and paymentCard. Check if
+    // the record exists in formAutofillStorage because it may be temporary.
+    if (shippingGUID && await formAutofillStorage.addresses.get(shippingGUID)) {
+      formAutofillStorage.addresses.notifyUsed(shippingGUID);
+    }
+    if (payerGUID && await formAutofillStorage.addresses.get(payerGUID)) {
+      formAutofillStorage.addresses.notifyUsed(payerGUID);
+    }
+    if (await formAutofillStorage.creditCards.get(paymentCardGUID)) {
+      formAutofillStorage.creditCards.notifyUsed(paymentCardGUID);
+    }
 
     this.pay({
       methodName: "basic-card",
       methodData,
       payerName,
       payerEmail,
       payerPhone,
     });
--- a/browser/components/payments/res/containers/payment-dialog.js
+++ b/browser/components/payments/res/containers/payment-dialog.js
@@ -97,27 +97,40 @@ export default class PaymentDialog exten
     event.preventDefault();
   }
 
   cancelRequest() {
     paymentRequest.cancel();
   }
 
   pay() {
+    let state = this.requestStore.getState();
     let {
       selectedPayerAddress,
       selectedPaymentCard,
       selectedPaymentCardSecurityCode,
-    } = this.requestStore.getState();
+      selectedShippingAddress,
+    } = state;
 
-    paymentRequest.pay({
-      selectedPayerAddressGUID: selectedPayerAddress,
+    let data = {
       selectedPaymentCardGUID: selectedPaymentCard,
       selectedPaymentCardSecurityCode,
-    });
+    };
+
+    data.selectedShippingAddressGUID =
+      state.request.paymentOptions.requestShipping ?
+      selectedShippingAddress :
+      null;
+
+    data.selectedPayerAddressGUID =
+      this._isPayerRequested(state.request.paymentOptions) ?
+      selectedPayerAddress :
+      null;
+
+    paymentRequest.pay(data);
   }
 
   changeShippingAddress(shippingAddressGUID) {
     paymentRequest.changeShippingAddress({
       shippingAddressGUID,
     });
   }
 
--- a/browser/components/payments/res/debugging.js
+++ b/browser/components/payments/res/debugging.js
@@ -203,130 +203,139 @@ let ADDRESSES_1 = {
     "email": "foo@bar.com",
     "family-name": "Smith",
     "given-name": "John",
     "guid": "48bnds6854t",
     "name": "John Smith",
     "postal-code": "90210",
     "street-address": "123 Sesame Street,\nApt 40",
     "tel": "+1 519 555-5555",
+    timeLastUsed: 50000,
   },
   "68gjdh354j": {
     "additional-name": "Z.",
     "address-level1": "CA",
     "address-level2": "Mountain View",
     "country": "US",
     "family-name": "Doe",
     "given-name": "Jane",
     "guid": "68gjdh354j",
     "name": "Jane Z. Doe",
     "postal-code": "94041",
     "street-address": "P.O. Box 123",
     "tel": "+1 650 555-5555",
+    timeLastUsed: 30000,
   },
   "abcde12345": {
     "address-level2": "Mountain View",
     "country": "US",
     "family-name": "Fields",
     "given-name": "Mrs.",
     "guid": "abcde12345",
     "name": "Mrs. Fields",
+    timeLastUsed: 70000,
   },
   "german1": {
     "additional-name": "Y.",
     "address-level1": "",
     "address-level2": "Berlin",
     "country": "DE",
     "email": "de@example.com",
     "family-name": "Mouse",
     "given-name": "Anon",
     "guid": "german1",
     "name": "Anon Y. Mouse",
     "organization": "Mozilla",
     "postal-code": "10997",
     "street-address": "Schlesische Str. 27",
     "tel": "+49 30 983333002",
+    timeLastUsed: 10000,
   },
   "missing-country": {
     "address-level1": "ON",
     "address-level2": "Toronto",
     "family-name": "Bogard",
     "given-name": "Kristin",
     "guid": "missing-country",
     "name": "Kristin Bogard",
     "postal-code": "H0H 0H0",
     "street-address": "123 Yonge Street\nSuite 2300",
     "tel": "+1 416 555-5555",
+    timeLastUsed: 90000,
   },
 };
 
 let DUPED_ADDRESSES = {
   "a9e830667189": {
     "street-address": "Unit 1\n1505 Northeast Kentucky Industrial Parkway \n",
     "address-level2": "Greenup",
     "address-level1": "KY",
     "postal-code": "41144",
     "country": "US",
     "email": "bob@example.com",
     "family-name": "Smith",
     "given-name": "Bob",
     "guid": "a9e830667189",
     "tel": "+19871234567",
     "name": "Bob Smith",
+    timeLastUsed: 10001,
   },
   "72a15aed206d": {
     "street-address": "1 New St",
     "address-level2": "York",
     "address-level1": "SC",
     "postal-code": "29745",
     "country": "US",
     "given-name": "Mary Sue",
     "guid": "72a15aed206d",
     "tel": "+19871234567",
     "name": "Mary Sue",
     "address-line1": "1 New St",
+    timeLastUsed: 10009,
   },
   "2b4dce0fbc1f": {
     "street-address": "123 Park St",
     "address-level2": "Springfield",
     "address-level1": "OR",
     "postal-code": "97403",
     "country": "US",
     "email": "rita@foo.com",
     "family-name": "Foo",
     "given-name": "Rita",
     "guid": "2b4dce0fbc1f",
     "name": "Rita Foo",
     "address-line1": "123 Park St",
+    timeLastUsed: 10005,
   },
   "46b2635a5b26": {
     "street-address": "432 Another St",
     "address-level2": "Springfield",
     "address-level1": "OR",
     "postal-code": "97402",
     "country": "US",
     "email": "rita@foo.com",
     "family-name": "Foo",
     "given-name": "Rita",
     "guid": "46b2635a5b26",
     "name": "Rita Foo",
     "address-line1": "432 Another St",
+    timeLastUsed: 10003,
   },
 };
 
 let BASIC_CARDS_1 = {
   "53f9d009aed2": {
     billingAddressGUID: "68gjdh354j",
     methodName: "basic-card",
     "cc-number": "************5461",
     "guid": "53f9d009aed2",
     "version": 1,
     "timeCreated": 1505240896213,
     "timeLastModified": 1515609524588,
-    "timeLastUsed": 0,
+    "timeLastUsed": 10000,
     "timesUsed": 0,
     "cc-name": "John Smith",
     "cc-exp-month": 6,
     "cc-exp-year": 2024,
     "cc-type": "visa",
     "cc-given-name": "John",
     "cc-additional-name": "",
     "cc-family-name": "Smith",
@@ -334,17 +343,17 @@ let BASIC_CARDS_1 = {
   },
   "9h5d4h6f4d1s": {
     methodName: "basic-card",
     "cc-number": "************0954",
     "guid": "9h5d4h6f4d1s",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 50000,
     "timesUsed": 0,
     "cc-name": "Jane Doe",
     "cc-exp-month": 5,
     "cc-exp-year": 2023,
     "cc-type": "mastercard",
     "cc-given-name": "Jane",
     "cc-additional-name": "",
     "cc-family-name": "Doe",
@@ -352,33 +361,33 @@ let BASIC_CARDS_1 = {
   },
   "123456789abc": {
     methodName: "basic-card",
     "cc-number": "************1234",
     "guid": "123456789abc",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 90000,
     "timesUsed": 0,
     "cc-name": "Jane Fields",
     "cc-given-name": "Jane",
     "cc-additional-name": "",
     "cc-family-name": "Fields",
     "cc-type": "discover",
   },
   "amex-card": {
     methodName: "basic-card",
     billingAddressGUID: "68gjdh354j",
     "cc-number": "************1941",
     "guid": "amex-card",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 70000,
     "timesUsed": 0,
     "cc-name": "Capt America",
     "cc-given-name": "Capt",
     "cc-additional-name": "",
     "cc-family-name": "America",
     "cc-type": "amex",
     "cc-exp-month": 6,
     "cc-exp-year": 2023,
@@ -386,17 +395,17 @@ let BASIC_CARDS_1 = {
   },
   "missing-cc-name": {
     methodName: "basic-card",
     "cc-number": "************8563",
     "guid": "missing-cc-name",
     "version": 1,
     "timeCreated": 1517890536491,
     "timeLastModified": 1517890564518,
-    "timeLastUsed": 0,
+    "timeLastUsed": 30000,
     "timesUsed": 0,
     "cc-exp-month": 8,
     "cc-exp-year": 2024,
     "cc-exp": "2024-08",
   },
 };
 
 let buttonActions = {
--- a/browser/components/payments/res/paymentRequest.js
+++ b/browser/components/payments/res/paymentRequest.js
@@ -268,22 +268,35 @@ var paymentRequest = {
     return state.request.paymentDetails.totalItem;
   },
 
   onPaymentRequestUnload() {
     // remove listeners that may be used multiple times here
     window.removeEventListener("paymentChromeToContent", this);
   },
 
+  _sortObjectsByTimeLastUsed(objects) {
+    let sortedValues = Object.values(objects).sort((a, b) => {
+      let aLastUsed = a.timeLastUsed || a.timeLastModified;
+      let bLastUsed = b.timeLastUsed || b.timeLastModified;
+      return bLastUsed - aLastUsed;
+    });
+    let sortedObjects = {};
+    for (let obj of sortedValues) {
+      sortedObjects[obj.guid] = obj;
+    }
+    return sortedObjects;
+  },
+
   getAddresses(state) {
     let addresses = Object.assign({}, state.savedAddresses, state.tempAddresses);
-    return addresses;
+    return this._sortObjectsByTimeLastUsed(addresses);
   },
 
   getBasicCards(state) {
     let cards = Object.assign({}, state.savedBasicCards, state.tempBasicCards);
-    return cards;
+    return this._sortObjectsByTimeLastUsed(cards);
   },
 };
 
 paymentRequest.init();
 
 export default paymentRequest;
--- a/browser/components/payments/test/browser/browser_change_shipping.js
+++ b/browser/components/payments/test/browser/browser_change_shipping.js
@@ -326,17 +326,19 @@ add_task(async function test_address_edi
 
     addressOptions =
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
     info("initial addressOptions: " + JSON.stringify(addressOptions));
     selectedIndex = addressOptions.selectedOptionIndex;
     let selectedAddressGuid = addressOptions.options[selectedIndex].guid;
     let selectedAddress = await formAutofillStorage.addresses.get(selectedAddressGuid);
 
-    is(selectedIndex, 0, "First address should be selected");
+    // US address is inserted first, then German address, so German address
+    // has more recent timeLastModified and will appear at the top of the list.
+    is(selectedIndex, 1, "Second address should be selected");
     ok(selectedAddress, "Selected address does exist in the address collection");
     is(selectedAddress.country, "US", "Expected initial country value");
 
     info("Updating the address directly in the store");
     await formAutofillStorage.addresses.update(selectedAddress.guid, {
       country: "CA",
     }, true);
 
@@ -344,17 +346,17 @@ add_task(async function test_address_edi
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
     info("updated addressOptions: " + JSON.stringify(addressOptions));
     selectedIndex = addressOptions.selectedOptionIndex;
     let newSelectedAddressGuid = addressOptions.options[selectedIndex].guid;
 
     is(newSelectedAddressGuid, selectedAddressGuid, "Selected guid hasnt changed");
     selectedAddress = await formAutofillStorage.addresses.get(selectedAddressGuid);
 
-    is(selectedIndex, 0, "First address should be selected");
+    is(selectedIndex, 1, "Second address should be selected");
     is(selectedAddress.country, "CA", "Expected changed country value");
 
     spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
 
   await cleanupFormAutofillStorage();
 });
@@ -378,17 +380,19 @@ add_task(async function test_address_rem
     await selectPaymentDialogShippingAddressByCountry(frame, "US");
 
     let addressOptions =
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
     info("initial addressOptions: " + JSON.stringify(addressOptions));
     let selectedIndex = addressOptions.selectedOptionIndex;
     let selectedAddressGuid = addressOptions.options[selectedIndex].guid;
 
-    is(selectedIndex, 0, "First address should be selected");
+    // US address is inserted first, then German address, so German address
+    // has more recent timeLastModified and will appear at the top of the list.
+    is(selectedIndex, 1, "Second address should be selected");
     is(addressOptions.options.length, 2, "Should be 2 address options initially");
 
     info("Remove the selected address from the store");
     await formAutofillStorage.addresses.remove(selectedAddressGuid);
 
     await ContentTask.spawn(browser, {
       eventName: "shippingaddresschange",
     }, PTU.ContentTasks.promisePaymentRequestEvent);
--- a/browser/components/payments/test/mochitest/test_address_picker.html
+++ b/browser/components/payments/test/mochitest/test_address_picker.html
@@ -52,41 +52,44 @@ add_task(async function test_initialSet(
         "address-level1": "MI",
         "address-level2": "Some City",
         "country": "US",
         "guid": "48bnds6854t",
         "name": "Mr. Foo",
         "postal-code": "90210",
         "street-address": "123 Sesame Street,\nApt 40",
         "tel": "+1 519 555-5555",
+        timeLastUsed: 200,
       },
       "68gjdh354j": {
         "address-level1": "CA",
         "address-level2": "Mountain View",
         "country": "US",
         "guid": "68gjdh354j",
         "name": "Mrs. Bar",
         "postal-code": "94041",
         "street-address": "P.O. Box 123",
         "tel": "+1 650 555-5555",
+        timeLastUsed: 300,
       },
       "abcde12345": {
         "address-level2": "Mountain View",
         "country": "US",
         "guid": "abcde12345",
         "name": "Mrs. Fields",
+        timeLastUsed: 100,
       },
     },
   });
   await asyncElementRendered();
   let options = picker1.dropdown.popupBox.children;
   is(options.length, 3, "Check dropdown has all addresses");
-  ok(options[0].textContent.includes("Mr. Foo"), "Check first address");
-  ok(options[1].textContent.includes("Mrs. Bar"), "Check second address");
-  ok(options[2].textContent.includes("Mrs. Fields"), "Check third address");
+  ok(options[0].textContent.includes("Mrs. Bar"), "Check first address based on timeLastUsed");
+  ok(options[1].textContent.includes("Mr. Foo"), "Check second address based on timeLastUsed");
+  ok(options[2].textContent.includes("Mrs. Fields"), "Check third address based on timeLastUsed");
 });
 
 add_task(async function test_update() {
   picker1.requestStore.setState({
     savedAddresses: {
       "48bnds6854t": {
         // Same GUID, different values to trigger an update
         "address-level1": "MI-edit",
--- a/browser/components/payments/test/mochitest/test_payment_method_picker.html
+++ b/browser/components/payments/test/mochitest/test_payment_method_picker.html
@@ -39,56 +39,59 @@ let picker1 = document.getElementById("p
 add_task(async function test_empty() {
   ok(picker1, "Check picker1 exists");
   let {savedBasicCards} = picker1.requestStore.getState();
   is(Object.keys(savedBasicCards).length, 0, "Check empty initial state");
   is(picker1.dropdown.popupBox.children.length, 0, "Check dropdown is empty");
 });
 
 add_task(async function test_initialSet() {
-  picker1.requestStore.setState({
+  await picker1.requestStore.setState({
     savedBasicCards: {
       "48bnds6854t": {
         "cc-exp": "2017-02",
         "cc-exp-month": 2,
         "cc-exp-year": 2017,
         "cc-name": "John Doe",
         "cc-number": "************9999",
         "cc-type": "mastercard",
         "guid": "48bnds6854t",
+        timeLastUsed: 300,
       },
       "68gjdh354j": {
         "cc-exp": "2017-08",
         "cc-exp-month": 8,
         "cc-exp-year": 2017,
         "cc-name": "J Smith",
         "cc-number": "***********1234",
         "cc-type": "visa",
         "guid": "68gjdh354j",
+        timeLastUsed: 100,
       },
       "123456789abc": {
         "cc-name": "Jane Fields",
         "cc-given-name": "Jane",
         "cc-additional-name": "",
         "cc-family-name": "Fields",
         "cc-number": "************9876",
         "guid": "123456789abc",
+        timeLastUsed: 200,
       },
     },
   });
   await asyncElementRendered();
   let options = picker1.dropdown.popupBox.children;
   is(options.length, 3, "Check dropdown has all three cards");
-  ok(options[0].textContent.includes("John Doe"), "Check first card");
-  ok(options[1].textContent.includes("J Smith"), "Check second card");
-  ok(options[2].textContent.includes("Jane Fields"), "Check third card");
+  ok(options[0].textContent.includes("John Doe"), "Check first card based on timeLastUsed");
+  ok(options[1].textContent.includes("Jane Fields"), "Check second card based on timeLastUsed");
+  ok(options[2].textContent.includes("J Smith"), "Check third card based on timeLastUsed");
 });
 
 add_task(async function test_update() {
-  picker1.requestStore.setState({
+  await picker1.requestStore.setState({
     savedBasicCards: {
       "48bnds6854t": {
         // Same GUID, different values to trigger an update
         "cc-exp": "2017-09",
         "cc-exp-month": 9,
         "cc-exp-year": 2017,
         // cc-name was cleared which means it's not returned
         "cc-number": "************9876",
@@ -178,17 +181,17 @@ add_task(async function test_change_sele
   picker1.securityCodeInput.querySelector("input").focus();
   sendString("836");
   sendKey("Tab");
   let state = await stateChangePromise;
   ok(state.selectedPaymentCardSecurityCode, "836", "Check security code in state");
 });
 
 add_task(async function test_delete() {
-  picker1.requestStore.setState({
+  await picker1.requestStore.setState({
     savedBasicCards: {
       // 48bnds6854t was deleted
       "68gjdh354j": {
         "cc-exp": "2017-08",
         "cc-exp-month": 8,
         "cc-exp-year": 2017,
         "cc-name": "J Smith",
         "cc-number": "***********1234",
--- a/browser/extensions/formautofill/content/manageDialog.js
+++ b/browser/extensions/formautofill/content/manageDialog.js
@@ -99,18 +99,22 @@ class ManageRecords {
 
     // For testing only: Notify when records are loaded
     this._elements.records.dispatchEvent(new CustomEvent("RecordsLoaded"));
   }
 
   async _loadRecords() {
     let storage = await this.getStorage();
     let records = await storage.getAll();
-    // Sort by last modified time starting with most recent
-    records.sort((a, b) => b.timeLastModified - a.timeLastModified);
+    // Sort by last used time starting with most recent
+    records.sort((a, b) => {
+      let aLastUsed = a.timeLastUsed || a.timeLastModified;
+      let bLastUsed = b.timeLastUsed || b.timeLastModified;
+      return bLastUsed - aLastUsed;
+    });
     await this.renderRecordElements(records);
     this.updateButtonsStates(this._selectedOptions.length);
   }
 
   /**
    * Render the records onto the page while maintaining selected options if
    * they still exist.
    *