[WebLayer] PRService takes over disconnectFromClientWithDebugMessage()

Currently, PRService delegates disconnectFromClientWithDebugMessage()
to CPRService which in turns calls PRService#onError and
PRService#close() to close the PRClient. This is unnecessary.
Also this indicates an layering violation. Since CPRService depends on
PRService, it's more natural for CPRService to call PRService's
disconnectFromClientWithDebugMessage()  than the other way around.

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

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

@@ -10,7 +10,6 @@ import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.collection.ArrayMap;
 
-import org.chromium.base.Log;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.app.ChromeActivity;
@@ -73,8 +72,6 @@ import java.util.Set;
 public class ChromePaymentRequestService implements BrowserPaymentRequest,
                                                     PaymentDetailsConverter.MethodChecker,
                                                     PaymentUiService.Delegate, PaymentUIsObserver {
-    private static final String TAG = "PaymentRequest";
-
     /**
      * Hold the currently showing PaymentRequest. Used to prevent showing more than one
      * PaymentRequest UI per browser process.
@@ -886,19 +883,12 @@ public class ChromePaymentRequestService implements BrowserPaymentRequest,
         disconnectFromClientWithDebugMessage(debugMessage, PaymentErrorReason.USER_CANCEL);
     }
 
-    // Implement BrowserPaymentRequest:
-    // This method is not supposed to be used outside this class and
-    // PaymentRequestService.
-    @Override
-    public void disconnectFromClientWithDebugMessage(String debugMessage, int reason) {
-        Log.d(TAG, debugMessage);
+    private void disconnectFromClientWithDebugMessage(String debugMessage, int reason) {
         if (mPaymentRequestService != null) {
-            mPaymentRequestService.onError(reason, debugMessage);
-        }
-        close();
-        if (PaymentRequestService.getNativeObserverForTest() != null) {
-            PaymentRequestService.getNativeObserverForTest().onConnectionTerminated();
+            mPaymentRequestService.disconnectFromClientWithDebugMessage(debugMessage, reason);
         }
+        // Either closed before this method or closed by mPaymentRequestService.
+        assert mHasClosed;
     }
 
     // Implement BrowserPaymentRequest:

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

@@ -8,7 +8,6 @@ import androidx.annotation.Nullable;
 
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.payments.mojom.PaymentDetails;
-import org.chromium.payments.mojom.PaymentErrorReason;
 import org.chromium.payments.mojom.PaymentMethodData;
 import org.chromium.payments.mojom.PaymentOptions;
 import org.chromium.payments.mojom.PaymentRequest;
@@ -64,13 +63,6 @@ public interface BrowserPaymentRequest {
      */
     void retry(PaymentValidationErrors errors);
 
-    /**
-     * Delegate to the same method of ChromePaymentRequestService.
-     * @param debugMessage The debug message shown for web developers.
-     * @param reason The reason of the disconnection defined in {@link PaymentErrorReason}.
-     */
-    void disconnectFromClientWithDebugMessage(String debugMessage, int reason);
-
     /**
      * Close this instance. The callers of this method should stop referencing this instance upon
      * calling it. This method can be called within itself without causing infinite loop.

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

@@ -349,6 +349,22 @@ public class PaymentRequestService
         service.create(/*delegate=*/this);
     }
 
+    /**
+     * Disconnects from the PaymentRequestClient with a debug message.
+     * @param debugMessage The debug message shown for web developers.
+     * @param reason The reason of the disconnection defined in {@link PaymentErrorReason}.
+     */
+    public void disconnectFromClientWithDebugMessage(String debugMessage, int reason) {
+        Log.d(TAG, debugMessage);
+        if (mClient != null) {
+            mClient.onError(reason, debugMessage);
+        }
+        close();
+        if (sNativeObserverForTest != null) {
+            sNativeObserverForTest.onConnectionTerminated();
+        }
+    }
+
     /** Abort the request, used before this class's instantiation. */
     private static void abortBeforeInstantiation(@Nullable PaymentRequestClient client,
             @Nullable JourneyLogger journeyLogger, String debugMessage, int reason) {
@@ -421,8 +437,7 @@ public class PaymentRequestService
             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,
+            disconnectFromClientWithDebugMessage(ErrorStrings.PROHIBITED_ORIGIN,
                     PaymentErrorReason.NOT_SUPPORTED_FOR_INVALID_ORIGIN_OR_SSL);
             return false;
         }
