[PRImpl] UIsManager accesses CPRImpl through PRImpl

Diagnosis for crbug.com/1123083:
It crashed when UIsManager tried to access CPRImpl#mClient which is
null after CPRImpl has closed. Since CPRImpl is not supposed to be
used after closed, UIsManager should not use PaymentUIsObserver
(implemented by CPRImpl) after CPRImpl is closed. However, the original
didn't allow CPRImpl to inform UIsManager of CPRImpl's closing, nor
should it because UIsManager is responsible for PaymentUIsObserver not
CPRImpl.

Change:
UIsManager no longer accesses CPRImpl directly. Instead, PRImpl is
used as middle man to handle it. The reason is that since CPRImpl,
PRImpl, UIsManager are structured in layers (low to high), CPRImpl
and UIsManager should not be aware of each other.

Acronyms:
CPRImpl = ComponentPaymentRequestImpl
UIsManager = PaymentUIsManager

Bug: 1123083

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

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

@@ -71,6 +71,7 @@ import org.chromium.components.payments.PaymentHandlerHost;
 import org.chromium.components.payments.PaymentOptionsUtils;
 import org.chromium.components.payments.PaymentRequestSpec;
 import org.chromium.components.payments.PaymentRequestUpdateEventListener;
+import org.chromium.components.payments.PaymentUIsObserver;
 import org.chromium.components.payments.PaymentValidator;
 import org.chromium.components.payments.Section;
 import org.chromium.components.payments.UrlUtil;
@@ -114,7 +115,7 @@ public class PaymentRequestImpl
                    PaymentApp.AbortCallback, PaymentApp.InstrumentDetailsCallback,
                    PaymentResponseHelper.PaymentResponseRequesterDelegate,
                    NormalizedAddressRequestDelegate, PaymentDetailsConverter.MethodChecker,
-                   PaymentUIsManager.Delegate {
+                   PaymentUIsManager.Delegate, PaymentUIsObserver {
     /**
      * A delegate to ask questions about the system, that allows tests to inject behaviour without
      * having to modify the entire system. This partially mirrors a similar C++
@@ -349,7 +350,7 @@ public class PaymentRequestImpl
         mComponentPaymentRequestImpl = componentPaymentRequestImpl;
         mPaymentUIsManager = new PaymentUIsManager(/*delegate=*/this,
                 /*params=*/this, mWebContents, mIsOffTheRecord, mJourneyLogger, mTopLevelOrigin,
-                mComponentPaymentRequestImpl);
+                /*observer=*/this);
         mComponentPaymentRequestImpl.registerPaymentRequestLifecycleObserver(mPaymentUIsManager);
     }
 
@@ -2209,4 +2210,11 @@ public class PaymentRequestImpl
     public static void setIsLocalCanMakePaymentQueryQuotaEnforcedForTest() {
         sIsLocalCanMakePaymentQueryQuotaEnforcedForTest = true;
     }
+
+    // Implement PaymentUIsObserver:
+    @Override
+    public void onPaymentRequestUIFaviconNotAvailable() {
+        if (mComponentPaymentRequestImpl == null) return;
+        mComponentPaymentRequestImpl.warnNoFavicon();
+    }
 }

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

@@ -40,7 +40,7 @@ import java.util.List;
  * PaymentRequestImpl. Note that the callers of the instances of this class need to close them with
  * {@link ComponentPaymentRequestImpl#close()}, after which no usage is allowed.
  */
-public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
+public class ComponentPaymentRequestImpl {
     private static final String TAG = "CompPaymentRequest";
     private static PaymentRequestServiceObserverForTest sObserverForTest;
     private static NativeObserverForTest sNativeObserverForTest;
@@ -487,7 +487,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mSkipUiForNonUrlPaymentMethodIdentifiers = true;
     }
 
-    /** Invokes {@link PaymentRequest.onPaymentMethodChange}. */
+    /** Invokes {@link PaymentRequestClient.onPaymentMethodChange}. */
     public void onPaymentMethodChange(String methodName, String stringifiedDetails) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -495,7 +495,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onPaymentMethodChange(methodName, stringifiedDetails);
     }
 
-    /** Invokes {@link PaymentRequest.onShippingAddressChange}. */
+    /** Invokes {@link PaymentRequestClient.onShippingAddressChange}. */
     public void onShippingAddressChange(PaymentAddress address) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -503,7 +503,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onShippingAddressChange(address);
     }
 
-    /** Invokes {@link PaymentRequest.onShippingOptionChange}. */
+    /** Invokes {@link PaymentRequestClient.onShippingOptionChange}. */
     public void onShippingOptionChange(String shippingOptionId) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -511,7 +511,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onShippingOptionChange(shippingOptionId);
     }
 
-    /** Invokes {@link PaymentRequest.onPayerDetailChange}. */
+    /** Invokes {@link PaymentRequestClient.onPayerDetailChange}. */
     public void onPayerDetailChange(PayerDetail detail) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -519,7 +519,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onPayerDetailChange(detail);
     }
 
-    /** Invokes {@link PaymentRequest.onPaymentResponse}. */
+    /** Invokes {@link PaymentRequestClient.onPaymentResponse}. */
     public void onPaymentResponse(PaymentResponse response) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -527,7 +527,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onPaymentResponse(response);
     }
 
-    /** Invokes {@link PaymentRequest.onError}. */
+    /** Invokes {@link PaymentRequestClient.onError}. */
     public void onError(int error, String errorMessage) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -535,7 +535,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onError(error, errorMessage);
     }
 
-    /** Invokes {@link PaymentRequest.onComplete}. */
+    /** Invokes {@link PaymentRequestClient.onComplete}. */
     public void onComplete() {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -543,7 +543,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onComplete();
     }
 
-    /** Invokes {@link PaymentRequest.onAbort}. */
+    /** Invokes {@link PaymentRequestClient.onAbort}. */
     public void onAbort(boolean abortedSuccessfully) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -551,7 +551,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onAbort(abortedSuccessfully);
     }
 
-    /** Invokes {@link PaymentRequest.onCanMakePayment}. */
+    /** Invokes {@link PaymentRequestClient.onCanMakePayment}. */
     public void onCanMakePayment(int result) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -559,7 +559,7 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onCanMakePayment(result);
     }
 
-    /** Invokes {@link PaymentRequest.onHasEnrolledInstrument}. */
+    /** Invokes {@link PaymentRequestClient.onHasEnrolledInstrument}. */
     public void onHasEnrolledInstrument(int result) {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;
@@ -567,10 +567,8 @@ public class ComponentPaymentRequestImpl implements PaymentUIsObserver {
         mClient.onHasEnrolledInstrument(result);
     }
 
-    // PaymentUIsObserver implementation.
-    @Override
-    /** Invokes {@link PaymentRequest.warnNoFavicon}. */
-    public void onPaymentRequestUIFaviconNotAvailable() {
+    /** Invokes {@link PaymentRequestClient.warnNoFavicon}. */
+    public void warnNoFavicon() {
         // Every caller should stop referencing this class once close() is called.
         assert mClient != null;