[PRImpl] PRImpl and CPRImpl mutual reference starts from constructor

Description:
PaymentRequestImpl(PRImpl) and ComponentPaymentRequestImpl(CPRImpl)
will have mutual reference starting from their constructors.

Change:
* Before - PRImpl was created at PRFactory#createImpl. After - PRImpl
is created at CPRImpl#init.
* Rename ComponentPaymentRequestDelegate BrowserPaymentRequest, and
make it an individual file. This gives the interface independent
semantics (no longer just part of CPRImpl) - "the browser part of the
PaymentRequest implementation"

Bug: 1102522

Change-Id: I19a5144c23f0f45a8ef7ce3d72752a2b90bef296
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340402
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796434}

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

@@ -186,6 +186,8 @@ public class PaymentRequestFactory implements InterfaceFactory<PaymentRequest> {
             delegate = new PaymentRequestDelegateImpl(mRenderFrameHost);
         }
 
-        return new ComponentPaymentRequestImpl(new PaymentRequestImpl(mRenderFrameHost, delegate));
+        return new ComponentPaymentRequestImpl((componentPaymentRequest)
+                                                       -> new PaymentRequestImpl(mRenderFrameHost,
+                                                               componentPaymentRequest, delegate));
     }
 }

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

@@ -53,9 +53,9 @@ import org.chromium.components.browser_ui.bottomsheet.BottomSheetControllerProvi
 import org.chromium.components.embedder_support.util.UrlConstants;
 import org.chromium.components.page_info.CertificateChainHelper;
 import org.chromium.components.payments.AbortReason;
+import org.chromium.components.payments.BrowserPaymentRequest;
 import org.chromium.components.payments.CanMakePaymentQuery;
 import org.chromium.components.payments.ComponentPaymentRequestImpl;
-import org.chromium.components.payments.ComponentPaymentRequestImpl.ComponentPaymentRequestDelegate;
 import org.chromium.components.payments.CurrencyFormatter;
 import org.chromium.components.payments.ErrorMessageUtil;
 import org.chromium.components.payments.ErrorStrings;
@@ -121,10 +121,9 @@ import java.util.Set;
  * living in {@link ComponentPaymentRequestImpl}.
  */
 public class PaymentRequestImpl
-        implements ComponentPaymentRequestDelegate, PaymentRequestUI.Client,
-                   PaymentAppFactoryDelegate, PaymentAppFactoryParams,
-                   PaymentRequestUpdateEventListener, PaymentApp.AbortCallback,
-                   PaymentApp.InstrumentDetailsCallback,
+        implements BrowserPaymentRequest, PaymentRequestUI.Client, PaymentAppFactoryDelegate,
+                   PaymentAppFactoryParams, PaymentRequestUpdateEventListener,
+                   PaymentApp.AbortCallback, PaymentApp.InstrumentDetailsCallback,
                    PaymentResponseHelper.PaymentResponseRequesterDelegate,
                    NormalizedAddressRequestDelegate, PaymentDetailsConverter.MethodChecker,
                    PaymentUIsManager.Delegate {
@@ -231,24 +230,16 @@ public class PaymentRequestImpl
     }
 
     private static final String TAG = "PaymentRequest";
-
-    private ComponentPaymentRequestImpl mComponentPaymentRequestImpl;
-
-    private PaymentOptions mPaymentOptions;
-    private boolean mRequestShipping;
-    private boolean mRequestPayerName;
-    private boolean mRequestPayerPhone;
-    private boolean mRequestPayerEmail;
-
     private static PaymentRequestServiceObserverForTest sObserverForTest;
     private static boolean sIsLocalCanMakePaymentQueryQuotaEnforcedForTest;
-
     /**
      * Hold the currently showing PaymentRequest. Used to prevent showing more than one
      * PaymentRequest UI per browser process.
      */
     private static PaymentRequestImpl sShowingPaymentRequest;
 
