Commit c3c1b628 authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Commit Bot

PRImpl#cst no longer directly depends on AddressEditor and CardEditor

Before:
PRImpl#constructor depended on AddressEditor and CardEditor directly.

After:
PRImpl#constructor depends on PaymentUIsManager which depends on
CardEditor and AddressEditor.

Change:
* Move mAddressEditor and mCardEditor instantiation into
PaymentUIsManager.
* Refactor CardEditor so that forTest observer is no longer set in
its constructor.
* Turn the test observer to static so that it can exist before there's
a PRImpl, PaymentUIsManager instance, which is required by tests[1].

[1] https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestTestRule.java;l=211;drc=26503120e38dd3c6b2dfa17dfbc65465ac475534

PaymentUIsManager set the test observer only in tests.

Bug: 1102522, 1107102
Binary-Size: Silence the "for-test" alert because PRImpl,
Change-Id: I47ae3d37d015f8ebb734ede2c90d590015a3a163
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324566
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarSahel Sharify <sahel@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792777}
parent bf18b9ed
...@@ -675,7 +675,7 @@ class AssistantCollectUserDataBinder ...@@ -675,7 +675,7 @@ class AssistantCollectUserDataBinder
/*deleteRunnable =*/null, profile)); /*deleteRunnable =*/null, profile));
CardEditor cardEditor = new CardEditor(webContents, addressEditor, CardEditor cardEditor = new CardEditor(webContents, addressEditor,
/* includeOrgLabel= */ false, /* observerForTest= */ null); /* includeOrgLabel= */ false);
List<String> supportedCardNetworks = List<String> supportedCardNetworks =
model.get(AssistantCollectUserDataModel.SUPPORTED_BASIC_CARD_NETWORKS); model.get(AssistantCollectUserDataModel.SUPPORTED_BASIC_CARD_NETWORKS);
if (supportedCardNetworks != null) { if (supportedCardNetworks != null) {
......
...@@ -12,6 +12,7 @@ import android.text.style.ForegroundColorSpan; ...@@ -12,6 +12,7 @@ import android.text.style.ForegroundColorSpan;
import android.util.Pair; import android.util.Pair;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.ApiCompatibilityUtils; import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.Callback; import org.chromium.base.Callback;
...@@ -110,7 +111,7 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument> ...@@ -110,7 +111,7 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument>
private final AddressEditor mAddressEditor; private final AddressEditor mAddressEditor;
/** An optional observer used by tests. */ /** An optional observer used by tests. */
@Nullable private final PaymentRequestServiceObserverForTest mObserverForTest; private static PaymentRequestServiceObserverForTest sObserverForTest;
/** /**
* A mapping from all card issuer networks recognized in Chrome to information about these * A mapping from all card issuer networks recognized in Chrome to information about these
...@@ -167,16 +168,14 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument> ...@@ -167,16 +168,14 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument>
* billing addresses. * billing addresses.
* @param includeOrgLabel Whether the labels in the billing address dropdown should include the * @param includeOrgLabel Whether the labels in the billing address dropdown should include the
* organization name. * organization name.
* @param observerForTest Optional observer for test.
*/ */
public CardEditor(WebContents webContents, AddressEditor addressEditor, boolean includeOrgLabel, public CardEditor(
@Nullable PaymentRequestServiceObserverForTest observerForTest) { WebContents webContents, AddressEditor addressEditor, boolean includeOrgLabel) {
assert webContents != null; assert webContents != null;
assert addressEditor != null; assert addressEditor != null;
mWebContents = webContents; mWebContents = webContents;
mAddressEditor = addressEditor; mAddressEditor = addressEditor;
mObserverForTest = observerForTest;
List<AutofillProfile> profiles = List<AutofillProfile> profiles =
PersonalDataManager.getInstance().getBillingAddressesToSuggest(includeOrgLabel); PersonalDataManager.getInstance().getBillingAddressesToSuggest(includeOrgLabel);
...@@ -269,6 +268,15 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument> ...@@ -269,6 +268,15 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument>
mIsIncognito = activity != null && activity.getCurrentTabModel().isIncognito(); mIsIncognito = activity != null && activity.getCurrentTabModel().isIncognito();
} }
/**
* Set an observer for test.
* @param observerForTest An observer for test.
*/
@VisibleForTesting
public static void setObserverForTest(PaymentRequestServiceObserverForTest observerForTest) {
sObserverForTest = observerForTest;
}
private boolean isCardNumberLengthMaximum(@Nullable CharSequence value) { private boolean isCardNumberLengthMaximum(@Nullable CharSequence value) {
if (TextUtils.isEmpty(value)) return false; if (TextUtils.isEmpty(value)) return false;
String cardNetwork = PersonalDataManager.getInstance().getBasicCardIssuerNetwork( String cardNetwork = PersonalDataManager.getInstance().getBasicCardIssuerNetwork(
...@@ -544,11 +552,11 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument> ...@@ -544,11 +552,11 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument>
R.string.payments_card_expiration_invalid_validation_message)); R.string.payments_card_expiration_invalid_validation_message));
mMonthField.setIsFullLine(false); mMonthField.setIsFullLine(false);
if (mObserverForTest != null) { if (sObserverForTest != null) {
mMonthField.setDropdownCallback(new Callback<Pair<String, Runnable>>() { mMonthField.setDropdownCallback(new Callback<Pair<String, Runnable>>() {
@Override @Override
public void onResult(final Pair<String, Runnable> eventData) { public void onResult(final Pair<String, Runnable> eventData) {
mObserverForTest.onPaymentRequestServiceExpirationMonthChange(); sObserverForTest.onPaymentRequestServiceExpirationMonthChange();
} }
}); });
} }
...@@ -675,8 +683,8 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument> ...@@ -675,8 +683,8 @@ public class CardEditor extends EditorBase<AutofillPaymentInstrument>
final boolean isSelectingIncompleteAddress = final boolean isSelectingIncompleteAddress =
mIncompleteProfilesForBillingAddress.containsKey(eventData.first); mIncompleteProfilesForBillingAddress.containsKey(eventData.first);
if (!isAddingNewAddress && !isSelectingIncompleteAddress) { if (!isAddingNewAddress && !isSelectingIncompleteAddress) {
if (mObserverForTest != null) { if (sObserverForTest != null) {
mObserverForTest.onPaymentRequestServiceBillingAddressChangeProcessed(); sObserverForTest.onPaymentRequestServiceBillingAddressChangeProcessed();
} }
return; return;
} }
......
...@@ -452,21 +452,13 @@ public class PaymentRequestImpl ...@@ -452,21 +452,13 @@ public class PaymentRequestImpl
mIsOffTheRecord = mDelegate.isOffTheRecord(ChromeActivity.fromWebContents(mWebContents)); mIsOffTheRecord = mDelegate.isOffTheRecord(ChromeActivity.fromWebContents(mWebContents));
// Do not persist changes on disk in OffTheRecord mode.
AddressEditor addressEditor = new AddressEditor(
AddressEditor.Purpose.PAYMENT_REQUEST, /*saveToDisk=*/!mIsOffTheRecord);
// PaymentRequest card editor does not show the organization name in the dropdown with the
// billing address labels.
CardEditor cardEditor = new CardEditor(
mWebContents, addressEditor, /*includeOrgLabel=*/false, sObserverForTest);
mJourneyLogger = new JourneyLogger(mIsOffTheRecord, mWebContents); mJourneyLogger = new JourneyLogger(mIsOffTheRecord, mWebContents);
mSkipUiForNonUrlPaymentMethodIdentifiers = mDelegate.skipUiForBasicCard(); mSkipUiForNonUrlPaymentMethodIdentifiers = mDelegate.skipUiForBasicCard();
if (sObserverForTest != null) sObserverForTest.onPaymentRequestCreated(this); if (sObserverForTest != null) sObserverForTest.onPaymentRequestCreated(this);
mPaymentUIsManager = new PaymentUIsManager(/*delegate=*/this, mPaymentUIsManager = new PaymentUIsManager(/*delegate=*/this,
/*params=*/this, addressEditor, cardEditor); /*params=*/this, mWebContents, mIsOffTheRecord);
mPaymentAppComparator = new PaymentAppComparator(/*params=*/this); mPaymentAppComparator = new PaymentAppComparator(/*params=*/this);
} }
...@@ -2764,6 +2756,7 @@ public class PaymentRequestImpl ...@@ -2764,6 +2756,7 @@ public class PaymentRequestImpl
@VisibleForTesting @VisibleForTesting
public static void setObserverForTest(PaymentRequestServiceObserverForTest observerForTest) { public static void setObserverForTest(PaymentRequestServiceObserverForTest observerForTest) {
sObserverForTest = observerForTest; sObserverForTest = observerForTest;
PaymentUIsManager.setObserverForTest(sObserverForTest);
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.payments.AutofillPaymentAppCreator; ...@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.payments.AutofillPaymentAppCreator;
import org.chromium.chrome.browser.payments.AutofillPaymentAppFactory; import org.chromium.chrome.browser.payments.AutofillPaymentAppFactory;
import org.chromium.chrome.browser.payments.CardEditor; import org.chromium.chrome.browser.payments.CardEditor;
import org.chromium.chrome.browser.payments.PaymentRequestImpl; import org.chromium.chrome.browser.payments.PaymentRequestImpl;
import org.chromium.chrome.browser.payments.PaymentRequestImpl.PaymentRequestServiceObserverForTest;
import org.chromium.chrome.browser.payments.SettingsAutofillAndPaymentsObserver; import org.chromium.chrome.browser.payments.SettingsAutofillAndPaymentsObserver;
import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator; import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator;
import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator.PaymentHandlerUiObserver; import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator.PaymentHandlerUiObserver;
...@@ -123,7 +124,7 @@ public class PaymentUIsManager implements SettingsAutofillAndPaymentsObserver.Ob ...@@ -123,7 +124,7 @@ public class PaymentUIsManager implements SettingsAutofillAndPaymentsObserver.Ob
} }
/** A callback invoked when the bottom sheet is hidden, to enforce the visibility rules. */ /** A callback invoked when the bottom sheet is hidden, to enforce the visibility rules. */
public void onBottomSheetClosed() { /* package */ void onBottomSheetClosed() {
mShowingBottomSheet = false; mShowingBottomSheet = false;
updatePaymentRequestDialogShowState(); updatePaymentRequestDialogShowState();
} }
...@@ -138,22 +139,34 @@ public class PaymentUIsManager implements SettingsAutofillAndPaymentsObserver.Ob ...@@ -138,22 +139,34 @@ public class PaymentUIsManager implements SettingsAutofillAndPaymentsObserver.Ob
* Create PaymentUIsManager. * Create PaymentUIsManager.
* @param delegate The delegate of this instance. * @param delegate The delegate of this instance.
* @param params The parameters of the payment request specified by the merchant. * @param params The parameters of the payment request specified by the merchant.
* @param addressEditor The AddressEditor of the PaymentRequest UI. * @param webContents The WebContents of the merchant page.
* @param cardEditor The CardEditor of the PaymentRequest UI. * @param isOffTheRecord Whether merchant page is in an isOffTheRecord tab.
*/ */
// TODO(crbug.com/1107102): AddressEditor and CardEditor should be initialized in this
// constructor instead of the caller of the constructor, once CardEditor's "ForTest" symbols
// have been removed from the production code.
public PaymentUIsManager(Delegate delegate, PaymentRequestParams params, public PaymentUIsManager(Delegate delegate, PaymentRequestParams params,
AddressEditor addressEditor, CardEditor cardEditor) { WebContents webContents, boolean isOffTheRecord) {
mDelegate = delegate; mDelegate = delegate;
mParams = params; mParams = params;
mAddressEditor = addressEditor;
mCardEditor = cardEditor; // Do not persist changes on disk in OffTheRecord mode.
mAddressEditor = new AddressEditor(
AddressEditor.Purpose.PAYMENT_REQUEST, /*saveToDisk=*/!isOffTheRecord);
// PaymentRequest card editor does not show the organization name in the dropdown with the
// billing address labels.
mCardEditor = new CardEditor(webContents, mAddressEditor, /*includeOrgLabel=*/false);
mPaymentUisShowStateReconciler = new PaymentUisShowStateReconciler(); mPaymentUisShowStateReconciler = new PaymentUisShowStateReconciler();
mCurrencyFormatterMap = new HashMap<>(); mCurrencyFormatterMap = new HashMap<>();
} }
/**
* Set an observer for test.
* @param observerForTest An observer for test.
*/
@VisibleForTesting
public static void setObserverForTest(PaymentRequestServiceObserverForTest observerForTest) {
CardEditor.setObserverForTest(observerForTest);
}
/** @return The PaymentRequestUI. */ /** @return The PaymentRequestUI. */
public PaymentRequestUI getPaymentRequestUI() { public PaymentRequestUI getPaymentRequestUI() {
return mPaymentRequestUI; return mPaymentRequestUI;
......
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