Redacted payment details has shipping info only if PH handles shipping.

During a payment process the merchant can change details of the
paymentRequest in response to changes in shipping address, etc. The
browser redacts the unnecessary fields from Updated payment details
before sending it to the Payment Handler.

This cl changes the redacting logic so that shipping information is
included in redacted details only when shipping is requested and the
payment handler is responsible for handling shipping address.

Bug: 984694
Change-Id: I0f5e93cb27e91565425167bfc796908c67dcd747
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1842292Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703453}

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

@@ -1242,9 +1242,11 @@ public class PaymentRequestImpl
             // opaque to Chrome sub-instruments inside, representing each card in the user account.
             // Hence Chrome forwards the updateWith() calls to the currently invoked
             // PaymentInstrument object.
+            // Todo(sahel): handlesShipping must be true when the payment handler is responsible for
+            // handling shipping. crbug.com/984694
             mInvokedPaymentInstrument.updateWith(
                     PaymentDetailsConverter.convertToPaymentMethodChangeResponse(
-                            details, this /* methodChecker */));
+                            details, false /* handlesShipping */, this /* methodChecker */));
             return;
         }
 

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

@@ -41,12 +41,14 @@ public class PaymentDetailsConverter {
      * sent to the payment handler.
      * @param details       The pre-validated payment details update from the merchant. Should not
      *                      be null.
+     * @param handlesShipping The shipping related information should get redacted when
+     *         handlesShipping is false.
      * @param methodChecker The object that can check whether the invoked payment instrument is
      *                      valid for the given payment method identifier. Should not be null.
      * @return The data structure that can be sent to the invoked payment handler.
      */
     public static PaymentMethodChangeResponse convertToPaymentMethodChangeResponse(
-            PaymentDetails details, MethodChecker methodChecker) {
+            PaymentDetails details, boolean handlesShipping, MethodChecker methodChecker) {
         // Keep in sync with components/payments/content/payment_details_converter.cc.
         assert details != null;
         assert methodChecker != null;
@@ -54,7 +56,7 @@ public class PaymentDetailsConverter {
         PaymentMethodChangeResponse response = new PaymentMethodChangeResponse();
         response.error = details.error;
         response.stringifiedPaymentMethodErrors = details.stringifiedPaymentMethodErrors;
-        response.shippingAddressErrors = details.shippingAddressErrors;
+        if (handlesShipping) response.shippingAddressErrors = details.shippingAddressErrors;
 
         if (details.total != null) response.total = details.total.amount;
 
@@ -83,7 +85,7 @@ public class PaymentDetailsConverter {
             response.modifiers = modifiers.toArray(new PaymentHandlerModifier[modifiers.size()]);
         }
 
-        if (details.shippingOptions != null) {
+        if (handlesShipping && details.shippingOptions != null) {
             ArrayList<PaymentShippingOption> options = new ArrayList<>();
             for (int i = 0; i < details.shippingOptions.length; i++) {
                 PaymentShippingOption option = new PaymentShippingOption();

components/payments/content/payment_details_converter.cc

@@ -16,6 +16,7 @@ namespace payments {
 mojom::PaymentMethodChangeResponsePtr
 PaymentDetailsConverter::ConvertToPaymentMethodChangeResponse(
     const mojom::PaymentDetailsPtr& details,
+    bool handles_shipping,
     const MethodChecker& method_checker) {
   DCHECK(details);
 
@@ -23,7 +24,7 @@ PaymentDetailsConverter::ConvertToPaymentMethodChangeResponse(
   response->error = details->error;
   response->stringified_payment_method_errors =
       details->stringified_payment_method_errors;
-  if (details->shipping_address_errors) {
+  if (handles_shipping && details->shipping_address_errors) {
     response->shipping_address_errors =
         details->shipping_address_errors.Clone();
   }
@@ -54,7 +55,7 @@ PaymentDetailsConverter::ConvertToPaymentMethodChangeResponse(
     }
   }
 
-  if (details->shipping_options) {
+  if (handles_shipping && details->shipping_options) {
     response->shipping_options = std::vector<mojom::PaymentShippingOptionPtr>();
     for (const auto& option : *details->shipping_options)
       response->shipping_options->emplace_back(option.Clone());

components/payments/content/payment_details_converter.h

@@ -24,11 +24,12 @@ class PaymentDetailsConverter {
   // call into a data structure that can be sent to the payment handler.
   //
   // The |details| should not be null.
-  //
+  // Shipping related information is redacted when |handles_shipping| is false.
   // The |method_checker| is not saved. It is used only for the duration of this
   // call.
   static mojom::PaymentMethodChangeResponsePtr
   ConvertToPaymentMethodChangeResponse(const mojom::PaymentDetailsPtr& details,
+                                       bool handles_shipping,
                                        const MethodChecker& method_checker);
 
  private:

components/payments/content/payment_request.cc

@@ -345,9 +345,10 @@ void PaymentRequest::UpdateWith(mojom::PaymentDetailsPtr details) {
       payment_handler_host_.is_changing()) {
     payment_handler_host_.UpdateWith(
         PaymentDetailsConverter::ConvertToPaymentMethodChangeResponse(
-            details, base::BindRepeating(
-                         &PaymentInstrument::IsValidForPaymentMethodIdentifier,
-                         state()->selected_instrument()->AsWeakPtr())));
+            details, state()->selected_instrument()->HandlesShippingAddress(),
+            base::BindRepeating(
+                &PaymentInstrument::IsValidForPaymentMethodIdentifier,
+                state()->selected_instrument()->AsWeakPtr())));
   }
 
   bool is_resolving_promise_passed_into_show_method = !spec_->IsInitialized();