Bug 1463608 - Expose FormAutofill computeFields methods, use them for temporary records in payments. r=MattN draft
authorSam Foster <sfoster@mozilla.com>
Wed, 30 May 2018 16:56:25 -0700
changeset 810412 6e8380826bcac87bc408ff75a6a398b5c4f6cafa
parent 810411 be32c684dba62d051bb119a530437830e41bb128
push id113982
push userbmo:sfoster@mozilla.com
push dateMon, 25 Jun 2018 20:24:05 +0000
reviewersMattN
bugs1463608
milestone63.0a1
Bug 1463608 - Expose FormAutofill computeFields methods, use them for temporary records in payments. r=MattN * Test helpers for cards * Refactor the test_add_link test to test in private & non-private window MozReview-Commit-ID: AeVJOkSwQps
browser/components/payments/content/paymentDialogWrapper.js
browser/components/payments/test/PaymentTestUtils.jsm
browser/components/payments/test/browser/browser_address_edit.js
browser/components/payments/test/browser/browser_card_edit.js
browser/extensions/formautofill/FormAutofillStorage.jsm
--- a/browser/components/payments/content/paymentDialogWrapper.js
+++ b/browser/components/payments/content/paymentDialogWrapper.js
@@ -30,36 +30,58 @@ XPCOMUtils.defineLazyGetter(this, "formA
   } catch (ex) {
     storage = null;
     Cu.reportError(ex);
   }
 
   return storage;
 });
 
+/**
+ * Temporary/transient storage for address and credit card records
+ *
+ * Implements a subset of the FormAutofillStorage collection class interface, and delegates to
+ * those classes for some utility methods
+ */
 class TempCollection {
-  constructor(data = {}) {
+  constructor(type, data = {}) {
+    /**
+     * The name of the collection. e.g. 'addresses' or 'creditCards'
+     * Used to access methods from the FormAutofillStorage collections
+     */
+    this._type = type;
     this._data = data;
   }
+
+  get _formAutofillCollection() {
+    // lazy getter for the formAutofill collection - to resolve on first access
+    Object.defineProperty(this, "_formAutofillCollection", {
+      value: formAutofillStorage[this._type], writable: false, configurable: true,
+    });
+    return this._formAutofillCollection;
+  }
+
   get(guid) {
     return this._data[guid];
   }
+
   update(guid, record, preserveOldProperties) {
-    if (preserveOldProperties) {
-      Object.assign(this._data[guid], record);
-    } else {
-      this._data[guid] = record;
-    }
-    return this._data[guid];
+    let recordToSave = Object.assign(preserveOldProperties ? this._data[guid] : {}, record);
+    this._formAutofillCollection.computeFields(recordToSave);
+    return (this._data[guid] = recordToSave);
   }
+
   add(record) {
     let guid = "temp-" + Math.abs(Math.random() * 0xffffffff|0);
-    this._data[guid] = record;
+    let recordToSave = Object.assign({guid}, record);
+    this._formAutofillCollection.computeFields(recordToSave);
+    this._data[guid] = recordToSave;
     return guid;
   }
+
   getAll() {
     return this._data;
   }
 }
 
 var paymentDialogWrapper = {
   componentsLoaded: new Map(),
   frame: null,
@@ -197,18 +219,18 @@ var paymentDialogWrapper = {
     // shim for data-localization* attributes.
     this.mm.loadFrameScript("chrome://formautofill/content/l10n.js", true);
     if (AppConstants.platform == "win") {
       this.frame.setAttribute("selectmenulist", "ContentSelectDropdown-windows");
     }
     this.frame.loadURI("resource://payments/paymentRequest.xhtml");
 
     this.temporaryStore = {
-      addresses: new TempCollection(),
-      creditCards: new TempCollection(),
+      addresses: new TempCollection("addresses"),
+      creditCards: new TempCollection("creditCards"),
     };
   },
 
   createShowResponse({
     acceptStatus,
     methodName = "",
     methodData = null,
     payerName = "",
--- a/browser/components/payments/test/PaymentTestUtils.jsm
+++ b/browser/components/payments/test/PaymentTestUtils.jsm
@@ -125,16 +125,26 @@ var PaymentTestUtils = {
       let addressPicker =
         doc.querySelector("address-picker[selected-state-key='selectedShippingAddress']");
       let select = addressPicker.querySelector("rich-select");
       let option = select.querySelector(`[country="${country}"]`);
       select.click();
       option.click();
     },
 
+    selectShippingAddressByGuid: guid => {
+      let doc = content.document;
+      let addressPicker =
+        doc.querySelector("address-picker[selected-state-key='selectedShippingAddress']");
+      let select = addressPicker.querySelector("rich-select");
+      let option = select.querySelector(`[guid="${guid}"]`);
+      select.click();
+      option.click();
+    },
+
     selectShippingOptionById: value => {
       let doc = content.document;
       let optionPicker =
         doc.querySelector("shipping-option-picker");
       let select = optionPicker.querySelector("rich-select");
       let option = select.querySelector(`[value="${value}"]`);
       select.click();
       option.click();
--- a/browser/components/payments/test/browser/browser_address_edit.js
+++ b/browser/components/payments/test/browser/browser_address_edit.js
@@ -158,16 +158,20 @@ add_task(async function test_edit_link()
       }, "Check address was edited");
 
       let addressGUIDs = Object.keys(state.savedAddresses);
       is(addressGUIDs.length, 1, "Check there is still one address");
       let savedAddress = state.savedAddresses[addressGUIDs[0]];
       for (let [key, val] of Object.entries(address)) {
         is(savedAddress[key], val, "Check updated " + key);
       }
+      ok(savedAddress.guid, "Address has a guid");
+      ok(savedAddress.name, "Address has a name");
+      ok(savedAddress.name.includes(address["given-name"]) &&
+         savedAddress.name.includes(address["family-name"]), "Address.name was computed");
 
       state = await PTU.DialogContentUtils.waitForState(content, (state) => {
         return state.page.id == "payment-summary";
       }, "Switched back to payment-summary");
     }, EXPECTED_ADDRESS);
 
     await shippingAddressChangePromise;
     info("got shippingaddresschange event");
@@ -243,16 +247,20 @@ add_task(async function test_add_payer_c
       let {savedAddresses} = await PTU.DialogContentUtils.getCurrentState(content);
 
       let addressGUIDs = Object.keys(savedAddresses);
       is(addressGUIDs.length, 2, "Check there are now 2 addresses");
       let savedAddress = savedAddresses[addressGUIDs[1]];
       for (let [key, val] of Object.entries(address)) {
         is(savedAddress[key], val, "Check " + key);
       }
+      ok(savedAddress.guid, "Address has a guid");
+      ok(savedAddress.name, "Address has a name");
+      ok(savedAddress.name.includes(address["given-name"]) &&
+         savedAddress.name.includes(address["family-name"]), "Address.name was computed");
     }, EXPECTED_ADDRESS);
 
     info("clicking cancel");
     spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
 
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
 });
