[Web Payment] Check for null origin.

Before this patch, a closed frame could return a null origin, but the
payment operation continued and eventually triggered an assert that
checked for non-null origin.

This patch checks for null origin and aborts payment if that's detected.
The origin and URL of the render frame host is marked nullable for
clarity.

After this patch, if a closed frame returns a null origin, then payment
is aborted, so the assert is not triggered.

Bug: 1129578
Change-Id: I0d6404a6f425ce2e814d8d6fa177609a52d14641
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2511710
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823316}

chrome/android/java/src/org/chromium/chrome/browser/browserservices/digitalgoods/DigitalGoodsImpl.java

@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.browserservices.digitalgoods;
 
 import android.net.Uri;
 
+import androidx.annotation.Nullable;
+
 import org.chromium.mojo.system.MojoException;
 import org.chromium.payments.mojom.DigitalGoods;
 import org.chromium.payments.mojom.DigitalGoods.AcknowledgeResponse;
@@ -21,26 +23,30 @@ public class DigitalGoodsImpl implements DigitalGoods {
 
     /** A Delegate that provides the current URL. */
     public interface Delegate {
+        /** @return The current URL or null when the frame is being destroyed. */
+        @Nullable
         String getUrl();
     }
 
     /** Constructs the object with a given adapter and delegate. */
-    public DigitalGoodsImpl(DigitalGoodsAdapter adapter,
-            Delegate delegate) {
+    public DigitalGoodsImpl(DigitalGoodsAdapter adapter, Delegate delegate) {
         mAdapter = adapter;
         mDelegate = delegate;
     }
 
     @Override
     public void getDetails(String[] itemIds, GetDetailsResponse callback) {
-        mAdapter.getDetails(Uri.parse(mDelegate.getUrl()), itemIds, callback);
+        String url = mDelegate.getUrl();
+        if (url != null) mAdapter.getDetails(Uri.parse(url), itemIds, callback);
     }
 
     @Override
-    public void acknowledge(String purchaseToken, boolean makeAvailableAgain,
-            AcknowledgeResponse callback) {
-        mAdapter.acknowledge(
-                Uri.parse(mDelegate.getUrl()), purchaseToken, makeAvailableAgain,callback);
+    public void acknowledge(
+            String purchaseToken, boolean makeAvailableAgain, AcknowledgeResponse callback) {
+        String url = mDelegate.getUrl();
+        if (url != null) {
+            mAdapter.acknowledge(Uri.parse(url), purchaseToken, makeAvailableAgain, callback);
+        }
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentAppServiceBridge.java

@@ -37,7 +37,13 @@ public class PaymentAppServiceBridge implements PaymentAppFactoryInterface {
     // PaymentAppFactoryInterface implementation.
     @Override
     public void create(PaymentAppFactoryDelegate delegate) {
-        if (delegate.getParams().hasClosed()) return;
+        if (delegate.getParams().hasClosed()
+                || delegate.getParams().getRenderFrameHost().getLastCommittedURL() == null
+                || delegate.getParams().getRenderFrameHost().getLastCommittedOrigin() == null
+                || delegate.getParams().getWebContents().isDestroyed()) {
+            return;
+        }
+
         assert delegate.getParams().getPaymentRequestOrigin().equals(
                 UrlFormatter.formatUrlForSecurityDisplay(
                         delegate.getParams().getRenderFrameHost().getLastCommittedURL()));

components/payments/content/android/java/src/org/chromium/components/payments/PaymentRequestService.java

@@ -124,7 +124,6 @@ public class PaymentRequestService {
      * A test-only observer for the PaymentRequest service implementation.
      */
     public interface PaymentRequestServiceObserverForTest {
-
         /**
          * Called when an abort request was denied.
          */
@@ -214,6 +213,13 @@ public class PaymentRequestService {
         assert browserPaymentRequestFactory != null;
         assert onClosedListener != null;
 
+        if (renderFrameHost.getLastCommittedOrigin() == null
+                || renderFrameHost.getLastCommittedURL() == null) {
+            abortBeforeInstantiation(/*client=*/null, /*journeyLogger=*/null, ErrorStrings.NO_FRAME,
+                    AbortReason.INVALID_DATA_FROM_RENDERER);
+            return null;
+        }
+
         WebContents webContents = WebContentsStatics.fromRenderFrameHost(renderFrameHost);
         if (webContents == null || webContents.isDestroyed()) {
             abortBeforeInstantiation(/*client=*/null, /*journeyLogger=*/null,
@@ -334,16 +340,12 @@ public class PaymentRequestService {
             PaymentMethodData[] methodData, PaymentDetails details,
             boolean googlePayBridgeEligible) {
         mBrowserPaymentRequest = factory.createBrowserPaymentRequest(this);
-        mJourneyLogger.recordCheckoutStep(
-                org.chromium.components.payments.CheckoutFunnelStep.INITIATED);
+        mJourneyLogger.recordCheckoutStep(CheckoutFunnelStep.INITIATED);
 
         if (!UrlUtil.isOriginAllowedToUseWebPaymentApis(mWebContents.getLastCommittedUrl())) {
-            Log.d(TAG, org.chromium.components.payments.ErrorStrings.PROHIBITED_ORIGIN);
-            Log.d(TAG,
-                    org.chromium.components.payments.ErrorStrings
-                            .PROHIBITED_ORIGIN_OR_INVALID_SSL_EXPLANATION);
-            mJourneyLogger.setAborted(
-                    org.chromium.components.payments.AbortReason.INVALID_DATA_FROM_RENDERER);
+            Log.d(TAG, ErrorStrings.PROHIBITED_ORIGIN);
+            Log.d(TAG, ErrorStrings.PROHIBITED_ORIGIN_OR_INVALID_SSL_EXPLANATION);
+            mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
             mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
                     ErrorStrings.PROHIBITED_ORIGIN,
                     PaymentErrorReason.NOT_SUPPORTED_FOR_INVALID_ORIGIN_OR_SSL);
@@ -356,11 +358,8 @@ public class PaymentRequestService {
         String rejectShowErrorMessage = mDelegate.getInvalidSslCertificateErrorMessage();
         if (!TextUtils.isEmpty(rejectShowErrorMessage)) {
             Log.d(TAG, rejectShowErrorMessage);
-            Log.d(TAG,
-                    org.chromium.components.payments.ErrorStrings
-                            .PROHIBITED_ORIGIN_OR_INVALID_SSL_EXPLANATION);
-            mJourneyLogger.setAborted(
-                    org.chromium.components.payments.AbortReason.INVALID_DATA_FROM_RENDERER);
+            Log.d(TAG, ErrorStrings.PROHIBITED_ORIGIN_OR_INVALID_SSL_EXPLANATION);
+            mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
             mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(rejectShowErrorMessage,
                     PaymentErrorReason.NOT_SUPPORTED_FOR_INVALID_ORIGIN_OR_SSL);
             return false;

components/payments/content/android/java_templates/ErrorStrings.java.tmpl

@@ -57,11 +57,13 @@ public abstract class ErrorStrings {{
     public static final String MINIMAL_UI_SUPPRESSED = "Payment minimal UI suppressed.";
 
     public static final String UNATHORIZED_SERVICE_REQUEST =
-            "Caller's signuature or package name does not match invoked app's.";
+            "Caller's signature or package name does not match invoked app's.";
 
     public static final String FAIL_TO_SHOW_PAYMENT_REQUEST_UI =
         "Fails to show payment request UI. Please try again.";
 
+    public static final String NO_FRAME = "The frame that initiated payment is gone.";
+
     // Prevent instantiation.
     private ErrorStrings() {{}}
 }}

content/public/android/java/src/org/chromium/content/browser/framehost/RenderFrameHostImpl.java

@@ -4,6 +4,8 @@
 
 package org.chromium.content.browser.framehost;
 
+import androidx.annotation.Nullable;
+
 import org.chromium.base.Callback;
 import org.chromium.base.UnguessableToken;
 import org.chromium.base.annotations.CalledByNative;
@@ -74,6 +76,7 @@ public class RenderFrameHostImpl implements RenderFrameHost {
     }
 
     @Override
+    @Nullable
     public String getLastCommittedURL() {
         if (mNativeRenderFrameHostAndroid == 0) return null;
         return RenderFrameHostImplJni.get().getLastCommittedURL(
@@ -81,6 +84,7 @@ public class RenderFrameHostImpl implements RenderFrameHost {
     }
 
     @Override
+    @Nullable
     public Origin getLastCommittedOrigin() {
         if (mNativeRenderFrameHostAndroid == 0) return null;
         return RenderFrameHostImplJni.get().getLastCommittedOrigin(
@@ -134,6 +138,7 @@ public class RenderFrameHostImpl implements RenderFrameHost {
     /**
      * Return the AndroidOverlay routing token for this RenderFrameHostImpl.
      */
+    @Nullable
     public UnguessableToken getAndroidOverlayRoutingToken() {
         if (mNativeRenderFrameHostAndroid == 0) return null;
         return RenderFrameHostImplJni.get().getAndroidOverlayRoutingToken(

content/public/android/java/src/org/chromium/content_public/browser/RenderFrameHost.java

@@ -4,6 +4,8 @@
 
 package org.chromium.content_public.browser;
 
+import androidx.annotation.Nullable;
+
 import org.chromium.base.Callback;
 import org.chromium.services.service_manager.InterfaceProvider;
 import org.chromium.url.Origin;
@@ -15,8 +17,9 @@ public interface RenderFrameHost {
     /**
      * Get the last committed URL of the frame.
      *
-     * @return The last committed URL of the frame.
+     * @return The last committed URL of the frame or null when being destroyed.
      */
+    @Nullable
     String getLastCommittedURL();
 
     /**
@@ -24,8 +27,9 @@ public interface RenderFrameHost {
      * of getLastCommittedURL(), since it can be an "opaque" origin in such cases as, for example,
      * sandboxed frame.
      *
-     * @return The last committed Origin of the frame.
+     * @return The last committed Origin of the frame or null when being destroyed.
      */
+    @Nullable
     Origin getLastCommittedOrigin();
 
     /**