Skip to content

GitLab

T
tangled
Commit 87eb2e54 authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Commit Bot
Browse files
Options

[PaymentHandler] Hide Payment Request UI when Payment Handler UI open 

In the new bottom-sheet based Payment Handler UI, the Payment Request
UI is currently obstructing the PH UI. This is because PR UI is a
Dialog, and the PH UI is a widget on the ChromeActivity - Dialogs
displayed on the top graphic layer of their activity.

To solve this issue, this CL change to hide the PR UI when opens
a PH UI, and reshow the PR UI when hides the PH UI. The way to
implement this behavior is to create a coordinator responsible for
coordinating the display of both UIs, so that:
* at most one of PH UI, PR UI is displayed at any moment
* PH UI is prioritized over PR UI
* the coordinating mechanism is robust to the external factors, e.g, a
timeout event tries to show the PR UI when it's not supposed to be
shown.

As a side issue this CL solves, clicking the PR UI's "Pay" after
hiding the PH UI used to fail. This was because the PH UI
reference wasn't cleaned up when hiding. This CL adds a cleanup
callback on PH UI closed to solve the issue.

Bug: 999196

Change-Id: Ic602378cc30e93d1ba318a55006100feab7eac50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1924600Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719994}
parent 0412fc7d
Hide whitespace changes
Inline Side-by-side
Showing with 135 additions and 20 deletions
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
View file @ 87eb2e54
......@@ -30,6 +30,7 @@ import org.chromium.chrome.browser.compositor.layouts.OverviewModeBehavior.Overv
import org.chromium.chrome.browser.favicon.FaviconHelper;
import org.chromium.chrome.browser.page_info.CertificateChainHelper;
import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator;
import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator.PaymentHandlerUiObserver;
import org.chromium.chrome.browser.payments.micro.MicrotransactionCoordinator;
import org.chromium.chrome.browser.payments.ui.ContactDetailsSection;
import org.chromium.chrome.browser.payments.ui.LineItem;
......@@ -113,7 +114,8 @@ public class PaymentRequestImpl
PaymentAppFactory.PaymentAppCreatedCallback,
PaymentResponseHelper.PaymentResponseRequesterDelegate, FocusChangedObserver,
NormalizedAddressRequestDelegate, SettingsAutofillAndPaymentsObserver.Observer,
PaymentHandlerHostDelegate, PaymentDetailsConverter.MethodChecker {
PaymentHandlerHostDelegate, PaymentDetailsConverter.MethodChecker,
PaymentHandlerUiObserver {
/**
* A delegate to ask questions about the system, that allows tests to inject behaviour without
* having to modify the entire system. This partially mirrors a similar C++
......@@ -146,6 +148,64 @@ public class PaymentRequestImpl
boolean skipUiForBasicCard();
}
/**
* This class is to coordinate the show state of the Payment Handler UI and the Payment
* Request UI so that these visibility rules are enforced:
* 1. at most one UI is shown at any moment in case the Payment Request UI obstructs the Payment
* Handler UI.
* 2. Payment Handler UI is prioritized to show over Payment Request UI
*/
public class PaymentUisShowStateReconciler {
// Whether the Payment Handler UI is showing.
private boolean mShowingHandlerUi;
// Whether to show the Payment Request UI when the Payment Handler is not being shown.
private boolean mShouldShowDialog;
/**
* Show the Payment Request UI dialog when Payment Handler UI is hidden, i.e., if Payment
* Handler UI is hidden, show the dialog immediately; otherwise, do it on Payment Handler UI
* hidden.
*/
public void showPaymentRequestDialogWhenNoPaymentHandlerUi() {
mShouldShowDialog = true;
updatePaymentRequestDialogShowState();
}
/** Hide the Payment Request UI dialog. */
public void hidePaymentRequestDialog() {
mShouldShowDialog = false;
updatePaymentRequestDialogShowState();
}
/** A callback invoked when the Payment Request UI is closed. */
/* package */ void onPaymentRequestUiClosed() {
assert mUI == null;
mShouldShowDialog = false;
}
/**
* A callback invoked when the Payment Handler UI is shown, to enforce the visibility rules.
*/
public void onPaymentHandlerUiShown() {
mShowingHandlerUi = true;
updatePaymentRequestDialogShowState();
}
/**
* A callback invoked when the Payment Handler UI is hidden, to enforce the visibility
* rules.
*/
public void onPaymentHandlerUiClosed() {
mShowingHandlerUi = false;
updatePaymentRequestDialogShowState();
}
private void updatePaymentRequestDialogShowState() {
if (mUI == null) return;
mUI.setVisible(!mShowingHandlerUi && mShouldShowDialog);
}
}
/**
* A test-only observer for the PaymentRequest service implementation.
*/
......@@ -423,6 +483,7 @@ public class PaymentRequestImpl
private Callback<PaymentInformation> mPaymentInformationCallback;
private PaymentInstrument mInvokedPaymentInstrument;
private PaymentHandlerCoordinator mPaymentHandlerUi;
private PaymentUisShowStateReconciler mPaymentUisShowStateReconciler;
private boolean mMerchantSupportsAutofillPaymentInstruments;
private boolean mUserCanAddCreditCard;
private boolean mHideServerAutofillInstruments;
......@@ -544,6 +605,8 @@ public class PaymentRequestImpl
mSkipUiForNonUrlPaymentMethodIdentifiers = mDelegate.skipUiForBasicCard();
if (sObserverForTest != null) sObserverForTest.onPaymentRequestCreated(this);
mPaymentUisShowStateReconciler = new PaymentUisShowStateReconciler();
}
/**
......@@ -797,7 +860,7 @@ public class PaymentRequestImpl
mUI = new PaymentRequestUI(activity, this, mMerchantSupportsAutofillPaymentInstruments,
!PaymentPreferencesUtil.isPaymentCompleteOnce(), mMerchantName, mTopLevelOrigin,
SecurityStateModel.getSecurityLevelForWebContents(mWebContents),
new ShippingStrings(mShippingType));
new ShippingStrings(mShippingType), mPaymentUisShowStateReconciler);
final FaviconHelper faviconHelper = new FaviconHelper();
faviconHelper.getLocalFaviconImageForURL(Profile.getLastUsedProfile(),
......@@ -1314,7 +1377,18 @@ public class PaymentRequestImpl
mPaymentHandlerUi = new PaymentHandlerCoordinator();
ChromeActivity chromeActivity = ChromeActivity.fromWebContents(mWebContents);
if (chromeActivity == null) return false;
return mPaymentHandlerUi.show(chromeActivity, url, mIsIncognito);
return mPaymentHandlerUi.show(chromeActivity, url, mIsIncognito, /*observer=*/this);
}
@Override
public void onPaymentHandlerUiClosed() {
mPaymentUisShowStateReconciler.onPaymentHandlerUiClosed();
mPaymentHandlerUi = null;
}
@Override
public void onPaymentHandlerUiShown() {
mPaymentUisShowStateReconciler.onPaymentHandlerUiShown();
}
@Override
......@@ -2845,6 +2919,7 @@ public class PaymentRequestImpl
closeClient();
});
mUI = null;
mPaymentUisShowStateReconciler.onPaymentRequestUiClosed();
}
setShowingPaymentRequest(null);
......
chrome/android/java/src/org/chromium/chrome/browser/payments/handler/PaymentHandlerCoordinator.java
View file @ 87eb2e54
......@@ -34,6 +34,14 @@ public class PaymentHandlerCoordinator {
assert isEnabled();
}
/** Observes the state changes of the payment-handler UI. */
public interface PaymentHandlerUiObserver {
/** Called when Payment Handler UI is closed. */
void onPaymentHandlerUiClosed();
/** Called when Payment Handler UI is shown. */
void onPaymentHandlerUiShown();
}
/**
* Shows the payment-handler UI.
*
......@@ -41,9 +49,11 @@ public class PaymentHandlerCoordinator {
* @param url The url of the payment handler app, i.e., that of
* "PaymentRequestEvent.openWindow(url)".
* @param isIncognito Whether the tab is in incognito mode.
* @param observer The {@link PaymentHandlerUiObserver} that observes this Payment Handler UI.
* @return Whether the payment-handler UI was shown. Can be false if the UI was suppressed.
*/
public boolean show(ChromeActivity activity, URI url, boolean isIncognito) {
public boolean show(ChromeActivity activity, URI url, boolean isIncognito,
PaymentHandlerUiObserver observer) {
assert mHider == null : "Already showing payment-handler UI";
WebContents webContents =
......@@ -56,7 +66,7 @@ public class PaymentHandlerCoordinator {
PropertyModel model = new PropertyModel.Builder(PaymentHandlerProperties.ALL_KEYS).build();
PaymentHandlerMediator mediator =
new PaymentHandlerMediator(model, this::hide, webContents);
new PaymentHandlerMediator(model, this::hide, webContents, observer);
BottomSheetController bottomSheetController = activity.getBottomSheetController();
bottomSheetController.addObserver(mediator);
webContents.addObserver(mediator);
......@@ -73,10 +83,9 @@ public class PaymentHandlerCoordinator {
bottomSheetController.removeObserver(mediator);
bottomSheetController.hideContent(/*content=*/view, /*animate=*/true);
webContents.destroy();
observer.onPaymentHandlerUiClosed();
};
boolean result = bottomSheetController.requestShowContent(view, /*animate=*/true);
if (result) bottomSheetController.expandSheet();
return result;
return bottomSheetController.requestShowContent(view, /*animate=*/true);
}
/** Hides the payment-handler UI. */
......
chrome/android/java/src/org/chromium/chrome/browser/payments/handler/PaymentHandlerMediator.java
View file @ 87eb2e54
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.payments.handler;
import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel.StateChangeReason;
import org.chromium.chrome.browser.payments.ServiceWorkerPaymentAppBridge;
import org.chromium.chrome.browser.payments.SslValidityChecker;
import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator.PaymentHandlerUiObserver;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetContent;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController.SheetState;
......@@ -28,6 +29,7 @@ import org.chromium.ui.modelutil.PropertyModel;
// mWebContents (a weak ref) requires null checks, while mWebContentsRef is guaranteed to be not
// null.
private final WebContents mWebContentsRef;
private final PaymentHandlerUiObserver mPaymentHandlerUiObserver;
/**
* Build a new mediator that handle events from outside the payment handler component.
......@@ -36,14 +38,16 @@ import org.chromium.ui.modelutil.PropertyModel;
* @param hider The callback to clean up {@link PaymentHandlerCoordinator} when the sheet is
* hidden.
* @param webContents The web-contents that loads the payment app.
* @param observer The {@link PaymentHandlerUiObserver} that observes this Payment Handler UI.
*/
/* package */ PaymentHandlerMediator(
PropertyModel model, Runnable hider, WebContents webContents) {
/* package */ PaymentHandlerMediator(PropertyModel model, Runnable hider,
WebContents webContents, PaymentHandlerUiObserver observer) {
super(webContents);
assert webContents != null;
mWebContentsRef = webContents;
mModel = model;
mHider = hider;
mPaymentHandlerUiObserver = observer;
}
// BottomSheetObserver:
......@@ -63,7 +67,9 @@ import org.chromium.ui.modelutil.PropertyModel;
}
@Override
public void onSheetOpened(@StateChangeReason int reason) {}
public void onSheetOpened(@StateChangeReason int reason) {
mPaymentHandlerUiObserver.onPaymentHandlerUiShown();
}
@Override
public void onSheetClosed(@StateChangeReason int reason) {
......
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
View file @ 87eb2e54
......@@ -39,6 +39,7 @@ import org.chromium.base.Callback;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeVersionInfo;
import org.chromium.chrome.browser.payments.PaymentRequestImpl.PaymentUisShowStateReconciler;
import org.chromium.chrome.browser.payments.ShippingStrings;
import org.chromium.chrome.browser.payments.ui.PaymentRequestSection.LineItemBreakdownSection;
import org.chromium.chrome.browser.payments.ui.PaymentRequestSection.OptionSection;
......@@ -280,7 +281,13 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
private final Context mContext;
private final Client mClient;
private final boolean mShowDataSource;
private final PaymentUisShowStateReconciler mPaymentUisShowStateReconciler;
/**
* The top level container of this UI. When needing to call show() or hide(), use {@link
* PaymentUisShowStateReconciler}'s showPaymentRequestDialogWhenNoPaymentHandlerUi() and
* hidePaymentRequestDialog() instead.
*/
private final DimmingDialog mDialog;
private final EditorDialog mEditorDialog;
private final EditorDialog mCardEditorDialog;
......@@ -345,7 +352,8 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
*/
public PaymentRequestUI(Activity activity, Client client, boolean canAddCards,
boolean showDataSource, String title, String origin, int securityLevel,
ShippingStrings shippingStrings) {
ShippingStrings shippingStrings,
PaymentUisShowStateReconciler paymentUisShowStateReconciler) {
mContext = activity;
mClient = client;
mShowDataSource = showDataSource;
......@@ -412,6 +420,7 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
}
mDialog = new DimmingDialog(activity, this);
mPaymentUisShowStateReconciler = paymentUisShowStateReconciler;
}
/**
......@@ -419,7 +428,7 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
*/
public void show() {
mDialog.addBottomSheetView(mRequestView);
mDialog.show();
mPaymentUisShowStateReconciler.showPaymentRequestDialogWhenNoPaymentHandlerUi();
mClient.getDefaultPaymentInformation(new Callback<PaymentInformation>() {
@Override
public void onResult(PaymentInformation result) {
......@@ -449,12 +458,14 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
/**
* Dim the background without showing any UI. No UI will be interactive. The dimming stops when
* close() is called. This is useful for launching a payment handler directly without showing a
* PaymentRequest UI first in cases where only one payment handler is available.
* close() is called. This is useful for the skip-ui scenario, i.e., launching a payment handler
* directly without showing a PaymentRequest UI first in cases where only one payment handler is
* available.
*/
public void dimBackground() {
// Intentionally do not add the bottom sheet view.
mDialog.show();
// Intentionally do not add the bottom sheet view to mDialog so that only the scrim part of
// the dialog will be shown.
mPaymentUisShowStateReconciler.showPaymentRequestDialogWhenNoPaymentHandlerUi();
}
/**
......@@ -951,7 +962,7 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
if (shouldShowSpinner) {
changeSpinnerVisibility(true);
} else {
mDialog.hide();
mPaymentUisShowStateReconciler.hidePaymentRequestDialog();
}
}
......@@ -962,7 +973,7 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
assert mIsProcessingPayClicked;
mIsProcessingPayClicked = false;
changeSpinnerVisibility(false);
mDialog.show();
mPaymentUisShowStateReconciler.showPaymentRequestDialogWhenNoPaymentHandlerUi();
updatePayButtonEnabled();
}
......@@ -984,7 +995,7 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
assert mIsProcessingPayClicked;
changeSpinnerVisibility(true);
mDialog.show();
mPaymentUisShowStateReconciler.showPaymentRequestDialogWhenNoPaymentHandlerUi();
}
private void changeSpinnerVisibility(boolean showSpinner) {
......@@ -1419,4 +1430,18 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
sPaymentRequestObserverForTest.onPaymentRequestSelectionChecked(this);
}
}
/**
* Set the visibility state of the dialog. Use {@link PaymentUisShowStateReconciler}'s
* showPaymentRequestDialogWhenNoPaymentHandlerUi() and hidePaymentRequestDialog() instead of
* calling this method directly.
* @param visible True to show the dialog, false to hide the dialog.
*/
public void setVisible(boolean visible) {
if (visible) {
mDialog.show();
} else {
mDialog.hide();
}
}
}
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