[WebLayer] Simplify PRService's init logic

Context:
This CL is to simplify the PRService's init logic and the dependency
relationship.

Before the CL,
* PRService creates MojoPRGateKeeper (in a static method) and
  MojoPRGateKeeper creates PRService (constructor). It formed a
  circular dependency loop.
* PRService's init procedure is over complicated as it includes
  multiple similar methods which confused code readers as to where to
  put certain validation codes. The init procedure includes:
 - createPaymentRequest
 - createIfParamsValid
 - PaymentRequestService's constructor
 - initAndValidate

After the CL,
* CPRFactory & WLPRFactory creates MojoPRGateKeeper, and
  MojoPRGateKeeper creates PRService. PaymentRequestService delegates
  CPRFactory/WLPRFactory to create CPRServic/WLPRService. The
  dependency relationship becomes more clear.
* PRService's init procedure get simplified, as it includes:
 - PaymentRequestService's constructor
 - init
 - initAndValidate (will be combined into init in follow-up CL)

Bug: 1157847
Change-Id: I083e0e162085e27736577d433caf952c59592207
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2592190Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837184}

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

@@ -10,7 +10,9 @@ import androidx.annotation.VisibleForTesting;
 import org.chromium.chrome.browser.app.ChromeActivity;
 import org.chromium.chrome.browser.preferences.Pref;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.components.payments.BrowserPaymentRequest;
 import org.chromium.components.payments.InvalidPaymentRequest;
+import org.chromium.components.payments.MojoPaymentRequestGateKeeper;
 import org.chromium.components.payments.OriginSecurityChecker;
 import org.chromium.components.payments.PaymentFeatureList;
 import org.chromium.components.payments.PaymentRequestService;
@@ -64,6 +66,12 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ
             mRenderFrameHost = renderFrameHost;
         }
 
+        @Override
+        public BrowserPaymentRequest createBrowserPaymentRequest(
+                PaymentRequestService paymentRequestService) {
+            return new ChromePaymentRequestService(paymentRequestService, this);
+        }
+
         @Override
         public boolean isOffTheRecord() {
             WebContents liveWebContents =
@@ -162,8 +170,8 @@ public class ChromePaymentRequestFactory implements InterfaceFactory<PaymentRequ
         WebContents webContents = WebContentsStatics.fromRenderFrameHost(mRenderFrameHost);
         if (webContents == null || webContents.isDestroyed()) return new InvalidPaymentRequest();
 
-        return PaymentRequestService.createPaymentRequest(mRenderFrameHost, delegate,
-                (paymentRequestService)
-                        -> new ChromePaymentRequestService(paymentRequestService, delegate));
+        return new MojoPaymentRequestGateKeeper(
+                (client, onClosed)
+                        -> new PaymentRequestService(mRenderFrameHost, client, onClosed, delegate));
     }
 }

chrome/test/android/test_support/src/org/chromium/chrome/test_support/PaymentRequestTestBridge.java

@@ -14,6 +14,7 @@ import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chrome.browser.payments.ChromePaymentRequestFactory;
 import org.chromium.chrome.browser.payments.ChromePaymentRequestService;
 import org.chromium.components.autofill.EditableOption;
+import org.chromium.components.payments.BrowserPaymentRequest;
 import org.chromium.components.payments.PaymentApp;
 import org.chromium.components.payments.PaymentRequestService;
 import org.chromium.components.payments.PaymentRequestService.NativeObserverForTest;
@@ -49,6 +50,12 @@ public class PaymentRequestTestBridge {
         public boolean skipUiForBasicCard() {
             return mSkipUiForBasicCard;
         }
+
+        @Override
+        public BrowserPaymentRequest createBrowserPaymentRequest(
+                PaymentRequestService paymentRequestService) {
+            return new ChromePaymentRequestService(paymentRequestService, this);
+        }
     }
 
     /**
@@ -56,7 +63,8 @@ public class PaymentRequestTestBridge {
      * answers about the state of the system, in order to control which paths should be tested in
      * the ChromePaymentRequestService.
      */
