[PRImpl] Move NativeObserverForTest

Change:
* Move NativeObserverForTest from PRImpl to CPRImpl.
* Use a visible-in-test setter to set the observer instead of passing
it from the constructor, so that ComponentPRImpl can own it (it's
PRFactory who instantiates PRImpl).
* Clean up unused variable PaymentRequestTestBridge#skipUiForBasicCard

Bug: 1107102

moving a "ForTest" class, not adding any new one.

Binary-Size: Silence the "for-test" alert because this CL is only
Change-Id: Id384208f27eaa1830288402c7dc30be8be24bebc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317835Reviewed-by: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791949}

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

@@ -40,7 +40,6 @@ import org.chromium.services.service_manager.InterfaceFactory;
 public class PaymentRequestFactory implements InterfaceFactory<PaymentRequest> {
     // Tests can inject behaviour on future PaymentRequests via these objects.
     public static PaymentRequestImpl.Delegate sDelegateForTest;
-    public static PaymentRequestImpl.NativeObserverForTest sNativeObserverForTest;
 
     private final RenderFrameHost mRenderFrameHost;
 
@@ -169,6 +168,7 @@ public class PaymentRequestFactory implements InterfaceFactory<PaymentRequest> {
 
     @Override
     public PaymentRequest createImpl() {
+        if (mRenderFrameHost == null) return new InvalidPaymentRequest();
         if (!mRenderFrameHost.isFeatureEnabled(FeaturePolicyFeature.PAYMENT)) {
             mRenderFrameHost.getRemoteInterfaces().onConnectionError(
                     new MojoException(MojoResult.PERMISSION_DENIED));
@@ -179,8 +179,6 @@ public class PaymentRequestFactory implements InterfaceFactory<PaymentRequest> {
             return new InvalidPaymentRequest();
         }
 
-        if (mRenderFrameHost == null) return new InvalidPaymentRequest();
-
         PaymentRequestImpl.Delegate delegate;
         if (sDelegateForTest != null) {
             delegate = sDelegateForTest;
@@ -188,11 +186,6 @@ public class PaymentRequestFactory implements InterfaceFactory<PaymentRequest> {
             delegate = new PaymentRequestDelegateImpl(mRenderFrameHost);
         }
 
-        return new ComponentPaymentRequestImpl(
-                new PaymentRequestImpl(mRenderFrameHost, delegate, sNativeObserverForTest));
-    }
-
-    private WebContents getWebContents() {
-        return WebContentsStatics.fromRenderFrameHost(mRenderFrameHost);
+        return new ComponentPaymentRequestImpl(new PaymentRequestImpl(mRenderFrameHost, delegate));
     }
 }

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

@@ -58,6 +58,7 @@ import org.chromium.components.page_info.CertificateChainHelper;
 import org.chromium.components.payments.AbortReason;
 import org.chromium.components.payments.ComponentPaymentRequestImpl;
 import org.chromium.components.payments.ComponentPaymentRequestImpl.ComponentPaymentRequestDelegate;
+import org.chromium.components.payments.ComponentPaymentRequestImpl.NativeObserverForTest;
 import org.chromium.components.payments.CurrencyFormatter;
 import org.chromium.components.payments.ErrorMessageUtil;
 import org.chromium.components.payments.ErrorStrings;
@@ -235,26 +236,6 @@ public class PaymentRequestImpl
         void onRendererClosedMojoConnection();
     }
 
-    /**
-     * An observer interface injected when running tests to allow them to observe events.
-     * This interface holds events that should be passed back to the native C++ test
-     * harness and mirrors the C++ PaymentRequest::ObserverForTest() interface. Its methods
-     * should be called in the same places that the C++ PaymentRequest object will call its
-     * ObserverForTest.
-     */
-    public interface NativeObserverForTest {
-        void onCanMakePaymentCalled();
-        void onCanMakePaymentReturned();
-        void onHasEnrolledInstrumentCalled();
-        void onHasEnrolledInstrumentReturned();
-        void onAppListReady(@Nullable List<EditableOption> paymentApps, PaymentItem total);
-        void onNotSupportedError();
-        void onConnectionTerminated();
-        void onAbortCalled();
-        void onCompleteCalled();
-        void onMinimalUIReady();
-    }
-
     /** Limit in the number of suggested items in a section. */
     public static final int SUGGESTIONS_LIMIT = 4;
 
@@ -314,7 +295,7 @@ public class PaymentRequestImpl
     private final Handler mHandler = new Handler();
     private final RenderFrameHost mRenderFrameHost;
     private final Delegate mDelegate;
-    private final NativeObserverForTest mNativeObserverForTest;
+    private NativeObserverForTest mNativeObserverForTest;
     private final WebContents mWebContents;
     private final String mTopLevelOrigin;
     private final String mPaymentRequestOrigin;
@@ -454,13 +435,11 @@ public class PaymentRequestImpl
      *
      * @param renderFrameHost The host of the frame that has invoked the PaymentRequest API.
      */
-    public PaymentRequestImpl(RenderFrameHost renderFrameHost, Delegate delegate,
-            NativeObserverForTest nativeObserver) {
+    public PaymentRequestImpl(RenderFrameHost renderFrameHost, Delegate delegate) {
         assert renderFrameHost != null;
 
         mRenderFrameHost = renderFrameHost;
         mDelegate = delegate;
-        mNativeObserverForTest = nativeObserver;
         mWebContents = WebContentsStatics.fromRenderFrameHost(renderFrameHost);
 
         mPaymentRequestOrigin =
@@ -502,6 +481,12 @@ public class PaymentRequestImpl
         mComponentPaymentRequestImpl = componentPaymentRequestImpl;
     }
 
+    // Implement ComponentPaymentRequestDelegate:
+    @Override
+    public void setNativeObserverForTest(NativeObserverForTest nativeObserverForTest) {
+        mNativeObserverForTest = nativeObserverForTest;
+    }
+
     // Implement ComponentPaymentRequestDelegate:
     /**
      * Called by the merchant website to initialize the payment request data.

chrome/test/android/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+components/browser_ui/android/bottomsheet",
   "+components/autofill/android/java/src/org/chromium/components/autofill",
   "+components/infobars/android/java",
+  "+components/payments/content/android",
   # This test code needs to depend on sync related classes and test tools.
   "+components/sync/android/java/src/org/chromium/components/sync/signin",
   "+components/sync/test/android/javatests/src/org/chromium/components/sync/test/util",
@@ -10,4 +11,3 @@ include_rules = [
   "+components/translate/content/android/java",
   "+components/webapk/android",
 ]
-

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

@@ -16,6 +16,8 @@ import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.payments.PaymentRequestFactory;
 import org.chromium.chrome.browser.payments.PaymentRequestImpl;
 import org.chromium.components.autofill.EditableOption;
+import org.chromium.components.payments.ComponentPaymentRequestImpl;
+import org.chromium.components.payments.ComponentPaymentRequestImpl.NativeObserverForTest;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.payments.mojom.PaymentItem;
 
@@ -36,17 +38,14 @@ public class PaymentRequestTestBridge {
         private final boolean mIsValidSsl;
         private final boolean mIsWebContentsActive;
         private final boolean mPrefsCanMakePayment;
-        private final boolean mSkipUiForBasicCard;
         private final String mTwaPackageName;
 
         PaymentRequestDelegateForTest(boolean isOffTheRecord, boolean isValidSsl,
-                boolean isWebContentsActive, boolean prefsCanMakePayment,
-                boolean skipUiForBasicCard, String twaPackageName) {
+                boolean isWebContentsActive, boolean prefsCanMakePayment, String twaPackageName) {
             mIsOffTheRecord = isOffTheRecord;
             mIsValidSsl = isValidSsl;
             mIsWebContentsActive = isWebContentsActive;
             mPrefsCanMakePayment = prefsCanMakePayment;
-            mSkipUiForBasicCard = skipUiForBasicCard;
             mTwaPackageName = twaPackageName;
         }
 
@@ -89,7 +88,7 @@ public class PaymentRequestTestBridge {
      * methods are called.
      */
     private static class PaymentRequestNativeObserverBridgeToNativeForTest
-            implements PaymentRequestImpl.NativeObserverForTest {
+            implements NativeObserverForTest {
         private final long mOnCanMakePaymentCalledPtr;
         private final long mOnCanMakePaymentReturnedPtr;
         private final long mOnHasEnrolledInstrumentCalledPtr;
@@ -199,9 +198,9 @@ public class PaymentRequestTestBridge {
             boolean isValidSsl, boolean isWebContentsActive, boolean prefsCanMakePayment,
             boolean skipUiForBasicCard, String twaPackageName) {
         if (useDelegate) {
-            PaymentRequestFactory.sDelegateForTest = new PaymentRequestDelegateForTest(
-                    isOffTheRecord, isValidSsl, isWebContentsActive, prefsCanMakePayment,
-                    skipUiForBasicCard, twaPackageName);
+            PaymentRequestFactory.sDelegateForTest =
+                    new PaymentRequestDelegateForTest(isOffTheRecord, isValidSsl,
+                            isWebContentsActive, prefsCanMakePayment, twaPackageName);
         } else {
             PaymentRequestFactory.sDelegateForTest = null;
         }
@@ -213,12 +212,12 @@ public class PaymentRequestTestBridge {
             long onHasEnrolledInstrumentReturnedPtr, long onAppListReadyPtr,
             long setAppDescriptionPtr, long onNotSupportedErrorPtr, long onConnectionTerminatedPtr,
             long onAbortCalledPtr, long onCompleteCalledPtr, long onMinimalUIReadyPtr) {
-        PaymentRequestFactory.sNativeObserverForTest =
+        ComponentPaymentRequestImpl.setNativeObserverForTest(
                 new PaymentRequestNativeObserverBridgeToNativeForTest(onCanMakePaymentCalledPtr,
                         onCanMakePaymentReturnedPtr, onHasEnrolledInstrumentCalledPtr,
                         onHasEnrolledInstrumentReturnedPtr, onAppListReadyPtr, setAppDescriptionPtr,
                         onNotSupportedErrorPtr, onConnectionTerminatedPtr, onAbortCalledPtr,
-                        onCompleteCalledPtr, onMinimalUIReadyPtr);
+                        onCompleteCalledPtr, onMinimalUIReadyPtr));
     }
 
     @CalledByNative

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

@@ -5,7 +5,9 @@
 package org.chromium.components.payments;
 
 import androidx.annotation.Nullable;
+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.PaymentMethodData;
@@ -14,6 +16,8 @@ import org.chromium.payments.mojom.PaymentRequest;
 import org.chromium.payments.mojom.PaymentRequestClient;
 import org.chromium.payments.mojom.PaymentValidationErrors;
 
+import java.util.List;
+
 /**
  * Android implementation of the PaymentRequest service defined in
  * third_party/blink/public/mojom/payments/payment_request.mojom. This component provides the parts
@@ -22,6 +26,7 @@ import org.chromium.payments.mojom.PaymentValidationErrors;
  */
 public class ComponentPaymentRequestImpl implements PaymentRequest {
     private final ComponentPaymentRequestDelegate mDelegate;
+    private static NativeObserverForTest sNativeObserverForTest;
     private PaymentRequestClient mClient;
     private PaymentRequestLifecycleObserver mPaymentRequestLifecycleObserver;
 
@@ -50,6 +55,14 @@ public class ComponentPaymentRequestImpl implements PaymentRequest {
         void setComponentPaymentRequestImpl(
                 ComponentPaymentRequestImpl componentPaymentRequestImpl);
 
+        /**
+         * Set a native side observer for the implementation of this interface, for testing purpose
+         * only.
+         * @param nativeObserverForTest The native side observer.
+         */
+        @VisibleForTesting
+        void setNativeObserverForTest(NativeObserverForTest nativeObserverForTest);
+
         /**
          * @return The JourneyLogger of PaymentRequestImpl.
          */
@@ -61,6 +74,27 @@ public class ComponentPaymentRequestImpl implements PaymentRequest {
         void disconnectFromClientWithDebugMessage(String debugMessage);
     }
 
+    /**
+     * An observer interface injected when running tests to allow them to observe events.
+     * This interface holds events that should be passed back to the native C++ test
+     * harness and mirrors the C++ PaymentRequest::ObserverForTest() interface. Its methods
+     * should be called in the same places that the C++ PaymentRequest object will call its
+     * ObserverForTest.
+     */
+    public interface NativeObserverForTest {
+        void onCanMakePaymentCalled();
+        void onCanMakePaymentReturned();
+        void onHasEnrolledInstrumentCalled();
+        void onHasEnrolledInstrumentReturned();
+        void onAppListReady(@Nullable List<EditableOption> paymentApps,
+                org.chromium.payments.mojom.PaymentItem total);
+        void onNotSupportedError();
+        void onConnectionTerminated();
+        void onAbortCalled();
+        void onCompleteCalled();
+        void onMinimalUIReady();
+    }
+
     /**
      * Build an instance of the PaymentRequest implementation.
      * @param delegate A delegate of the instance.
@@ -68,6 +102,17 @@ public class ComponentPaymentRequestImpl implements PaymentRequest {
     public ComponentPaymentRequestImpl(ComponentPaymentRequestDelegate delegate) {
         mDelegate = delegate;
         mDelegate.setComponentPaymentRequestImpl(this);
+        mDelegate.setNativeObserverForTest(sNativeObserverForTest);
+    }
+
+    /**
+     * Set a native-side observer for PaymentRequest implementations. This observer should be set
+     * before PaymentRequest implementations are instantiated.
+     * @param nativeObserverForTest The native-side observer.
+     */
+    @VisibleForTesting
+    public static void setNativeObserverForTest(NativeObserverForTest nativeObserverForTest) {
+        sNativeObserverForTest = nativeObserverForTest;
     }
 
     @Override