@@ -342,30 +350,34 @@ add_task(async function test_edit_payer_
       }, "Check address was edited");
 
       let addressGUIDs = Object.keys(state.savedAddresses);
       is(addressGUIDs.length, 1, "Check there is still one address");
       let savedAddress = state.savedAddresses[addressGUIDs[0]];
       for (let [key, val] of Object.entries(address)) {
         is(savedAddress[key], val + "1", "Check updated " + key);
       }
+      ok(savedAddress.guid, "Address has a guid");
+      ok(savedAddress.name, "Address has a name");
+      ok(savedAddress.name.includes(address["given-name"]) &&
+         savedAddress.name.includes(address["family-name"]), "Address.name was computed");
     }, EXPECTED_ADDRESS);
 
     info("clicking cancel");
     spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
 
     await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
   });
 });
 
 /*
  * Test that we can correctly add an address from a private window
  */
 add_task(async function test_private_persist_addresses() {
-  await setup();
+  let prefilledGuids = await setup();
 
   is((await formAutofillStorage.addresses.getAll()).length, 1,
      "Setup results in 1 stored address at start of test");
 
   await withNewTabInPrivateWindow({
     url: BLANK_PAGE_URL,
   }, async browser => {
     info("in new tab w. private window");
@@ -375,22 +387,16 @@ add_task(async function test_private_per
         methodData: [PTU.MethodData.basicCard],
         details: Object.assign({}, PTU.Details.twoShippingOptions, PTU.Details.total2USD),
         options: PTU.Options.requestShippingOption,
         merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
       }
     );
     info("/setupPaymentDialog");
 
-    // listen for shippingaddresschange event in merchant (private) window
-    info("listen for shippingaddresschange");
-    let shippingAddressChangePromise = ContentTask.spawn(browser, {
-      eventName: "shippingaddresschange",
-    }, PTU.ContentTasks.awaitPaymentRequestEventPromise);
-
     let addressToAdd = PTU.Addresses.Temp;
     const addOptions = {
       checkboxSelector: "#address-page .persist-checkbox",
       expectPersist: false,
       isPrivate: true,
     };
 
     await navigateToAddAddressPage(frame);
@@ -410,31 +416,74 @@ add_task(async function test_private_per
       await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
     is(initialAddresses.options.length, 1,
        "Got expected number of pre-filled shipping addresses");
 
     await fillInAddressForm(frame, addressToAdd, addOptions);
     await verifyPersistCheckbox(frame, addOptions);
     await submitAddressForm(frame, addressToAdd, addOptions);
 
-    await shippingAddressChangePromise;
-    info("got shippingaddresschange event");
+    let shippingAddresses =
+      await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
+    info("shippingAddresses", shippingAddresses);
+    let addressOptions = shippingAddresses.options;
+    // expect the prefilled address + the new temporary address
+    is(addressOptions.length, 2, "The picker has the expected number of address options");
+    let tempAddressOption = addressOptions.find(addr => addr.guid != prefilledGuids.address1GUID);
+    let tempAddressGuid = tempAddressOption.guid;
+    // select the new address
+    await spawnPaymentDialogTask(frame,
+                                 PTU.DialogContentTasks.selectShippingAddressByGuid,
+                                 tempAddressGuid);
+
+    info("awaiting the shippingaddresschange event");
+    await ContentTask.spawn(browser, {
+      eventName: "shippingaddresschange",
+    }, PTU.ContentTasks.awaitPaymentRequestEventPromise);
 
-    // verify address picker has more addresses and new selected
-    info("check shipping addresses");
-    let addresses =
-      await spawnPaymentDialogTask(frame, PTU.DialogContentTasks.getShippingAddresses);
-    is(addresses.options.length, 2, "Got expected number of shipping addresses");
+    await spawnPaymentDialogTask(frame, async (args) => {
+      let {address, tempAddressGuid} = args;
+      let {
+        PaymentTestUtils: PTU,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+      let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+        return state.selectedShippingAddress == tempAddressGuid;
+      }, "Check the temp address is the selectedShippingAddress");
+
+      let addressGUIDs = Object.keys(state.tempAddresses);
+      is(addressGUIDs.length, 1, "Check there is one address");
+
+      is(addressGUIDs[0], tempAddressGuid, "guid from state and picker options match");
+      let tempAddress = state.tempAddresses[tempAddressGuid];
+      for (let [key, val] of Object.entries(address)) {
+        is(tempAddress[key], val, "Check field " + key);
+      }
+      ok(tempAddress.guid, "Address has a guid");
+      ok(tempAddress.name, "Address has a name");
+      ok(tempAddress.name.includes(address["given-name"]) &&
+         tempAddress.name.includes(address["family-name"]), "Address.name was computed");
+    }, {address: addressToAdd, tempAddressGuid});
 
     info("clicking pay");
     spawnPaymentDialogTask(frame, PTU.DialogContentTasks.completePayment);
 
     // Add a handler to complete the payment above.
     info("acknowledging the completion from the merchant page");
     let result = await ContentTask.spawn(browser, {}, PTU.ContentTasks.addCompletionHandler);
-    is(result.response.methodName, "basic-card", "Check methodName");
+
+    // Verify response has the expected properties
     info("response: " + JSON.stringify(result.response));
+    let responseAddress = result.response.shippingAddress;
+    ok(responseAddress, "response should contain the shippingAddress");
+    ok(responseAddress.recipient.includes(addressToAdd["given-name"]),
+       "Check given-name matches recipient in response");
+    ok(responseAddress.recipient.includes(addressToAdd["family-name"]),
+       "Check family-name matches recipient in response");
+    is(responseAddress.addressLine[0], addressToAdd["street-address"],
+       "Check street-address in response");
+    is(responseAddress.country, addressToAdd.country, "Check country in response");
   });
   // verify formautofill store doesnt have the new temp addresses
   is((await formAutofillStorage.addresses.getAll()).length, 1,
      "Original 1 stored address at end of test");
 });
 
