[Payment Request][Android] Enhance SkipToGPay experiment flow.

This patch splits the SkipToGPay experiment into two phases that can be
tested separately per request from early partners:

1) kSkipToGPayIfNoCard: enables skip-to-GPay if and only if user doesn't
   have a usable autofill instrument.
2) kSkipToGPay: enables skip-to-GPay regardless of availability of
   autofill instruments in the user's profile.

Part (1) required an extra lookup from the PersonalDataManager before
dispatching payment app creation. Manual profiling on a Nexus 5 showed
that this added 0.3-19ms extra main thread processing, with average
around 3ms for profiles that contain at least one credit card. This is a
relatively small performance penalty.

Bug: 877284
Change-Id: If465bb1fe0c1f79979cc3743af5474662bd6dd68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1936107Reviewed-by: Sahel Sharify <sahel@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719402}

chrome/android/chrome_test_java_sources.gni

@@ -306,6 +306,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinderTest.java",
   "javatests/src/org/chromium/chrome/browser/payments/CurrencyFormatterTest.java",
   "javatests/src/org/chromium/chrome/browser/payments/MockPackageManagerDelegate.java",
+  "javatests/src/org/chromium/chrome/browser/payments/SkipToGPayHelperTest.java",
   "javatests/src/org/chromium/chrome/browser/payments/PaymentHandlerChangePaymentMethodTest.java",
   "javatests/src/org/chromium/chrome/browser/payments/PaymentHandlerEnableDelegationsTest.java",
   "javatests/src/org/chromium/chrome/browser/payments/PaymentManifestDownloaderTest.java",

chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java

