Bug 1480872 - paymentDetails attributes should default to an empty arrays. r=edenchuang
authorMarcos Cáceres <marcos@marcosc.com>
Sun, 19 Aug 2018 19:11:00 +0300
changeset 432336 3378c38648970f1040a5db0e8f2cc72d308dea29
parent 432318 f2c0be296f6f10567864fcd0f4d2e7c5da342c34
child 432337 dcdd7d0adcc07d883c971b11e1d33a0212f63327
push id34474
push usershindli@mozilla.com
push dateMon, 20 Aug 2018 16:31:49 +0000
treeherdermozilla-central@1ba33950cff3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedenchuang
bugs1480872
milestone63.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 1480872 - paymentDetails attributes should default to an empty arrays. r=edenchuang
dom/payments/PaymentRequestData.cpp
dom/payments/PaymentRequestManager.cpp
dom/payments/ipc/PPaymentRequest.ipdl
dom/payments/test/ConstructorChromeScript.js
dom/payments/test/browser_payment_in_different_tabs.js
dom/payments/test/head.js
dom/payments/test/simple_payment_request.html
--- a/dom/payments/PaymentRequestData.cpp
+++ b/dom/payments/PaymentRequestData.cpp
@@ -360,68 +360,62 @@ PaymentDetails::Create(const IPCPaymentD
 
   nsCOMPtr<nsIPaymentItem> total;
   nsresult rv = PaymentItem::Create(aIPCDetails.total(), getter_AddRefs(total));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   nsCOMPtr<nsIArray> displayItems;
-  if (aIPCDetails.displayItemsPassed()) {
-    nsCOMPtr<nsIMutableArray> items = do_CreateInstance(NS_ARRAY_CONTRACTID);
-    MOZ_ASSERT(items);
-    for (const IPCPaymentItem& displayItem : aIPCDetails.displayItems()) {
-      nsCOMPtr<nsIPaymentItem> item;
-      rv = PaymentItem::Create(displayItem, getter_AddRefs(item));
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
-      rv = items->AppendElement(item);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
+  nsCOMPtr<nsIMutableArray> items = do_CreateInstance(NS_ARRAY_CONTRACTID);
+  MOZ_ASSERT(items);
+  for (const IPCPaymentItem& displayItem : aIPCDetails.displayItems()) {
+    nsCOMPtr<nsIPaymentItem> item;
+    rv = PaymentItem::Create(displayItem, getter_AddRefs(item));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
     }
-    displayItems = items.forget();
+    rv = items->AppendElement(item);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
+  displayItems = items.forget();
 
   nsCOMPtr<nsIArray> shippingOptions;
-  if (aIPCDetails.shippingOptionsPassed()) {
-    nsCOMPtr<nsIMutableArray> options = do_CreateInstance(NS_ARRAY_CONTRACTID);
-    MOZ_ASSERT(options);
-    for (const IPCPaymentShippingOption& shippingOption : aIPCDetails.shippingOptions()) {
-      nsCOMPtr<nsIPaymentShippingOption> option;
-      rv = PaymentShippingOption::Create(shippingOption, getter_AddRefs(option));
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
-      rv = options->AppendElement(option);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
+  nsCOMPtr<nsIMutableArray> options = do_CreateInstance(NS_ARRAY_CONTRACTID);
+  MOZ_ASSERT(options);
+  for (const IPCPaymentShippingOption& shippingOption : aIPCDetails.shippingOptions()) {
+    nsCOMPtr<nsIPaymentShippingOption> option;
+    rv = PaymentShippingOption::Create(shippingOption, getter_AddRefs(option));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
     }
-    shippingOptions = options.forget();
+    rv = options->AppendElement(option);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
+  shippingOptions = options.forget();
 
   nsCOMPtr<nsIArray> modifiers;
-  if (aIPCDetails.modifiersPassed()) {
-    nsCOMPtr<nsIMutableArray> detailsModifiers = do_CreateInstance(NS_ARRAY_CONTRACTID);
-    MOZ_ASSERT(detailsModifiers);
-    for (const IPCPaymentDetailsModifier& modifier : aIPCDetails.modifiers()) {
-      nsCOMPtr<nsIPaymentDetailsModifier> detailsModifier;
-      rv = PaymentDetailsModifier::Create(modifier, getter_AddRefs(detailsModifier));
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
-      rv = detailsModifiers->AppendElement(detailsModifier);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
+  nsCOMPtr<nsIMutableArray> detailsModifiers = do_CreateInstance(NS_ARRAY_CONTRACTID);
+  MOZ_ASSERT(detailsModifiers);
+  for (const IPCPaymentDetailsModifier& modifier : aIPCDetails.modifiers()) {
+    nsCOMPtr<nsIPaymentDetailsModifier> detailsModifier;
+    rv = PaymentDetailsModifier::Create(modifier, getter_AddRefs(detailsModifier));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
     }
-    modifiers = detailsModifiers.forget();
+    rv = detailsModifiers->AppendElement(detailsModifier);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
+  modifiers = detailsModifiers.forget();
 
   nsCOMPtr<nsIPaymentDetails> details =
     new PaymentDetails(aIPCDetails.id(), total, displayItems, shippingOptions,
                        modifiers, aIPCDetails.error(),
                        aIPCDetails.shippingAddressErrors());
 
   details.forget(aDetails);
   return NS_OK;
--- a/dom/payments/PaymentRequestManager.cpp
+++ b/dom/payments/PaymentRequestManager.cpp
@@ -173,20 +173,17 @@ ConvertDetailsInit(JSContext* aCx,
   ConvertItem(aDetails.mTotal, total);
 
   aIPCDetails = IPCPaymentDetails(id,
                                   total,
                                   displayItems,
                                   shippingOptions,
                                   modifiers,
                                   EmptyString(), // error message
-                                  EmptyString(), // shippingAddressErrors
-                                  aDetails.mDisplayItems.WasPassed(),
-                                  aDetails.mShippingOptions.WasPassed(),
-                                  aDetails.mModifiers.WasPassed());
+                                  EmptyString()); // shippingAddressErrors
   return NS_OK;
 }
 
 nsresult
 ConvertDetailsUpdate(JSContext* aCx,
                      const PaymentDetailsUpdate& aDetails,
                      IPCPaymentDetails& aIPCDetails,
                      bool aRequestShipping)
@@ -218,20 +215,17 @@ ConvertDetailsUpdate(JSContext* aCx,
   }
 
   aIPCDetails = IPCPaymentDetails(EmptyString(), // id
                                   total,
                                   displayItems,
                                   shippingOptions,
                                   modifiers,
                                   error,
-                                  shippingAddressErrors,
-                                  aDetails.mDisplayItems.WasPassed(),
-                                  aDetails.mShippingOptions.WasPassed(),
-                                  aDetails.mModifiers.WasPassed());
+                                  shippingAddressErrors);
   return NS_OK;
 }
 
 void
 ConvertOptions(const PaymentOptions& aOptions,
                IPCPaymentOptions& aIPCOption)
 {
   uint8_t shippingTypeIndex = static_cast<uint8_t>(aOptions.mShippingType);
--- a/dom/payments/ipc/PPaymentRequest.ipdl
+++ b/dom/payments/ipc/PPaymentRequest.ipdl
@@ -51,19 +51,16 @@ struct IPCPaymentDetails
 {
   nsString id;
   IPCPaymentItem total;
   IPCPaymentItem[] displayItems;
   IPCPaymentShippingOption[] shippingOptions;
   IPCPaymentDetailsModifier[] modifiers;
   nsString error;
   nsString shippingAddressErrors;
-  bool displayItemsPassed;
-  bool shippingOptionsPassed;
-  bool modifiersPassed;
 };
 
 struct IPCPaymentOptions
 {
   bool requestPayerName;
   bool requestPayerEmail;
   bool requestPayerPhone;
   bool requestShipping;
--- a/dom/payments/test/ConstructorChromeScript.js
+++ b/dom/payments/test/ConstructorChromeScript.js
@@ -40,24 +40,24 @@ function checkSimplestRequest(payRequest
   }
   if (details.totalItem.amount.currency != "USD") {
     emitTestFail("total item's currency should be 'USD'.");
   }
   if (details.totalItem.amount.value != "1.00") {
     emitTestFail("total item's value should be '1.00'.");
   }
 
-  if (details.displayItems) {
-    emitTestFail("details.displayItems should be undefined.");
+  if (details.displayItems.length !== 0) {
+    emitTestFail("details.displayItems should be an empty array.");
   }
-  if (details.modifiers) {
-    emitTestFail("details.displayItems should be undefined.");
+  if (details.modifiers.length !== 0) {
+    emitTestFail("details.modifiers should be an empty array.");
   }
-  if (details.shippingOptions) {
-    emitTestFail("details.shippingOptions should be undefined.");
+  if (details.shippingOptions.length !== 0) {
+    emitTestFail("details.shippingOptions should be an empty array.");
   }
 
   // checking the default generated PaymentOptions parameter
   const paymentOptions = payRequest.paymentOptions;
   if (paymentOptions.requestPayerName) {
     emitTestFail("requestPayerName option should be false.");
   }
   if (paymentOptions.requestPayerEmail) {
@@ -314,24 +314,24 @@ function checkNonBasicCardRequest(payReq
   }
   if (details.totalItem.amount.currency != "USD") {
     emitTestFail("total item's currency should be 'USD'.");
   }
   if (details.totalItem.amount.value != "1.00") {
     emitTestFail("total item's value should be '1.00'.");
   }
 
-  if (details.displayItems) {
-    emitTestFail("details.displayItems should be undefined.");
+  if (details.displayItems.length !== 0) {
+    emitTestFail("details.displayItems should be an zero length array.");
   }
-  if (details.modifiers) {
-    emitTestFail("details.displayItems should be undefined.");
+  if (details.displayItems.length !== 0) {
+    emitTestFail("details.modifiers should be an zero length array.");
   }
-  if (details.shippingOptions) {
-    emitTestFail("details.shippingOptions should be undefined.");
+  if (details.displayItems.length !== 0) {
+    emitTestFail("details.shippingOptions should be an zero length array.");
   }
 
   // checking the default generated PaymentOptions parameter
   const paymentOptions = payRequest.paymentOptions;
   if (paymentOptions.requestPayerName) {
     emitTestFail("requestPayerName option should be false.");
   }
   if (paymentOptions.requestPayerEmail) {
--- a/dom/payments/test/browser_payment_in_different_tabs.js
+++ b/dom/payments/test/browser_payment_in_different_tabs.js
@@ -1,31 +1,39 @@
 "use strict";
 
 // kTestRoot is from head.js
 const kTestPage = kTestRoot + "simple_payment_request.html";
-
-add_task(async function() {
+const TABS_TO_OPEN = 5;
+add_task(async () => {
   Services.prefs.setBoolPref("dom.payments.request.enabled", true);
-  await BrowserTestUtils.withNewTab(kTestPage,
-    async function(browser) {
-      await BrowserTestUtils.withNewTab(kTestPage,
-        function(browser) {
-          const paymentSrv = Cc["@mozilla.org/dom/payments/payment-request-service;1"].getService(Ci.nsIPaymentRequestService);
-          ok(paymentSrv, "Fail to get PaymentRequestService.");
-
-          const paymentEnum = paymentSrv.enumerate();
-          ok(paymentEnum.hasMoreElements(), "PaymentRequestService should have at least one payment request.");
-          let tabIds = [];
-          while (paymentEnum.hasMoreElements()) {
-            let payment = paymentEnum.getNext().QueryInterface(Ci.nsIPaymentRequest);
-            ok(payment, "Fail to get existing payment request.");
-            checkSimplePayment(payment);
-            tabIds.push(payment.tabId);
-          }
-          is(tabIds.length, 2, "TabId array length should be 2.");
-          ok(tabIds[0] != tabIds[1], "TabIds should be different.");
-          Services.prefs.setBoolPref("dom.payments.request.enabled", false);
-        }
-      );
-    }
+  const tabs = [];
+  const options = {
+    gBrowser: Services.wm.getMostRecentWindow("navigator:browser").gBrowser,
+    url: kTestPage,
+  };
+  for (let i = 0; i < TABS_TO_OPEN; i++) {
+    const tab = await BrowserTestUtils.openNewForegroundTab(options);
+    tabs.push(tab);
+  }
+  const paymentSrv = Cc[
+    "@mozilla.org/dom/payments/payment-request-service;1"
+  ].getService(Ci.nsIPaymentRequestService);
+  ok(paymentSrv, "Fail to get PaymentRequestService.");
+  const paymentEnum = paymentSrv.enumerate();
+  ok(
+    paymentEnum.hasMoreElements(),
+    "PaymentRequestService should have at least one payment request."
   );
+  const payments = new Set();
+  while (paymentEnum.hasMoreElements()) {
+    const payment = paymentEnum.getNext().QueryInterface(Ci.nsIPaymentRequest);
+    ok(payment, "Fail to get existing payment request.");
+    checkSimplePayment(payment);
+    payments.add(payment);
+  }
+  is(payments.size, TABS_TO_OPEN, `Should be ${TABS_TO_OPEN} unique objects.`);
+  tabs.forEach(async tab => {
+    await TestUtils.waitForTick();
+    BrowserTestUtils.removeTab(tab);
+  });
+  Services.prefs.setBoolPref("dom.payments.request.enabled", false);
 });
--- a/dom/payments/test/head.js
+++ b/dom/payments/test/head.js
@@ -12,19 +12,19 @@ function checkSimplePayment(aSimplePayme
 
   // checking the passed PaymentDetails parameter
   const details = aSimplePayment.paymentDetails;
   is(details.id, "simple details", "details.id should be 'simple details'.");
   is(details.totalItem.label, "Donation", "total item's label should be 'Donation'.");
   is(details.totalItem.amount.currency, "USD", "total item's currency should be 'USD'.");
   is(details.totalItem.amount.value, "55.00", "total item's value should be '55.00'.");
 
-  ok(!details.displayItems, "details.displayItems should be undefined.");
-  ok(!details.modifiers, "details.modifiers should be undefined.");
-  ok(!details.shippingOptions, "details.shippingOptions should be undefined.");
+  is(details.displayItems.length, 0, "details.displayItems should be a zero length array.");
+  is(details.modifiers.length, 0, "details.modifiers should be a zero length array.");
+  is(details.shippingOptions.length, 0, "details.shippingOptions should be a zero length array.");
 
   // checking the default generated PaymentOptions parameter
   const paymentOptions = aSimplePayment.paymentOptions;
   ok(!paymentOptions.requestPayerName, "payerName option should be false");
   ok(!paymentOptions.requestPayerEmail, "payerEmail option should be false");
   ok(!paymentOptions.requestPayerPhone, "payerPhone option should be false");
   ok(!paymentOptions.requestShipping, "requestShipping option should be false");
   is(paymentOptions.shippingType, "shipping", "shippingType option should be 'shipping'");
--- a/dom/payments/test/simple_payment_request.html
+++ b/dom/payments/test/simple_payment_request.html
@@ -1,47 +1,42 @@
-<!DOCTYPE html>
-<html>
-  <head>
-    <title>Payment Request Testing</title>
-    <meta content="text/html;charset=utf-8" http-equiv="Content-Type">
-    <meta content="utf-8" http-equiv="encoding">
-  </head>
-  <body>
-    <h1>simple payment request.html</h1>
-    <script type="text/javascript">
-
-    const supportedInstruments = [{
-      supportedMethods: "basic-card",
-    }];
-    const details = {
-      id: "simple details",
-      total: {
-        label: "Donation",
-        amount: { currency: "USD", value: "55.00" }
-      },
-    };
+<!doctype html>
+<meta charset="utf-8">
+<title>Payment Request Testing</title>
+<script>
+const methods = [
+  {
+    supportedMethods: "basic-card",
+  },
+];
+const details = {
+  id: "simple details",
+  total: {
+    label: "Donation",
+    amount: { currency: "USD", value: "55.00" },
+  },
+};
 
-    try {
-      const payRequest = new PaymentRequest(supportedInstruments, details);
-      window.onmessage = async (e) => {
-        if (e.data === "show PaymentRequest") {
-          if (payRequest) {
-            payRequest.show();
-            window.parent.postMessage("successful", '*');
-          } else {
-            window.parent.postMessage("PaymentRequest does not exist", "*");
-          }
-        }
-      }
+let request;
+let msg = "successful";
+try {
+  request = new PaymentRequest(methods, details);
+} catch (err) {
+  msg = err.name;
+}
+window.parent.postMessage(msg, "*");
 
-      if(window.parent) {
-        window.parent.postMessage("successful", '*');
-      }
-    } catch(err) {
-      if(window.parent) {
-        window.parent.postMessage(err.name, '*');
-      }
+if (request) {
+  window.onmessage = async ({ data: action }) => {
+    switch (action) {
+      case "show PaymentRequest":
+        const responsePromise = request.show();
+        window.parent.postMessage("successful", "*");
+        try {
+          await responsePromise;
+        } catch (err) { /* graceful abort */ }
+        break;
+      default:
+        window.parent.postMessage(`fail - unknown postmessage action: ${action}`, "*");
     }
-
-    </script>
-  </body>
-</html>
+  };
+}
+</script>