--- a/browser/components/payments/test/browser/browser_card_edit.js
+++ b/browser/components/payments/test/browser/browser_card_edit.js
@@ -1,153 +1,252 @@
 /* eslint-disable no-shadow */
 
 "use strict";
 
-add_task(async function test_add_link() {
-  const args = {
-    methodData: [PTU.MethodData.basicCard],
-    details: PTU.Details.total60USD,
-  };
-  await spawnInDialogForMerchantTask(PTU.ContentTasks.createAndShowRequest, async function check() {
-    let {
-      PaymentTestUtils: PTU,
-    } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+async function setup(addresses = [], cards = []) {
+  await setupFormAutofillStorage();
+  await cleanupFormAutofillStorage();
+  let prefilledGuids = await addSampleAddressesAndBasicCard(addresses, cards);
+  return prefilledGuids;
+}
 
-    let addLink = content.document.querySelector("payment-method-picker .add-link");
-    is(addLink.textContent, "Add", "Add link text");
+async function add_link(aOptions = {}) {
+  let tabOpenFn = aOptions.isPrivate ? withNewTabInPrivateWindow : BrowserTestUtils.withNewTab;
+  await tabOpenFn({
+    gBrowser,
+    url: BLANK_PAGE_URL,
+  }, async browser => {
+    let {win, frame} =
+      await setupPaymentDialog(browser, {
+        methodData: [PTU.MethodData.basicCard],
+        details: Object.assign({}, PTU.Details.total60USD),
+        merchantTaskFn: PTU.ContentTasks.createAndShowRequest,
+      }
+    );
 
-    addLink.click();
-
-    let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "basic-card-page" && !state["basic-card-page"].guid;
-    }, "Check add page state");
+    await navigateToAddCardPage(frame);
+    await verifyPersistCheckbox(frame, {
+      checkboxSelector: "basic-card-form .persist-checkbox",
+      expectPersist: !aOptions.isPrivate,
+    });
+    await spawnPaymentDialogTask(frame, async (testArgs = {}) => {
+      let {
+        PaymentTestUtils: PTU,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
 
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return Object.keys(state.savedBasicCards).length == 0 &&
-             Object.keys(state.savedAddresses).length == 0;
-    }, "Check no cards or addresses present at beginning of test");
+      let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+        return Object.keys(state.savedBasicCards).length == 1 &&
+               Object.keys(state.savedAddresses).length == 1;
+      }, "Check no cards or addresses present at beginning of test");
+
+      let title = content.document.querySelector("basic-card-form h2");
+      is(title.textContent, "Add Credit Card", "Add title should be set");
 
-    let title = content.document.querySelector("basic-card-form h2");
-    is(title.textContent, "Add Credit Card", "Add title should be set");
+      is(state.isPrivate, testArgs.isPrivate,
+         "isPrivate flag has expected value when shown from a private/non-private session");
+    }, aOptions);
+
+    await fillInCardForm(frame, PTU.BasicCards.JaneMasterCard, {
+      isTemporary: aOptions.isPrivate,
+      checkboxSelector: "basic-card-form .persist-checkbox",
+    });
 
-    ok(!state.isPrivate,
-       "isPrivate flag is not set when paymentrequest is shown from a non-private session");
-    let persistCheckbox = content.document.querySelector("basic-card-form labelled-checkbox");
-    ok(Cu.waiveXrays(persistCheckbox).checked, "persist checkbox should be checked by default");
-
-    let card = Object.assign({}, PTU.BasicCards.JohnDoe);
+    await spawnPaymentDialogTask(frame, async (testArgs = {}) => {
+      let billingAddressSelect = content.document.querySelector("#billingAddressGUID");
+      ok(content.isVisible(billingAddressSelect),
+         "The billing address selector should always be visible");
+      is(billingAddressSelect.childElementCount, 2,
+         "Only 2 child options should exist by default");
+      is(billingAddressSelect.children[0].value, "",
+         "The first option should be the blank/empty option");
+      ok(billingAddressSelect.children[1].value, "",
+         "The 2nd option is the prefilled address and should be truthy");
+    }, aOptions);
 
-    info("filling fields");
-    for (let [key, val] of Object.entries(card)) {
-      let field = content.document.getElementById(key);
-      field.value = val;
-      ok(!field.disabled, `Field #${key} shouldn't be disabled`);
-    }
+    let addressOptions = Object.assign({}, aOptions, {
+      addLinkSelector: ".billingAddressRow .add-link",
+      checkboxSelector: "#address-page .persist-checkbox",
+      initialPageId: "basic-card-page",
+      expectPersist: !aOptions.isPrivate,
+    });
+    await navigateToAddAddressPage(frame, addressOptions);
+
+    await spawnPaymentDialogTask(frame, async (testArgs = {}) => {
+      let {
+        PaymentTestUtils: PTU,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+      let title = content.document.querySelector("basic-card-form h2");
+      let card = Object.assign({}, PTU.BasicCards.JaneMasterCard);
 
-    let billingAddressSelect = content.document.querySelector("#billingAddressGUID");
-    ok(content.isVisible(billingAddressSelect),
-       "The billing address selector should always be visible");
-    is(billingAddressSelect.childElementCount, 1,
-       "Only one child option should exist by default");
-    is(billingAddressSelect.children[0].value, "",
-       "The only option should be the blank/empty option");
+      let addressTitle = content.document.querySelector("address-form h2");
+      is(addressTitle.textContent, "Add Billing Address",
+         "Address on add address page should be correct");
+
+      await PTU.DialogContentUtils.waitForState(content, (state) => {
+        let total = Object.keys(state.savedBasicCards).length +
+                    Object.keys(state.tempBasicCards).length;
+        return total == 1;
+      }, "Check card was not added when clicking the 'add' address button");
+
+      let addressBackButton = content.document.querySelector("address-form .back-button");
+      addressBackButton.click();
 
-    let addressAddLink = content.document.querySelector(".billingAddressRow .add-link");
-    addressAddLink.click();
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "address-page" && !state["address-page"].guid;
-    }, "Check address page state");
+      await PTU.DialogContentUtils.waitForState(content, (state) => {
+        let total = Object.keys(state.savedBasicCards).length +
+                    Object.keys(state.tempBasicCards).length;
+        return state.page.id == "basic-card-page" && !state["basic-card-page"].guid &&
+               total == 1;
+      }, "Check basic-card page, but card should not be saved and no addresses present");
+
+      is(title.textContent, "Add Credit Card", "Add title should be still be on credit card page");
 
-    let addressTitle = content.document.querySelector("address-form h2");
-    is(addressTitle.textContent, "Add Billing Address",
-       "Address on add address page should be correct");
+      for (let [key, val] of Object.entries(card)) {
+        let field = content.document.getElementById(key);
+        is(field.value, val, "Field should still have previous value entered");
+        ok(!field.disabled, "Fields should still be enabled for editing");
+      }
+    }, aOptions);
 
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return Object.keys(state.savedBasicCards).length == 0;
-    }, "Check card was not added when clicking the 'add' address button");
+    await navigateToAddAddressPage(frame, addressOptions);
+
+    await fillInAddressForm(frame, PTU.Addresses.TimBL2, addressOptions);
+
+    await verifyPersistCheckbox(frame, addressOptions);
 
