Commit 530f25db authored by Sahel Sharify's avatar Sahel Sharify Committed by Commit Bot

[Payment] changePaymentMethod should work when details is empty or null

According to PH w3 spec the "details" input of the changePaymentMethod
should be optional[1]. Even though payment_request_event.idl honors the
spec[2],the blink implementation and the mojom file treat "details"
as a mandatory field. This cl fixes the issue.


Test coverage:
I reproduced the bug by removing the "details" field from
changePaymentMethod call[3]; The following two tests pass only after
applying the fix in this cl and fail without it:
MerchantResponse/PaymentHandlerChangePaymentMethodTest.Test/0
MerchantResponse/PaymentHandlerChangePaymentMethodTest.Test/1

[1]https://w3c.github.io/payment-handler/#the-paymentrequestevent
[2]https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/payments/payment_request_event.idl?l=23
[3]https://chromium-review.googlesource.com/c/chromium/src/+/2254084/2/components/test/data/payments/change_payment_method_app.js#32

Bug: 1083193
Change-Id: Ia479401d021be2d110a7a7ad4fe652d2eae4d88c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2254084
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarLiquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781380}
parent b5afe61e
...@@ -124,7 +124,7 @@ void PaymentHandlerHost::UpdateWith( ...@@ -124,7 +124,7 @@ void PaymentHandlerHost::UpdateWith(
data.emplace(prefix + " Method Name", data.emplace(prefix + " Method Name",
modifier->method_data->method_name); modifier->method_data->method_name);
data.emplace(prefix + " Method Data", data.emplace(prefix + " Method Data",
modifier->method_data->stringified_data); modifier->method_data->stringified_data.value_or("{}"));
if (!modifier->total) if (!modifier->total)
continue; continue;
data.emplace(prefix + " Total Currency", modifier->total->currency); data.emplace(prefix + " Total Currency", modifier->total->currency);
...@@ -187,8 +187,10 @@ void PaymentHandlerHost::ChangePaymentMethod( ...@@ -187,8 +187,10 @@ void PaymentHandlerHost::ChangePaymentMethod(
return; return;
} }
const std::string stringified_data =
method_data->stringified_data.value_or("{}");
if (!delegate_->ChangePaymentMethod(method_data->method_name, if (!delegate_->ChangePaymentMethod(method_data->method_name,
method_data->stringified_data)) { stringified_data)) {
RunCallbackWithError(errors::kInvalidState, std::move(callback)); RunCallbackWithError(errors::kInvalidState, std::move(callback));
return; return;
} }
...@@ -202,7 +204,7 @@ void PaymentHandlerHost::ChangePaymentMethod( ...@@ -202,7 +204,7 @@ void PaymentHandlerHost::ChangePaymentMethod(
"Change payment method", "Change payment method",
/*instance_id=*/payment_request_id_for_logs_, /*instance_id=*/payment_request_id_for_logs_,
{{"Method Name", method_data->method_name}, {{"Method Name", method_data->method_name},
{"Method Data", method_data->stringified_data}}); {"Method Data", stringified_data}});
} }
change_payment_request_details_callback_ = std::move(callback); change_payment_request_details_callback_ = std::move(callback);
......
...@@ -16,7 +16,7 @@ self.addEventListener('canmakepayment', (event) => { ...@@ -16,7 +16,7 @@ self.addEventListener('canmakepayment', (event) => {
* @param {PaymentRequestEvent} event - The event to respond. * @param {PaymentRequestEvent} event - The event to respond.
* @return {PamentDetailsUpdate} - The update to the payment details. * @return {PamentDetailsUpdate} - The update to the payment details.
*/ */
async function responder(event) { async function responder(event) { // eslint-disable-line no-unused-vars
const methodName = event.methodData[0].supportedMethods; const methodName = event.methodData[0].supportedMethods;
if (!event.changePaymentMethod) { if (!event.changePaymentMethod) {
return { return {
...@@ -29,9 +29,7 @@ async function responder(event) { ...@@ -29,9 +29,7 @@ async function responder(event) {
} }
let changePaymentMethodReturned; let changePaymentMethodReturned;
try { try {
const response = await event.changePaymentMethod(methodName, { const response = await event.changePaymentMethod(methodName);
country: 'US',
});
changePaymentMethodReturned = response; changePaymentMethodReturned = response;
} catch (error) { } catch (error) {
changePaymentMethodReturned = error.message; changePaymentMethodReturned = error.message;
......
...@@ -20,7 +20,7 @@ struct PaymentHandlerMethodData { ...@@ -20,7 +20,7 @@ struct PaymentHandlerMethodData {
// object, so more specific types cannot be used. A simple example: // object, so more specific types cannot be used. A simple example:
// //
// {"supportedNetworks": ["visa"]} // {"supportedNetworks": ["visa"]}
string stringified_data; string? stringified_data;
}; };
struct PaymentHandlerModifier { struct PaymentHandlerModifier {
......
...@@ -185,14 +185,6 @@ ScriptPromise PaymentRequestEvent::openWindow(ScriptState* script_state, ...@@ -185,14 +185,6 @@ ScriptPromise PaymentRequestEvent::openWindow(ScriptState* script_state,
return promise; return promise;
} }
ScriptPromise PaymentRequestEvent::changePaymentMethod(
ScriptState* script_state,
const String& method_name,
ExceptionState& exception_state) {
return changePaymentMethod(script_state, method_name, ScriptValue(),
exception_state);
}
ScriptPromise PaymentRequestEvent::changePaymentMethod( ScriptPromise PaymentRequestEvent::changePaymentMethod(
ScriptState* script_state, ScriptState* script_state,
const String& method_name, const String& method_name,
...@@ -213,7 +205,8 @@ ScriptPromise PaymentRequestEvent::changePaymentMethod( ...@@ -213,7 +205,8 @@ ScriptPromise PaymentRequestEvent::changePaymentMethod(
} }
auto method_data = payments::mojom::blink::PaymentHandlerMethodData::New(); auto method_data = payments::mojom::blink::PaymentHandlerMethodData::New();
if (!method_details.IsEmpty()) { if (!method_details.IsNull()) {
DCHECK(!method_details.IsEmpty());
PaymentsValidators::ValidateAndStringifyObject( PaymentsValidators::ValidateAndStringifyObject(
script_state->GetIsolate(), "Method details", method_details, script_state->GetIsolate(), "Method details", method_details,
method_data->stringified_data, exception_state); method_data->stringified_data, exception_state);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment