[Payment Request] Update the Contacts section after editing address

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 90a8c8e276eed9aea80206a6d381087f4da562c8..2066c7c45cdcd0415294c1ec4b2d6a32a3116492 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
@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.autofill.PersonalDataManager;
 import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
 import org.chromium.chrome.browser.favicon.FaviconHelper;
 import org.chromium.chrome.browser.payments.ui.Completable;
+import org.chromium.chrome.browser.payments.ui.ContactDetailsSection;
 import org.chromium.chrome.browser.payments.ui.LineItem;
 import org.chromium.chrome.browser.payments.ui.PaymentInformation;
 import org.chromium.chrome.browser.payments.ui.PaymentOption;
@@ -161,11 +162,10 @@ public class PaymentRequestImpl
         }
     };
 
-    private static final String TAG = "cr_PaymentRequest";
-    private static final String ANDROID_PAY_METHOD_NAME = "https://android.com/pay";
-    private static final String PAYMENT_COMPLETE_ONCE = "payment_complete_once";
-    private static final int SUGGESTIONS_LIMIT = 4;
-    private static final Comparator<Completable> COMPLETENESS_COMPARATOR =
+    /** Limit in the number of suggested items in a section. */
+    public static final int SUGGESTIONS_LIMIT = 4;
+    /** Comparator used to sort by item completeness. */
+    public static final Comparator<Completable> COMPLETENESS_COMPARATOR =
             new Comparator<Completable>() {
                 @Override
                 public int compare(Completable a, Completable b) {
@@ -173,6 +173,10 @@ public class PaymentRequestImpl
                 }
             };
 
+    private static final String TAG = "cr_PaymentRequest";
+    private static final String ANDROID_PAY_METHOD_NAME = "https://android.com/pay";
+    private static final String PAYMENT_COMPLETE_ONCE = "payment_complete_once";
+
     /** Every origin can call canMakePayment() every 30 minutes. */
     private static final int CAN_MAKE_PAYMENT_QUERY_PERIOD_MS = 30 * 60 * 1000;
 
@@ -249,7 +253,7 @@ public class PaymentRequestImpl
 
     private Map<String, PaymentMethodData> mMethodData;
     private SectionInformation mShippingAddressesSection;
-    private SectionInformation mContactSection;
+    private ContactDetailsSection mContactDetailsSection;

[please use gerrit instead, 2016/12/20 18:45:21]

Your match will be smaller if you keep the name mContectSection.

[Mathieu, 2016/12/20 20:21:41]

Done.

     private List<PaymentApp> mApps;
     private boolean mAllAppsCreated;
     private List<PaymentApp> mPendingApps;
@@ -286,8 +290,8 @@ public class PaymentRequestImpl
 
         mMerchantName = webContents.getTitle();
         // The feature is available only in secure context, so it's OK to not show HTTPS.
-        mOrigin = UrlFormatter.formatUrlForSecurityDisplay(
-                webContents.getLastCommittedUrl(), false);
+        mOrigin =

[please use gerrit instead, 2016/12/20 18:45:21]

Please don't reformat lines that did not change.

[Mathieu, 2016/12/20 20:21:41]

Done.

+                UrlFormatter.formatUrlForSecurityDisplay(webContents.getLastCommittedUrl(), false);
 
         final FaviconHelper faviconHelper = new FaviconHelper();
         faviconHelper.getLocalFaviconImageForURL(Profile.getLastUsedProfile(),
@@ -361,7 +365,9 @@ public class PaymentRequestImpl
         }
 
         if (requestPayerName || requestPayerPhone || requestPayerEmail) {
-            createContactSection(profiles, requestPayerName, requestPayerPhone, requestPayerEmail);
+            mContactEditor =
+                    new ContactEditor(requestPayerName, requestPayerPhone, requestPayerEmail);
+            mContactDetailsSection = new ContactDetailsSection(mContext, profiles, mContactEditor);
         }
 
         mUI = new PaymentRequestUI(mContext, this, requestShipping,
@@ -381,8 +387,8 @@ public class PaymentRequestImpl
         mCardEditor.setEditorView(mUI.getCardEditorView());
         if (mContactEditor != null) mContactEditor.setEditorView(mUI.getEditorView());
 
-        PaymentRequestMetrics.recordRequestedInformationHistogram(requestPayerEmail,
-                requestPayerPhone, requestShipping, requestPayerName);
+        PaymentRequestMetrics.recordRequestedInformationHistogram(
+                requestPayerEmail, requestPayerPhone, requestShipping, requestPayerName);
     }
 
     private void createShippingSection(List<AutofillProfile> profiles) {
@@ -434,63 +440,6 @@ public class PaymentRequestImpl
                 PaymentRequestUI.TYPE_SHIPPING_ADDRESSES, firstCompleteAddressIndex, addresses);
     }
 