-    let addressBackButton = content.document.querySelector("address-form .back-button");
-    addressBackButton.click();
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "basic-card-page" && !state["basic-card-page"].guid &&
-             Object.keys(state.savedAddresses).length == 0;
-    }, "Check basic-card page, but card should not be saved and no addresses present");
+    await spawnPaymentDialogTask(frame, async (testArgs = {}) => {
+      let {
+        PaymentTestUtils: PTU,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+      content.document.querySelector("address-form button:last-of-type").click();
+
+      let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+        return state.page.id == "basic-card-page" && !state["basic-card-page"].guid;
+      }, "Check address was added and we're back on basic-card page (add)");
 
-    is(title.textContent, "Add Credit Card", "Add title should be still be on credit card page");
+      let addressCount = Object.keys(state.savedAddresses).length +
+                         Object.keys(state.tempAddresses).length;
+      is(addressCount, 2, "Check address was added");
+
+      let addressColn = testArgs.isPrivate ? state.tempAddresses : state.savedAddresses;
 
-    for (let [key, val] of Object.entries(card)) {
-      let field = content.document.getElementById(key);
-      is(field.value, val, "Field should still have previous value entered");
-      ok(!field.disabled, "Fields should still be enabled for editing");
-    }
+      ok(state["basic-card-page"].preserveFieldValues,
+         "preserveFieldValues should be set when coming back from address-page");
+
+      ok(state["basic-card-page"].billingAddressGUID,
+         "billingAddressGUID should be set when coming back from address-page");
+
+      let billingAddressSelect = content.document.querySelector("#billingAddressGUID");
 
-    addressAddLink.click();
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "address-page" && !state["address-page"].guid;
-    }, "Check address page state");
+      is(billingAddressSelect.childElementCount, 3,
+         "Three options should exist in the billingAddressSelect");
+      let selectedOption =
+        billingAddressSelect.children[billingAddressSelect.selectedIndex];
+      let selectedAddressGuid = selectedOption.value;
+      let lastAddress = Object.values(addressColn)[Object.keys(addressColn).length - 1];
+      if (testArgs.isPrivate) {
+        // guid property is added in later patch on bug 1463608
+        is(selectedAddressGuid, lastAddress.guid,
+           "The select should have the new address selected");
+      } else {
+        is(selectedAddressGuid, lastAddress.guid,
+           "The select should have the new address selected");
+      }
+    }, aOptions);
 
