Commit 0cd018c7 authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Chromium LUCI CQ

[WebLayer] Remove pendingApps from disconnectIfNoPaymentMethodsSupported

Motivation:
This CL lays the preparatory work to create onShowCalledAndAppQueried()
which will encapsulate the following methods:
- disconnectIfNoPaymentMethodsSupported
- showAppSelector
- triggerPaymentAppUiSkipIfApplicable

Behavioural changes:
* When show() is called before app querying is finished, after the CL,
  notifyPaymentUiOfPendingApps() is triggered, and so
  mJourneyLogger.setNumberOfSuggestionsShown() get recorded, and so
  the NEEDS_COMPLETION_PAYMENT event is recorded. Before the CL,
  notifyPaymentUiOfPendingApps() was not triggered in this condition
  and setNumberOfSuggestionsShown() was not recorded.
* When show() is called after app querying is finished, the CL doesn't
  change any behaviour (i.e., setNumberOfSuggestionsShown gets
  recorded).

Note that the behavioural change is positive, because after the CL,
setNumberOfSuggestionsShown gets recorded regardless of the order of
show and app query finishing. The system has less states and thus
less complicated.

Structural changes:
* Whether the payment methods are supported by any payment apps becomes
  solely decided by BrowserPaymentRequest.hasAvailableApps(). But this
  will be temporary, because hasAvailableApps() can be further broken
  down into whether regular apps are available and whether users can
  manually type in the payment information.
* disconnectIfNoPaymentMethodsSupported() is moved together with
  showAppSelector() so they are ready to be encapsulated by
  onShowCalledAndAppQueried().

Bug: 1153353
Change-Id: I9ff13fdac6dd07c44b6c6c57d3353b4b5cca2c2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2562837
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832210}
parent 449a12f7
...@@ -346,8 +346,8 @@ public class PaymentRequestMetricsTest implements MainActivityStartCallback { ...@@ -346,8 +346,8 @@ public class PaymentRequestMetricsTest implements MainActivityStartCallback {
"PaymentRequest.TransactionAmount.Completed")); "PaymentRequest.TransactionAmount.Completed"));
// Make sure the events were logged correctly. // Make sure the events were logged correctly.
int expectedSample = int expectedSample = Event.REQUEST_SHIPPING | Event.REQUEST_METHOD_GOOGLE
Event.REQUEST_SHIPPING | Event.REQUEST_METHOD_GOOGLE | Event.COULD_NOT_SHOW; | Event.COULD_NOT_SHOW | Event.NEEDS_COMPLETION_PAYMENT;
Assert.assertEquals(1, Assert.assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
"PaymentRequest.Events", expectedSample)); "PaymentRequest.Events", expectedSample));
...@@ -386,8 +386,8 @@ public class PaymentRequestMetricsTest implements MainActivityStartCallback { ...@@ -386,8 +386,8 @@ public class PaymentRequestMetricsTest implements MainActivityStartCallback {
"PaymentRequest.TransactionAmount.Completed")); "PaymentRequest.TransactionAmount.Completed"));
// Make sure the events were logged correctly. // Make sure the events were logged correctly.
int expectedSample = int expectedSample = Event.REQUEST_SHIPPING | Event.REQUEST_METHOD_OTHER
Event.REQUEST_SHIPPING | Event.REQUEST_METHOD_OTHER | Event.COULD_NOT_SHOW; | Event.COULD_NOT_SHOW | Event.NEEDS_COMPLETION_PAYMENT;
Assert.assertEquals(1, Assert.assertEquals(1,
RecordHistogram.getHistogramValueCountForTesting( RecordHistogram.getHistogramValueCountForTesting(
"PaymentRequest.Events", expectedSample)); "PaymentRequest.Events", expectedSample));
......
...@@ -746,11 +746,10 @@ public class PaymentRequestService ...@@ -746,11 +746,10 @@ public class PaymentRequestService
// Always return false when can make payment is disabled. // Always return false when can make payment is disabled.
mHasEnrolledInstrument &= mDelegate.prefsCanMakePayment(); mHasEnrolledInstrument &= mDelegate.prefsCanMakePayment();
if (mIsShowCalled && disconnectIfNoPaymentMethodsSupported()) return;
mBrowserPaymentRequest.notifyPaymentUiOfPendingApps(mPendingApps); mBrowserPaymentRequest.notifyPaymentUiOfPendingApps(mPendingApps);
mPendingApps.clear(); mPendingApps.clear();
if (mIsShowCalled) { if (mIsShowCalled) {
if (disconnectIfNoPaymentMethodsSupported()) return;
String error = mBrowserPaymentRequest.showAppSelector(mIsShowWaitingForUpdatedDetails, String error = mBrowserPaymentRequest.showAppSelector(mIsShowWaitingForUpdatedDetails,
mSpec.getRawTotal(), mSpec.getPaymentOptions(), mIsUserGestureShow); mSpec.getRawTotal(), mSpec.getPaymentOptions(), mIsUserGestureShow);
if (error != null) { if (error != null) {
...@@ -791,8 +790,9 @@ public class PaymentRequestService ...@@ -791,8 +790,9 @@ public class PaymentRequestService
* @return Whether client has been disconnected. * @return Whether client has been disconnected.
*/ */
private boolean disconnectIfNoPaymentMethodsSupported() { private boolean disconnectIfNoPaymentMethodsSupported() {
if (!mCanMakePayment assert mIsShowCalled;
|| (mPendingApps.isEmpty() && !mBrowserPaymentRequest.hasAvailableApps())) { assert mIsFinishedQueryingPaymentApps;
if (!mCanMakePayment || !mBrowserPaymentRequest.hasAvailableApps()) {
// All factories have responded, but none of them have apps. It's possible to add credit // All factories have responded, but none of them have apps. It's possible to add credit
// cards, but the merchant does not support them either. The payment request must be // cards, but the merchant does not support them either. The payment request must be
// rejected. // rejected.
......
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