-    private void createContactSection(List<AutofillProfile> profiles, boolean requestName,
-            boolean requestPhone, boolean requestEmail) {
-        Set<String> uniqueContactInfos = new HashSet<>();
-        mContactEditor = new ContactEditor(requestName, requestPhone, requestEmail);
-        List<AutofillContact> contacts = new ArrayList<>();
-
-        for (int i = 0; i < profiles.size(); i++) {
-            AutofillProfile profile = profiles.get(i);
-            String name = requestName && !TextUtils.isEmpty(profile.getFullName())
-                    ? profile.getFullName()
-                    : null;
-            String phone = requestPhone && !TextUtils.isEmpty(profile.getPhoneNumber())
-                    ? profile.getPhoneNumber()
-                    : null;
-            String email = requestEmail && !TextUtils.isEmpty(profile.getEmailAddress())
-                    ? profile.getEmailAddress()
-                    : null;
-            mContactEditor.addPayerNameIfValid(name);
-            mContactEditor.addPhoneNumberIfValid(phone);
-            mContactEditor.addEmailAddressIfValid(email);
-
-            if (name != null || phone != null || email != null) {
-                // Different profiles can have identical contact info. Do not add the same
-                // contact info to the list twice.
-                String uniqueContactInfo = name + phone + email;
-                if (!uniqueContactInfos.contains(uniqueContactInfo)) {
-                    uniqueContactInfos.add(uniqueContactInfo);
-
-                    @ContactEditor.CompletionStatus
-                    int completionStatus =
-                            mContactEditor.checkContactCompletionStatus(name, phone, email);
-                    contacts.add(new AutofillContact(
-                            mContext, profile, name, phone, email, completionStatus));
-                }
-            }
-        }
-
-        // Suggest complete contact infos first.
-        Collections.sort(contacts, COMPLETENESS_COMPARATOR);
-
-        // Limit the number of suggestions.
-        contacts = contacts.subList(0, Math.min(contacts.size(), SUGGESTIONS_LIMIT));
-
-        // Log the number of suggested contact infos.
-        mJourneyLogger.setNumberOfSuggestionsShown(
-                PaymentRequestJourneyLogger.SECTION_CONTACT_INFO, contacts.size());
-
-        // Automatically select the first address if it is complete.
-        int firstCompleteContactIndex = SectionInformation.NO_SELECTION;
-        if (!contacts.isEmpty() && contacts.get(0).isComplete()) {
-            firstCompleteContactIndex = 0;
-        }
-
-        mContactSection = new SectionInformation(
-                PaymentRequestUI.TYPE_CONTACT_DETAILS, firstCompleteContactIndex, contacts);
-    }
-
     /**
      * Called by the merchant website to show the payment request to the user.
      */