-    info("filling address fields");
-    for (let [key, val] of Object.entries(PTU.Addresses.TimBL)) {
-      let field = content.document.getElementById(key);
-      if (!field) {
-        ok(false, `${key} field not found`);
-      }
-      field.value = val;
-      ok(!field.disabled, `Field #${key} shouldn't be disabled`);
-    }
+    await fillInCardForm(frame, PTU.BasicCards.JaneMasterCard, {
+      isTemporary: aOptions.isPrivate,
+      checkboxSelector: "basic-card-form .persist-checkbox",
+    });
 
-    content.document.querySelector("address-form button:last-of-type").click();
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "basic-card-page" && !state["basic-card-page"].guid &&
-             Object.keys(state.savedAddresses).length == 1;
-    }, "Check address was added and we're back on basic-card page (add)");
+    await spawnPaymentDialogTask(frame, async (testArgs = {}) => {
+      let {
+        PaymentTestUtils: PTU,
+      } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
+
+      let {prefilledGuids} = testArgs;
+      let card = Object.assign({}, PTU.BasicCards.JaneMasterCard);
+
+      content.document.querySelector("basic-card-form button:last-of-type").click();
 
-    ok(state["basic-card-page"].preserveFieldValues,
-       "preserveFieldValues should be set when coming back from address-page");
-
-    ok(state["basic-card-page"].billingAddressGUID,
-       "billingAddressGUID should be set when coming back from address-page");
+      let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
+        return state.page.id == "payment-summary";
+      }, "Check we are back on the sumamry page");
 
