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

[ExpandablePaymentHandler] Remove posted tasks of PH Mediator on hidden

Context:
PaymentHandlerMediator does not stop the posted tasks on PH hidden.
In the example of the bug, PaymentHandlerMediator has found the page
to be insecure in several WebContents observer hooks. Each of them
posts a hide() task. When several of such task is posted, the first
task would release WebContents; the subsequent tasks who tries to
access WebContents would trigger the crash.

Before Change:
Clank would crash when users clicking an http link in a payment app.

After Change:
No crash happens.

Change:
* In ServiceWorkerPaymentAppBridge, whenever it tries to access
WebContents, check whether if it has been destroyed.
* PaymentHandlerMediator stops the pending posted tasks on PH hidden.
* (side) fix CCT PH UI for always closing on open.

Bug: 1049262

Change-Id: I9f849854c48e92e06ada1348a80d761a0b4db744
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2039995
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738713}
parent 64f2b8bc
...@@ -412,6 +412,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface ...@@ -412,6 +412,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface
String paymentRequestId, PaymentInstrument.AbortCallback callback) { String paymentRequestId, PaymentInstrument.AbortCallback callback) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (webContents.isDestroyed()) return;
ServiceWorkerPaymentAppBridgeJni.get().abortPaymentApp( ServiceWorkerPaymentAppBridgeJni.get().abortPaymentApp(
webContents, registrationId, swScope, paymentRequestId, callback); webContents, registrationId, swScope, paymentRequestId, callback);
} }
...@@ -425,7 +426,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface ...@@ -425,7 +426,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface
public static void addTabObserverForPaymentRequestTab(Tab tab) { public static void addTabObserverForPaymentRequestTab(Tab tab) {
tab.addObserver(new EmptyTabObserver() { tab.addObserver(new EmptyTabObserver() {
@Override @Override
public void onDidFinishNavigation(Tab tab, NavigationHandle navigationHandle){} { public void onDidFinishNavigation(Tab tab, NavigationHandle navigationHandle) {
// Notify closing payment app window so as to abort payment if unsecure. // Notify closing payment app window so as to abort payment if unsecure.
WebContents webContents = tab.getWebContents(); WebContents webContents = tab.getWebContents();
if (!SslValidityChecker.isValidPageInPaymentHandlerWindow(webContents)) { if (!SslValidityChecker.isValidPageInPaymentHandlerWindow(webContents)) {
...@@ -455,6 +456,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface ...@@ -455,6 +456,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface
* @param webContents The web contents in the opened window. * @param webContents The web contents in the opened window.
*/ */
public static void onClosingPaymentAppWindowForInsecureNavigation(WebContents webContents) { public static void onClosingPaymentAppWindowForInsecureNavigation(WebContents webContents) {
if (webContents.isDestroyed()) return;
ServiceWorkerPaymentAppBridgeJni.get().onClosingPaymentAppWindow( ServiceWorkerPaymentAppBridgeJni.get().onClosingPaymentAppWindow(
webContents, PaymentEventResponseType.PAYMENT_HANDLER_INSECURE_NAVIGATION); webContents, PaymentEventResponseType.PAYMENT_HANDLER_INSECURE_NAVIGATION);
} }
...@@ -465,6 +467,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface ...@@ -465,6 +467,7 @@ public class ServiceWorkerPaymentAppBridge implements PaymentAppFactoryInterface
* @param webContents The web contents in the opened window. * @param webContents The web contents in the opened window.
*/ */
public static void onClosingPaymentAppWindow(WebContents webContents) { public static void onClosingPaymentAppWindow(WebContents webContents) {
if (webContents.isDestroyed()) return;
ServiceWorkerPaymentAppBridgeJni.get().onClosingPaymentAppWindow( ServiceWorkerPaymentAppBridgeJni.get().onClosingPaymentAppWindow(
webContents, PaymentEventResponseType.PAYMENT_HANDLER_WINDOW_CLOSING); webContents, PaymentEventResponseType.PAYMENT_HANDLER_WINDOW_CLOSING);
} }
......
...@@ -98,6 +98,7 @@ public class PaymentHandlerCoordinator { ...@@ -98,6 +98,7 @@ public class PaymentHandlerCoordinator {
bottomSheetController.hideContent(/*content=*/view, /*animate=*/true); bottomSheetController.hideContent(/*content=*/view, /*animate=*/true);
mWebContents.destroy(); mWebContents.destroy();
uiObserver.onPaymentHandlerUiClosed(); uiObserver.onPaymentHandlerUiClosed();
mediator.destroy();
}; };
return bottomSheetController.requestShowContent(view, /*animate=*/true); return bottomSheetController.requestShowContent(view, /*animate=*/true);
} }
......
...@@ -58,6 +58,12 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -58,6 +58,12 @@ import org.chromium.ui.modelutil.PropertyModel;
mPaymentHandlerUiObserver = observer; mPaymentHandlerUiObserver = observer;
} }
@Override
public void destroy() {
super.destroy(); // Stops observing the web contents and cleans up associated references.
mHandler.removeCallbacksAndMessages(null);
}
// BottomSheetObserver: // BottomSheetObserver:
@Override @Override
public void onSheetStateChanged(@SheetState int newState) { public void onSheetStateChanged(@SheetState int newState) {
......
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