@@ -560,8 +509,8 @@ public class PaymentRequestImpl
         Map<PaymentApp, Map<String, PaymentMethodData>> queryApps = new ArrayMap<>();
         for (int i = 0; i < mApps.size(); i++) {
             PaymentApp app = mApps.get(i);
-            Map<String, PaymentMethodData> appMethods = filterMerchantMethodData(mMethodData,
-                    app.getAppMethodNames());
+            Map<String, PaymentMethodData> appMethods =
+                    filterMerchantMethodData(mMethodData, app.getAppMethodNames());
             if (appMethods == null || !app.supportsMethodsAndData(appMethods)) {
                 mPendingApps.remove(app);
             } else {
@@ -642,13 +591,12 @@ public class PaymentRequestImpl
         }
 
         if (mFormatter == null) {
-            mFormatter = new CurrencyStringFormatter(details.total.amount.currency,
-                    Locale.getDefault());
+            mFormatter =
+                    new CurrencyStringFormatter(details.total.amount.currency, Locale.getDefault());
         }
 
         // Total is never pending.
-        LineItem uiTotal = new LineItem(
-                details.total.label, mFormatter.getFormattedCurrencyCode(),
+        LineItem uiTotal = new LineItem(details.total.label, mFormatter.getFormattedCurrencyCode(),
                 mFormatter.format(details.total.amount.value), /* isPending */ false);
 
         List<LineItem> uiLineItems = getLineItems(details.displayItems, mFormatter);
@@ -682,8 +630,9 @@ public class PaymentRequestImpl
         for (int i = 0; i < mPaymentMethodsSection.getSize(); i++) {
             PaymentInstrument instrument = (PaymentInstrument) mPaymentMethodsSection.getItem(i);
             PaymentDetailsModifier modifier = getModifier(instrument);
-            instrument.setModifiedTotal(modifier == null || modifier.total == null ? null
-                    : mFormatter.format(modifier.total.amount.value));
+            instrument.setModifiedTotal(modifier == null || modifier.total == null
+                            ? null
+                            : mFormatter.format(modifier.total.amount.value));
         }
 
         updateOrderSummary((PaymentInstrument) mPaymentMethodsSection.getSelectedItem());
@@ -699,13 +648,15 @@ public class PaymentRequestImpl
 
         mUiShoppingCart.setTotal(new LineItem(total.label, mFormatter.getFormattedCurrencyCode(),
                 mFormatter.format(total.amount.value), false /* isPending */));
-        mUiShoppingCart.setAdditionalContents(modifier == null ? null
-                : getLineItems(modifier.additionalDisplayItems, mFormatter));
+        mUiShoppingCart.setAdditionalContents(modifier == null
+                        ? null
+                        : getLineItems(modifier.additionalDisplayItems, mFormatter));
         mUI.updateOrderSummarySection(mUiShoppingCart);
     }
 
     /** @return The first modifier that matches the given instrument, or null. */
-    @Nullable private PaymentDetailsModifier getModifier(@Nullable PaymentInstrument instrument) {
+    @Nullable
+    private PaymentDetailsModifier getModifier(@Nullable PaymentInstrument instrument) {
         if (instrument == null) return null;
         Set<String> methodNames = instrument.getInstrumentMethodNames();
         methodNames.retainAll(mModifiers.keySet());
@@ -753,8 +704,8 @@ public class PaymentRequestImpl
         int selectedItemIndex = SectionInformation.NO_SELECTION;
         for (int i = 0; i < options.length; i++) {
             PaymentShippingOption option = options[i];
-            result.add(new PaymentOption(option.id, option.label,
-                    formatter.format(option.amount.value), null));
+            result.add(new PaymentOption(
+                    option.id, option.label, formatter.format(option.amount.value), null));
             if (option.selected) selectedItemIndex = i;
         }
 
@@ -782,7 +733,7 @@ public class PaymentRequestImpl
     private void providePaymentInformation() {
         mPaymentInformationCallback.onResult(
                 new PaymentInformation(mUiShoppingCart, mShippingAddressesSection,
-                        mUiShippingOptions, mContactSection, mPaymentMethodsSection));
+                        mUiShippingOptions, mContactDetailsSection, mPaymentMethodsSection));
         mPaymentInformationCallback = null;
     }
 
@@ -807,7 +758,7 @@ public class PaymentRequestImpl
                 } else if (optionType == PaymentRequestUI.TYPE_SHIPPING_OPTIONS) {
                     callback.onResult(mUiShippingOptions);
                 } else if (optionType == PaymentRequestUI.TYPE_CONTACT_DETAILS) {
-                    callback.onResult(mContactSection);
+                    callback.onResult(mContactDetailsSection);
                 } else if (optionType == PaymentRequestUI.TYPE_PAYMENT_METHODS) {
                     assert mPaymentMethodsSection != null;
                     callback.onResult(mPaymentMethodsSection);
@@ -849,7 +800,7 @@ public class PaymentRequestImpl
             AutofillContact contact = (AutofillContact) option;
 
             if (contact.isComplete()) {
-                mContactSection.setSelectedItem(option);
+                mContactDetailsSection.setSelectedItem(option);
             } else {
                 editContact(contact);
                 return PaymentRequestUI.SELECTION_RESULT_EDITOR_LAUNCH;
@@ -952,20 +903,28 @@ public class PaymentRequestImpl
                         // information when cancelled).
                         mShippingAddressesSection.setSelectedItemIndex(
                                 SectionInformation.NO_SELECTION);
-                        providePaymentInformation();
                     } else {
                         if (toEdit == null) {
-                            // Address is complete and we were in the "Add flow": add an item to the
-                            // list.
+                            // Address is complete and user was in the "Add flow": add an item to
+                            // the list.
                             mShippingAddressesSection.addAndSelectItem(editedAddress);
                         }
+
+                        if (mContactDetailsSection != null) {
+                            // Update |mContactDetailsSection| with the new/edited address, which
+                            // will

[please use gerrit instead, 2016/12/20 18:45:21]

reflow comment please

[Mathieu, 2016/12/20 20:21:41]

Done.

+                            // recompute the contacts section items.
+                            mContactDetailsSection.updateWithAutofillAddress(editedAddress);
+                            mUI.updateSection(
+                                    PaymentRequestUI.TYPE_CONTACT_DETAILS, mContactDetailsSection);
+                        }
+
                         // This updates the line items and the shipping options asynchronously by
                         // sending the new address to the merchant website.
                         mClient.onShippingAddressChange(editedAddress.toPaymentAddress());
                     }
-                } else {
-                    providePaymentInformation();
                 }