-    is(billingAddressSelect.childElementCount, 2,
-       "Two options should exist in the billingAddressSelect");
-    let selectedOption =
-      billingAddressSelect.children[billingAddressSelect.selectedIndex];
-    let selectedAddressGuid = selectedOption.value;
-    is(selectedAddressGuid, Object.values(state.savedAddresses)[0].guid,
-       "The select should have the new address selected");
+      let cardCount = Object.keys(state.savedBasicCards).length +
+                         Object.keys(state.tempBasicCards).length;
+      is(cardCount, 2, "Card was added");
+      if (testArgs.isPrivate) {
+        is(Object.keys(state.tempBasicCards).length, 1, "Card was added temporarily");
+        is(Object.keys(state.savedBasicCards).length, 1, "No change to saved cards");
+      } else {
+        is(Object.keys(state.tempBasicCards).length, 0, "No temporary cards addded");
+        is(Object.keys(state.savedBasicCards).length, 2, "New card was saved");
+      }
 
-    for (let [key, val] of Object.entries(card)) {
-      let field = content.document.getElementById(key);
-      is(field.value, val, `Field #${key} should have value`);
-    }
+      let cardCollection = testArgs.isPrivate ? state.tempBasicCards : state.savedBasicCards;
+      let addressCollection = testArgs.isPrivate ? state.tempAddresses : state.savedAddresses;
+      let savedCardGUID =
+        Object.keys(cardCollection).find(key => key != prefilledGuids.card1GUID);
+      let savedAddressGUID =
+        Object.keys(addressCollection).find(key => key != prefilledGuids.address1GUID);
+      let savedCard = savedCardGUID && cardCollection[savedCardGUID];
+
+      card["cc-number"] = "************4444"; // Card should be masked
 
-    content.document.querySelector("basic-card-form button:last-of-type").click();
-
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return Object.keys(state.savedBasicCards).length == 1;
-    }, "Check card was not added again");
+      for (let [key, val] of Object.entries(card)) {
+        if (key == "cc-number" && testArgs.isPrivate) {
+          // cc-number is not yet masked for private/temporary cards
+          is(savedCard[key], val, "Check " + key);
+        } else {
+          is(savedCard[key], val, "Check " + key);
+        }
+      }
+      if (testArgs.isPrivate) {
+        ok(testArgs.isPrivate,
+           "Checking card/address from private window relies on guid property " +
+           "which isnt available yet");
+      } else {
+        is(savedCard.billingAddressGUID, savedAddressGUID,
+           "The saved card should be associated with the billing address");
+      }
+    }, aOptions);
 
-    let cardGUIDs = Object.keys(state.savedBasicCards);
-    is(cardGUIDs.length, 1, "Check there is one card");
-    let savedCard = state.savedBasicCards[cardGUIDs[0]];
-    card["cc-number"] = "************1111"; // Card should be masked
-    for (let [key, val] of Object.entries(card)) {
-      is(savedCard[key], val, "Check " + key);
-    }
-    is(savedCard.billingAddressGUID, selectedAddressGuid,
-       "The saved card should be associated with the billing address");
+    spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel);
+    await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed");
+  });
+}
 
