Commit d77ef780 authored by Danyao Wang's avatar Danyao Wang Committed by Commit Bot

[Payment Request][Android] Fix strict hasEnrolledInstrument checking.

Without this patch, an incomplete autofill instrument is still offered
to the user even when kStrictHasEnrolledAutofillInstrument is enabled.
This is due a bug in PaymentRequestImpl.show() that checks the strict
criteria before the correct value of |mIsUserGestureShow| is updated.

This patch moves |mIsUserGestureShow| update to earlier in the call
flow to fix the bug. A browsertest is added to guard against regression.

Bug: 1028114
Change-Id: Id0f89e8d8f7180cc3a85c0f30ee8e05a1d849042
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1934510
Commit-Queue: Danyao Wang <danyao@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718824}
parent babfdaf7
...@@ -914,6 +914,9 @@ public class PaymentRequestImpl ...@@ -914,6 +914,9 @@ public class PaymentRequestImpl
setShowingPaymentRequest(this); setShowingPaymentRequest(this);
mIsCurrentPaymentRequestShowing = true; mIsCurrentPaymentRequestShowing = true;
mIsUserGestureShow = isUserGesture;
mWaitForUpdatedDetails = waitForUpdatedDetails;
mJourneyLogger.setTriggerTime(); mJourneyLogger.setTriggerTime();
if (disconnectIfNoPaymentMethodsSupported()) return; if (disconnectIfNoPaymentMethodsSupported()) return;
...@@ -925,9 +928,6 @@ public class PaymentRequestImpl ...@@ -925,9 +928,6 @@ public class PaymentRequestImpl
return; return;
} }
mIsUserGestureShow = isUserGesture;
mWaitForUpdatedDetails = waitForUpdatedDetails;
if (isFinishedQueryingPaymentApps()) { if (isFinishedQueryingPaymentApps()) {
// Calculate skip ui and build ui only after all payment instruments are ready and // Calculate skip ui and build ui only after all payment instruments are ready and
// request.show() is called. // request.show() is called.
......
...@@ -113,6 +113,7 @@ class HasEnrolledInstrumentTest ...@@ -113,6 +113,7 @@ class HasEnrolledInstrumentTest
"PaymentRequest.CheckoutFunnel.NoShow", "PaymentRequest.CheckoutFunnel.NoShow",
JourneyLogger::NOT_SHOWN_REASON_NO_SUPPORTED_PAYMENT_METHOD); JourneyLogger::NOT_SHOWN_REASON_NO_SUPPORTED_PAYMENT_METHOD);
// Check code path where show() is called before instruments are ready.
EXPECT_EQ(not_supported_message(), EXPECT_EQ(not_supported_message(),
content::EvalJs(GetActiveWebContents(), "show()")); content::EvalJs(GetActiveWebContents(), "show()"));
// TODO(crbug.com/1027322): Fix NoShow logging on Android. // TODO(crbug.com/1027322): Fix NoShow logging on Android.
...@@ -129,6 +130,16 @@ class HasEnrolledInstrumentTest ...@@ -129,6 +130,16 @@ class HasEnrolledInstrumentTest
EXPECT_EQ(not_supported_message(), EXPECT_EQ(not_supported_message(),
content::EvalJs(GetActiveWebContents(), content::EvalJs(GetActiveWebContents(),
"show({requestPayerEmail:true})")); "show({requestPayerEmail:true})"));
// Check code path where show() is called after instruments are ready.
EXPECT_EQ(not_supported_message(),
content::EvalJs(GetActiveWebContents(), "delayedShow()"));
EXPECT_EQ(not_supported_message(),
content::EvalJs(GetActiveWebContents(),
"delayedShow({requestShipping:true})"));
EXPECT_EQ(not_supported_message(),
content::EvalJs(GetActiveWebContents(),
"delayedShow({requestPayerEmail:true})"));
} }
} }
......
...@@ -44,3 +44,24 @@ async function show(options) { // eslint-disable-line no-unused-vars ...@@ -44,3 +44,24 @@ async function show(options) { // eslint-disable-line no-unused-vars
return e.toString(); return e.toString();
} }
} }
/**
* A version of show(options) that inserts a delay between creating the request
* and calling request.show().
* This is a regression test for https://crbug.com/1028114.
* @param {PaymentOptions} options - The payment options to use.
* @return {Promise<string>} The error message string, if any.
*/
async function delayedShow(options) { // eslint-disable-line no-unused-vars
let request = buildPaymentRequest(options);
try {
// Block on hasEnrolledInstrument() to make sure when show() is called,
// all instruments are available.
await request.hasEnrolledInstrument();
await request.show();
return '';
} catch (e) {
return e.toString();
}
}
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