[PRImpl] Move parseAndValidateDetails() from PRImpl to PRSpec

Change:
* Move parseAndValidateDetails() into PRSpec.
* Assign id in PRSpec constructor as opposed to after parsing details.

Bug: 1102522

Change-Id: Ie292c723ee26c4f6237115f6d1dc367319aa031a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391264Reviewed-by: Sahel Sharify <sahel@chromium.org>
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805862}

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

@@ -86,7 +86,6 @@ import org.chromium.url.GURL;
 import org.chromium.url.Origin;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -341,9 +340,9 @@ public class PaymentRequestImpl
             mQueryForQuota.put("basic-card-payment-options", paymentMethodData);
         }
 
-        mSpec = new PaymentRequestSpec(mPaymentOptions, methodData);
-        if (parseAndValidateDetails(details)
-                && parseAndValidateDetailsForSkipToGPayHelper(details)) {
+        mSpec = PaymentRequestSpec.createAndValidate(
+                details, mPaymentOptions, methodData, LocaleUtils.getDefaultLocaleString());
+        if (mSpec != null && parseAndValidateDetailsForSkipToGPayHelper(details)) {
             mPaymentUIsManager.updateDetailsOnPaymentRequestUI(
                     details, mSpec.getRawTotal(), mSpec.getRawLineItems());
         } else {
@@ -351,14 +350,12 @@ public class PaymentRequestImpl
             disconnectFromClientWithDebugMessage(ErrorStrings.INVALID_PAYMENT_DETAILS);
             return false;
         }
-        mSpec.createNative(details, LocaleUtils.getDefaultLocaleString());
 
         if (mSpec.getRawTotal() == null) {
             mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
             disconnectFromClientWithDebugMessage(ErrorStrings.TOTAL_REQUIRED);
             return false;
         }
-        mSpec.setId(details.id);
 
         // The first time initializations and validation of all of the parameters of {@link
         // PaymentRequestParams} should be done before {@link
@@ -840,7 +837,7 @@ public class PaymentRequestImpl
             return;
         }
 