+                providePaymentInformation();

[please use gerrit instead, 2016/12/20 18:45:21]

What problem does moving this statement out of "else" solve?

[Mathieu, 2016/12/20 20:21:41]

The "Checking..." spinner would endlessly show if providePaymentInformation()
was not called in the 957-964 block.

[please use gerrit instead, 2016/12/21 13:38:38]

The "Checking..." spinner is removed in updateWith() call on line 549. The
spinner is necessary to block user interaction with UI while the website
verifies the new shipping address. If your "Checking..." spinner is spinning
forever, then your JavaScript did not register for shipping address change
events or did not respond to those events in 60 seconds. Please fix your test
instead JavaScript code instead.

             }
         });
     }
@@ -987,11 +946,12 @@ public class PaymentRequestImpl
                     if (!editedContact.isComplete()) {
                         // If the contact is not complete according to the requirements of the flow,
                         // unselect it (editor can return incomplete information when cancelled).
-                        mContactSection.setSelectedItemIndex(SectionInformation.NO_SELECTION);
+                        mContactDetailsSection.setSelectedItemIndex(
+                                SectionInformation.NO_SELECTION);
                     } else if (toEdit == null) {
                         // Contact is complete and we were in the "Add flow": add an item to the
                         // list.
-                        mContactSection.addAndSelectItem(editedContact);
+                        mContactDetailsSection.addAndSelectItem(editedContact);
                     }
                     // If contact is complete and (toEdit != null), no action needed: the contact
                     // was already selected in the UI.
@@ -999,7 +959,7 @@ public class PaymentRequestImpl
                 // If |editedContact| is null, the user has cancelled out of the "Add flow". No
                 // action to take (if a contact was selected in the UI, it will stay selected).
 
-                mUI.updateSection(PaymentRequestUI.TYPE_CONTACT_DETAILS, mContactSection);
+                mUI.updateSection(PaymentRequestUI.TYPE_CONTACT_DETAILS, mContactDetailsSection);
             }
         });
     }
@@ -1055,8 +1015,8 @@ public class PaymentRequestImpl
         PaymentInstrument instrument = (PaymentInstrument) selectedPaymentMethod;
         mPaymentAppRunning = true;
 
-        PaymentOption selectedContact = mContactSection != null ? mContactSection.getSelectedItem()
-                : null;
+        PaymentOption selectedContact =
+                mContactDetailsSection != null ? mContactDetailsSection.getSelectedItem() : null;
         mPaymentResponseHelper = new PaymentResponseHelper(
                 selectedShippingAddress, selectedShippingOption, selectedContact, this);
 
@@ -1182,7 +1142,7 @@ public class PaymentRequestImpl
 
     private void respondCanMakePaymentQuery(boolean response) {
         mClient.onCanMakePayment(response ? CanMakePaymentQueryResult.CAN_MAKE_PAYMENT
-                : CanMakePaymentQueryResult.CANNOT_MAKE_PAYMENT);
+                                          : CanMakePaymentQueryResult.CANNOT_MAKE_PAYMENT);
         if (sObserverForTest != null) {
             sObserverForTest.onPaymentRequestServiceCanMakePaymentQueryResponded();
         }