+    private final ComponentPaymentRequestImpl mComponentPaymentRequestImpl;
+
     /** Monitors changes in the TabModelSelector. */
     private final TabModelSelectorObserver mSelectorObserver = new EmptyTabModelSelectorObserver() {
         @Override
@@ -291,6 +282,14 @@ public class PaymentRequestImpl
     private final JourneyLogger mJourneyLogger;
     private final boolean mIsOffTheRecord;
 
+    private final PaymentUIsManager mPaymentUIsManager;
+
+    private PaymentOptions mPaymentOptions;
+    private boolean mRequestShipping;
+    private boolean mRequestPayerName;
+    private boolean mRequestPayerPhone;
+    private boolean mRequestPayerEmail;
+
     private boolean mIsCanMakePaymentResponsePending;
     private boolean mIsHasEnrolledInstrumentResponsePending;
     private boolean mHasEnrolledInstrumentUsesPerMethodQuota;
@@ -328,7 +327,6 @@ public class PaymentRequestImpl
     private int mShippingType;
     private boolean mIsFinishedQueryingPaymentApps;
     private List<PaymentApp> mPendingApps = new ArrayList<>();
-    private final PaymentUIsManager mPaymentUIsManager;
     private MinimalUICoordinator mMinimalUi;
     private PaymentApp mInvokedPaymentApp;
     private boolean mHideServerAutofillCards;
@@ -413,9 +411,13 @@ public class PaymentRequestImpl
      * Builds the PaymentRequest service implementation.
      *
      * @param renderFrameHost The host of the frame that has invoked the PaymentRequest API.
+     * @param componentPaymentRequestImpl The component side of the PaymentRequest implementation.
      */
-    public PaymentRequestImpl(RenderFrameHost renderFrameHost, Delegate delegate) {
+    public PaymentRequestImpl(RenderFrameHost renderFrameHost,
+            ComponentPaymentRequestImpl componentPaymentRequestImpl, Delegate delegate) {
         assert renderFrameHost != null;
+        assert componentPaymentRequestImpl != null;
+        assert delegate != null;
 
         mRenderFrameHost = renderFrameHost;
         mDelegate = delegate;
@@ -440,18 +442,10 @@ public class PaymentRequestImpl
         if (sObserverForTest != null) sObserverForTest.onPaymentRequestCreated(this);
         mPaymentUIsManager = new PaymentUIsManager(/*delegate=*/this,
                 /*params=*/this, mWebContents, mIsOffTheRecord, mJourneyLogger);
-    }
-
-    // Implement ComponentPaymentRequestDelegate:
-    @Override
-    public void setComponentPaymentRequestImpl(
-            ComponentPaymentRequestImpl componentPaymentRequestImpl) {
-        assert mComponentPaymentRequestImpl == null;
-        assert componentPaymentRequestImpl != null;
         mComponentPaymentRequestImpl = componentPaymentRequestImpl;
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called by the merchant website to initialize the payment request data.
      */
@@ -712,7 +706,7 @@ public class PaymentRequestImpl
         return true;
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called by the merchant website to show the payment request to the user.
      */
@@ -1089,7 +1083,7 @@ public class PaymentRequestImpl
                 && mInvokedPaymentApp.isValidForPaymentMethodData(methodName, null);
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called by merchant to update the shipping options and line items after the user has selected
      * their shipping address or shipping option.
@@ -1193,7 +1187,7 @@ public class PaymentRequestImpl
         }
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called when the merchant received a new shipping address, shipping option, or payment method
      * info, but did not update the payment details in response.
@@ -1545,7 +1539,7 @@ public class PaymentRequestImpl
         disconnectFromClientWithDebugMessage(ErrorStrings.USER_CANCELLED);
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     // This method is not supposed to be used outside this class and
     // ComponentPaymentRequestImpl.
     @Override
@@ -1564,7 +1558,7 @@ public class PaymentRequestImpl
         }
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called by the merchant website to abort the payment.
      */
@@ -1597,7 +1591,7 @@ public class PaymentRequestImpl
         }
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called when the merchant website has processed the payment.
      */
@@ -1639,7 +1633,7 @@ public class PaymentRequestImpl
         closeUIAndDestroyNativeObjects();
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     @Override
     public void retry(PaymentValidationErrors errors) {
         if (getClient() == null) return;
@@ -1731,7 +1725,7 @@ public class PaymentRequestImpl
         settingsLauncher.launchSettingsActivity(context);
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /** Called by the merchant website to check if the user has complete payment apps. */
     @Override
     public void canMakePayment() {
@@ -1768,7 +1762,7 @@ public class PaymentRequestImpl
         }
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /** Called by the merchant website to check if the user has complete payment instruments. */
     @Override
     public void hasEnrolledInstrument(boolean perMethodQuota) {
@@ -1830,7 +1824,7 @@ public class PaymentRequestImpl
                 || sIsLocalCanMakePaymentQueryQuotaEnforcedForTest;
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called when the renderer closes the Mojo connection.
      */
@@ -1846,7 +1840,7 @@ public class PaymentRequestImpl
         }
     }
 
