[Payments] Field trial and flag to abort payment request if no card.

[Payments] Field trial and flag to abort payment request if no card.

BUG=657980
Committed: https://crrev.com/d9595aa60bba38244a0099891f8b982575b5e390
Cr-Commit-Position: refs/heads/master@{#427361}

[sebsg, 2016-10-20 20:03:33]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 20:04:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sebsg, 2016-10-20 20:08:36]

Patchset #1 (id:1) has been deleted

[sebsg, 2016-10-20 20:09:47]

sebsg@chromium.org changed reviewers:
+ rouslan@chromium.org

[sebsg, 2016-10-20 20:09:48]

Hi Rouslan could you please take a look? Thanks!

[please use gerrit instead, 2016-10-20 20:11:09]

Description was changed from

==========
[Payments] Field trial and flag to abort payment request if no card.
==========

to

==========
[Payments] Field trial and flag to abort payment request if no card.

BUG=657980
==========

[please use gerrit instead, 2016-10-20 20:27:44]

Good work!

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeSwitches.java
(right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeSwitches.java:100:
public static final String DISABLE_NO_CARD_ABORT = "disabled-no-card-abort";
Please use
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...
instead. It's the new way to manage flags.

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1025:
private boolean disconnectIfNoPaymentMethodsSupported() {
The finch experiment should be in here instead, because we need to consider all
payment methods, not just credit cards. (If users has not credit cards, but has
Android Pay, for example, don't abort the PaymentRequst.)

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java
(right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:1:
// Copyright 2016 The Chromium Authors. All rights reserved.
This file is not necessary with ChromeFeatureList.isEnabled(). See:

https://cs.chromium.org/chromium/src/base/feature_list.h?rcl=0&l=59

// Behind the scenes, the above call would take into account any command-line
// flags to enable or disable the feature, any experiments that may control it
// and finally its default state (in that order of priority), to determine
// whether the feature is on.

https://codereview.chromium.org/2437593007/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2076: {"enable-no-card-abort",
IDS_FLAGS_NO_CARD_ABORT,
The "enable-no-card-abort" is used to generate a quick access URL for the
feature:

chrome://flags#enable-no-card-abort

The feature can be disabled, but the URL will remain the same, with "enable"
keyword in there. That's inconsistent. Let's remove the "enable" keyword from
the URL.

chrome://flags#no-card-abort

https://codereview.chromium.org/2437593007/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2078:
ENABLE_DISABLE_VALUE_TYPE(switches::kEnablePaymentRequestNoCardAbort,
You can use FEATURE_VALUE_TYPE() with ChromeFeatureList.

[commit-bot: I haz the power, 2016-10-20 22:07:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-20 22:07:48]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[sebsg, 2016-10-20 23:10:37]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 23:11:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 01:25:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 01:25:44]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sebsg, 2016-10-21 02:20:32]

Thanks! Another look?

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeSwitches.java
(right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeSwitches.java:100:
public static final String DISABLE_NO_CARD_ABORT = "disabled-no-card-abort";
On 2016/10/20 20:27:43, rouslan wrote:
> Please use
>
https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...
> instead. It's the new way to manage flags.

Done.

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1025:
private boolean disconnectIfNoPaymentMethodsSupported() {
On 2016/10/20 20:27:44, rouslan wrote:
> The finch experiment should be in here instead, because we need to consider
all
> payment methods, not just credit cards. (If users has not credit cards, but
has
> Android Pay, for example, don't abort the PaymentRequst.)

Done.

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java
(right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:1:
// Copyright 2016 The Chromium Authors. All rights reserved.
On 2016/10/20 20:27:44, rouslan wrote:
> This file is not necessary with ChromeFeatureList.isEnabled(). See:
> 
> https://cs.chromium.org/chromium/src/base/feature_list.h?rcl=0&l=59
> 
> // Behind the scenes, the above call would take into account any command-line
> // flags to enable or disable the feature, any experiments that may control it
> // and finally its default state (in that order of priority), to determine
> // whether the feature is on.

Done.

https://codereview.chromium.org/2437593007/diff/20001/chrome/browser/about_fl...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2437593007/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2076: {"enable-no-card-abort",
IDS_FLAGS_NO_CARD_ABORT,
On 2016/10/20 20:27:44, rouslan wrote:
> The "enable-no-card-abort" is used to generate a quick access URL for the
> feature:
> 
> chrome://flags#enable-no-card-abort
> 
> The feature can be disabled, but the URL will remain the same, with "enable"
> keyword in there. That's inconsistent. Let's remove the "enable" keyword from
> the URL.
> 
> chrome://flags#no-card-abort

Done.

https://codereview.chromium.org/2437593007/diff/20001/chrome/browser/about_fl...
chrome/browser/about_flags.cc:2078:
ENABLE_DISABLE_VALUE_TYPE(switches::kEnablePaymentRequestNoCardAbort,
On 2016/10/20 20:27:44, rouslan wrote:
> You can use FEATURE_VALUE_TYPE() with ChromeFeatureList.

Done.

[sebsg, 2016-10-21 02:38:41]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 02:39:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 03:35:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 03:35:48]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[please use gerrit instead, 2016-10-21 13:36:27]

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java:45:
public static final String NO_CREDIT_CARD_ABORT = "NoCreditCardAbort";
Please re-sort this list.

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1017:

Easier to do this:

bool userCanAddCreditCards = mMerchantSupportsAutofillPaymentInstruments
        && !ChromeFeatureList.isEnabled(ChromeFeatureList.NO_CREDIT_CARD_ABORT);

... and replace "!mMerchantSupportsAutofillPaymentInstruments" with
"!userCanAddCreditCards" on line 1020.

Then you don't need lines 1033-1048. This eliminates lines 1041-1046, which are
copy-paste of lines 1024-1030. The end result is the same, however.

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:18:
* A payment integration test for a merchant that provides free shipping
regardless of address.
This comment does not describe the purpose of this test very well.

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:22:
// This merchant provides free shipping worldwide.
The point of having a separate payment_request_bobpay_and_cards_test.html is a
merchant that accepts both credit cards and Bob Pay. Shipping is not relevant to
this test.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.cc (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:60: &kNoCreditCardAbort,
Keep this list in order please.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:108:
base::FEATURE_DISABLED_BY_DEFAULT};
Order.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.h (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.h:29: extern const base::Feature
kNoCreditCardAbort;
Re-sort this list please.

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:1099: const char
kDisablePaymentRequestNoCardAbort[] = "disable-no-card-abort";
Please remove this.

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
chrome/common/chrome_switches.h:316: extern const char
kDisablePaymentRequestNoCardAbort[];
These two are no longer necessary.

https://codereview.chromium.org/2437593007/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2437593007/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:89977: +  <int value="2141463682"
label="no-card-abort"/>
Run the AboutFlagsHistogramTest unittest to determine the correct # for
no-card-abort. Then run tools/metrics/histograms/pretty_print.py to put
no-card-abort in the correct location in the list.

[sebsg, 2016-10-21 16:41:28]

Patchset #4 (id:80001) has been deleted

[sebsg, 2016-10-21 16:46:43]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[sebsg, 2016-10-21 16:46:59]

Patchset #4 (id:100001) has been deleted

[commit-bot: I haz the power, 2016-10-21 16:47:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sebsg, 2016-10-21 16:49:54]

Thanks!

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java:45:
public static final String NO_CREDIT_CARD_ABORT = "NoCreditCardAbort";
On 2016/10/21 13:36:26, rouslan wrote:
> Please re-sort this list.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1017:

On 2016/10/21 13:36:27, rouslan wrote:
> Easier to do this:
> 
> bool userCanAddCreditCards = mMerchantSupportsAutofillPaymentInstruments
>         &&
!ChromeFeatureList.isEnabled(ChromeFeatureList.NO_CREDIT_CARD_ABORT);
> 
> ... and replace "!mMerchantSupportsAutofillPaymentInstruments" with
> "!userCanAddCreditCards" on line 1020.
> 
> Then you don't need lines 1033-1048. This eliminates lines 1041-1046, which
are
> copy-paste of lines 1024-1030. The end result is the same, however.

I tend to keep experimental stuff separate since it's easy to see and remove,
but yeah, this is much cleaner and better. Thanks

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:18:
* A payment integration test for a merchant that provides free shipping
regardless of address.
On 2016/10/21 13:36:27, rouslan wrote:
> This comment does not describe the purpose of this test very well.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:22:
// This merchant provides free shipping worldwide.
On 2016/10/21 13:36:27, rouslan wrote:
> The point of having a separate payment_request_bobpay_and_cards_test.html is a
> merchant that accepts both credit cards and Bob Pay. Shipping is not relevant
to
> this test.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.cc (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:60: &kNoCreditCardAbort,
On 2016/10/21 13:36:27, rouslan wrote:
> Keep this list in order please.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:108:
base::FEATURE_DISABLED_BY_DEFAULT};
On 2016/10/21 13:36:27, rouslan wrote:
> Order.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.h (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.h:29: extern const base::Feature
kNoCreditCardAbort;
On 2016/10/21 13:36:27, rouslan wrote:
> Re-sort this list please.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:1099: const char
kDisablePaymentRequestNoCardAbort[] = "disable-no-card-abort";
On 2016/10/21 13:36:27, rouslan wrote:
> Please remove this.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
chrome/common/chrome_switches.h:316: extern const char
kDisablePaymentRequestNoCardAbort[];
On 2016/10/21 13:36:27, rouslan wrote:
> These two are no longer necessary.

Done.

https://codereview.chromium.org/2437593007/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2437593007/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:89977: +  <int value="2141463682"
label="no-card-abort"/>
On 2016/10/21 13:36:27, rouslan wrote:
> Run the AboutFlagsHistogramTest unittest to determine the correct # for
> no-card-abort. Then run tools/metrics/histograms/pretty_print.py to put
> no-card-abort in the correct location in the list.

Done.

[sebsg, 2016-10-21 16:49:55]

Thanks!

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java:45:
public static final String NO_CREDIT_CARD_ABORT = "NoCreditCardAbort";
On 2016/10/21 13:36:26, rouslan wrote:
> Please re-sort this list.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1017:

On 2016/10/21 13:36:27, rouslan wrote:
> Easier to do this:
> 
> bool userCanAddCreditCards = mMerchantSupportsAutofillPaymentInstruments
>         &&
!ChromeFeatureList.isEnabled(ChromeFeatureList.NO_CREDIT_CARD_ABORT);
> 
> ... and replace "!mMerchantSupportsAutofillPaymentInstruments" with
> "!userCanAddCreditCards" on line 1020.
> 
> Then you don't need lines 1033-1048. This eliminates lines 1041-1046, which
are
> copy-paste of lines 1024-1030. The end result is the same, however.

I tend to keep experimental stuff separate since it's easy to see and remove,
but yeah, this is much cleaner and better. Thanks

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java
(right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:18:
* A payment integration test for a merchant that provides free shipping
regardless of address.
On 2016/10/21 13:36:27, rouslan wrote:
> This comment does not describe the purpose of this test very well.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java:22:
// This merchant provides free shipping worldwide.
On 2016/10/21 13:36:27, rouslan wrote:
> The point of having a separate payment_request_bobpay_and_cards_test.html is a
> merchant that accepts both credit cards and Bob Pay. Shipping is not relevant
to
> this test.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.cc (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:60: &kNoCreditCardAbort,
On 2016/10/21 13:36:27, rouslan wrote:
> Keep this list in order please.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.cc:108:
base::FEATURE_DISABLED_BY_DEFAULT};
On 2016/10/21 13:36:27, rouslan wrote:
> Order.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
File chrome/browser/android/chrome_feature_list.h (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/browser/android/...
chrome/browser/android/chrome_feature_list.h:29: extern const base::Feature
kNoCreditCardAbort;
On 2016/10/21 13:36:27, rouslan wrote:
> Re-sort this list please.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:1099: const char
kDisablePaymentRequestNoCardAbort[] = "disable-no-card-abort";
On 2016/10/21 13:36:27, rouslan wrote:
> Please remove this.

Done.

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/2437593007/diff/60001/chrome/common/chrome_sw...
chrome/common/chrome_switches.h:316: extern const char
kDisablePaymentRequestNoCardAbort[];
On 2016/10/21 13:36:27, rouslan wrote:
> These two are no longer necessary.

Done.

https://codereview.chromium.org/2437593007/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2437593007/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:89977: +  <int value="2141463682"
label="no-card-abort"/>
On 2016/10/21 13:36:27, rouslan wrote:
> Run the AboutFlagsHistogramTest unittest to determine the correct # for
> no-card-abort. Then run tools/metrics/histograms/pretty_print.py to put
> no-card-abort in the correct location in the list.

Done.

[sebsg, 2016-10-21 18:41:33]

sebsg@chromium.org changed reviewers:
+ dfalcantara@chromium.org, rkaplow@chromium.org

[sebsg, 2016-10-21 18:41:34]

rkaplow@chromium.org: Could you please review changes in 
histograms.xml and fieldtrial_testing_config.json ?

dfalcantara@chromium.org: Could you please review changes in 
chrome/browser/* and ChromeFeatureList.java ?

[please use gerrit instead, 2016-10-21 19:00:20]

https://codereview.chromium.org/2437593007/diff/120001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2437593007/diff/120001/chrome/app/generated_r...
chrome/app/generated_resources.grd:15570: +    <!---<if expr="is_android">-->
You seem to have forgotten to remove <!--->

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/about_f...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/about_f...
chrome/browser/about_flags.cc:2079: #endif  // OS_ANDROID
btw, you can reuse the OS_ANDROID block above

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/android...
File chrome/browser/android/chrome_feature_list.cc (right):

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/android...
chrome/browser/android/chrome_feature_list.cc:48:
&autofill::kAutofillScanCardholderName,
What kind of sorting puts "a" below "k"? :-P

[sebsg, 2016-10-21 19:08:43]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 19:09:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 19:33:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 19:33:55]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[rkaplow, 2016-10-21 20:19:57]

lgtm

[sebsg, 2016-10-21 21:11:04]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 21:11:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sebsg, 2016-10-21 21:14:46]

Patchset #6 (id:160001) has been deleted

[sebsg, 2016-10-21 21:15:07]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 21:15:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 21:48:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 21:48:11]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sebsg, 2016-10-24 12:40:15]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 12:40:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-24 13:02:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-24 13:02:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[gone, 2016-10-24 19:01:07]

lgtm

https://codereview.chromium.org/2437593007/diff/200001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2437593007/diff/200001/chrome/app/generated_r...
chrome/app/generated_resources.grd:15568: +          No Card Abort
1) Can you make this more obviously about credit cards?  You use "No credit card
abort" in other places.
2) Indentation seems off.

[sebsg, 2016-10-24 21:25:00]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 21:25:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-24 21:34:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-24 21:34:20]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[sebsg, 2016-10-24 22:08:42]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-24 22:09:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sebsg, 2016-10-24 22:14:21]

Patchset #9 (id:240001) has been deleted

[sebsg, 2016-10-24 22:14:31]

Patchset #6 (id:180001) has been deleted

[commit-bot: I haz the power, 2016-10-25 00:29:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-25 00:29:56]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sebsg, 2016-10-25 14:14:38]

The CQ bit was checked by sebsg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 14:14:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sebsg, 2016-10-25 15:13:10]

Hi Rouslan, one last look?

https://codereview.chromium.org/2437593007/diff/120001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2437593007/diff/120001/chrome/app/generated_r...
chrome/app/generated_resources.grd:15570: +    <!---<if expr="is_android">-->
On 2016/10/21 19:00:20, rouslan wrote:
> You seem to have forgotten to remove <!--->

I'm really sloppy on this one. Sorry, I'll take more time to review.

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/about_f...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/about_f...
chrome/browser/about_flags.cc:2079: #endif  // OS_ANDROID
On 2016/10/21 19:00:20, rouslan wrote:
> btw, you can reuse the OS_ANDROID block above

Done.

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/android...
File chrome/browser/android/chrome_feature_list.cc (right):

https://codereview.chromium.org/2437593007/diff/120001/chrome/browser/android...
chrome/browser/android/chrome_feature_list.cc:48:
&autofill::kAutofillScanCardholderName,
On 2016/10/21 19:00:20, rouslan wrote:
> What kind of sorting puts "a" below "k"? :-P

I saw it like the include files where you would include
   chrome/browser/text.h 
before
   chrome/browser/test/test.h

Happy to change it

https://codereview.chromium.org/2437593007/diff/200001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2437593007/diff/200001/chrome/app/generated_r...
chrome/app/generated_resources.grd:15568: +          No Card Abort
On 2016/10/24 19:01:07, dfalcantara (slow 10.24) wrote:
> 1) Can you make this more obviously about credit cards?  You use "No credit
card
> abort" in other places.
> 2) Indentation seems off.

Done.

[please use gerrit instead, 2016-10-25 15:25:55]

lgtm

[commit-bot: I haz the power, 2016-10-25 15:28:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-25 15:28:37]

Dry run: This issue passed the CQ dry run.

[sebsg, 2016-10-25 15:29:08]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-10-25 15:29:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
rkaplow@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2437593007/#ps280001
(title: "Disabled experiment for tests")

[commit-bot: I haz the power, 2016-10-25 15:29:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 15:34:27]

Description was changed from

==========
[Payments] Field trial and flag to abort payment request if no card.

BUG=657980
==========

to

==========
[Payments] Field trial and flag to abort payment request if no card.

BUG=657980
==========

[commit-bot: I haz the power, 2016-10-25 15:34:28]

Committed patchset #9 (id:280001)

[commit-bot: I haz the power, 2016-10-25 15:43:59]

Description was changed from

==========
[Payments] Field trial and flag to abort payment request if no card.

BUG=657980
==========

to

==========
[Payments] Field trial and flag to abort payment request if no card.

BUG=657980

Committed: https://crrev.com/d9595aa60bba38244a0099891f8b982575b5e390
Cr-Commit-Position: refs/heads/master@{#427361}
==========

[commit-bot: I haz the power, 2016-10-25 15:44:00]

Patchset 9 (id:??) landed as
https://crrev.com/d9595aa60bba38244a0099891f8b982575b5e390
Cr-Commit-Position: refs/heads/master@{#427361}