@@ -1223,8 +1183,8 @@ public class PaymentRequestImpl
         if (instruments != null) {
             for (int i = 0; i < instruments.size(); i++) {
                 PaymentInstrument instrument = instruments.get(i);
-                Set<String> instrumentMethodNames = new HashSet<>(
-                        instrument.getInstrumentMethodNames());
+                Set<String> instrumentMethodNames =
+                        new HashSet<>(instrument.getInstrumentMethodNames());
                 instrumentMethodNames.retainAll(mMethodData.keySet());
                 if (!instrumentMethodNames.isEmpty()) {
                     addPendingInstrument(instrument);
@@ -1245,8 +1205,8 @@ public class PaymentRequestImpl
         for (int i = 0; i < mPendingAutofillInstruments.size(); ++i) {
             assert mPendingAutofillInstruments.get(i) instanceof AutofillPaymentInstrument;
 
-            String countryCode = AutofillAddress
-                    .getCountryCode(((AutofillPaymentInstrument) mPendingAutofillInstruments.get(
+            String countryCode = AutofillAddress.getCountryCode((
+                    (AutofillPaymentInstrument) mPendingAutofillInstruments.get(
                             i)).getBillingAddress());
             if (!uniqueCountryCodes.contains(countryCode)) {
                 uniqueCountryCodes.add(countryCode);
@@ -1281,8 +1241,8 @@ public class PaymentRequestImpl
         if (query != null) query.setResponse(selection == 0);
 
         // The list of payment instruments is ready to display.
-        mPaymentMethodsSection = new SectionInformation(PaymentRequestUI.TYPE_PAYMENT_METHODS,
-                selection, mPendingInstruments);
+        mPaymentMethodsSection = new SectionInformation(
+                PaymentRequestUI.TYPE_PAYMENT_METHODS, selection, mPendingInstruments);
 
         mPendingInstruments.clear();
 
@@ -1303,8 +1263,8 @@ public class PaymentRequestImpl
             return false;
         }
 
-        boolean foundPaymentMethods = mPaymentMethodsSection != null
-                && !mPaymentMethodsSection.isEmpty();
+        boolean foundPaymentMethods =
+                mPaymentMethodsSection != null && !mPaymentMethodsSection.isEmpty();
         boolean userCanAddCreditCard = mMerchantSupportsAutofillPaymentInstruments
                 && !ChromeFeatureList.isEnabled(ChromeFeatureList.NO_CREDIT_CARD_ABORT);
 
@@ -1316,8 +1276,8 @@ public class PaymentRequestImpl
             disconnectFromClientWithDebugMessage("Requested payment methods have no instruments",
                     PaymentErrorReason.NOT_SUPPORTED);
             recordAbortReasonHistogram(mArePaymentMethodsSupported
-                    ? PaymentRequestMetrics.ABORT_REASON_NO_MATCHING_PAYMENT_METHOD
-                    : PaymentRequestMetrics.ABORT_REASON_NO_SUPPORTED_PAYMENT_METHOD);
+                            ? PaymentRequestMetrics.ABORT_REASON_NO_MATCHING_PAYMENT_METHOD
+                            : PaymentRequestMetrics.ABORT_REASON_NO_SUPPORTED_PAYMENT_METHOD);
             if (sObserverForTest != null) sObserverForTest.onPaymentRequestServiceShowFailed();
             return true;
         }
@@ -1387,15 +1347,14 @@ public class PaymentRequestImpl
     }
 
     @Override
-    public void onFocusChanged(
-            @PaymentRequestUI.DataType int dataType, boolean willFocus) {
+    public void onFocusChanged(@PaymentRequestUI.DataType int dataType, boolean willFocus) {
         assert dataType == PaymentRequestUI.TYPE_SHIPPING_ADDRESSES;
 
         if (mShippingAddressesSection.getSelectedItem() == null) return;
 
         assert mShippingAddressesSection.getSelectedItem() instanceof AutofillAddress;
-        AutofillAddress selectedAddress = (AutofillAddress) mShippingAddressesSection
-                .getSelectedItem();
+        AutofillAddress selectedAddress =
+                (AutofillAddress) mShippingAddressesSection.getSelectedItem();
 
         // The label should only include the country if the view is focused.
         if (willFocus) {
@@ -1483,9 +1442,8 @@ public class PaymentRequestImpl
         if (mHasRecordedAbortReason) return;
 
         mHasRecordedAbortReason = true;
-        RecordHistogram.recordEnumeratedHistogram(
-                "PaymentRequest.CheckoutFunnel.Aborted", abortReason,
-                PaymentRequestMetrics.ABORT_REASON_MAX);
+        RecordHistogram.recordEnumeratedHistogram("PaymentRequest.CheckoutFunnel.Aborted",
+                abortReason, PaymentRequestMetrics.ABORT_REASON_MAX);
 
         if (abortReason == PaymentRequestMetrics.ABORT_REASON_ABORTED_BY_USER) {
             mJourneyLogger.recordJourneyStatsHistograms("UserAborted");