-    // Implement ComponentPaymentRequestDelegate:
+    // Implement BrowserPaymentRequest:
     /**
      * Called when the Mojo connection encounters an error.
      */

components/payments/content/android/BUILD.gn

@@ -78,6 +78,7 @@ android_library("java") {
   annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
   sources = [
     "java/src/org/chromium/components/payments/Address.java",
+    "java/src/org/chromium/components/payments/BrowserPaymentRequest.java",
     "java/src/org/chromium/components/payments/CanMakePaymentQuery.java",
     "java/src/org/chromium/components/payments/ComponentPaymentRequestImpl.java",
     "java/src/org/chromium/components/payments/CurrencyFormatter.java",

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

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.components.payments;
+
+import org.chromium.mojo.system.MojoException;
+import org.chromium.payments.mojom.PaymentDetails;
+import org.chromium.payments.mojom.PaymentMethodData;
+import org.chromium.payments.mojom.PaymentOptions;
+import org.chromium.payments.mojom.PaymentValidationErrors;
+
+/**
+ * The browser part of the PaymentRequest implementation. The browser here can be either the
+ * Android Chrome browser or the WebLayer "browser".
+ */
+public interface BrowserPaymentRequest {
+    /**
+     * The browser part of the {@link PaymentRequest#init} implementation.
+     * @param methodData The supported methods specified by the merchant.
+     * @param details The payment details specified by the merchant.
+     * @param options The payment options specified by the merchant.
+     * @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
+     */
+    void init(PaymentMethodData[] methodData, PaymentDetails details, PaymentOptions options,
+            boolean googlePayBridgeEligible);
+
+    /**
+     * The browser part of the {@link PaymentRequest#show} implementation.
+     * @param isUserGesture Whether this method is triggered from a user gesture.
+     * @param waitForUpdatedDetails Whether to wait for updated details. It's true when merchant
+     *         passed in a promise into PaymentRequest.show(), so Chrome should disregard the
+     *         initial payment details and show a spinner until the promise resolves with the
+     *         correct payment details.
+     */
+    void show(boolean isUserGesture, boolean waitForUpdatedDetails);
+
+    /**
+     * The browser part of the {@link PaymentRequest#updateWith} implementation.
+     * @param details The details that the merchant provides to update the payment request.
+     */
+    void updateWith(PaymentDetails details);
+
+    /** The browser part of the {@link PaymentRequest#onPaymentDetailsNotUpdated} implementation. */
+    void onPaymentDetailsNotUpdated();
+
+    /** The browser part of the {@link PaymentRequest#abort} implementation. */
+    void abort();
+
+    /** The browser part of the {@link PaymentRequest#complete} implementation. */
+    void complete(int result);
+
+    /**
+     * The browser part of the {@link PaymentRequest#retry} implementation.
+     * @param errors The merchant-defined error message strings, which are used to indicate to the
+     *         end-user that something is wrong with the data of the payment response.
+     */
+    void retry(PaymentValidationErrors errors);
+
+    /**
+     * The browser part of the {@link PaymentRequest#hasEnrolledInstrument} implementation.
+     * @param perMethodQuota Whether to query with per-method quota.
+     */
+    void hasEnrolledInstrument(boolean perMethodQuota);
+
+    /** The browser part of the {@link PaymentRequest#canMakePayment} implementation. */
+    void canMakePayment();
+
+    /** The browser part of the {@link PaymentRequest#close} implementation. */
+    void close();
+
+    /**
+     * The browser part of the {@link PaymentRequest#onConnectionError} implementation.
+     * @param e The detail of the error.
+     */
+    void onConnectionError(MojoException e);
+
+    /** @return The JourneyLogger of PaymentRequestImpl. */
+    JourneyLogger getJourneyLogger();
+
+    /** Delegate to the same method of PaymentRequestImpl. */
+    void disconnectFromClientWithDebugMessage(String debugMessage);
+}

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

@@ -10,6 +10,7 @@ import androidx.annotation.VisibleForTesting;
 import org.chromium.components.autofill.EditableOption;
 import org.chromium.mojo.system.MojoException;
 import org.chromium.payments.mojom.PaymentDetails;
+import org.chromium.payments.mojom.PaymentItem;
 import org.chromium.payments.mojom.PaymentMethodData;
 import org.chromium.payments.mojom.PaymentOptions;
 import org.chromium.payments.mojom.PaymentRequest;
@@ -25,45 +26,22 @@ import java.util.List;
  * org.chromium.chrome.browser.payments.PaymentRequestImpl.
  */
 public class ComponentPaymentRequestImpl implements PaymentRequest {
-    private final ComponentPaymentRequestDelegate mDelegate;
     private static NativeObserverForTest sNativeObserverForTest;
+    private final BrowserPaymentRequestFactory mBrowserPaymentRequestFactory;
+    private BrowserPaymentRequest mBrowserPaymentRequest;
     private PaymentRequestClient mClient;
     private PaymentRequestLifecycleObserver mPaymentRequestLifecycleObserver;
 
+    /** The factory that creates an instance of {@link BrowserPaymentRequest}. */
+    public interface BrowserPaymentRequestFactory {
         /**
-     * The delegate of {@link ComponentPaymentRequestImpl}.
+         * Create an instance of {@link BrowserPaymentRequest}, and have it working together with an
+         * instance of {@link ComponentPaymentRequestImpl}.
+         * @param componentPaymentRequestImpl The ComponentPaymentRequestImpl to work together with
+         *         the BrowserPaymentRequest instance.
          */
-    public interface ComponentPaymentRequestDelegate {
-        // The implementation of the same methods in {@link PaymentRequest).
-        void init(PaymentMethodData[] methodData, PaymentDetails details, PaymentOptions options,
-                boolean googlePayBridgeEligible);
-        void show(boolean isUserGesture, boolean waitForUpdatedDetails);
-        void updateWith(PaymentDetails details);
-        void onPaymentDetailsNotUpdated();
-        void abort();
-        void complete(int result);
-        void retry(PaymentValidationErrors errors);
-        void hasEnrolledInstrument(boolean perMethodQuota);
-        void canMakePayment();
-        void close();
-        void onConnectionError(MojoException e);
-
-        /**
-         * Set a weak reference to the client of this delegate.
-         * @param componentPaymentRequestImpl The client of this delegate.
-         */
-        void setComponentPaymentRequestImpl(
+        BrowserPaymentRequest createBrowserPaymentRequest(
                 ComponentPaymentRequestImpl componentPaymentRequestImpl);
-
-        /**
-         * @return The JourneyLogger of PaymentRequestImpl.
-         */
-        JourneyLogger getJourneyLogger();
-
-        /**
-         * Delegate to the same method of PaymentRequestImpl.
-         */
-        void disconnectFromClientWithDebugMessage(String debugMessage);
     }
 
     /**
@@ -78,8 +56,7 @@ public class ComponentPaymentRequestImpl implements PaymentRequest {
         void onCanMakePaymentReturned();
         void onHasEnrolledInstrumentCalled();
         void onHasEnrolledInstrumentReturned();
-        void onAppListReady(@Nullable List<EditableOption> paymentApps,
-                org.chromium.payments.mojom.PaymentItem total);
+        void onAppListReady(@Nullable List<EditableOption> paymentApps, PaymentItem total);
         void onNotSupportedError();
         void onConnectionTerminated();
         void onAbortCalled();
@@ -89,11 +66,11 @@ public class ComponentPaymentRequestImpl implements PaymentRequest {
 
     /**
      * Build an instance of the PaymentRequest implementation.
-     * @param delegate A delegate of the instance.
+     * @param browserPaymentRequestFactory The factory that generates an instance of
+     *         BrowserPaymentRequest to work with this ComponentPaymentRequestImpl instance.
      */
-    public ComponentPaymentRequestImpl(ComponentPaymentRequestDelegate delegate) {
-        mDelegate = delegate;
-        mDelegate.setComponentPaymentRequestImpl(this);
+    public ComponentPaymentRequestImpl(BrowserPaymentRequestFactory browserPaymentRequestFactory) {
+        mBrowserPaymentRequestFactory = browserPaymentRequestFactory;
     }
 
     /**
@@ -115,75 +92,76 @@ public class ComponentPaymentRequestImpl implements PaymentRequest {
     @Override
     public void init(PaymentRequestClient client, PaymentMethodData[] methodData,
             PaymentDetails details, PaymentOptions options, boolean googlePayBridgeEligible) {
+        mBrowserPaymentRequest = mBrowserPaymentRequestFactory.createBrowserPaymentRequest(this);
+        assert mBrowserPaymentRequest != null;
         if (mClient != null) {
-            mDelegate.getJourneyLogger().setAborted(
-                    org.chromium.components.payments.AbortReason.INVALID_DATA_FROM_RENDERER);
-            mDelegate.disconnectFromClientWithDebugMessage(
-                    org.chromium.components.payments.ErrorStrings.ATTEMPTED_INITIALIZATION_TWICE);
+            mBrowserPaymentRequest.getJourneyLogger().setAborted(
+                    AbortReason.INVALID_DATA_FROM_RENDERER);
+            mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+                    ErrorStrings.ATTEMPTED_INITIALIZATION_TWICE);
             return;
         }
 
         if (client == null) {
-            mDelegate.getJourneyLogger().setAborted(
-                    org.chromium.components.payments.AbortReason.INVALID_DATA_FROM_RENDERER);
-            mDelegate.disconnectFromClientWithDebugMessage(
-                    org.chromium.components.payments.ErrorStrings.INVALID_STATE);
+            mBrowserPaymentRequest.getJourneyLogger().setAborted(
+                    AbortReason.INVALID_DATA_FROM_RENDERER);
+            mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(ErrorStrings.INVALID_STATE);
             return;
         }
 
         mClient = client;
 
-        mDelegate.init(methodData, details, options, googlePayBridgeEligible);
+        mBrowserPaymentRequest.init(methodData, details, options, googlePayBridgeEligible);
     }
 
     @Override
     public void show(boolean isUserGesture, boolean waitForUpdatedDetails) {
-        mDelegate.show(isUserGesture, waitForUpdatedDetails);
+        mBrowserPaymentRequest.show(isUserGesture, waitForUpdatedDetails);
     }
 
     @Override
     public void updateWith(PaymentDetails details) {
-        mDelegate.updateWith(details);
+        mBrowserPaymentRequest.updateWith(details);
     }
 
     @Override
     public void onPaymentDetailsNotUpdated() {
-        mDelegate.onPaymentDetailsNotUpdated();
+        mBrowserPaymentRequest.onPaymentDetailsNotUpdated();
     }
 
     @Override
     public void abort() {
-        mDelegate.abort();
+        mBrowserPaymentRequest.abort();
     }
 
     @Override
     public void complete(int result) {
-        mDelegate.complete(result);
+        mBrowserPaymentRequest.complete(result);
     }
 
     @Override
     public void retry(PaymentValidationErrors errors) {
-        mDelegate.retry(errors);
+        mBrowserPaymentRequest.retry(errors);
     }
 
     @Override
     public void canMakePayment() {
-        mDelegate.canMakePayment();
+        mBrowserPaymentRequest.canMakePayment();
     }
 
     @Override
     public void hasEnrolledInstrument(boolean perMethodQuota) {
-        mDelegate.hasEnrolledInstrument(perMethodQuota);
+        mBrowserPaymentRequest.hasEnrolledInstrument(perMethodQuota);
     }
 
     @Override
     public void close() {
-        mDelegate.close();
+        mBrowserPaymentRequest.close();
     }
 
     @Override
     public void onConnectionError(MojoException e) {
-        mDelegate.onConnectionError(e);
+        mBrowserPaymentRequest.onConnectionError(e);
     }
 
     /**

third_party/blink/public/mojom/payments/payment_request.mojom

@@ -218,10 +218,17 @@ interface PaymentRequest {
        [EnableIf=is_android] bool google_pay_bridge_eligible);
 
   // Shows the user interface with the payment details.
+  // |is_user_gesture|: Whether the show is triggered from a user gesture.
+  // |wait_for_updated_details|: It's true when merchant passed in a promise
+  // into PaymentRequest.show(), so Chrome should disregard the initial payment
+  // details and show a spinner until the promise resolves with the correct
+  // payment details.
   Show(bool is_user_gesture, bool wait_for_updated_details);
 
   // Updates the payment details in response to new shipping address or shipping
   // option.
+  // |details|: The details that the merchant provides to update the payment
+  // request.
   UpdateWith(PaymentDetails details);
 
   // Called when the merchant received a new shipping address or shipping