@@ -435,7 +450,7 @@ public class PaymentRequestService
             Log.d(TAG, rejectShowErrorMessage);
             Log.d(TAG, ErrorStrings.PROHIBITED_ORIGIN_OR_INVALID_SSL_EXPLANATION);
             mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
-            mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(rejectShowErrorMessage,
+            disconnectFromClientWithDebugMessage(rejectShowErrorMessage,
                     PaymentErrorReason.NOT_SUPPORTED_FOR_INVALID_ORIGIN_OR_SSL);
             return false;
         }
@@ -447,7 +462,7 @@ public class PaymentRequestService
         mBrowserPaymentRequest.modifyMethodData(methodData);
         if (methodData == null) {
             mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
-            mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+            disconnectFromClientWithDebugMessage(
                     ErrorStrings.INVALID_PAYMENT_METHODS_OR_DATA, PaymentErrorReason.USER_CANCEL);
             return false;
         }
@@ -458,7 +473,7 @@ public class PaymentRequestService
 
         if (!PaymentValidator.validatePaymentDetails(details)) {
             mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
-            mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+            disconnectFromClientWithDebugMessage(
                     ErrorStrings.INVALID_PAYMENT_DETAILS, PaymentErrorReason.USER_CANCEL);
             return false;
         }
@@ -472,7 +487,7 @@ public class PaymentRequestService
                 methodData.values(), LocaleUtils.getDefaultLocaleString());
         if (spec.getRawTotal() == null) {
             mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
-            mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+            disconnectFromClientWithDebugMessage(
                     ErrorStrings.TOTAL_REQUIRED, PaymentErrorReason.USER_CANCEL);
             return false;
         }
@@ -485,7 +500,7 @@ public class PaymentRequestService
     @Override
     public void onPaymentResponseReady(PaymentResponse response) {
         if (!mBrowserPaymentRequest.patchPaymentResponseIfNeeded(response)) {
-            mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+            disconnectFromClientWithDebugMessage(
                     ErrorStrings.PAYMENT_APP_INVALID_RESPONSE, PaymentErrorReason.NOT_SUPPORTED);
             // Intentionally do not early-return.
         }
@@ -609,7 +624,7 @@ public class PaymentRequestService
             if (mDelegate.isOffTheRecord()) {
                 // If the user is in the OffTheRecord mode, hide the absence of their payment
                 // methods from the merchant site.
-                mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+                disconnectFromClientWithDebugMessage(
                         ErrorStrings.USER_CANCELLED, PaymentErrorReason.USER_CANCEL);
             } else {
                 if (sNativeObserverForTest != null) {
@@ -620,7 +635,7 @@ public class PaymentRequestService
                         && mSpec.getMethodData().get(MethodStrings.GOOGLE_PLAY_BILLING) != null) {
                     mRejectShowErrorMessage = ErrorStrings.APP_STORE_METHOD_ONLY_SUPPORTED_IN_TWA;
                 }
-                mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+                disconnectFromClientWithDebugMessage(
                         ErrorMessageUtil.getNotSupportedErrorMessage(mSpec.getMethodData().keySet())
                                 + (TextUtils.isEmpty(mRejectShowErrorMessage)
                                                 ? ""
@@ -652,7 +667,7 @@ public class PaymentRequestService
             sObserverForTest.onPaymentRequestServiceShowFailed();
         }
         mRejectShowErrorMessage = ErrorStrings.STRICT_BASIC_CARD_SHOW_REJECT;
-        mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
+        disconnectFromClientWithDebugMessage(
                 ErrorMessageUtil.getNotSupportedErrorMessage(mSpec.getMethodData().keySet()) + " "
                         + mRejectShowErrorMessage,
                 PaymentErrorReason.NOT_SUPPORTED);
@@ -972,8 +987,7 @@ public class PaymentRequestService
         assert mBrowserPaymentRequest != null;
 
         mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
-        mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
-                debugMessage, PaymentErrorReason.USER_CANCEL);
+        disconnectFromClientWithDebugMessage(debugMessage, PaymentErrorReason.USER_CANCEL);
     }
 
     /**

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

@@ -48,11 +48,6 @@ public class WebLayerPaymentRequestService implements BrowserPaymentRequest {
         assert false : "Not implemented yet";
     }
 
-    @Override
-    public void disconnectFromClientWithDebugMessage(String debugMessage, int reason) {
-        assert false : "Not implemented yet";
-    }
-
     @Override
     public void close() {
         assert false : "Not implemented yet";