Commit 40310ba0 authored by Rouslan Solomakhin's avatar Rouslan Solomakhin Committed by Commit Bot

[Web Payment] Improved error message for absence of instruments.

Before this patch, invoking PaymentRequest.show() for secure payment
confirmation on a device without credentials would reject with "User
closed PaymentRequest UI" error message instead of the more correct
"Payment method not supported", if show() was called after
PaymentRequest.canMakePayment() returned. This is detrimental to
measuring the success of the secure payment confirmation dialog.

This patch changes the secure payment confirmation UI controller to not
invoke PaymentRequest::UserCancelled() when PaymentRequestState has
finished looking for credentials and found none. This allows
payment_request.cc to correctly the no-apps state by rejecting
PaymentRequest.show() with the correct error message.

After this patch, a device without credentials always rejects
PaymentRequest.show() with "Payment method not supported" without
implying that the user interacted with the UI dialog and cancelled it.

Bug: 1123082
Change-Id: Idba82354f413c926c1f835ec34e50d3c29080bb5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2383812
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: default avatarNick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802955}
parent 08a45ff6
......@@ -134,9 +134,13 @@ IN_PROC_BROWSER_TEST_F(SecurePaymentConfirmationTest, NoAuthenticator) {
// TODO(https://crbug.com/1110320): Implement SetHasAuthenticator() for Android,
// so this behavior can be tested on Android as well.
#define MAYBE_NoInstrumentInStorage DISABLED_NoInstrumentInStorage
#define MAYBE_CheckInstrumentInStorageAfterCanMakePayment \
DISABLED_CheckInstrumentInStorageAfterCanMakePayment
#define MAYBE_PaymentSheetShowsApp DISABLED_PaymentSheetShowsApp
#else
#define MAYBE_NoInstrumentInStorage NoInstrumentInStorage
#define MAYBE_CheckInstrumentInStorageAfterCanMakePayment \
CheckInstrumentInStorageAfterCanMakePayment
#define MAYBE_PaymentSheetShowsApp PaymentSheetShowsApp
#endif // OS_ANDROID
......@@ -152,6 +156,21 @@ IN_PROC_BROWSER_TEST_F(SecurePaymentConfirmationTest,
getInvokePaymentRequestSnippet()));
}
IN_PROC_BROWSER_TEST_F(SecurePaymentConfirmationTest,
MAYBE_CheckInstrumentInStorageAfterCanMakePayment) {
test_controller()->SetHasAuthenticator(true);
NavigateTo("a.com", "/payment_handler_status.html");
// EvalJs waits for JavaScript promise to resolve.
EXPECT_EQ(
"The payment method \"secure-payment-confirmation\" is not supported.",
content::EvalJs(
GetActiveWebContents(),
base::StringPrintf("getStatusForMethodDataAfterCanMakePayment(%s, "
"/*checkCanMakePaymentFirst=*/true)",
kTestMethodData)));
}
IN_PROC_BROWSER_TEST_F(SecurePaymentConfirmationTest,
MAYBE_PaymentSheetShowsApp) {
test_controller()->SetHasAuthenticator(true);
......
......@@ -54,8 +54,15 @@ void SecurePaymentConfirmationController::ShowDialog() {
void SecurePaymentConfirmationController::
SetupModelAndShowDialogIfApplicable() {
DCHECK(!view_);
if (!request_ || !request_->web_contents() || !request_->state() ||
!request_->state()->selected_app() ||
// If no apps are available then don't show any UI. The payment_request.cc
// code will reject the PaymentRequest.show() call with appropriate error
// message on its own.
if (!request_ || !request_->state() ||
request_->state()->available_apps().empty()) {
return;
}
if (!request_->web_contents() || !request_->state()->selected_app() ||
request_->state()->selected_app()->type() != PaymentApp::Type::INTERNAL ||
request_->state()->selected_app()->GetAppMethodNames().size() != 1 ||
*request_->state()->selected_app()->GetAppMethodNames().begin() !=
......
......@@ -35,10 +35,27 @@ async function getStatusList(methods) { // eslint-disable-line no-unused-vars
* @return {string} - The status field or error message.
*/
async function getStatusForMethodData(methodData) {
return getStatusForMethodDataAfterCanMakePayment(methodData, false);
}
/**
* Returns the status field from the payment handler's response for given
* payment method data.
* @param {array<PaymentMethodData>} methodData - The method data to use.
* @param {bool} checkCanMakePaymentFirst - Whether to wait for canMakePayment()
* to complete before invoking show(). The return value of canMakePayment() is
* ignored.
* @return {string} - The status field or error message.
*/
async function getStatusForMethodDataAfterCanMakePayment(
methodData, checkCanMakePaymentFirst) {
try {
const request = new PaymentRequest(
methodData,
{total: {label: 'TEST', amount: {currency: 'USD', value: '0.01'}}});
if (checkCanMakePaymentFirst) {
await request.canMakePayment(); // Ignore the result.
}
const response = await request.show();
await response.complete();
if (!response.details.status) {
......
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