PaymentRequest: Make modifiers member optional
Before this patch, when calling updateWith() w/o modifiers member in
PaymentRequestUpdateEvent, it will reset modifiers(total,
additionalDisplayItems and so on) in PaymentRequest UI. This is a
implementation bug and doesn't match behavior with the spec[1].
So, this patch makes the modifiers member optional when updateWith()
called.
[1] https://w3c.github.io/payment-request/#update-a-paymentrequest-s-details-algorithm
Bug: 902291
Change-Id: Ia8b55234e0416d86b2f12452ac0f914f877434c8
Reviewed-on: https://chromium-review.googlesource.com/c/1477612
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634119}diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestUpdateWithTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestUpdateWithTest.java
index 69f5896..b11962b 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestUpdateWithTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestUpdateWithTest.java
@@ -64,12 +64,11 @@
Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
- mRule.clickInShippingAddressAndWait(R.id.payments_section, mRule.getReadyToPay());
- mRule.clickOnShippingAddressSuggestionOptionAndWait(1, mRule.getReadyToPay());
- mRule.clickInOrderSummaryAndWait(mRule.getReadyToPay());
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
mRule.clickAndWait(R.id.button_primary, mRule.getReadyForUnmaskInput());
mRule.setTextInCardUnmaskDialogAndWait(
R.id.card_unmask_input, "123", mRule.getReadyToUnmask());
@@ -88,12 +87,14 @@
Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
mRule.clickInShippingAddressAndWait(R.id.payments_section, mRule.getReadyToPay());
mRule.clickOnShippingAddressSuggestionOptionAndWait(1, mRule.getReadyToPay());
mRule.clickInOrderSummaryAndWait(mRule.getReadyToPay());
Assert.assertEquals("USD $10.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
mRule.clickAndWait(R.id.button_primary, mRule.getReadyForUnmaskInput());
mRule.setTextInCardUnmaskDialogAndWait(
R.id.card_unmask_input, "123", mRule.getReadyToUnmask());
@@ -112,12 +113,14 @@
Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
mRule.clickInShippingAddressAndWait(R.id.payments_section, mRule.getReadyToPay());
mRule.clickOnShippingAddressSuggestionOptionAndWait(1, mRule.getReadyToPay());
mRule.clickInOrderSummaryAndWait(mRule.getReadyToPay());
Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$3.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$2.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
mRule.clickAndWait(R.id.button_primary, mRule.getReadyForUnmaskInput());
mRule.setTextInCardUnmaskDialogAndWait(
R.id.card_unmask_input, "123", mRule.getReadyToUnmask());
@@ -136,12 +139,14 @@
Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
mRule.clickInShippingAddressAndWait(R.id.payments_section, mRule.getReadyToPay());
mRule.clickOnShippingAddressSuggestionOptionAndWait(1, mRule.getReadyToPay());
mRule.clickInOrderSummaryAndWait(mRule.getReadyToPay());
Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
mRule.clickAndWait(R.id.button_primary, mRule.getReadyForUnmaskInput());
mRule.setTextInCardUnmaskDialogAndWait(
R.id.card_unmask_input, "123", mRule.getReadyToUnmask());
@@ -149,4 +154,30 @@
ModalDialogProperties.ButtonType.POSITIVE, mRule.getDismissed());
mRule.expectResultContains(new String[] {"updatedShipping"});
}
+
+ /** A merchant that calls updateWith() with modifiers will not cause timeouts in UI. */
+ @Test
+ @MediumTest
+ @Feature({"Payments"})
+ public void testUpdateWithModifiers() throws Throwable {
+ mRule.triggerUIAndWait("updateWithModifiers", mRule.getReadyToPay());
+ mRule.clickInOrderSummaryAndWait(mRule.getReadyToPay());
+ Assert.assertEquals("USD $5.00", mRule.getOrderSummaryTotal());
+ Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
+ Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("$0.00", mRule.getLineItemAmount(2));
+ mRule.clickInShippingAddressAndWait(R.id.payments_section, mRule.getReadyToPay());
+ mRule.clickOnShippingAddressSuggestionOptionAndWait(1, mRule.getReadyToPay());
+ mRule.clickInOrderSummaryAndWait(mRule.getReadyToPay());
+ Assert.assertEquals("USD $4.00", mRule.getOrderSummaryTotal());
+ Assert.assertEquals("$2.00", mRule.getLineItemAmount(0));
+ Assert.assertEquals("$3.00", mRule.getLineItemAmount(1));
+ Assert.assertEquals("-$1.00", mRule.getLineItemAmount(2));
+ mRule.clickAndWait(R.id.button_primary, mRule.getReadyForUnmaskInput());
+ mRule.setTextInCardUnmaskDialogAndWait(
+ R.id.card_unmask_input, "123", mRule.getReadyToUnmask());
+ mRule.clickCardUnmaskButtonAndWait(
+ ModalDialogProperties.ButtonType.POSITIVE, mRule.getDismissed());
+ mRule.expectResultContains(new String[] {"freeShipping"});
+ }
}
diff --git a/chrome/browser/ui/views/payments/payment_request_update_with_browsertest.cc b/chrome/browser/ui/views/payments/payment_request_update_with_browsertest.cc
index b3ad958..e636197b 100644
--- a/chrome/browser/ui/views/payments/payment_request_update_with_browsertest.cc
+++ b/chrome/browser/ui/views/payments/payment_request_update_with_browsertest.cc
@@ -48,6 +48,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
OpenShippingAddressSectionScreen();
@@ -72,6 +74,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
PayWithCreditCardAndWait(base::ASCIIToUTF16("123"));
@@ -97,6 +101,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
OpenShippingAddressSectionScreen();
@@ -121,6 +127,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
PayWithCreditCardAndWait(base::ASCIIToUTF16("123"));
@@ -146,6 +154,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
OpenShippingAddressSectionScreen();
@@ -170,6 +180,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$2.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
PayWithCreditCardAndWait(base::ASCIIToUTF16("123"));
@@ -196,6 +208,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
OpenShippingAddressSectionScreen();
@@ -220,6 +234,8 @@
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
ClickOnBackArrow();
PayWithCreditCardAndWait(base::ASCIIToUTF16("123"));
@@ -227,4 +243,57 @@
ExpectBodyContains({"updatedShipping"});
}
+IN_PROC_BROWSER_TEST_F(PaymentRequestUpdateWithTest, UpdateWithModifiers) {
+ NavigateTo("/payment_request_update_with_test.html");
+ autofill::AutofillProfile billing_address = autofill::test::GetFullProfile();
+ AddAutofillProfile(billing_address);
+ AddAutofillProfile(autofill::test::GetFullProfile2());
+ autofill::CreditCard card = autofill::test::GetCreditCard();
+ card.set_billing_address_id(billing_address.guid());
+ AddCreditCard(card);
+
+ RunJavaScriptFunctionToOpenPaymentRequestUI("updateWithModifiers");
+
+ OpenOrderSummaryScreen();
+ EXPECT_EQ(base::ASCIIToUTF16("$5.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_TOTAL_AMOUNT_LABEL));
+ EXPECT_EQ(base::ASCIIToUTF16("$2.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
+ EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("$0.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
+ ClickOnBackArrow();
+
+ OpenShippingAddressSectionScreen();
+ ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+ DialogEvent::PROCESSING_SPINNER_HIDDEN,
+ DialogEvent::SPEC_DONE_UPDATING,
+ DialogEvent::BACK_NAVIGATION});
+ ClickOnChildInListViewAndWait(
+ /* child_index=*/1, /*total_num_children=*/2,
+ DialogViewID::SHIPPING_ADDRESS_SHEET_LIST_VIEW,
+ /*wait_for_animation=*/false);
+ // Wait for the animation here explicitly, otherwise
+ // ClickOnChildInListViewAndWait tries to install an AnimationDelegate before
+ // the animation is kicked off (since that's triggered off of the spec being
+ // updated) and this hits a DCHECK.
+ WaitForAnimation();
+
+ OpenOrderSummaryScreen();
+ EXPECT_EQ(base::ASCIIToUTF16("$4.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_TOTAL_AMOUNT_LABEL));
+ EXPECT_EQ(base::ASCIIToUTF16("$2.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_1));
+ EXPECT_EQ(base::ASCIIToUTF16("$3.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_2));
+ EXPECT_EQ(base::ASCIIToUTF16("-$1.00"),
+ GetLabelText(DialogViewID::ORDER_SUMMARY_LINE_ITEM_3));
+ ClickOnBackArrow();
+
+ PayWithCreditCardAndWait(base::ASCIIToUTF16("123"));
+
+ ExpectBodyContains({"freeShipping"});
+}
+
} // namespace payments
diff --git a/components/payments/content/payment_request_spec.cc b/components/payments/content/payment_request_spec.cc
index 7a6a1529..c8dee60 100644
--- a/components/payments/content/payment_request_spec.cc
+++ b/components/payments/content/payment_request_spec.cc
@@ -106,7 +106,8 @@
details_->display_items = std::move(details->display_items);
if (details->shipping_options)
details_->shipping_options = std::move(details->shipping_options);
- details_->modifiers = std::move(details->modifiers);
+ if (!details->modifiers.empty())
+ details_->modifiers = std::move(details->modifiers);
details_->error = std::move(details->error);
if (details->shipping_address_errors)
details_->shipping_address_errors =
diff --git a/components/test/data/payments/payment_request_update_with_test.html b/components/test/data/payments/payment_request_update_with_test.html
index 87fa0e6d..c30f3ab 100644
--- a/components/test/data/payments/payment_request_update_with_test.html
+++ b/components/test/data/payments/payment_request_update_with_test.html
@@ -16,6 +16,7 @@
<button class="small" onclick="updateWithTotal()" id="updateWithTotal">updateWithTotal</button>
<button class="small" onclick="updateWithDisplayItems()" id="updateWithDisplayItems">updateWithDisplayItems</button>
<button class="small" onclick="updateWithShippingOptions()" id="updateWithShippingOptions">updateWithShippingOptions</button>
+<button class="small" onclick="updateWithModifiers()" id="updateWithModifiers">updateWithModifiers</button>
<pre id="result"></pre>
<script src="util.js"></script>
<script src="update_with.js"></script>
diff --git a/components/test/data/payments/update_with.js b/components/test/data/payments/update_with.js
index 93f810b..9791173 100644
--- a/components/test/data/payments/update_with.js
+++ b/components/test/data/payments/update_with.js
@@ -23,6 +23,13 @@
label: 'Free shipping',
amount: {currency: 'USD', value: '0.00'},
}],
+ modifiers: [{
+ supportedMethods: 'basic-card',
+ additionalDisplayItems: [{
+ label: 'Discount',
+ amount: {currency: 'USD', value: '0.00'},
+ }],
+ }],
},
{requestShipping: true});
} catch (error) {
@@ -123,3 +130,30 @@
});
showPaymentRequest(pr);
}
+
+/**
+ * Calls updateWith() with modifiers
+ */
+function updateWithModifiers() { // eslint-disable-line no-unused-vars
+ var pr = buildPaymentRequest();
+ var updatedDetails = {
+ modifiers: [{
+ supportedMethods: 'basic-card',
+ total: {
+ label: 'Modifier total',
+ amount: {currency: 'USD', value: '4.00'},
+ },
+ additionalDisplayItems: [{
+ label: 'Discount',
+ amount: {currency: 'USD', value: '-1.00'},
+ }],
+ }],
+ };
+ pr.addEventListener('shippingaddresschange', function(e) {
+ e.updateWith(updatedDetails);
+ });
+ pr.addEventListener('shippingoptionchange', function(e) {
+ e.updateWith(updatedDetails);
+ });
+ showPaymentRequest(pr);
+}