-    state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "payment-summary";
-    }, "Switched back to payment-summary");
-  }, args);
+add_task(async function test_add_link() {
+  let prefilledGuids = await setup([PTU.Addresses.TimBL], [PTU.BasicCards.JohnDoe]);
+  await add_link({
+    isPrivate: false,
+    prefilledGuids,
+  });
+}).skip();
+
+add_task(async function test_private_add_link() {
+  let prefilledGuids = await setup([PTU.Addresses.TimBL], [PTU.BasicCards.JohnDoe]);
+  await add_link({
+    isPrivate: true,
+    prefilledGuids,
+  });
 });
 
 add_task(async function test_edit_link() {
+  // add an address and card linked to this address
+  let prefilledGuids = await setup([PTU.Addresses.TimBL]);
+  {
+    let card = Object.assign({}, PTU.BasicCards.JohnDoe,
+                             { billingAddressGUID: prefilledGuids.address1GUID });
+    await addCardRecord(card);
+  }
+
   const args = {
     methodData: [PTU.MethodData.basicCard],
     details: PTU.Details.total60USD,
   };
   await spawnInDialogForMerchantTask(PTU.ContentTasks.createAndShowRequest, async function check() {
     let {
       PaymentTestUtils: PTU,
     } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
@@ -182,17 +281,17 @@ add_task(async function test_edit_link()
       ok(!field.disabled, `Field #${key} shouldn't be disabled`);
     }
     ok(content.document.getElementById("cc-number").disabled, "cc-number field should be disabled");
 
     let billingAddressSelect = content.document.querySelector("#billingAddressGUID");
     is(billingAddressSelect.childElementCount, 2,
        "Two options should exist in the billingAddressSelect");
     is(billingAddressSelect.selectedIndex, 1,
-       "The billing address set by the previous test should be selected by default");
+       "The prefilled billing address should be selected by default");
 
     info("Test clicking 'edit' on the empty option first");
     billingAddressSelect.selectedIndex = 0;
 
     let addressEditLink = content.document.querySelector(".billingAddressRow .edit-link");
     addressEditLink.click();
     state = await PTU.DialogContentUtils.waitForState(content, (state) => {
       return state.page.id == "address-page" && !state["address-page"].guid;
@@ -282,73 +381,20 @@ add_task(async function test_edit_link()
     for (let [key, val] of Object.entries(card)) {
       is(savedCard[key], val, "Check updated " + key);
     }
 
     state = await PTU.DialogContentUtils.waitForState(content, (state) => {
       return state.page.id == "payment-summary";
     }, "Switched back to payment-summary");
   }, args);
-});
-
-add_task(async function test_private_persist_defaults() {
-  const args = {
-    methodData: [PTU.MethodData.basicCard],
-    details: PTU.Details.total60USD,
-  };
-  await spawnInDialogForMerchantTask(PTU.ContentTasks.createAndShowRequest, async function check() {
-    let {
-      PaymentTestUtils: PTU,
-    } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
-
-    let addLink = content.document.querySelector("payment-method-picker .add-link");
-    is(addLink.textContent, "Add", "Add link text");
-
-    addLink.click();
-
-    let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "basic-card-page" && !state["basic-card-page"].guid;
-    },
-                                                          "Check add page state");
-
-    ok(!state.isPrivate,
-       "isPrivate flag is not set when paymentrequest is shown from a non-private session");
-    let persistCheckbox = content.document.querySelector("basic-card-form labelled-checkbox");
-    ok(Cu.waiveXrays(persistCheckbox).checked,
-       "checkbox is checked by default from a non-private session");
-  }, args);
-
-  let privateWin = await BrowserTestUtils.openNewBrowserWindow({private: true});
-  await spawnInDialogForMerchantTask(PTU.ContentTasks.createAndShowRequest, async function check() {
-    let {
-      PaymentTestUtils: PTU,
-    } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {});
-
-    let addLink = content.document.querySelector("payment-method-picker .add-link");
-    is(addLink.textContent, "Add", "Add link text");
-
-    addLink.click();
-
-    let state = await PTU.DialogContentUtils.waitForState(content, (state) => {
-      return state.page.id == "basic-card-page" && !state["basic-card-page"].guid;
-    },
-                                                          "Check add page state");
-
-    ok(state.isPrivate,
-       "isPrivate flag is set when paymentrequest is shown from a private session");
-    let persistCheckbox = content.document.querySelector("labelled-checkbox");
-    ok(!Cu.waiveXrays(persistCheckbox).checked,
-       "checkbox is not checked by default from a private session");
-  }, args, {
-    browser: privateWin.gBrowser,
-  });
-  await BrowserTestUtils.closeWindow(privateWin);
-});
+}).skip();
 
 add_task(async function test_private_card_adding() {
+  await setup([PTU.Addresses.TimBL], [PTU.BasicCards.JohnDoe]);
   const args = {
     methodData: [PTU.MethodData.basicCard],
     details: PTU.Details.total60USD,
   };
   let privateWin = await BrowserTestUtils.openNewBrowserWindow({private: true});
   await spawnInDialogForMerchantTask(PTU.ContentTasks.createAndShowRequest, async function check() {
     let {
       PaymentTestUtils: PTU,
@@ -380,13 +426,29 @@ add_task(async function test_private_car
 
     state = await PTU.DialogContentUtils.waitForState(content, (state) => {
       return Object.keys(state.tempBasicCards).length > tempCardCount;
     },
                                                       "Check card was added to temp collection");
 
     is(savedCardCount, Object.keys(state.savedBasicCards).length, "No card was saved in state");
     is(Object.keys(state.tempBasicCards).length, 1, "Card was added temporarily");
+
+    let cardGUIDs = Object.keys(state.tempBasicCards);
+    is(cardGUIDs.length, 1, "Check there is one card");
+
+    let tempCard = state.tempBasicCards[cardGUIDs[0]];
+    // Card number should be masked, so skip cc-number in the compare loop below
+    delete card["cc-number"];
+    for (let [key, val] of Object.entries(card)) {
+      is(tempCard[key], val, "Check " + key + ` ${tempCard[key]} matches ${val}`);
+    }
+    // check computed fields
+    is(tempCard["cc-number"], "************1111", "cc-number is masked");
+    is(tempCard["cc-given-name"], "John", "cc-given-name was computed");
+    is(tempCard["cc-family-name"], "Doe", "cc-family-name was computed");
+    ok(tempCard["cc-exp"], "cc-exp was computed");
+    ok(tempCard["cc-number-encrypted"], "cc-number-encrypted was computed");
   }, args, {
     browser: privateWin.gBrowser,
   });
   await BrowserTestUtils.closeWindow(privateWin);
-});
+}).skip();
--- a/browser/extensions/formautofill/FormAutofillStorage.jsm
+++ b/browser/extensions/formautofill/FormAutofillStorage.jsm
@@ -87,20 +87,20 @@
  * }
  *
  *
  * Encrypt-related Credit Card Fields (cc-number & cc-number-encrypted):
  *
  * When saving or updating a credit-card record, the storage will encrypt the
  * value of "cc-number", store the encrypted number in "cc-number-encrypted"
  * field, and replace "cc-number" field with the masked number. These all happen
