[Payments] Use the correct index when checking addresses

We have an off-by-one error now that the mOptionLayout can have multiple children in it above the actual options.

* Use the correct index when indexing into the options layout.

* Add a test that checks for the crash and additionally checks that the "merchant doesn't accept address" string is
correctly displayed.

* Plumbs a new callback that indicates that the client has finished
checking the selection.

BUG=630348

Review-Url: https://codereview.chromium.org/2170603006
Cr-Commit-Position: refs/heads/master@{#407197}

chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestSection.java

@@ -540,7 +540,7 @@ public abstract class PaymentRequestSection extends LinearLayout {
          * + The "label" is text describing the row.
          * + The "icon" is a logo representing the option, like a credit card.
          */
-        private class OptionRow {
+        public class OptionRow {
             private static final int OPTION_ROW_TYPE_OPTION = 0;
             private static final int OPTION_ROW_TYPE_ADD = 1;
             private static final int OPTION_ROW_TYPE_DESCRIPTION = 2;
@@ -589,6 +589,12 @@ public abstract class PaymentRequestSection extends LinearLayout {
                 mButton.setId(id);
             }
 
+            /** @return the label for the row. */
+            @VisibleForTesting
+            public CharSequence getLabelText() {
+                return mLabel.getText();
+            }
+
             private View createButton(
                     GridLayout parent, int rowIndex, boolean isSelected, boolean isEnabled) {
                 if (mRowType == OPTION_ROW_TYPE_DESCRIPTION) return null;
@@ -881,21 +887,22 @@ public abstract class PaymentRequestSection extends LinearLayout {
             mLabelsForTest.clear();
 
             // Show any additional text requested by the layout.
-            if (!TextUtils.isEmpty(mDelegate.getAdditionalText(this))) {
+            String additionalText = mDelegate.getAdditionalText(this);
+            if (!TextUtils.isEmpty(additionalText)) {
                 OptionRow descriptionRow = new OptionRow(mOptionLayout,
-                        mOptionLayout.getChildCount(),
+                        mOptionRows.size(),
                         mDelegate.isAdditionalTextDisplayingWarning(this)
                                 ? OptionRow.OPTION_ROW_TYPE_WARNING
                                 : OptionRow.OPTION_ROW_TYPE_DESCRIPTION,
                                 null, false);
                 mOptionRows.add(descriptionRow);
-                descriptionRow.setLabel(mDelegate.getAdditionalText(this));
+                descriptionRow.setLabel(additionalText);
             }
 
             // List out known payment options.
             int firstOptionIndex = INVALID_OPTION_INDEX;
             for (int i = 0; i < information.getSize(); i++) {
-                int currentRow = mOptionLayout.getChildCount();
+                int currentRow = mOptionRows.size();
                 if (firstOptionIndex == INVALID_OPTION_INDEX) firstOptionIndex = currentRow;
 
                 PaymentOption item = information.getItem(i);
@@ -957,6 +964,12 @@ public abstract class PaymentRequestSection extends LinearLayout {
         public int getNumberOfOptionLabelsForTest() {
             return mLabelsForTest.size();
         }
+
+        /** Returns the OptionRow at the specified |index|. */
+        @VisibleForTesting
+        public OptionRow getOptionRowAtIndex(int index) {
+            return mOptionRows.get(index);
+        }
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java

@@ -193,6 +193,11 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
          */
         void onPaymentRequestReadyToPay(PaymentRequestUI ui);
 
+        /**
+         * Called when the UI has been updated to reflect checking a selected option.
+         */
+        void onPaymentRequestSelectionChecked(PaymentRequestUI ui);
+
         /**
          * Called when edit dialog is showing.
          */
@@ -329,6 +334,7 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
                     expand(null);
                 }
                 updatePayButtonEnabled();
+                notifySelectionChecked();
             }
         };
 
@@ -1168,4 +1174,10 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
             sObserverForTest.onPaymentRequestReadyToClose(this);
         }
     }
+
+    private void notifySelectionChecked() {
+        if (sObserverForTest != null) {
+            sObserverForTest.onPaymentRequestSelectionChecked(this);
+        }
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestDynamicShippingMultipleAddressesTest.java

@@ -9,8 +9,8 @@ import android.test.suitebuilder.annotation.MediumTest;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.autofill.AutofillTestHelper;
 import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
-import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard;
 
+import java.util.ArrayList;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
 
@@ -19,6 +19,37 @@ import java.util.concurrent.TimeoutException;
  * and user that has 5 addresses stored in autofill settings.
  */
 public class PaymentRequestDynamicShippingMultipleAddressesTest extends PaymentRequestTestBase {
+    private static final AutofillProfile[] AUTOFILL_PROFILES = {
+        // Incomplete profile (missing phone number)
+        new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
+                "Bart Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
+                "90210", "", "US", "", "bart@simpson.com", ""),
+
+        // Incomplete profile.
+        new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
+                "Homer Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
+                "90210", "", "US", "", "homer@simpson.com", ""),
+
+        // Complete profile.
+        new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
+                "Lisa Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
+                "90210", "", "US", "555 123-4567", "lisa@simpson.com", ""),
+
+        // Complete profile in another country.
+        new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
+                "Maggie Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
+                "90210", "", "Uzbekistan", "555 123-4567", "maggie@simpson.com", ""),
+
+        // Incomplete profile.
+        new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
+                "Marge Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
+                "90210", "", "US", "", "marge@simpson.com", "")
+    };
+
+    private AutofillProfile[] mProfilesToAdd;
+    private int[] mCountsToSet;
+    private int[] mDatesToSet;
+
     public PaymentRequestDynamicShippingMultipleAddressesTest() {
         // This merchant requests the shipping address first before providing any shipping options.
         super("payment_request_dynamic_shipping_test.html");
@@ -28,48 +59,17 @@ public class PaymentRequestDynamicShippingMultipleAddressesTest extends PaymentR
     public void onMainActivityStarted()
             throws InterruptedException, ExecutionException, TimeoutException {
         AutofillTestHelper helper = new AutofillTestHelper();
-        // Create an incomplete (no phone) profile with the highest frecency score.
-        String guid1 = helper.setProfile(
-                new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
-                        "Bart Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
-                        "90210", "", "US", "", "bart@simpson.com", ""));
-
-        // Create an incomplete (no phone) profile with a the second highest frecency score.
-        String guid2 = helper.setProfile(
-                new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
-                        "Homer Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
-                        "90210", "", "US", "", "homer@simpson.com", ""));
-
-        // Create a complete profile with a middle frecency score.
-        String guid3 = helper.setProfile(
-                new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
-                        "Lisa Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
-                        "90210", "", "US", "555 123-4567", "lisa@simpson.com", ""));
-
-        // Create a complete profile with the second lowest frecency score.
-        String guid4 = helper.setProfile(
-                new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
-                        "Maggie Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
-                        "90210", "", "US", "555 123-4567", "maggie@simpson.com", ""));
-
-        // Create an incomplete profile with the lowest frecency score.
-        String guid5 = helper.setProfile(
-                new AutofillProfile("" /* guid */, "https://www.example.com" /* origin */,
-                        "Marge Simpson", "Acme Inc.", "123 Main", "California", "Los Angeles", "",
-                        "90210", "", "US", "", "marge@simpson.com", ""));
-
-        // Create a credit card associated witht the fourth profile.
-        helper.setCreditCard(new CreditCard("", "https://example.com", true, true, "Jon Doe",
-                "4111111111111111", "1111", "12", "2050", "visa", R.drawable.pr_visa,
-                guid4));
-
-        // Set the use stats so that profile1 has the highest frecency score, profile2 the second
-        // highest, profile 3 the second lowest and profile4 the lowest.
-        helper.setProfileUseStatsForTesting(guid1, 20, 5000);
-        helper.setProfileUseStatsForTesting(guid2, 15, 5000);
-        helper.setProfileUseStatsForTesting(guid3, 10, 5000);
-        helper.setProfileUseStatsForTesting(guid4, 5, 5000);
-        helper.setProfileUseStatsForTesting(guid5, 1, 1);
+
+        // Add the profiles.
+        ArrayList<String> guids = new ArrayList<>();
+        for (int i = 0; i < mProfilesToAdd.length; i++) {
+            guids.add(helper.setProfile(mProfilesToAdd[i]));
+        }
+
+        // Set up the profile use stats.
+        for (int i = 0; i < guids.size(); i++) {
+            helper.setProfileUseStatsForTesting(guids.get(i), mCountsToSet[i], mDatesToSet[i]);
+        }
     }
 
     /**
@@ -80,6 +80,14 @@ public class PaymentRequestDynamicShippingMultipleAddressesTest extends PaymentR
     @MediumTest
     public void testShippingAddressSuggestionOrdering()
             throws InterruptedException, ExecutionException, TimeoutException {
+        // Create a bunch of profiles, some complete, some incomplete.  Values are set so that the
+        // profiles are ordered by frecency.
+        mProfilesToAdd = new AutofillProfile[] {
+                AUTOFILL_PROFILES[0], AUTOFILL_PROFILES[1], AUTOFILL_PROFILES[2],
+                AUTOFILL_PROFILES[3], AUTOFILL_PROFILES[4]};
+        mCountsToSet = new int[] {20, 15, 10, 5, 1};
+        mDatesToSet = new int[] {5000, 5000, 5000, 5000, 1};
+
         triggerUIAndWait(mReadyForInput);
         clickInShippingSummaryAndWait(R.id.payments_section, mReadyForInput);
         assertEquals(4, getNumberOfShippingAddressSuggestions());
@@ -88,4 +96,30 @@ public class PaymentRequestDynamicShippingMultipleAddressesTest extends PaymentR
         assertTrue(getShippingAddressSuggestionLabel(2).contains("Bart Simpson"));
         assertTrue(getShippingAddressSuggestionLabel(3).contains("Homer Simpson"));
     }
+
+    /**
+     * Select a shipping address that the website refuses to accept, which should force the dialog
+     * to show an error.
+     */
+    @MediumTest
+    public void testShippingAddresNotAcceptedByMerchant()
+            throws InterruptedException, ExecutionException, TimeoutException {
+        // Add a profile that is not accepted by the website.
+        mProfilesToAdd = new AutofillProfile[] {AUTOFILL_PROFILES[3]};
+        mCountsToSet = new int[] {5};
+        mDatesToSet = new int[] {5000};
+
+        // Click on the unacceptable shipping address.
+        triggerUIAndWait(mReadyForInput);
+        clickInShippingSummaryAndWait(R.id.payments_section, mReadyForInput);
+        assertTrue(getShippingAddressSuggestionLabel(0).contains(
+                AUTOFILL_PROFILES[3].getFullName()));
+        clickOnShippingAddressSuggestionOptionAndWait(0, mSelectionChecked);
+
+        // The string should indicate that the shipping address isn't valid.
+        CharSequence actualString = getShippingAddressOptionRowAtIndex(0).getLabelText();
+        CharSequence expectedString = getInstrumentation().getTargetContext().getString(
+                R.string.payments_unsupported_shipping_address);
+        assertEquals(expectedString, actualString);
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestTestBase.java

@@ -22,6 +22,7 @@ import org.chromium.chrome.browser.payments.PaymentAppFactory.PaymentAppFactoryA
 import org.chromium.chrome.browser.payments.PaymentRequestImpl.PaymentRequestServiceObserverForTest;
 import org.chromium.chrome.browser.payments.ui.EditorTextField;
 import org.chromium.chrome.browser.payments.ui.PaymentRequestSection.OptionSection;
+import org.chromium.chrome.browser.payments.ui.PaymentRequestSection.OptionSection.OptionRow;
 import org.chromium.chrome.browser.payments.ui.PaymentRequestUI;
 import org.chromium.chrome.browser.payments.ui.PaymentRequestUI.PaymentRequestObserverForTest;
 import org.chromium.chrome.test.ChromeActivityTestCaseBase;
@@ -64,6 +65,7 @@ abstract class PaymentRequestTestBase extends ChromeActivityTestCaseBase<ChromeA
     protected final PaymentsCallbackHelper<PaymentRequestUI> mReadyForInput;
     protected final PaymentsCallbackHelper<PaymentRequestUI> mReadyToPay;
     protected final PaymentsCallbackHelper<PaymentRequestUI> mReadyToClose;
+    protected final PaymentsCallbackHelper<PaymentRequestUI> mSelectionChecked;
     protected final PaymentsCallbackHelper<PaymentRequestUI> mResultReady;
     protected final PaymentsCallbackHelper<CardUnmaskPrompt> mReadyForUnmaskInput;
     protected final PaymentsCallbackHelper<CardUnmaskPrompt> mReadyToUnmask;
@@ -86,6 +88,7 @@ abstract class PaymentRequestTestBase extends ChromeActivityTestCaseBase<ChromeA
         mReadyForInput = new PaymentsCallbackHelper<>();
         mReadyToPay = new PaymentsCallbackHelper<>();
         mReadyToClose = new PaymentsCallbackHelper<>();
+        mSelectionChecked = new PaymentsCallbackHelper<>();
         mResultReady = new PaymentsCallbackHelper<>();
         mReadyForUnmaskInput = new PaymentsCallbackHelper<>();
         mReadyToUnmask = new PaymentsCallbackHelper<>();
@@ -325,7 +328,28 @@ abstract class PaymentRequestTestBase extends ChromeActivityTestCaseBase<ChromeA
     }
 
     /**
-     *  Returns the the number of shipping address suggestions,
+     *  Clicks on the label corresponding to the shipping address suggestion at the specified
+     *  |suggestionIndex|.
+     * @throws InterruptedException
+     */
+    protected void clickOnShippingAddressSuggestionOptionAndWait(
+            final int suggestionIndex, CallbackHelper helper)
+                    throws ExecutionException, TimeoutException, InterruptedException {
+        assert (suggestionIndex < getNumberOfShippingAddressSuggestions());
+
+        int callCount = helper.getCallCount();
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                ((OptionSection) mUI.getShippingAddressSectionForTest())
+                        .getOptionLabelsForTest(suggestionIndex).performClick();
+            }
+        });
+        helper.waitForCallback(callCount);
+    }
+
+    /**
+     *  Returns the the number of shipping address suggestions.
      */
     protected int getNumberOfShippingAddressSuggestions() throws ExecutionException {
         return ThreadUtils.runOnUiThreadBlocking(new Callable<Integer>() {
@@ -337,6 +361,18 @@ abstract class PaymentRequestTestBase extends ChromeActivityTestCaseBase<ChromeA
         });
     }
 
+    /** Returns the {@link OptionRow} at the given index for the shipping address section. */
+    protected OptionRow getShippingAddressOptionRowAtIndex(final int index)
+            throws ExecutionException {
+        return ThreadUtils.runOnUiThreadBlocking(new Callable<OptionRow>() {
+            @Override
+            public OptionRow call() {
+                return ((OptionSection) mUI.getShippingAddressSectionForTest())
+                        .getOptionRowAtIndex(index);
+            }
+        });
+    }
+
     /** Selects the spinner value in the editor UI for credit cards. */
     protected void setSpinnerSelectionsInCardEditorAndWait(final int[] selections,
             CallbackHelper helper) throws InterruptedException, TimeoutException {
@@ -496,6 +532,12 @@ abstract class PaymentRequestTestBase extends ChromeActivityTestCaseBase<ChromeA
         mReadyToClose.notifyCalled(ui);
     }
 
+    @Override
+    public void onPaymentRequestSelectionChecked(PaymentRequestUI ui) {
+        ThreadUtils.assertOnUiThread();
+        mSelectionChecked.notifyCalled(ui);
+    }
+
     @Override
     public void onPaymentRequestResultReady(PaymentRequestUI ui) {
         ThreadUtils.assertOnUiThread();