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

[PaymentHandler] Avoid destroy FrameTreeNode in WebContents' methods

Context: In the expandable PaymentHandler UI, users who click on a http
link in a payment app get a crash. The cause is that the WebContents'
didFinishLoad callback destroy FrameTreeNode when http content is
detected. Since the subsequent execution of WebContents still relies on
FrameTreeNode, it causes a null pointer crash afterwards.

Before change: users click on a http link in a payment app get a crash.

After change: no crash in the same scenario any more.

Change: when closing PH UI is needed in WebContentsObserver callbacks,
postpone the closing until the main thread is free.

Bug: 1044077

Change-Id: If574cb9b23f4ff6b176f49daa8fcb47b25572ad3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2013440
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733761}
parent ec45afcc
...@@ -26,6 +26,7 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -26,6 +26,7 @@ import org.chromium.ui.modelutil.PropertyModel;
/* package */ class PaymentHandlerMediator /* package */ class PaymentHandlerMediator
extends WebContentsObserver implements BottomSheetObserver, PaymentHandlerToolbarObserver { extends WebContentsObserver implements BottomSheetObserver, PaymentHandlerToolbarObserver {
private final PropertyModel mModel; private final PropertyModel mModel;
// Whenever invoked, invoked outside of the WebContentsObserver callbacks.
private final Runnable mHider; private final Runnable mHider;
// Postfixed with "Ref" to distinguish from mWebContent in WebContentsObserver. Although // Postfixed with "Ref" to distinguish from mWebContent in WebContentsObserver. Although
// referencing the same object, mWebContentsRef is preferable to WebContents here because // referencing the same object, mWebContentsRef is preferable to WebContents here because
...@@ -33,6 +34,8 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -33,6 +34,8 @@ import org.chromium.ui.modelutil.PropertyModel;
// null. // null.
private final WebContents mWebContentsRef; private final WebContents mWebContentsRef;
private final PaymentHandlerUiObserver mPaymentHandlerUiObserver; private final PaymentHandlerUiObserver mPaymentHandlerUiObserver;
// Used to postpone execution of a callback to avoid destroy objects (e.g., WebContents) in
// their own methods.
private final Handler mHandler = new Handler(); private final Handler mHandler = new Handler();
/** /**
...@@ -54,6 +57,14 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -54,6 +57,14 @@ import org.chromium.ui.modelutil.PropertyModel;
mPaymentHandlerUiObserver = observer; mPaymentHandlerUiObserver = observer;
} }
private void closeUIForInsecureNavigation() {
mHandler.post(() -> {
ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindowForInsecureNavigation(
mWebContentsRef);
mHider.run();
});
}
// BottomSheetObserver: // BottomSheetObserver:
@Override @Override
public void onSheetStateChanged(@SheetState int newState) { public void onSheetStateChanged(@SheetState int newState) {
...@@ -91,25 +102,23 @@ import org.chromium.ui.modelutil.PropertyModel; ...@@ -91,25 +102,23 @@ import org.chromium.ui.modelutil.PropertyModel;
@Override @Override
public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) { public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
if (!SslValidityChecker.isValidPageInPaymentHandlerWindow(mWebContentsRef)) { if (!SslValidityChecker.isValidPageInPaymentHandlerWindow(mWebContentsRef)) {
ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindowForInsecureNavigation( closeUIForInsecureNavigation();
mWebContentsRef);
mHandler.post(mHider);
} }
} }
@Override @Override
public void didAttachInterstitialPage() { public void didAttachInterstitialPage() {
ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindowForInsecureNavigation( closeUIForInsecureNavigation();
mWebContentsRef);
mHandler.post(mHider);
} }
@Override @Override
public void didFailLoad( public void didFailLoad(
boolean isMainFrame, int errorCode, String description, String failingUrl) { boolean isMainFrame, int errorCode, String description, String failingUrl) {
// TODO(crbug.com/1017926): Respond to service worker with the net error. mHandler.post(() -> {
ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindow(mWebContentsRef); // TODO(crbug.com/1017926): Respond to service worker with the net error.
mHandler.post(mHider); ServiceWorkerPaymentAppBridge.onClosingPaymentAppWindow(mWebContentsRef);
mHider.run();
});
} }
// PaymentHandlerToolbarObserver: // PaymentHandlerToolbarObserver:
......
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