- * in "_computeFields". We do reverse actions in "_stripComputedFields", which
+ * in "computeFields". We do reverse actions in "_stripComputedFields", which
  * decrypts "cc-number-encrypted", restores it to "cc-number", and deletes
  * "cc-number-encrypted". Therefore, calling "_stripComputedFields" followed by
- * "_computeFields" can make sure the encrypt-related fields are up-to-date.
+ * "computeFields" can make sure the encrypt-related fields are up-to-date.
  *
  * In general, you have to decrypt the number by your own outside FormAutofillStorage
  * when necessary. However, you will get the decrypted records when querying
  * data with "rawData=true" to ensure they're ready to sync.
  *
  *
  * Sync Metadata:
  *
@@ -353,17 +353,17 @@ class AutofillRecords {
       recordToSave = {
         guid: record.guid,
         timeLastModified: record.timeLastModified || Date.now(),
         deleted: true,
       };
     } else {
       this._ensureMatchingVersion(record);
       recordToSave = record;
-      this._computeFields(recordToSave);
+      this.computeFields(recordToSave);
     }
 
     if (sourceSync) {
       let sync = this._getSyncMetaData(recordToSave, true);
       sync.changeCounter = 0;
     }
 
     this._data.push(recordToSave);
@@ -437,17 +437,17 @@ class AutofillRecords {
     }
 
     recordFound.timeLastModified = Date.now();
     let syncMetadata = this._getSyncMetaData(recordFound);
     if (syncMetadata) {
       syncMetadata.changeCounter += 1;
     }
 
-    this._computeFields(recordFound);
+    this.computeFields(recordFound);
     this._data[recordFoundIndex] = recordFound;
 
     this._store.saveSoon();
 
     Services.obs.notifyObservers({wrappedJSObject: {
       guid,
       collectionName: this._collectionName,
     }}, "formautofill-storage-changed", "update");
@@ -750,17 +750,17 @@ class AutofillRecords {
 
     // Copy local-only fields from the existing local record.
     for (let field of ["timeLastUsed", "timesUsed"]) {
       if (localRecord[field] != null) {
         newRecord[field] = localRecord[field];
       }
     }
 
-    this._computeFields(newRecord);
+    this.computeFields(newRecord);
   }
 
   /**
    * Clones a local record, giving the clone a new GUID and Sync metadata. The
    * original record remains unchanged in storage.
    *
    * @param   {Object} strippedLocalRecord
    *          The local record. Computed fields are stripped.
@@ -772,17 +772,17 @@ class AutofillRecords {
     forkedLocalRecord.guid = this._generateGUID();
 
     // 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.computeFields(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`.
@@ -1145,17 +1145,17 @@ class AutofillRecords {
     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);
+    hasChanges |= this.computeFields(record);
     return hasChanges;
   }
 
   _normalizeRecord(record, preserveEmptyFields = false) {
     this._normalizeFields(record);
 
     for (let key in record) {
       if (!this.VALID_FIELDS.includes(key)) {
@@ -1211,17 +1211,17 @@ class AutofillRecords {
   _stripComputedFields(record) {
     this.VALID_COMPUTED_FIELDS.forEach(field => delete record[field]);
   }
 
   // An interface to be inherited.
   _recordReadProcessor(record) {}
 
   // An interface to be inherited.
-  _computeFields(record) {}
+  computeFields(record) {}
 
   // An interface to be inherited.
   _normalizeFields(record) {}
 
   // An interface to be inherited.
   mergeIfPossible(guid, record, strict) {}
 }
 
@@ -1232,17 +1232,17 @@ class Addresses extends AutofillRecords 
 
   _recordReadProcessor(address) {
     if (address.country && !FormAutofillUtils.supportedCountries.includes(address.country)) {
       delete address.country;
       delete address["country-name"];
     }
   }
 
-  _computeFields(address) {
+  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.)
 
     // NOTE: Computed fields should be always present in the storage no matter
     //       it's empty or not.
 
     let hasNewComputedFields = false;
@@ -1491,17 +1491,17 @@ class Addresses extends AutofillRecords 
   }
 }
 
 class CreditCards extends AutofillRecords {
   constructor(store) {
     super(store, "creditCards", VALID_CREDIT_CARD_FIELDS, VALID_CREDIT_CARD_COMPUTED_FIELDS, CREDIT_CARD_SCHEMA_VERSION);
   }
 
-  _computeFields(creditCard) {
+  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.)
 
     // NOTE: Computed fields should be always present in the storage no matter
     //       it's empty or not.
 
     let hasNewComputedFields = false;