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

[ExpandablePaymentHandler] Resolve OpenWindow on WebContents created

Context:
PaymentRequestEvent.OpenWindow() promise should be resolved to be a
window client. With ExpandablePaymentHandler enabled, OpenWindow is
never resolved. This makes a payment app service worker fail to send a
message to the payment app js since it relies on the resolved
WindowClient.

Before Change:
With ExpandablePaymentHandler enabled, PaymentRequestEvent.OpenWindow()
is never resolved.

After Change:
With ExpandablePaymentHandler enabled, OpenWindow is resolved upon
initializing the WebContents of PaymentHandler.

Change:
Notify ServiceTabLaunch when Expandable PH UI has initialized
WebContents, so that ServiceTabLaunch can resolve the OpenWindow()
promise.

Bug: 1045955

Change-Id: I3fae5f90d144d27e97044f175478072743de05aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037643Reviewed-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@{#738572}
parent dc4af251
...@@ -81,9 +81,13 @@ public class ServiceTabLauncher { ...@@ -81,9 +81,13 @@ public class ServiceTabLauncher {
if (disposition == WindowOpenDisposition.NEW_POPUP) { if (disposition == WindowOpenDisposition.NEW_POPUP) {
boolean success = false; boolean success = false;
try { try {
success = PaymentHandlerCoordinator.isEnabled() if (PaymentHandlerCoordinator.isEnabled()) {
? PaymentRequestImpl.openPaymentHandlerWindow(new URI(url)) success = PaymentRequestImpl.openPaymentHandlerWindow(new URI(url),
: createPopupCustomTab(requestId, url, incognito); (webContents)
-> onWebContentsForRequestAvailable(requestId, webContents));
} else {
success = createPopupCustomTab(requestId, url, incognito);
}
} catch (URISyntaxException e) { /* Intentionally leave blank, so success is false. */ } catch (URISyntaxException e) { /* Intentionally leave blank, so success is false. */
} }
if (!success) { if (!success) {
......
...@@ -32,6 +32,7 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList; ...@@ -32,6 +32,7 @@ import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.page_info.CertificateChainHelper; import org.chromium.chrome.browser.page_info.CertificateChainHelper;
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;
import org.chromium.chrome.browser.payments.handler.PaymentHandlerCoordinator.PaymentHandlerWebContentsObserver;
import org.chromium.chrome.browser.payments.micro.MicrotransactionCoordinator; import org.chromium.chrome.browser.payments.micro.MicrotransactionCoordinator;
import org.chromium.chrome.browser.payments.ui.ContactDetailsSection; import org.chromium.chrome.browser.payments.ui.ContactDetailsSection;
import org.chromium.chrome.browser.payments.ui.LineItem; import org.chromium.chrome.browser.payments.ui.LineItem;
...@@ -1293,19 +1294,26 @@ public class PaymentRequestImpl ...@@ -1293,19 +1294,26 @@ public class PaymentRequestImpl
/** /**
* Called to open a new PaymentHandler UI on the showing PaymentRequest. * Called to open a new PaymentHandler UI on the showing PaymentRequest.
* @param url The url of the payment app to be displayed in the UI. * @param url The url of the payment app to be displayed in the UI.
* @param paymentHandlerWebContentsObserver The observer of the WebContents of the
* PaymentHandler.
* @return Whether the opening is successful. * @return Whether the opening is successful.
*/ */
public static boolean openPaymentHandlerWindow(URI url) { public static boolean openPaymentHandlerWindow(
URI url, PaymentHandlerWebContentsObserver paymentHandlerWebContentsObserver) {
return sShowingPaymentRequest != null return sShowingPaymentRequest != null
&& sShowingPaymentRequest.openPaymentHandlerWindowInternal(url); && sShowingPaymentRequest.openPaymentHandlerWindowInternal(
url, paymentHandlerWebContentsObserver);
} }
/** /**
* Called to open a new PaymentHandler UI on this PaymentRequest. * Called to open a new PaymentHandler UI on this PaymentRequest.
* @param url The url of the payment app to be displayed in the UI. * @param url The url of the payment app to be displayed in the UI.
* @param paymentHandlerWebContentsObserver The observer of the WebContents of the
* PaymentHandler.
* @return Whether the opening is successful. * @return Whether the opening is successful.
*/ */
private boolean openPaymentHandlerWindowInternal(URI url) { private boolean openPaymentHandlerWindowInternal(
URI url, PaymentHandlerWebContentsObserver paymentHandlerWebContentsObserver) {
assert mInvokedPaymentInstrument != null; assert mInvokedPaymentInstrument != null;
assert mInvokedPaymentInstrument instanceof ServiceWorkerPaymentApp; assert mInvokedPaymentInstrument instanceof ServiceWorkerPaymentApp;
assert org.chromium.chrome.browser.browserservices.Origin.create(url.toString()) assert org.chromium.chrome.browser.browserservices.Origin.create(url.toString())
...@@ -1318,7 +1326,8 @@ public class PaymentRequestImpl ...@@ -1318,7 +1326,8 @@ public class PaymentRequestImpl
mPaymentHandlerUi = new PaymentHandlerCoordinator(); mPaymentHandlerUi = new PaymentHandlerCoordinator();
ChromeActivity chromeActivity = ChromeActivity.fromWebContents(mWebContents); ChromeActivity chromeActivity = ChromeActivity.fromWebContents(mWebContents);
if (chromeActivity == null) return false; if (chromeActivity == null) return false;
return mPaymentHandlerUi.show(chromeActivity, url, mIsIncognito, /*observer=*/this); return mPaymentHandlerUi.show(chromeActivity, url, mIsIncognito,
paymentHandlerWebContentsObserver, /*uiObserver=*/this);
} }
@Override @Override
......
...@@ -44,6 +44,15 @@ public class PaymentHandlerCoordinator { ...@@ -44,6 +44,15 @@ public class PaymentHandlerCoordinator {
void onPaymentHandlerUiShown(); void onPaymentHandlerUiShown();
} }
/** Observes the WebContents of the payment-handler UI. */
public interface PaymentHandlerWebContentsObserver {
/**
* Called when the WebContents has been initialized.
* @param webContents The WebContents of the PaymentHandler.
*/
void onWebContentsInitialized(WebContents webContents);
}
/** /**
* Shows the payment-handler UI. * Shows the payment-handler UI.
* *
...@@ -51,11 +60,14 @@ public class PaymentHandlerCoordinator { ...@@ -51,11 +60,14 @@ public class PaymentHandlerCoordinator {
* @param url The url of the payment handler app, i.e., that of * @param url The url of the payment handler app, i.e., that of
* "PaymentRequestEvent.openWindow(url)". * "PaymentRequestEvent.openWindow(url)".
* @param isIncognito Whether the tab is in incognito mode. * @param isIncognito Whether the tab is in incognito mode.
* @param observer The {@link PaymentHandlerUiObserver} that observes this Payment Handler UI. * @param webContentsObserver The observer of the WebContents of the
* PaymentHandler.
* @param uiObserver 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. * @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) { PaymentHandlerWebContentsObserver webContentsObserver,
PaymentHandlerUiObserver uiObserver) {
assert mHider == null : "Already showing payment-handler UI"; assert mHider == null : "Already showing payment-handler UI";
mWebContents = WebContentsFactory.createWebContents(isIncognito, /*initiallyHidden=*/false); mWebContents = WebContentsFactory.createWebContents(isIncognito, /*initiallyHidden=*/false);
...@@ -63,11 +75,12 @@ public class PaymentHandlerCoordinator { ...@@ -63,11 +75,12 @@ public class PaymentHandlerCoordinator {
mWebContents.initialize(ChromeVersionInfo.getProductVersion(), mWebContents.initialize(ChromeVersionInfo.getProductVersion(),
ViewAndroidDelegate.createBasicDelegate(webContentView), webContentView, ViewAndroidDelegate.createBasicDelegate(webContentView), webContentView,
activity.getWindowAndroid(), WebContents.createDefaultInternalsHolder()); activity.getWindowAndroid(), WebContents.createDefaultInternalsHolder());
webContentsObserver.onWebContentsInitialized(mWebContents);
mWebContents.getNavigationController().loadUrl(new LoadUrlParams(url.toString())); mWebContents.getNavigationController().loadUrl(new LoadUrlParams(url.toString()));
PropertyModel model = new PropertyModel.Builder(PaymentHandlerProperties.ALL_KEYS).build(); PropertyModel model = new PropertyModel.Builder(PaymentHandlerProperties.ALL_KEYS).build();
PaymentHandlerMediator mediator = PaymentHandlerMediator mediator =
new PaymentHandlerMediator(model, this::hide, mWebContents, observer); new PaymentHandlerMediator(model, this::hide, mWebContents, uiObserver);
BottomSheetController bottomSheetController = activity.getBottomSheetController(); BottomSheetController bottomSheetController = activity.getBottomSheetController();
bottomSheetController.addObserver(mediator); bottomSheetController.addObserver(mediator);
mWebContents.addObserver(mediator); mWebContents.addObserver(mediator);
...@@ -84,7 +97,7 @@ public class PaymentHandlerCoordinator { ...@@ -84,7 +97,7 @@ public class PaymentHandlerCoordinator {
bottomSheetController.removeObserver(mediator); bottomSheetController.removeObserver(mediator);
bottomSheetController.hideContent(/*content=*/view, /*animate=*/true); bottomSheetController.hideContent(/*content=*/view, /*animate=*/true);
mWebContents.destroy(); mWebContents.destroy();
observer.onPaymentHandlerUiClosed(); uiObserver.onPaymentHandlerUiClosed();
}; };
return bottomSheetController.requestShowContent(view, /*animate=*/true); return bottomSheetController.requestShowContent(view, /*animate=*/true);
} }
......
...@@ -118,5 +118,20 @@ IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest, ...@@ -118,5 +118,20 @@ IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest,
GetActiveWebContents(), GetActiveWebContents(),
"launchAndWaitUntilReady('" + GetHttpPageUrl().spec() + "')")); "launchAndWaitUntilReady('" + GetHttpPageUrl().spec() + "')"));
} }
// Make sure openWindow() can be resolved into window client.
IN_PROC_BROWSER_TEST_F(ExpandablePaymentHandlerBrowserTest, WindowClientReady) {
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(true,
content::EvalJs(test_controller_.GetPaymentHandlerWebContents(),
"isWindowClientReady()"));
}
} // namespace } // namespace
} // namespace payments } // namespace payments
...@@ -38,13 +38,17 @@ self.addEventListener('paymentrequest', (evt) => { ...@@ -38,13 +38,17 @@ self.addEventListener('paymentrequest', (evt) => {
evt.respondWith(new Promise((responder) => { evt.respondWith(new Promise((responder) => {
paymentRequestResponder = responder; paymentRequestResponder = responder;
const errorString = 'open_window_failed'; const errorString = 'open_window_failed';
evt.openWindow(url).then((result) => { evt.openWindow(url)
if (!result) { .then((windowClient) => {
if (!windowClient) {
paymentRequestEvent.changePaymentMethod(methodName, { paymentRequestEvent.changePaymentMethod(methodName, {
status: errorString, status: errorString,
}); });
} else {
windowClient.postMessage('window_client_ready');
} }
}).catch((error) => { })
.catch((error) => {
paymentRequestEvent.changePaymentMethod(methodName, { paymentRequestEvent.changePaymentMethod(methodName, {
status: errorString, status: errorString,
}); });
......
...@@ -16,6 +16,8 @@ found in the LICENSE file. ...@@ -16,6 +16,8 @@ found in the LICENSE file.
</body> </body>
<script> <script>
let window_client_ready = false;
/** /**
* Insert a message to the widget called 'log'. * Insert a message to the widget called 'log'.
* @param {string} text - the text that is intended to be inserted * @param {string} text - the text that is intended to be inserted
...@@ -44,8 +46,24 @@ function cancel() { ...@@ -44,8 +46,24 @@ function cancel() {
return 'canceled'; return 'canceled';
} }
function isWindowClientReady() {
return window_client_ready;
}
window.onload = function() { window.onload = function() {
navigator.serviceWorker.controller.postMessage('app_is_ready'); navigator.serviceWorker.controller.postMessage('app_is_ready');
updateLogView('app is ready.'); updateLogView('app is ready.');
} }
navigator.serviceWorker.addEventListener('message', (evt) => {
if (!evt.data) {
updateLogView('Received an empty message');
return;
}
if (evt.data === 'window_client_ready') {
window_client_ready = true;
}
updateLogView(evt.data);
});
</script> </script>
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