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

[PaymentRequest] Remove Error View from Android

Before change:
* With Expandable PH UI enabled, if merchant pages call
complete("fail"), the screen would be covered under scrim and Clank
would crash on refresh.
* With CCT PH UI enabled, if merchant pages call complete("fail"), it
would open Error View after closing PH UI.

After change:
The Error View is no longer shown on complete("fail").

Changes:
* remove Error View from Android Payment Request UI.
* Tests
 * add a test to ensure http would be rejected for openWindow(url)
 * add a test to cover complete ("unknown").

Bug: 1044077, 1047237, 1042892, 1045474

Change-Id: I0123e163d4cd0a97d1536d5c4baefa0e646c50d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2019922Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737634}
parent bb8ca8b7
......@@ -1293,7 +1293,6 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestHeader.java",
"java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java",
"java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java",
"java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUiErrorView.java",
"java/src/org/chromium/chrome/browser/payments/ui/SectionInformation.java",
"java/src/org/chromium/chrome/browser/payments/ui/SectionUiUtils.java",
"java/src/org/chromium/chrome/browser/payments/ui/ShoppingCart.java",
......
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2016 The Chromium Authors. All rights reserved.
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file. -->
<!-- PaymentRequest error dialog
The Java code inflating this layout manages the hiding and adjustment of unnecessary elements
in the layout, like the close button. In lieu of being able to set the elevation, we use a
drop shadow background.
-->
<org.chromium.chrome.browser.payments.ui.PaymentRequestUiErrorView
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
android:orientation="vertical"
android:gravity="center"
android:background="@drawable/popup_bg_tinted"
app:maxWidthLandscape="@dimen/payments_ui_max_dialog_width"
app:maxWidthPortrait="@dimen/payments_ui_max_dialog_width" >
<include layout="@layout/payment_request_header" />
<!-- Error message. -->
<TextView
android:id="@+id/message"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="20dp"
android:layout_marginStart="@dimen/editor_dialog_section_large_spacing"
android:layout_marginEnd="@dimen/editor_dialog_section_large_spacing"
android:layout_marginBottom="@dimen/payments_section_largest_spacing"
android:text="@string/payments_error_message"
android:textAppearance="@style/TextAppearance.PaymentRequestErrorText" />
<!-- Dismisses the dialog. -->
<org.chromium.ui.widget.ButtonCompat
android:id="@+id/ok_button"
android:text="@string/ok"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="end"
android:layout_marginEnd="@dimen/editor_dialog_section_small_spacing"
android:layout_marginBottom="@dimen/editor_dialog_section_small_spacing"
style="@style/TextButton" />
</org.chromium.chrome.browser.payments.ui.PaymentRequestUiErrorView>
......@@ -1139,13 +1139,13 @@ public class PaymentRequestImpl
private void onMicrotransactionUiErroredAndClosed() {
closeClient();
closeUIAndDestroyNativeObjects(/*immediateClose=*/true);
closeUIAndDestroyNativeObjects();
}
private void onMicrotransactionUiCompletedAndClosed() {
if (mClient != null) mClient.onComplete();
closeClient();
closeUIAndDestroyNativeObjects(/*immediateClose=*/true);
closeUIAndDestroyNativeObjects();
}
private static Map<String, PaymentMethodData> getValidatedMethodData(
......@@ -2160,7 +2160,7 @@ public class PaymentRequestImpl
Log.d(TAG, debugMessage);
if (mClient != null) mClient.onError(reason, debugMessage);
closeClient();
closeUIAndDestroyNativeObjects(/*immediateClose=*/true);
closeUIAndDestroyNativeObjects();
if (mNativeObserverForTest != null) mNativeObserverForTest.onConnectionTerminated();
}
......@@ -2185,7 +2185,7 @@ public class PaymentRequestImpl
if (abortSucceeded) {
closeClient();
mJourneyLogger.setAborted(AbortReason.ABORTED_BY_MERCHANT);
closeUIAndDestroyNativeObjects(/*immediateClose=*/true);
closeUIAndDestroyNativeObjects();
} else {
if (sObserverForTest != null) sObserverForTest.onPaymentRequestServiceUnableToAbort();
}
......@@ -2234,7 +2234,7 @@ public class PaymentRequestImpl
mNativeObserverForTest.onCompleteCalled();
}
closeUIAndDestroyNativeObjects(/*immediateClose=*/PaymentComplete.FAIL != result);
closeUIAndDestroyNativeObjects();
}
@Override
......@@ -2483,7 +2483,7 @@ public class PaymentRequestImpl
closeClient();
mJourneyLogger.setAborted(AbortReason.MOJO_RENDERER_CLOSING);
if (sObserverForTest != null) sObserverForTest.onRendererClosedMojoConnection();
closeUIAndDestroyNativeObjects(/*immediateClose=*/true);
closeUIAndDestroyNativeObjects();
if (mNativeObserverForTest != null) mNativeObserverForTest.onConnectionTerminated();
}
......@@ -2495,7 +2495,7 @@ public class PaymentRequestImpl
if (mClient == null) return;
closeClient();
mJourneyLogger.setAborted(AbortReason.MOJO_CONNECTION_ERROR);
closeUIAndDestroyNativeObjects(/*immediateClose=*/true);
closeUIAndDestroyNativeObjects();
if (mNativeObserverForTest != null) mNativeObserverForTest.onConnectionTerminated();
}
......@@ -2921,16 +2921,8 @@ public class PaymentRequestImpl
* Closes the UI and destroys native objects. If the client is still connected, then it's
* notified of UI hiding. This PaymentRequestImpl object can't be reused after this function is
* called.
*
* @param immediateClose If true, then UI immediately closes. If false, the UI shows the error
* message "There was an error processing your order." This message
* implies that the merchant attempted to process the order, failed, and
* called complete("fail") to notify the user. Therefore, this parameter
* may be "false" only when called from
* {@link PaymentRequestImpl#complete(int)}. All other callers should
* always pass "true."
*/
private void closeUIAndDestroyNativeObjects(boolean immediateClose) {
private void closeUIAndDestroyNativeObjects() {
ensureHideAndResetPaymentHandlerUi();
if (mMicrotransactionUi != null) {
mMicrotransactionUi.hide();
......@@ -2938,13 +2930,12 @@ public class PaymentRequestImpl
}
if (mUI != null) {
mUI.close(immediateClose, () -> {
if (mClient != null) {
if (sObserverForTest != null) sObserverForTest.onCompleteReplied();
mClient.onComplete();
}
closeClient();
});
mUI.close();
if (mClient != null) {
if (sObserverForTest != null) sObserverForTest.onCompleteReplied();
mClient.onComplete();
}
closeClient();
mUI = null;
mPaymentUisShowStateReconciler.onPaymentRequestUiClosed();
}
......
......@@ -292,7 +292,6 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
private final EditorDialog mEditorDialog;
private final EditorDialog mCardEditorDialog;
private final ViewGroup mRequestView;
private final PaymentRequestUiErrorView mErrorView;
private final Callback<PaymentInformation> mUpdateSectionsCallback;
private final ShippingStrings mShippingStrings;
......@@ -360,10 +359,6 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
mAnimatorTranslation = mContext.getResources().getDimensionPixelSize(
R.dimen.payments_ui_translation);
mErrorView = (PaymentRequestUiErrorView) LayoutInflater.from(mContext).inflate(
R.layout.payment_request_error, null);
mErrorView.initialize(title, origin, securityLevel);
mReadyToPayNotifierForTest = new NotifierForTest(new Runnable() {
@Override
public void run() {
......@@ -594,32 +589,11 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
* Does not call Client.onDismissed().
*
* Should not be called multiple times.
*
* @param shouldCloseImmediately If true, this function will immediately dismiss the dialog
* without describing the error.
* @param callback The callback to notify of finished animations.
*/
public void close(boolean shouldCloseImmediately, final Runnable callback) {
public void close() {
mIsClientClosing = true;
Runnable dismissRunnable = new Runnable() {
@Override
public void run() {
dismissDialog(false);
if (callback != null) callback.run();
}
};
if (shouldCloseImmediately) {
// The shouldCloseImmediately boolean is true when the merchant calls
// instrumentResponse.complete("success") or instrumentResponse.complete("")
// in JavaScript.
dismissRunnable.run();
} else {
// Show the error dialog.
mDialog.showOverlay(mErrorView);
mErrorView.setDismissRunnable(dismissRunnable);
}
dismissDialog(false);
if (sPaymentRequestObserverForTest != null) {
sPaymentRequestObserverForTest.onPaymentRequestResultReady(this);
......@@ -644,7 +618,6 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
*/
public void setTitleBitmap(Bitmap bitmap) {
((PaymentRequestHeader) mRequestView.findViewById(R.id.header)).setTitleBitmap(bitmap);
mErrorView.setBitmap(bitmap);
}
/**
......
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.payments.ui;
import android.content.Context;
import android.graphics.Bitmap;
import android.support.v4.view.MarginLayoutParamsCompat;
import android.util.AttributeSet;
import android.view.View;
import android.view.ViewGroup;
import org.chromium.chrome.R;
import org.chromium.components.browser_ui.widget.BoundedLinearLayout;
/**
* Displays the status of a payment request to the user.
*/
public class PaymentRequestUiErrorView extends BoundedLinearLayout {
public PaymentRequestUiErrorView(Context context, AttributeSet attrs) {
super(context, attrs);
}
/**
* Initializes the view with the correct strings.
*
* @param title Title of the webpage.
* @param origin Origin of the webpage.
* @param securityLevel The security level of the page that invoked PaymentRequest.
*/
public void initialize(String title, String origin, int securityLevel) {
((PaymentRequestHeader) findViewById(R.id.header))
.setTitleAndOrigin(title, origin, securityLevel);
// Remove the close button, then expand the page information to take up the space formerly
// occupied by the X.
View toRemove = findViewById(R.id.close_button);
((ViewGroup) toRemove.getParent()).removeView(toRemove);
int titleEndMargin = getContext().getResources().getDimensionPixelSize(
R.dimen.editor_dialog_section_large_spacing);
View pageInfoGroup = findViewById(R.id.page_info);
MarginLayoutParamsCompat.setMarginEnd(
(MarginLayoutParams) pageInfoGroup.getLayoutParams(), titleEndMargin);
}
/**
* Sets the callback to run upon hitting the OK button.
*
* @param callback Callback to run upon hitting the OK button.
*/
public void setDismissRunnable(final Runnable callback) {
// Make the user explicitly click on the OK button to dismiss the dialog.
View confirmButton = findViewById(R.id.ok_button);
confirmButton.setOnClickListener(new OnClickListener() {
@Override
public void onClick(View v) {
callback.run();
}
});
}
/**
* Sets what icon is displayed in the header.
*
* @param bitmap Icon to display.
*/
public void setBitmap(Bitmap bitmap) {
((PaymentRequestHeader) findViewById(R.id.header)).setTitleBitmap(bitmap);
}
}
......@@ -20,7 +20,6 @@ import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard;
import org.chromium.chrome.browser.payments.PaymentRequestTestRule.MainActivityStartCallback;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.modaldialog.ModalDialogProperties;
import org.chromium.ui.test.util.DisableAnimationsTestRule;
......@@ -64,15 +63,6 @@ public class PaymentRequestFailCompleteTest implements MainActivityStartCallback
ModalDialogProperties.ButtonType.POSITIVE,
mPaymentRequestTestRule.getResultReady());
// Dismiss the error message overlay.
int callCount = mPaymentRequestTestRule.getDismissed().getCallCount();
TestThreadUtils.runOnUiThreadBlocking(
(Runnable) () -> mPaymentRequestTestRule.getPaymentRequestUI()
.getDialogForTest()
.findViewById(R.id.ok_button)
.performClick());
mPaymentRequestTestRule.getDismissed().waitForCallback(callCount);
mPaymentRequestTestRule.expectResultContains(new String[] {"Transaction failed"});
}
}
......@@ -54,14 +54,9 @@ public class PaymentRequestPaymentAppUiSkipTest {
public void testFail() throws TimeoutException {
mPaymentRequestTestRule.installPaymentApp(HAVE_INSTRUMENTS, IMMEDIATE_RESPONSE);
// Wait for the error message overlay.
mPaymentRequestTestRule.triggerUIAndWait(
"buyFail", mPaymentRequestTestRule.getResultReady());
// Dismiss the error message overlay.
mPaymentRequestTestRule.clickErrorOverlayAndWait(
R.id.ok_button, mPaymentRequestTestRule.getCompleteReplied());
mPaymentRequestTestRule.expectResultContains(new String[] {"Transaction failed"});
}
......
......@@ -329,18 +329,6 @@ public class PaymentRequestTestRule extends ChromeTabbedActivityTestRule
helper.waitForCallback(callCount);
}
/** Clicks on an element in the error overlay. */
protected void clickErrorOverlayAndWait(int resourceId, CallbackHelper helper)
throws TimeoutException {
int callCount = helper.getCallCount();
ThreadUtils.runOnUiThreadBlocking(() -> {
// Error overlay always allows clicks and is not taken into account in
// isAcceptingUserInput().
mUI.getDialogForTest().findViewById(resourceId).performClick();
});
helper.waitForCallback(callCount);
}
/** Clicks on an element in the "Order summary" section of the payments UI. */
protected void clickInOrderSummaryAndWait(CallbackHelper helper) throws TimeoutException {
int callCount = helper.getCallCount();
......
......@@ -18,7 +18,8 @@ namespace {
class ExpandablePaymentHandlerBrowserTest : public PlatformBrowserTest {
public:
ExpandablePaymentHandlerBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS),
http_server_(net::EmbeddedTestServer::TYPE_HTTP) {
scoped_feature_list_.InitWithFeatures(
/*enabled_features=*/{chrome::android::kScrollToExpandPaymentHandler},
/*disabled_features=*/{});
......@@ -36,10 +37,17 @@ class ExpandablePaymentHandlerBrowserTest : public PlatformBrowserTest {
ASSERT_TRUE(content::NavigateToURL(
GetActiveWebContents(),
https_server_.GetURL("/maxpay.com/merchant.html")));
http_server_.ServeFilesFromSourceDirectory(
"components/test/data/payments/");
ASSERT_TRUE(http_server_.Start());
test_controller_.SetUpOnMainThread();
PlatformBrowserTest::SetUpOnMainThread();
}
GURL GetHttpPageUrl() {
return http_server_.GetURL("/maxpay.com/merchant.html");
}
content::WebContents* GetActiveWebContents() {
return chrome_test_utils::GetActiveWebContents(this);
}
......@@ -48,14 +56,18 @@ class ExpandablePaymentHandlerBrowserTest : public PlatformBrowserTest {
private:
net::EmbeddedTestServer https_server_;
net::EmbeddedTestServer http_server_;
base::test::ScopedFeatureList scoped_feature_list_;
};
// Make sure merchants can confirm the payment.
IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest, ConfirmPayment) {
std::string expected = "success";
EXPECT_EQ(expected, content::EvalJs(GetActiveWebContents(), "install()"));
EXPECT_EQ("app_is_ready", content::EvalJs(GetActiveWebContents(),
"launchAndWaitUntilReady()"));
EXPECT_EQ("app_is_ready",
content::EvalJs(
GetActiveWebContents(),
"launchAndWaitUntilReady('./payment_handler_window.html')"));
DCHECK(test_controller_.GetPaymentHandlerWebContents());
EXPECT_EQ("confirmed",
......@@ -64,16 +76,47 @@ IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest, ConfirmPayment) {
EXPECT_EQ("success", content::EvalJs(GetActiveWebContents(), "getResult()"));
}
// Make sure merchants can cancel the payment.
IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest, CancelPayment) {
std::string expected = "success";
EXPECT_EQ(expected, content::EvalJs(GetActiveWebContents(), "install()"));
EXPECT_EQ("app_is_ready", content::EvalJs(GetActiveWebContents(),
"launchAndWaitUntilReady()"));
EXPECT_EQ("app_is_ready",
content::EvalJs(
GetActiveWebContents(),
"launchAndWaitUntilReady('./payment_handler_window.html')"));
DCHECK(test_controller_.GetPaymentHandlerWebContents());
EXPECT_EQ("canceled",
content::EvalJs(test_controller_.GetPaymentHandlerWebContents(),
"cancel()"));
EXPECT_EQ("unknown", content::EvalJs(GetActiveWebContents(), "getResult()"));
}
// Make sure merchants can fail the payment.
IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest, PaymentFailed) {
std::string expected = "success";
EXPECT_EQ(expected, content::EvalJs(GetActiveWebContents(), "install()"));
EXPECT_EQ("app_is_ready",
content::EvalJs(
GetActiveWebContents(),
"launchAndWaitUntilReady('./payment_handler_window.html')"));
DCHECK(test_controller_.GetPaymentHandlerWebContents());
EXPECT_EQ("failed",
content::EvalJs(test_controller_.GetPaymentHandlerWebContents(),
"fail()"));
EXPECT_EQ("fail", content::EvalJs(GetActiveWebContents(), "getResult()"));
}
// Make sure payment apps served from an http connection are rejected.
IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest,
OpenWindowRejectHttp) {
std::string expected = "success";
EXPECT_EQ(expected, content::EvalJs(GetActiveWebContents(), "install()"));
EXPECT_EQ("open_window_failed",
content::EvalJs(
GetActiveWebContents(),
"launchAndWaitUntilReady('" + GetHttpPageUrl().spec() + "')"));
}
} // namespace
} // namespace payments
......@@ -76,40 +76,47 @@ async function uninstall() { // eslint-disable-line no-unused-vars
/**
* Launches the payment handler and waits until its window is ready.
* @param {string} url - open a specified url in payment handler window.
* @return {Promise<string>} - the message about the launch result.
*/
function launchAndWaitUntilReady() { // eslint-disable-line no-unused-vars
function launchAndWaitUntilReady( // eslint-disable-line no-unused-vars
url = './payment_handler_window.html') {
let appReadyResolver;
appReadyPromise = new Promise((r) => {
appReadyResolver = r;
});
try {
const request = new PaymentRequest([{supportedMethods: methodName}], {
total: {label: 'Total', amount: {currency: 'USD', value: '0.01'}},
});
const request = new PaymentRequest(
[{supportedMethods: methodName, data: {url}}],
{total: {label: 'Total', amount: {currency: 'USD', value: '0.01'}}});
request.onpaymentmethodchange = (event) => {
appReadyResolver(event.methodDetails.status);
};
resultPromise = request.show();
updateLogView('payment handler is shown.');
updateLogView('launched and waiting until app gets ready.');
} catch (e) {
appReadyResolver(e.message);
}
return appReadyPromise;
}
/**
* Gets the result of the PaymentRequest.show() from launchAndWaitUntilReady().
* Precondition: called only when launchAndWaitUntilReady() returns
* 'app_is_ready'.
* @return {Promise<string>} - the payment handler's response to the
* 'paymentrequest' event.
*/
async function getResult() { // eslint-disable-line no-unused-vars
try {
const response = await resultPromise;
updateLogView(response.details.status);
await response.complete(response.details.status);
return response.details.status;
const result = response.details.status;
updateLogView(result);
await response.complete(result);
return result;
} catch (e) {
return e.message;
updateLogView(e.message);
return e.message;
}
}
......
......@@ -29,11 +29,28 @@ found in the LICENSE file.
}
</style>
<body>
<script>
async function onInstallClicked() {
const result = await install();
updateLogView(result);
}
async function onUninstallClicked() {
const result = await uninstall();
updateLogView(result);
}
async function onLaunchClicked(url) {
const result = await launchAndWaitUntilReady(url);
updateLogView(result);
if (result !== 'app_is_ready') return;
updateLogView(await getResult());
}
</script>
<h1>Use Max Pay</h1>
<div id="controllers">
<button onclick="updateLogView(install())" id="install">Install</button>
<button onclick="updateLogView(uninstall())" id="uninstall">Uninstall</button>
<button onclick="updateLogView(launchAndWaitUntilReady())" id="testNoHandler">Launch</button>
<button onclick="onInstallClicked()">Install</button>
<button onclick="onUninstallClicked()">Uninstall</button>
<button onclick="onLaunchClicked()">Launch</button>
<button onclick="onLaunchClicked('http://info.cern.ch')">Launch http app</button>
</div>
<div>
<div>Installation Status: <span id="installationStatus"></span></div>
......
......@@ -17,9 +17,12 @@ self.addEventListener('message', (evt) => {
if (evt.data === 'confirm') {
paymentRequestResponder({methodName, details: {status: 'success'}});
return;
} else if (evt.data === 'cancel') {
} else if (evt.data === 'fail') {
paymentRequestResponder({methodName, details: {status: 'fail'}});
return;
} else if (evt.data === 'cancel') {
paymentRequestResponder({methodName, details: {status: 'unknown'}});
return;
} else if (evt.data === 'app_is_ready') {
paymentRequestEvent.changePaymentMethod(methodName, {
status: evt.data,
......@@ -31,8 +34,20 @@ self.addEventListener('message', (evt) => {
self.addEventListener('paymentrequest', (evt) => {
paymentRequestEvent = evt;
methodName = evt.methodData[0].supportedMethods;
url = evt.methodData[0].data.url;
evt.respondWith(new Promise((responder) => {
paymentRequestResponder = responder;
evt.openWindow('./payment_handler_window.html');
const errorString = 'open_window_failed';
evt.openWindow(url).then((result) => {
if (!result) {
paymentRequestEvent.changePaymentMethod(methodName, {
status: errorString,
});
}
}).catch((error) => {
paymentRequestEvent.changePaymentMethod(methodName, {
status: errorString,
});
});
}));
});
......@@ -10,6 +10,7 @@ found in the LICENSE file.
<body>
<button onclick="confirm()">confirm</button>
<button onclick="cancel()">cancel</button>
<button onclick="fail()">fail</button>
<div>Messages:</div>
<pre id="log"></pre>
</body>
......@@ -31,6 +32,12 @@ function confirm() {
return 'confirmed';
}
function fail() {
navigator.serviceWorker.controller.postMessage('fail');
updateLogView('fail is invoked.');
return 'failed';
}
function cancel() {
navigator.serviceWorker.controller.postMessage('cancel');
updateLogView('cancel is invoked.');
......
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