-        if (parseAndValidateDetails(details)
+        if (mSpec.parseAndValidateDetails(details)
                 && parseAndValidateDetailsForSkipToGPayHelper(details)) {
             mPaymentUIsManager.updateDetailsOnPaymentRequestUI(
                     details, mSpec.getRawTotal(), mSpec.getRawLineItems());
@@ -889,7 +886,7 @@ public class PaymentRequestImpl
             return;
         }
 
-        if (parseAndValidateDetails(details)
+        if (mSpec.parseAndValidateDetails(details)
                 && parseAndValidateDetailsForSkipToGPayHelper(details)) {
             mPaymentUIsManager.updateDetailsOnPaymentRequestUI(
                     details, mSpec.getRawTotal(), mSpec.getRawLineItems());
@@ -986,51 +983,6 @@ public class PaymentRequestImpl
         return mSkipToGPayHelper == null || mSkipToGPayHelper.setShippingOptionIfValid(details);
     }
 
-    /**
-     * Sets the total, display line items, and shipping options based on input and returns the
-     * status boolean. That status is true for valid data, false for invalid data. If the input is
-     * invalid, disconnects from the client. Both raw and UI versions of data are updated.
-     *
-     * @param details The total, line items, and shipping options to parse, validate, and save in
-     *                member variables.
-     * @return True if the data is valid. False if the data is invalid.
-     */
-    private boolean parseAndValidateDetails(PaymentDetails details) {
-        if (!PaymentValidator.validatePaymentDetails(details)) return false;
-
-        if (details.total != null) {
-            mSpec.setRawTotal(details.total);
-        }
-
-        if (mSpec.getRawLineItems() == null || details.displayItems != null) {
-            mSpec.setRawLineItems(Collections.unmodifiableList(details.displayItems != null
-                            ? Arrays.asList(details.displayItems)
-                            : new ArrayList<>()));
-        }
-
-        if (details.modifiers != null) {
-            if (details.modifiers.length == 0) mSpec.getModifiers().clear();
-
-            for (int i = 0; i < details.modifiers.length; i++) {
-                PaymentDetailsModifier modifier = details.modifiers[i];
-                String method = modifier.methodData.supportedMethod;
-                mSpec.getModifiers().put(method, modifier);
-            }
-        }
-
-        if (details.shippingOptions != null) {
-            mSpec.setRawShippingOptions(
-                    Collections.unmodifiableList(Arrays.asList(details.shippingOptions)));
-        } else if (mSpec.getRawShippingOptions() == null) {
-            mSpec.setRawShippingOptions(Collections.unmodifiableList(new ArrayList<>()));
-        }
-
-        assert mSpec.getRawTotal() != null;
-        assert mSpec.getRawLineItems() != null;
-
-        return true;
-    }
-
     /**
      * Called to retrieve the data to show in the initial PaymentRequest UI.
      */

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

@@ -19,6 +19,9 @@ import org.chromium.payments.mojom.PaymentShippingOption;
 import org.chromium.payments.mojom.PaymentValidationErrors;
 
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -31,36 +34,40 @@ import java.util.Map;
 public class PaymentRequestSpec {
     // TODO(crbug.com/1124917): these parameters duplicate with those in payment_request_spec from
     // the blink side. We need to de-dup them.
+    private final Map<String, PaymentMethodData> mMethodData;
+    private final String mId;
     private Map<String, PaymentDetailsModifier> mModifiers = new ArrayMap<>();
-    private String mId;
     private List<PaymentShippingOption> mRawShippingOptions;
     private List<PaymentItem> mRawLineItems;
     private PaymentItem mRawTotal;
-    private final PaymentOptions mOptions;
-    private final Map<String, PaymentMethodData> mMethodData;
     private long mNativePointer;
 
     /**
-     * Creates an instance to store the information received from the renderer that invoked the
-     * Payment Request API.
+     * Creates a valid instance of PaymentRequestSpec.
+     * @param details The payment details, e.g., the total amount.
      * @param options The payment options, e.g., whether shipping is requested.
      * @param methodData The map of supported payment method identifiers and corresponding payment
-     * method specific data.
+     * @param appLocale The current application locale.
+     * @return The created instance if valid; null otherwise.
      */
-    public PaymentRequestSpec(PaymentOptions options, Map<String, PaymentMethodData> methodData) {
-        mOptions = options;
+    @Nullable
+    public static PaymentRequestSpec createAndValidate(PaymentDetails details,
+            PaymentOptions options, Map<String, PaymentMethodData> methodData, String appLocale) {
+        PaymentRequestSpec spec = new PaymentRequestSpec(details.id, methodData);
+        if (!spec.parseAndValidateDetails(details)) return null;
+        spec.createNative(details, options, appLocale);
+        return spec;
+    }
+
+    private PaymentRequestSpec(String id, Map<String, PaymentMethodData> methodData) {
+        mId = id;
         mMethodData = methodData;
     }
 
-    /**
-     * Creates an instance of native payment_request_spec.cc with the existing and the given
-     * parameters.
-     * @param details The payment details, e.g., the total amount.
-     * @param appLocale The current application locale.
-     */
-    public void createNative(PaymentDetails details, String appLocale) {
+    private void createNative(PaymentDetails details, PaymentOptions options, String appLocale) {
+        assert mNativePointer == 0;
         mNativePointer =
-                PaymentRequestSpecJni.get().create(mOptions.serialize(), details.serialize(),
+                PaymentRequestSpecJni.get().create(options.serialize(), details.serialize(),
                         MojoStructCollection.serialize(mMethodData.values()), appLocale);
     }
 
@@ -95,11 +102,6 @@ public class PaymentRequestSpec {
         return mId;
     }
 
-    /** Set the id found from PaymentDetails. */
-    public void setId(String id) {
-        mId = id;
-    }
-
     /**
      * The raw shipping options, as it was received from the website. This data is passed to the
      * payment app when the app is responsible for handling shipping address. This value is still
@@ -109,11 +111,6 @@ public class PaymentRequestSpec {
         return mRawShippingOptions;
     }
 
-    /** Set the raw shipping options found from PaymentDetails. */
-    public void setRawShippingOptions(List<PaymentShippingOption> rawShippingOptions) {
-        mRawShippingOptions = rawShippingOptions;
-    }
-
     /**
      * The raw items in the shopping cart, as they were received from the website. This data is
      * passed to the payment app. This value is still available after the instance is destroyed.
@@ -122,11 +119,6 @@ public class PaymentRequestSpec {
         return mRawLineItems;
     }
 
-    /** Set the raw payment items found in PaymentDetails. */
-    public void setRawLineItems(List<PaymentItem> rawLineItems) {
-        mRawLineItems = rawLineItems;
-    }
-
     /**
      * The raw total amount being charged, as it was received from the website. This data is passed
      * to the payment app. This value is still available after the instance is destroyed.
@@ -135,9 +127,49 @@ public class PaymentRequestSpec {
         return mRawTotal;
     }
 
-    /** Set the total property found from PaymentDetails. */
-    public void setRawTotal(PaymentItem rawTotal) {
-        mRawTotal = rawTotal;
+    /**
+     * Sets the total, display line items, and shipping options based on input and returns the
+     * status boolean. That status is true for valid data, false for invalid data. If the input is
+     * invalid, disconnects from the client. Both raw and UI versions of data are updated.
+     *
+     * @param details The total, line items, and shipping options to parse, validate, and save in
+     *                member variables.
+     * @return True if the data is valid. False if the data is invalid.
+     */
+    public boolean parseAndValidateDetails(PaymentDetails details) {
+        if (!PaymentValidator.validatePaymentDetails(details)) return false;
+
+        if (details.total != null) {
+            mRawTotal = details.total;
+        }
+
+        if (mRawLineItems == null || details.displayItems != null) {
+            mRawLineItems = Collections.unmodifiableList(details.displayItems != null
+                            ? Arrays.asList(details.displayItems)
+                            : new ArrayList<>());
+        }
+
+        if (details.modifiers != null) {
+            if (details.modifiers.length == 0) mModifiers.clear();
+
+            for (int i = 0; i < details.modifiers.length; i++) {
+                PaymentDetailsModifier modifier = details.modifiers[i];
+                String method = modifier.methodData.supportedMethod;
+                mModifiers.put(method, modifier);
+            }
+        }
+
+        if (details.shippingOptions != null) {
+            mRawShippingOptions =
+                    Collections.unmodifiableList(Arrays.asList(details.shippingOptions));
+        } else if (mRawShippingOptions == null) {
+            mRawShippingOptions = Collections.unmodifiableList(new ArrayList<>());
+        }
+
+        assert mRawTotal != null;
+        assert mRawLineItems != null;
+
+        return true;
     }
 
     /**