@@ -301,6 +301,8 @@ public abstract class ChromeFeatureList {
             "PasswordManagerOnboardingAndroid";
     public static final String PAY_WITH_GOOGLE_V1 = "PayWithGoogleV1";
     public static final String PAYMENT_REQUEST_SKIP_TO_GPAY = "PaymentRequestSkipToGPay";
+    public static final String PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD =
+            "PaymentRequestSkipToGPayIfNoCard";
     public static final String PERMISSION_DELEGATION = "PermissionDelegation";
     public static final String PER_METHOD_CAN_MAKE_PAYMENT_QUOTA =
             "WebPaymentsPerMethodCanMakePaymentQuota";

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

@@ -161,10 +161,14 @@ public class AutofillPaymentInstrument extends PaymentInstrument
     public boolean canMakePayment() {
         return PaymentsExperimentalFeatures.isEnabled(
                        ChromeFeatureList.STRICT_HAS_ENROLLED_AUTOFILL_INSTRUMENT)
-                ? getMissingFields() == CompletionStatus.COMPLETE && mHaveRequestedAutofillData
+                ? strictCanMakePayment()
                 : mHasValidNumberAndName;
     }
 
+    public boolean strictCanMakePayment() {
+        return getMissingFields() == CompletionStatus.COMPLETE && mHaveRequestedAutofillData;
+    }
+
     @Override
     public boolean canPreselect() {
         return mIsComplete && mIsMatchingMerchantsRequestedCardType;

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

@@ -611,8 +611,7 @@ public class PaymentRequestImpl
         }
 
         boolean googlePayBridgeActivated = googlePayBridgeEligible
-                && PaymentsExperimentalFeatures.isEnabled(
-                        ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY);
+                && SkipToGPayHelper.canActivateExperiment(mWebContents, methodData);
 
         mMethodData = getValidatedMethodData(methodData, googlePayBridgeActivated, mCardEditor);
         if (googlePayBridgeActivated) {

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

@@ -4,17 +4,25 @@
 
 package org.chromium.chrome.browser.payments;
 
+import android.support.v4.util.ArrayMap;
+
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.json.JSONTokener;
 
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.components.payments.MethodStrings;
+import org.chromium.content_public.browser.WebContents;
 import org.chromium.payments.mojom.GooglePaymentMethodData;
 import org.chromium.payments.mojom.PaymentAddress;
 import org.chromium.payments.mojom.PaymentDetails;
+import org.chromium.payments.mojom.PaymentMethodData;
 import org.chromium.payments.mojom.PaymentOptions;
 import org.chromium.payments.mojom.PaymentResponse;
 
+import java.util.List;
+import java.util.Map;
+
 /**
  * A helper to manage the request / response patching for the Skip-to-GPay experimental flow.
  * TODO(crbug.com/984694): Retire this helper after general delegation of shipping and contact
@@ -42,6 +50,55 @@ public class SkipToGPayHelper {
      */
     private String mSelectedShippingOptionId;
 
+    /**
+     * Returns whether the skip-to-GPay experiment should be enabled.
+     * @param webContents The WebContents that triggered the PaymentRequest.
+     * @param rawMethodData The PaymentMethodData[] provided to PaymentRequest constructor.
+     * @return True if either of the two skip-to-GPay experiment flow can be enabled.
+     */
+    public static boolean canActivateExperiment(
+            WebContents webContents, PaymentMethodData[] rawMethodData) {
+        if (rawMethodData == null || rawMethodData.length == 0) return false;
+
+        Map<String, PaymentMethodData> methodData = new ArrayMap<>();
+        for (int i = 0; i < rawMethodData.length; i++) {
+            String method = rawMethodData[i].supportedMethod;
+            if (method.equals(MethodStrings.BASIC_CARD)) {
+                methodData.put(method, rawMethodData[i]);
+                break;
+            }
+        }
+        if (methodData.isEmpty()) return false;
+
+        // V2 experiment: enable skip-to-GPay regardless of usable basic-card.
+        if (PaymentsExperimentalFeatures.isEnabled(
+                    ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY)) {
+            return true;
+        }
+
+        // This check for autofill instrument is duplicate work if skip-to-GPay ends up not being
+        // enabled and adds a small delay (average ~3ms with first time ) to all hybrid request
+        // flows. However, this is the cleanest way to implement SKIP_TO_GPAY_IF_NO_CARD.
+        AutofillPaymentApp app = new AutofillPaymentApp(webContents);
+        List<PaymentInstrument> instruments =
+                app.getInstruments(methodData, /*forceReturnServerCards=*/false);
+        boolean hasUsableBasicCard = false;
+        for (int i = 0; i < instruments.size(); i++) {
+            PaymentInstrument instrument = instruments.get(i);
+            instrument.setHaveRequestedAutofillData(true);
+            if (instrument.isAutofillInstrument()
+                    && ((AutofillPaymentInstrument) instrument).strictCanMakePayment()) {
+                hasUsableBasicCard = true;
+                break;
+            }
+        }
+
+        // V1 experiment: only enable skip-to-GPay if no usable basic-card exists.
+        return !hasUsableBasicCard
+                && PaymentsExperimentalFeatures.isEnabled(
+                        ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD);
+    }
+
     /**
      * Constructs a SkipToGPayHeper.
      * @param options The PaymentOptions specified by the merchant when constructing the

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.payments;
+
+import android.support.test.filters.SmallTest;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.Feature;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.autofill.AutofillTestHelper;
+import org.chromium.chrome.browser.autofill.CardType;
+import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
+import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard;
+import org.chromium.chrome.test.ChromeActivityTestRule;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.components.payments.MethodStrings;
+import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.payments.mojom.PaymentMethodData;
+
+import java.util.concurrent.TimeoutException;
+
+/**
+ * An integration test for determining eligibility of SkipToGPay experimental flow.
+ */
+@RunWith(ChromeJUnit4ClassRunner.class)
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
+public final class SkipToGPayHelperTest {
+    @Rule
+    public ChromeActivityTestRule<ChromeActivity> mRule =
+            new ChromeActivityTestRule<>(ChromeActivity.class);
+
+    // A test PaymentMethodData[] shared by all test instances.
+    private PaymentMethodData[] mTestMethodData;
+    private AutofillTestHelper mHelper;
+
+    @Before
+    public void setUp() {
+        mRule.startMainActivityOnBlankPage();
+        mHelper = new AutofillTestHelper();
+
+        // Set up a test PaymentMethodData that requests "basic-card".
+        mTestMethodData = new PaymentMethodData[1];
+        mTestMethodData[0] = new PaymentMethodData();
+        mTestMethodData[0].supportedMethod = MethodStrings.BASIC_CARD;
+    }
+
+    private AutofillProfile makeCompleteProfile() {
+        return new AutofillProfile("", "https://example.com", true, "Jon Doe", "Google",
+                "340 Main St", "CA", "Los Angeles", "", "90291", "", "US", "650-253-0000",
+                "jon.doe@gmail.com", "en-US");
+    }
+
+    private CreditCard makeCreditCard(String billingAddressProfileId) {
+        return new CreditCard("", "https://example.com", true, true, "Jon Doe", "4111111111111111",
+                "1111", "12", "2050", "amex", R.drawable.amex_card, CardType.UNKNOWN,
+                billingAddressProfileId, /*serverId=*/"");
+    }
+
+    /**
+     * Helper function that runs canActivateExperiment() on the main thread and checks that it
+     * returns |expected|.
+     */
+    void assertCanActivateExperiment(boolean expected) {
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            Assert.assertEquals(expected,
+                    SkipToGPayHelper.canActivateExperiment(
+                            mRule.getWebContents(), mTestMethodData));
+        });
+    }
+
+    /**
+     * Verifies that when PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD is enabled, experiment is not
+     * activiated if user has a complete autofill instrument.
+     */
+    @Test
+    @SmallTest
+    @Feature({"Payments"})
+    @CommandLineFlags.Add({"disable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY,
+            "enable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD})
+    public void
+    testSkipToGPayIfNoCard_HasCard() throws TimeoutException {
+        String billingAddressProfileId = mHelper.setProfile(makeCompleteProfile());
+        mHelper.setCreditCard(makeCreditCard(billingAddressProfileId));
+        assertCanActivateExperiment(false);
+    }
+
+    /**
+     * Verifies that when PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD is enabled, experiment is
+     * activated if user doesn't have a complete autofill instrument.
+     */
+    @Test
+    @SmallTest
+    @Feature({"Payments"})
+    @CommandLineFlags.Add({"disable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY,
+            "enable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD})
+    public void
+    testSkipToGPayIfNoCard_IncompleteCard() throws TimeoutException {
+        mHelper.setCreditCard(makeCreditCard(/*billingAddressProfileId=*/""));
+        assertCanActivateExperiment(true);
+    }
+
+    /**
+     * Verifies that when PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD is enabled, experiment is
+     * activated if user doesn't have any autofill instrument.
+     */
+    @Test
+    @SmallTest
+    @Feature({"Payments"})
+    @CommandLineFlags.Add({"disable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY,
+            "enable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD})
+    public void
+    testSkipToGPayIfNoCard_NoCard() throws TimeoutException {
+        assertCanActivateExperiment(true);
+    }
+
+    /**
+     * Verifies that when PAYMENT_REQUEST_SKIP_TO_GPAY is enabled, experiment is activated
+     * regardless whether user has a complete autofill instrument or not.
+     */
+    @Test
+    @SmallTest
+    @Feature({"Payments"})
+    @CommandLineFlags.Add({"enable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY})
+    public void testSkipToGPay_AlwaysEnabled() throws TimeoutException {
+        // At this point, there is no card in the profile.
+        assertCanActivateExperiment(true);
+
+        // Add a credit card with a complete profile.
+        String billingAddressProfileId = mHelper.setProfile(makeCompleteProfile());
+        mHelper.setCreditCard(makeCreditCard(billingAddressProfileId));
+        assertCanActivateExperiment(true);
+    }
+
+    /**
+     * Verifies that when both experiment flags are disabled, experiment is not activated when user
+     * doesn't have any autofill instrument.
+     */
+    @Test
+    @SmallTest
+    @Feature({"Payments"})
+    @CommandLineFlags.Add({"disable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY
+            + "," + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD})
+    public void
+    testSkipToGPayDisabled_NoCard() throws TimeoutException {
+        assertCanActivateExperiment(false);
+    }
+
+    /**
+     * Verifies that when both experiment flags are disabled, experiment is not activated when user
+     * has a complete autofill instrument.
+     */
+    @Test
+    @SmallTest
+    @Feature({"Payments"})
+    @CommandLineFlags.Add({"disable-features=" + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY
+            + "," + ChromeFeatureList.PAYMENT_REQUEST_SKIP_TO_GPAY_IF_NO_CARD})
+    public void
+    testSkipToGPayDisabled_HasCard() throws TimeoutException {
+        String billingAddressProfileId = mHelper.setProfile(makeCompleteProfile());
+        mHelper.setCreditCard(makeCreditCard(billingAddressProfileId));
+        assertCanActivateExperiment(false);
+    }
+}

chrome/browser/android/chrome_feature_list.cc

@@ -196,6 +196,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &net::features::kCookiesWithoutSameSiteMustBeSecure,
     &payments::features::kAlwaysAllowJustInTimePaymentApp,
     &payments::features::kPaymentRequestSkipToGPay,
+    &payments::features::kPaymentRequestSkipToGPayIfNoCard,
     &payments::features::kReturnGooglePayInBasicCard,
     &payments::features::kStrictHasEnrolledAutofillInstrument,
     &payments::features::kWebPaymentMicrotransaction,

components/payments/core/features.cc

@@ -50,6 +50,9 @@ const base::Feature kStrictHasEnrolledAutofillInstrument{
 const base::Feature kPaymentRequestSkipToGPay{
     "PaymentRequestSkipToGPay", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kPaymentRequestSkipToGPayIfNoCard{
+    "PaymentRequestSkipToGPayIfNoCard", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const base::Feature kWebPaymentMicrotransaction{
     "WebPaymentMicrotransaction", base::FEATURE_DISABLED_BY_DEFAULT};
 

components/payments/core/features.h

@@ -53,9 +53,13 @@ extern const base::Feature kWebPaymentsRedactShippingAddress;
 // hasEnrolledInstrument() queries.
 extern const base::Feature kStrictHasEnrolledAutofillInstrument;
 
-// Used to enable skip-to-GPay experimental flow.
+// Enables skip-to-GPay experimental flow.
 extern const base::Feature kPaymentRequestSkipToGPay;
 
+// Enables skip-to-GPay experimental flow, but only if user doesn't have an
+// eligible credit card.
+extern const base::Feature kPaymentRequestSkipToGPayIfNoCard;
+
 // Controls whether the microtransaction features are enabled.
 extern const base::Feature kWebPaymentMicrotransaction;