[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..fcea802f283ebeaf2494fadf7ce19d9e57eb1e4e 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
@@ -15,6 +15,7 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.autofill.NormalizedAddressRequester;
 import org.chromium.chrome.browser.autofill.PersonalDataManager;
 import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
 import org.chromium.chrome.browser.favicon.FaviconHelper;
@@ -66,8 +67,9 @@ import java.util.Set;
  * Android implementation of the PaymentRequest service defined in
  * third_party/WebKit/public/platform/modules/payments/payment_request.mojom.
  */
-public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Client,
-        PaymentApp.InstrumentsCallback, PaymentInstrument.DetailsCallback {
+public class PaymentRequestImpl

[please use gerrit instead, 2016/09/23 09:04:40]

Please don't put a newline before "implements".

[sebsg, 2016/09/27 18:48:37]

Done.

+        implements PaymentRequest, PaymentRequestUI.Client, PaymentApp.InstrumentsCallback,
+                   PaymentInstrument.DetailsCallback, NormalizedAddressRequester {
     /**
      * Observer to be notified when PaymentRequest UI has been dismissed.
      */
@@ -189,6 +191,12 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     /** True if show() was called. */
     private boolean mIsShowing;
 
+    private boolean mIsWaitingForNormalization;
+    private PaymentResponse mPendingPaymentResponse;
+    private boolean mIsPendingResponseProfileComplete;
+    private Handler mNormalizationTimer;

[please use gerrit instead, 2016/09/23 09:04:40]

Reuse mHandler instead.

[sebsg, 2016/09/27 18:48:37]

Done.

+    private static final int NORMALIZATION_TIMEOUT_MS = 5000;

[please use gerrit instead, 2016/09/23 09:04:40]

Put all "private static final" variables together. (Line 112 for example.)

[sebsg, 2016/09/27 18:48:38]

Done.

+
     /**
      * Builds the PaymentRequest service implementation.
      *
@@ -290,6 +298,12 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
             // Limit the number of suggestions.
             addresses = addresses.subList(0, Math.min(addresses.size(), SUGGESTIONS_LIMIT));
 
+            // Start loading the validation rules for the regions associated with the profiles.
+            for (int i = 0; i < addresses.size(); ++i) {
+                PersonalDataManager.getInstance().loadRulesForRegion(
+                        AutofillAddress.getCountryCode(addresses.get(i).getProfile()));
+            }

[please use gerrit instead, 2016/09/23 09:04:40]

If you have 4 address from "CA", then you will call loadRulesForRegion("CA") 4
times, whereas it should really be called only once. Please put the country
codes into a Set and iterate over that.

[sebsg, 2016/09/27 18:48:37]

Done.

+
             // 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 +1027,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) {
@@ -1054,6 +1052,48 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
                     PaymentRequestMetrics.SELECTED_METHOD_OTHER_PAYMENT_APP);
         }
 

[please use gerrit instead, 2016/09/23 09:04:39]

Write "mUI.showProcessingMessage();" here and remove the two instances of it
below.

[sebsg, 2016/09/27 18:48:37]

Done.

+        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();
+
+                mUI.showProcessingMessage();
+
+                // Create the normalization task.
+                mIsWaitingForNormalization = true;
+                mPendingPaymentResponse = response;
+                mIsPendingResponseProfileComplete = selectedAutofillAddress.isComplete();

[please use gerrit instead, 2016/09/23 09:04:40]

This should always be true. In fact you can write "assert
selectedAutofillAddress.isComplete();" if you want to feel safer.

[sebsg, 2016/09/27 18:48:37]

Done.

+                PersonalDataManager.getInstance().startAddressNormalizationTask(
+                        selectedAutofillAddress.getProfile().getGUID(),
+                        AutofillAddress.getCountryCode(selectedAutofillAddress.getProfile()), this);
+
+                // Check if the normalization is already done.
+                if (!mIsWaitingForNormalization) {

[please use gerrit instead, 2016/09/23 09:04:40]

Seems like incorrect logic here.

if (!mIsWaitingForNormalization) return;

Right?

[sebsg, 2016/09/27 18:48:38]

Done.

+                    // Start a timer to cancel the normalization if it takes too long.
+                    mNormalizationTimer = new Handler();

[please use gerrit instead, 2016/09/23 09:04:40]

Reuse mHandler. No need to create a new object.

mHandler.postDelayed(new Runnable() {
    @Override
    public void run() {
        onAddressNormalized(null);
    }
}, NORMALIZATION_TIMEOUT_MS);

[sebsg, 2016/09/27 18:48:37]

Done.

+                    Runnable normalizationTimeout = new Runnable() {
+                        @Override
+                        public void run() {
+                            onAddressNormalized(null);
+                        }
+                    };
+                    mNormalizationTimer.postDelayed(normalizationTimeout, NORMALIZATION_TIMEOUT_MS);
+                }
+
+                // The payment response will be sent to the merchant in onAddressNormalized instead.
+                return;
+            }
+        }
+
         mUI.showProcessingMessage();
         mClient.onPaymentResponse(response);
 
@@ -1061,6 +1101,40 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     }
 
     /**
+     * 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.
+     */
+    public void onAddressNormalized(AutofillProfile profile) {
+        // Check if the response was already sent to the merchant.
+        if (!mIsWaitingForNormalization || mClient == null || mPendingPaymentResponse == null) {
+            return;
+        }
+
+        mIsWaitingForNormalization = false;
+
+        if (profile != null && !TextUtils.isEmpty(profile.getGUID())) {
+            // The normalization finished first: use the normalized address.
+            mPendingPaymentResponse.shippingAddress =
+                    new AutofillAddress(profile, mIsPendingResponseProfileComplete)

[please use gerrit instead, 2016/09/23 09:04:40]

s/mIsPendingResponseProfileComplete/true/

...because this address is always complete.

[sebsg, 2016/09/27 18:48:37]

Done.

+                            .toPaymentAddress();
+        } else {
+            // The timeout triggered first: cancel the normalization task.
+            PersonalDataManager.getInstance().cancelAddressNormalizationTask();
+        }
+
+        // Send the payment response to the merchant.
+        mClient.onPaymentResponse(mPendingPaymentResponse);
+
+        mPendingPaymentResponse = null;
+
+        recordSuccessFunnelHistograms("ReceivedInstrumentDetails");
+    }
+
+    /**
      * Called if unable to retrieve instrument details.
      */
     @Override
@@ -1108,6 +1182,7 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     private void closeClient() {
         if (mClient != null) mClient.close();
         mClient = null;
+        mIsWaitingForNormalization = false;

[please use gerrit instead, 2016/09/23 09:04:40]

Did you find this necessary? If this does not solve a problem that you've
encountered, let's not add it.

[sebsg, 2016/09/27 18:48:37]

Done.

         mDismissObserver.onPaymentRequestDismissed();
     }