-    private static class PaymentRequestDelegateForTest implements PaymentRequestService.Delegate {
+    private abstract static class PaymentRequestDelegateForTest
+            implements PaymentRequestService.Delegate {
         private final boolean mIsOffTheRecord;
         private final boolean mIsValidSsl;
         private final boolean mPrefsCanMakePayment;

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

@@ -25,18 +25,6 @@ import java.util.Map;
  * Android Chrome browser or the WebLayer "browser".
  */
 public interface BrowserPaymentRequest {
-    /** The factory that creates an instance of {@link BrowserPaymentRequest}. */
-    interface Factory {
-        /**
-         * Create an instance of {@link BrowserPaymentRequest}.
-         * @param paymentRequestService The PaymentRequestService to work together with
-         *         the BrowserPaymentRequest instance, cannot be null.
-         * @return An instance of BrowserPaymentRequest, cannot be null.
-         */
-        BrowserPaymentRequest createBrowserPaymentRequest(
-                PaymentRequestService paymentRequestService);
-    }
-
     /**
      * The client of the interface calls this when it has received the payment details update
      * from the merchant and has updated the PaymentRequestSpec.

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

@@ -16,39 +16,29 @@ import org.chromium.payments.mojom.PaymentValidationErrors;
  * Guards against invalid mojo parameters and enforces correct call sequence from mojo IPC in the
  * untrusted renderer, so PaymentRequestService does not have to.
  */
-/* package */ class MojoPaymentRequestGateKeeper implements PaymentRequest {
-    private final PaymentRequestServiceCreator mPaymentRequestServiceCreator;
+public class MojoPaymentRequestGateKeeper implements PaymentRequest {
+    private final Delegate mDelegate;
     private PaymentRequestService mPaymentRequestService;
 
-    /** The creator of PaymentRequestService. */
-    /* package */ interface PaymentRequestServiceCreator {
+    /** The delegate of the class. */
+    public interface Delegate {
         /**
-         * Create an instance of PaymentRequestService if the parameters are valid.
+         * Create an instance of PaymentRequestService.
          * @param client The client of the renderer PaymentRequest, need validation before usage.
-         * @param methodData The supported methods specified by the merchant, need validation before
-         *         usage.
-         * @param details The payment details specified by the merchant, need validation before
-         *         usage.
-         * @param options The payment options specified by the merchant, need validation before
-         *         usage.
-         * @param googlePayBridgeEligible True when the renderer process deems the current request
-         *         eligible for the skip-to-GPay experimental flow. It is ultimately up to the
-         * browser process to determine whether to trigger it.
          * @param onClosedListener The listener that's invoked when PaymentRequestService has
          *         just closed.
          * @return The created instance, if the parameters are valid; otherwise, null.
          */
-        PaymentRequestService createPaymentRequestServiceIfParamsValid(PaymentRequestClient client,
-                PaymentMethodData[] methodData, PaymentDetails details, PaymentOptions options,
-                boolean googlePayBridgeEligible, Runnable onClosedListener);
+        PaymentRequestService createPaymentRequestService(
+                PaymentRequestClient client, Runnable onClosedListener);
     }
 
     /**
      * Create an instance of MojoPaymentRequestGateKeeper.
-     * @param creator The creator of PaymentRequestService.
+     * @param delegate The delegate of the instance.
      */
-    /* package */ MojoPaymentRequestGateKeeper(PaymentRequestServiceCreator creator) {
-        mPaymentRequestServiceCreator = creator;
+    public MojoPaymentRequestGateKeeper(Delegate delegate) {
+        mDelegate = delegate;
     }
 
     // Implement PaymentRequest:
@@ -62,11 +52,10 @@ import org.chromium.payments.mojom.PaymentValidationErrors;
             return;
         }
 
-        // Note that a null value would be assigned when the params are invalid.
-        mPaymentRequestService =
-                mPaymentRequestServiceCreator.createPaymentRequestServiceIfParamsValid(client,
-                        methodData, details, options, googlePayBridgeEligible,
-                        this::onPaymentRequestServiceClosed);
+        PaymentRequestService service =
+                mDelegate.createPaymentRequestService(client, this::onPaymentRequestServiceClosed);
+        if (!service.init(methodData, details, options, googlePayBridgeEligible)) return;
+        mPaymentRequestService = service;
     }
 
     private void onPaymentRequestServiceClosed() {

components/payments/content/android/junit/src/org/chromium/components/payments/PaymentRequestServiceBuilder.java

@@ -8,7 +8,6 @@ import androidx.annotation.Nullable;
 
 import org.mockito.Mockito;
 
-import org.chromium.components.payments.BrowserPaymentRequest.Factory;
 import org.chromium.components.payments.PaymentRequestService.Delegate;
 import org.chromium.content_public.browser.RenderFrameHost;
 import org.chromium.content_public.browser.WebContents;
@@ -31,7 +30,7 @@ public class PaymentRequestServiceBuilder implements PaymentRequestService.Deleg
     private final RenderFrameHost mRenderFrameHost;
     private final Runnable mOnClosedListener;
     private final PaymentAppService mPaymentAppService;
-    private final Factory mBrowserPaymentRequestFactory;
+    private final BrowserPaymentRequest mBrowserPaymentRequest;
     private WebContents mWebContents;
     private boolean mIsOffTheRecord = true;
     private PaymentRequestClient mClient;
@@ -84,12 +83,18 @@ public class PaymentRequestServiceBuilder implements PaymentRequestService.Deleg
         Mockito.doReturn(total).when(mSpec).getRawTotal();
         Map<String, PaymentMethodData> methodDataMap = new HashMap<>();
         Mockito.doReturn(methodDataMap).when(mSpec).getMethodData();
-        mBrowserPaymentRequestFactory = (paymentRequestService) -> browserPaymentRequest;
+        mBrowserPaymentRequest = browserPaymentRequest;
         mOnClosedListener = onClosedListener;
         mClient = client;
         mPaymentAppService = appService;
     }
 
+    @Override
+    public BrowserPaymentRequest createBrowserPaymentRequest(
+            PaymentRequestService paymentRequestService) {
+        return mBrowserPaymentRequest;
+    }
+
     @Override
     public boolean isOffTheRecord() {
         return mIsOffTheRecord;
@@ -258,8 +263,9 @@ public class PaymentRequestServiceBuilder implements PaymentRequestService.Deleg
     }
 
     /* package */ PaymentRequestService build() {
-        return PaymentRequestService.createIfParamsValid(mRenderFrameHost,
-                mBrowserPaymentRequestFactory, mClient, mMethodData, mDetails, mOptions,
-                mGooglePayBridgeEligible, mOnClosedListener, mDelegate);
+        PaymentRequestService service =
+                new PaymentRequestService(mRenderFrameHost, mClient, mOnClosedListener, mDelegate);
+        boolean success = service.init(mMethodData, mDetails, mOptions, mGooglePayBridgeEligible);
+        return success ? service : null;
     }
 }

weblayer/browser/java/org/chromium/weblayer_private/payments/WebLayerPaymentRequestFactory.java

@@ -7,7 +7,9 @@ package org.chromium.weblayer_private.payments;
 import androidx.annotation.Nullable;
 
 import org.chromium.components.embedder_support.browser_context.BrowserContextHandle;
+import org.chromium.components.payments.BrowserPaymentRequest;
 import org.chromium.components.payments.InvalidPaymentRequest;
+import org.chromium.components.payments.MojoPaymentRequestGateKeeper;
 import org.chromium.components.payments.OriginSecurityChecker;
 import org.chromium.components.payments.PaymentFeatureList;
 import org.chromium.components.payments.PaymentRequestService;
@@ -42,6 +44,12 @@ public class WebLayerPaymentRequestFactory implements InterfaceFactory<PaymentRe
             mRenderFrameHost = renderFrameHost;
         }
 
+        @Override
+        public BrowserPaymentRequest createBrowserPaymentRequest(
+                PaymentRequestService paymentRequestService) {
+            return new WebLayerPaymentRequestService(paymentRequestService, this);
+        }
+
         @Override
         public boolean isOffTheRecord() {
             ProfileImpl profile = getProfile();
@@ -112,9 +120,8 @@ public class WebLayerPaymentRequestFactory implements InterfaceFactory<PaymentRe
 
         WebContents webContents = WebContentsStatics.fromRenderFrameHost(mRenderFrameHost);
         if (webContents == null || webContents.isDestroyed()) return new InvalidPaymentRequest();
-
-        return PaymentRequestService.createPaymentRequest(mRenderFrameHost, delegate,
-                (paymentRequestService)
-                        -> new WebLayerPaymentRequestService(paymentRequestService, delegate));
+        return new MojoPaymentRequestGateKeeper(
+                (client, onClosed)
+                        -> new PaymentRequestService(mRenderFrameHost, client, onClosed, delegate));
     }
 }