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

[Payments] Clean up for mJourneyLogger.recordTransactionAmount

Context:
Now that mJourneyLogger.recordTransactionAmount() no longer needs to
be recorded with Event.SHOWN, the code structures that supported that
can be removed, which includes:
* mDidRecordShowEvent: used to guarantee that Event.SHOWN was recorded
  only once so recordTransactionAmount() was invoked once. Since
  JourneyLogger is designed in a way that Event.SHOWN can be recorded
  multiple times, this structure actually applied for
  recordTransactionAmount() only.
* enableAndUpdatePaymentRequestUIWithPaymentInfo()'s return: this was
  introduced to call recordTransactionAmount()
  after providePaymentInformationToPaymentRequestUI(). Since all
  providePaymentInformationToPaymentRequestUI() happened with
  recordShowEventAndTransactionAmount(), this CL combines the two
  methods.

Changes:
* Removed mDidRecordShowEvent
* Inlined recordShowEventAndTransactionAmount() and merge it to
  providePaymentInformationToPaymentRequestUI().
* Makes enableAndUpdatePaymentRequestUIWithPaymentInfo return void

Bug: 1149936
Change-Id: I5097d92e6d4eeda0e582a19a9312a8d54ed68eab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580589
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835680}
parent 746a2e20
...@@ -85,12 +85,6 @@ public class ChromePaymentRequestService ...@@ -85,12 +85,6 @@ public class ChromePaymentRequestService
private boolean mHideServerAutofillCards; private boolean mHideServerAutofillCards;
private PaymentHandlerHost mPaymentHandlerHost; private PaymentHandlerHost mPaymentHandlerHost;
/**
* There are a few situations were the Payment Request can appear, from a code perspective, to
* be shown more than once. This boolean is used to make sure it is only logged once.
*/
private boolean mDidRecordShowEvent;
/** A helper to manage the Skip-to-GPay experimental flow. */ /** A helper to manage the Skip-to-GPay experimental flow. */
private SkipToGPayHelper mSkipToGPayHelper; private SkipToGPayHelper mSkipToGPayHelper;
private boolean mIsGooglePayBridgeActivated; private boolean mIsGooglePayBridgeActivated;
...@@ -296,7 +290,6 @@ public class ChromePaymentRequestService ...@@ -296,7 +290,6 @@ public class ChromePaymentRequestService
() ()
-> onUiAborted(AbortReason.ABORTED_BY_USER, -> onUiAborted(AbortReason.ABORTED_BY_USER,
ErrorStrings.USER_CANCELLED))) { ErrorStrings.USER_CANCELLED))) {
mDidRecordShowEvent = true;
mJourneyLogger.setEventOccurred(Event.SHOWN); mJourneyLogger.setEventOccurred(Event.SHOWN);
return null; return null;
} else { } else {
...@@ -307,7 +300,6 @@ public class ChromePaymentRequestService ...@@ -307,7 +300,6 @@ public class ChromePaymentRequestService
assert !mPaymentUiService.getPaymentApps().isEmpty(); assert !mPaymentUiService.getPaymentApps().isEmpty();
PaymentApp selectedApp = mPaymentUiService.getSelectedPaymentApp(); PaymentApp selectedApp = mPaymentUiService.getSelectedPaymentApp();
dimBackgroundIfNotBottomSheetPaymentHandler(selectedApp); dimBackgroundIfNotBottomSheetPaymentHandler(selectedApp);
mDidRecordShowEvent = true;
mJourneyLogger.setEventOccurred(Event.SKIPPED_SHOW); mJourneyLogger.setEventOccurred(Event.SKIPPED_SHOW);
invokePaymentApp(null /* selectedShippingAddress */, null /* selectedShippingOption */, invokePaymentApp(null /* selectedShippingAddress */, null /* selectedShippingOption */,
selectedApp); selectedApp);
...@@ -398,10 +390,7 @@ public class ChromePaymentRequestService ...@@ -398,10 +390,7 @@ public class ChromePaymentRequestService
String detailsError = mSpec.getPaymentDetails().error; String detailsError = mSpec.getPaymentDetails().error;
mPaymentUiService.showShippingAddressErrorIfApplicable(detailsError); mPaymentUiService.showShippingAddressErrorIfApplicable(detailsError);
boolean providedInformationToPaymentRequestUI =
mPaymentUiService.enableAndUpdatePaymentRequestUIWithPaymentInfo(); mPaymentUiService.enableAndUpdatePaymentRequestUIWithPaymentInfo();
if (providedInformationToPaymentRequestUI) recordShowEventAndTransactionAmount();
} }
// Implements BrowserPaymentRequest: // Implements BrowserPaymentRequest:
...@@ -413,9 +402,7 @@ public class ChromePaymentRequestService ...@@ -413,9 +402,7 @@ public class ChromePaymentRequestService
mPaymentUiService.updateDetailsOnPaymentRequestUI(mSpec.getPaymentDetails()); mPaymentUiService.updateDetailsOnPaymentRequestUI(mSpec.getPaymentDetails());
if (isFinishedQueryingPaymentApps && !mHasSkippedAppSelector) { if (isFinishedQueryingPaymentApps && !mHasSkippedAppSelector) {
boolean providedInformationToPaymentRequestUI =
mPaymentUiService.enableAndUpdatePaymentRequestUIWithPaymentInfo(); mPaymentUiService.enableAndUpdatePaymentRequestUIWithPaymentInfo();
if (providedInformationToPaymentRequestUI) recordShowEventAndTransactionAmount();
} }
return null; return null;
} }
...@@ -424,10 +411,7 @@ public class ChromePaymentRequestService ...@@ -424,10 +411,7 @@ public class ChromePaymentRequestService
@Override @Override
public void onPaymentDetailsNotUpdated(@Nullable String selectedShippingOptionError) { public void onPaymentDetailsNotUpdated(@Nullable String selectedShippingOptionError) {
mPaymentUiService.showShippingAddressErrorIfApplicable(selectedShippingOptionError); mPaymentUiService.showShippingAddressErrorIfApplicable(selectedShippingOptionError);
boolean providedInformationToPaymentRequestUI =
mPaymentUiService.enableAndUpdatePaymentRequestUIWithPaymentInfo(); mPaymentUiService.enableAndUpdatePaymentRequestUIWithPaymentInfo();
if (providedInformationToPaymentRequestUI) recordShowEventAndTransactionAmount();
} }
// Implements BrowserPaymentRequest: // Implements BrowserPaymentRequest:
...@@ -436,14 +420,6 @@ public class ChromePaymentRequestService ...@@ -436,14 +420,6 @@ public class ChromePaymentRequestService
return mSkipToGPayHelper == null || mSkipToGPayHelper.setShippingOptionIfValid(details); return mSkipToGPayHelper == null || mSkipToGPayHelper.setShippingOptionIfValid(details);
} }
// Implements PaymentUiService.Delegate:
@Override
public void recordShowEventAndTransactionAmount() {
if (mDidRecordShowEvent) return;
mDidRecordShowEvent = true;
mJourneyLogger.setEventOccurred(Event.SHOWN);
}
// Implements BrowserPaymentRequest: // Implements BrowserPaymentRequest:
@Override @Override
public void onInstrumentDetailsLoading() { public void onInstrumentDetailsLoading() {
......
...@@ -59,6 +59,7 @@ import org.chromium.components.payments.AbortReason; ...@@ -59,6 +59,7 @@ import org.chromium.components.payments.AbortReason;
import org.chromium.components.payments.BasicCardUtils; import org.chromium.components.payments.BasicCardUtils;
import org.chromium.components.payments.CurrencyFormatter; import org.chromium.components.payments.CurrencyFormatter;
import org.chromium.components.payments.ErrorStrings; import org.chromium.components.payments.ErrorStrings;
import org.chromium.components.payments.Event;
import org.chromium.components.payments.JourneyLogger; import org.chromium.components.payments.JourneyLogger;
import org.chromium.components.payments.PaymentApp; import org.chromium.components.payments.PaymentApp;
import org.chromium.components.payments.PaymentAppType; import org.chromium.components.payments.PaymentAppType;
...@@ -156,8 +157,7 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs ...@@ -156,8 +157,7 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs
public interface Delegate { public interface Delegate {
/** Dispatch the payer detail change event if needed. */ /** Dispatch the payer detail change event if needed. */
void dispatchPayerDetailChangeEventIfNeeded(PayerDetail detail); void dispatchPayerDetailChangeEventIfNeeded(PayerDetail detail);
/** Record the show event to the journey logger and record the transaction amount. */
void recordShowEventAndTransactionAmount();
/** /**
* @return Whether {@link ChromePaymentRequestService#onRetry} has been * @return Whether {@link ChromePaymentRequestService#onRetry} has been
* called. * called.
...@@ -921,14 +921,10 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs ...@@ -921,14 +921,10 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs
* Update Payment Request UI with the update event's information and enable the UI. This method * Update Payment Request UI with the update event's information and enable the UI. This method
* should be called when the user interface is disabled with a "↻" spinner being displayed. The * should be called when the user interface is disabled with a "↻" spinner being displayed. The
* user is unable to interact with the user interface until this method is called. * user is unable to interact with the user interface until this method is called.
* @return Whether this is the first time that payment information has been provided to the user
* interface, which indicates that the "UI shown" event should be recorded now.
*/ */
public boolean enableAndUpdatePaymentRequestUIWithPaymentInfo() { public void enableAndUpdatePaymentRequestUIWithPaymentInfo() {
boolean isFirstUpdate = false;
if (mPaymentInformationCallback != null && mPaymentMethodsSection != null) { if (mPaymentInformationCallback != null && mPaymentMethodsSection != null) {
providePaymentInformationToPaymentRequestUI(); providePaymentInformationToPaymentRequestUI();
isFirstUpdate = true;
} else { } else {
mPaymentRequestUI.updateOrderSummarySection(mUiShoppingCart); mPaymentRequestUI.updateOrderSummarySection(mUiShoppingCart);
if (shouldShowShippingSection()) { if (shouldShowShippingSection()) {
...@@ -936,7 +932,6 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs ...@@ -936,7 +932,6 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs
PaymentRequestUI.DataType.SHIPPING_OPTIONS, mUiShippingOptions); PaymentRequestUI.DataType.SHIPPING_OPTIONS, mUiShippingOptions);
} }
} }
return isFirstUpdate;
} }
/** /**
...@@ -1051,6 +1046,7 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs ...@@ -1051,6 +1046,7 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs
new PaymentInformation(mUiShoppingCart, mShippingAddressesSection, new PaymentInformation(mUiShoppingCart, mShippingAddressesSection,
mUiShippingOptions, mContactSection, mPaymentMethodsSection)); mUiShippingOptions, mContactSection, mPaymentMethodsSection));
mPaymentInformationCallback = null; mPaymentInformationCallback = null;
mJourneyLogger.setEventOccurred(Event.SHOWN);
} }
/** /**
...@@ -1120,7 +1116,6 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs ...@@ -1120,7 +1116,6 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs
mShippingAddressesSection.setSelectedItemIndex( mShippingAddressesSection.setSelectedItemIndex(
SectionInformation.NO_SELECTION); SectionInformation.NO_SELECTION);
providePaymentInformationToPaymentRequestUI(); providePaymentInformationToPaymentRequestUI();
mDelegate.recordShowEventAndTransactionAmount();
} else { } else {
if (toEdit == null) { if (toEdit == null) {
// Address is complete and user was in the "Add flow": add an item to // Address is complete and user was in the "Add flow": add an item to
...@@ -1141,7 +1136,6 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs ...@@ -1141,7 +1136,6 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs
} }
} else { } else {
providePaymentInformationToPaymentRequestUI(); providePaymentInformationToPaymentRequestUI();
mDelegate.recordShowEventAndTransactionAmount();
} }
if (!mRetryQueue.isEmpty()) mHandler.post(mRetryQueue.remove()); if (!mRetryQueue.isEmpty()) mHandler.post(mRetryQueue.remove());
...@@ -1544,10 +1538,7 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs ...@@ -1544,10 +1538,7 @@ public class PaymentUiService implements SettingsAutofillAndPaymentsObserver.Obs
if (isShowWaitingForUpdatedDetails) return; if (isShowWaitingForUpdatedDetails) return;
mHandler.post(() -> { mHandler.post(() -> {
if (mPaymentRequestUI != null) { if (mPaymentRequestUI != null) providePaymentInformationToPaymentRequestUI();
providePaymentInformationToPaymentRequestUI();
mDelegate.recordShowEventAndTransactionAmount();
}
}); });
} }
......
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