[Payments] Normalize addresses before passing them to merchants.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
index ae353c0e2cf68353cfe9a94a62c9b88d6fb5662c..cd182042af41cf90e44e24860ce9673aac11569f 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
@@ -17,6 +17,7 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.autofill.PersonalDataManager;
 import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
+import org.chromium.chrome.browser.autofill.PersonalDataManager.NormalizedAddressRequestDelegate;
 import org.chromium.chrome.browser.favicon.FaviconHelper;
 import org.chromium.chrome.browser.payments.ui.Completable;
 import org.chromium.chrome.browser.payments.ui.LineItem;
@@ -67,7 +68,8 @@ import java.util.Set;
  * third_party/WebKit/public/platform/modules/payments/payment_request.mojom.
  */
 public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Client,
-        PaymentApp.InstrumentsCallback, PaymentInstrument.DetailsCallback {
+        PaymentApp.InstrumentsCallback, PaymentInstrument.DetailsCallback,
+        NormalizedAddressRequestDelegate {
     /**
      * Observer to be notified when PaymentRequest UI has been dismissed.
      */
@@ -107,6 +109,7 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     private static final String TAG = "cr_PaymentRequest";
     private static final String ANDROID_PAY_METHOD_NAME = "https://android.com/pay";
     private static final int SUGGESTIONS_LIMIT = 4;
+    private static final int NORMALIZATION_TIMEOUT_MS = 5000;
     private static final Comparator<Completable> COMPLETENESS_COMPARATOR =
             new Comparator<Completable>() {
                 @Override
@@ -189,6 +192,9 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     /** True if show() was called. */
     private boolean mIsShowing;
 
+    private boolean mIsWaitingForNormalization;
+    private PaymentResponse mPendingPaymentResponse;
+
     /**
      * Builds the PaymentRequest service implementation.
      *
@@ -290,6 +296,16 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
             // Limit the number of suggestions.
             addresses = addresses.subList(0, Math.min(addresses.size(), SUGGESTIONS_LIMIT));
 
+            // Load the validation rules for each unique region code.
+            Set<String> uniqueCountryCodes = new HashSet<>();
+            for (int i = 0; i < addresses.size(); ++i) {
+                String countryCode = AutofillAddress.getCountryCode(addresses.get(i).getProfile());
+                if (!uniqueCountryCodes.contains(countryCode)) {
+                    uniqueCountryCodes.add(countryCode);
+                    PersonalDataManager.getInstance().loadRulesForRegion(countryCode);
+                }
+            }
+
             // Automatically select the first address if one is complete and if the merchant does
             // not require a shipping address to calculate shipping costs.
             int firstCompleteAddressIndex = SectionInformation.NO_SELECTION;
@@ -1013,22 +1029,6 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
             }
         }
 
-        if (mShippingAddressesSection != null) {
-            PaymentOption selectedShippingAddress = mShippingAddressesSection.getSelectedItem();
-            if (selectedShippingAddress != null) {
-                // Shipping addresses are created in show(). These should all be instances of
-                // AutofillAddress.
-                assert selectedShippingAddress instanceof AutofillAddress;
-                AutofillAddress selectedAutofillAddress = (AutofillAddress) selectedShippingAddress;
-
-                // Record the use of the profile.
-                PersonalDataManager.getInstance().recordAndLogProfileUse(
-                        selectedAutofillAddress.getProfile().getGUID());
-
-                response.shippingAddress = selectedAutofillAddress.toPaymentAddress();
-            }
-        }
-
         if (mUiShippingOptions != null) {
             PaymentOption selectedShippingOption = mUiShippingOptions.getSelectedItem();
             if (selectedShippingOption != null && selectedShippingOption.getIdentifier() != null) {
@@ -1055,7 +1055,81 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
         }
 
         mUI.showProcessingMessage();
+
+        if (mShippingAddressesSection != null) {
+            PaymentOption selectedShippingAddress = mShippingAddressesSection.getSelectedItem();
+            if (selectedShippingAddress != null) {
+                // Shipping addresses are created in show(). These should all be instances of
+                // AutofillAddress.
+                assert selectedShippingAddress instanceof AutofillAddress;
+                AutofillAddress selectedAutofillAddress = (AutofillAddress) selectedShippingAddress;
+
+                // Addresses to be sent to the merchant should always be complete.
+                assert selectedAutofillAddress.isComplete();
+
+                // Record the use of the profile.
+                PersonalDataManager.getInstance().recordAndLogProfileUse(
+                        selectedAutofillAddress.getProfile().getGUID());
+
+                response.shippingAddress = selectedAutofillAddress.toPaymentAddress();
+
+                // Create the normalization task.
+                mIsWaitingForNormalization = true;
+                mPendingPaymentResponse = response;
+                PersonalDataManager.getInstance().normalizeAddressAsync(
+                        selectedAutofillAddress.getProfile().getGUID(),
+                        AutofillAddress.getCountryCode(selectedAutofillAddress.getProfile()), this);
+
+                if (mIsWaitingForNormalization) {

[gone, 2016/09/28 18:04:31]

Given that this will set mIsWaitingForNormalization to true immediately above,
when would this not trigger?  If normalizeAddressAsync can actually return
synchronously and set mIsWaitingForNormalization to false before this point,
then you need to rename that function to reflect that.

[sebsg, 2016/09/28 18:50:39]

Ah I see, it's not clear enough. If the rules have finished loading when this is
called, it will normalize synchronously, otherwise it will normalize when they
have finished loading. So it can be either.

Would something like normalizeAddressOrSetupTask work?

[gone, 2016/09/28 19:00:52]

I know you argued against it earlier, but I think that normalizeAddress() would
still sense, given that it can be either async or sync.  Would be better if
normalizeAddress returned whether or not it was being handled asynchronously,
which would let you set that boolean properly.

+                    // If the normalization is not already completed, start a timer to cancel the
+                    // normalization if it takes too long.
+                    mHandler.postDelayed(new Runnable() {
+                        @Override
+                        public void run() {
+                            onAddressNormalized(null);
+                        }
+                    }, NORMALIZATION_TIMEOUT_MS);
+                }
+
+                // The payment response will be sent to the merchant in onAddressNormalized instead.
+                return;
+            }
+        }
+
         mClient.onPaymentResponse(response);
+        recordSuccessFunnelHistograms("ReceivedInstrumentDetails");
+    }
+
+    /**
+     * Callback method called either when the address has finished normalizing or when the timeout
+     * triggers. Replaces the address in the response with the normalized version if present and
+     * sends the response to the merchant.
+     *
+     * @param profile The profile with the address normalized or a null profile if the timeout
+     *                triggered first.
+     */
+    @Override
+    public void onAddressNormalized(AutofillProfile profile) {
+        // Check if the response was already sent to the merchant.
+        if (!mIsWaitingForNormalization || mClient == null || mPendingPaymentResponse == null) {

[gone, 2016/09/28 18:04:31]

Should you play it safe and set mIsWaitingForNormalization = false here?

Something like this:

if (!mIsWaitingForNormalization) return;
mIsWaitingForNormalization = false;

if (mClient == null || mPendingPaymentResponse == null) return;

[sebsg, 2016/09/28 18:50:39]

Done.

+            return;
+        }
+
+        mIsWaitingForNormalization = false;
+
+        if (profile != null && !TextUtils.isEmpty(profile.getGUID())) {
+            // The normalization finished first: use the normalized address.
+            mPendingPaymentResponse.shippingAddress =
+                    new AutofillAddress(profile, true /* isComplete */).toPaymentAddress();
+        } else {
+            // The timeout triggered first: cancel the normalization task.
+            PersonalDataManager.getInstance().cancelPendingAddressNormalization();
+        }
+
+        // Send the payment response to the merchant.
+        mClient.onPaymentResponse(mPendingPaymentResponse);
+
+        mPendingPaymentResponse = null;
 
         recordSuccessFunnelHistograms("ReceivedInstrumentDetails");
     }