[Payments] Add Payment Request metrics for selected payment method.

Added the metric to record the selected payment method of a Payment Request transaction.

BUG=635006
Committed: https://crrev.com/6b9ad19cdc0ac674121b1ba30b0c85e2119fcfd4
Cr-Commit-Position: refs/heads/master@{#410947}

[sebsg, 2016-08-05 21:37:35]

Patchset #1 (id:1) has been deleted

[sebsg, 2016-08-05 21:38:44]

Description was changed from

==========
[Payments] Add Payment Request metrics for selected payment method.

BUG=635006
==========

to

==========
Added the metric to record the selected payment method of a Payment Request
transaction.

BUG=635006
==========

[sebsg, 2016-08-05 21:41:40]

Patchset #1 (id:20001) has been deleted

[sebsg, 2016-08-05 21:44:03]

Patchset #1 (id:40001) has been deleted

[sebsg, 2016-08-05 21:44:35]

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

[sebsg, 2016-08-05 21:44:35]

Hi Rouslan, could you please take a look?

Thanks

[please use gerrit instead, 2016-08-05 22:08:27]

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

https://codereview.chromium.org/2220713002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1044:
} else {
Instead of this:

if () {
} else {
  if () {
  } else {
  }
}

It's cleaner to write this:

if () {
} else if () {
} else {
}

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

https://codereview.chromium.org/2220713002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestMetrics.java:80:
}
I'm not a fan of global modifiable state, if I can avoid it. Let's hardcode
"https://android.com/pay" for metrics and use it in your metrics tests as well.
Add an optional "methodName" parameter to installPaymentApp() function. So your
new tests can call something like:

installPaymentApp("https://android.com/pay", IMMEDIATE_RESPONSE,
HAVE_INSTRUMENT);

Then you can hard-code the comparison in PaymentRequestImpl:

} else if (methodName.equals("https://android.com/pay")) {
    ...
}

[sebsg, 2016-08-08 16:15:08]

Patchset #2 (id:80001) has been deleted

[sebsg, 2016-08-08 16:16:51]

Thanks, another look?

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

https://codereview.chromium.org/2220713002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1044:
} else {
On 2016/08/05 22:08:26, rouslan wrote:
> Instead of this:
> 
> if () {
> } else {
>   if () {
>   } else {
>   }
> }
> 
> It's cleaner to write this:
> 
> if () {
> } else if () {
> } else {
> }

Done.

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

https://codereview.chromium.org/2220713002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestMetrics.java:80:
}
On 2016/08/05 22:08:26, rouslan wrote:
> I'm not a fan of global modifiable state, if I can avoid it. Let's hardcode
> "https://android.com/pay" for metrics and use it in your metrics tests as
well.
> Add an optional "methodName" parameter to installPaymentApp() function. So
your
> new tests can call something like:
> 
> installPaymentApp("https://android.com/pay", IMMEDIATE_RESPONSE,
> HAVE_INSTRUMENT);
> 
> Then you can hard-code the comparison in PaymentRequestImpl:
> 
> } else if (methodName.equals("https://android.com/pay")) {
>     ...
> }

Done.

[please use gerrit instead, 2016-08-08 16:30:55]

lgtm

[sebsg, 2016-08-08 16:34:15]

sebsg@chromium.org changed reviewers:
+ bauerb@chromium.org, jwd@chromium.org

[sebsg, 2016-08-08 16:34:16]

bauerb@: Could you please review the changes in chrome/android/* ? Thanks!

jwd@: Could you please review the changes in histograms.xml? Thanks!

[Bernhard Bauer, 2016-08-09 08:51:40]

lgtm

https://codereview.chromium.org/2220713002/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2220713002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1037:
if (mPaymentMethodsSection.getSelectedItem() instanceof
AutofillPaymentInstrument) {
Nit: I would extract the selected item to a local variable maybe?

https://codereview.chromium.org/2220713002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1044:
} else if (methodName.equals("https://android.com/pay")) {
Extract the string to a constant?

[jwd, 2016-08-09 14:59:05]

lgtm

[sebsg, 2016-08-09 18:00:53]

Thanks for the review

https://codereview.chromium.org/2220713002/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2220713002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1037:
if (mPaymentMethodsSection.getSelectedItem() instanceof
AutofillPaymentInstrument) {
On 2016/08/09 08:51:40, Bernhard Bauer wrote:
> Nit: I would extract the selected item to a local variable maybe?

Done.

https://codereview.chromium.org/2220713002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1044:
} else if (methodName.equals("https://android.com/pay")) {
On 2016/08/09 08:51:40, Bernhard Bauer wrote:
> Extract the string to a constant?

Done.

[sebsg, 2016-08-09 18:00:59]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2016-08-09 18:00:59]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
rouslan@chromium.org, jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2220713002/#ps120001
(title: "Addressed comments")

[commit-bot: I haz the power, 2016-08-09 18:01:26]

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-08-09 20:09:28]

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

[commit-bot: I haz the power, 2016-08-09 20:09:29]

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-08-10 02:52:46]

The CQ bit was checked by sebsg@chromium.org

[commit-bot: I haz the power, 2016-08-10 02:53:33]

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-08-10 03:45:32]

Description was changed from

==========
Added the metric to record the selected payment method of a Payment Request
transaction.

BUG=635006
==========

to

==========
Added the metric to record the selected payment method of a Payment Request
transaction.

BUG=635006
==========

[commit-bot: I haz the power, 2016-08-10 03:45:33]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2016-08-10 03:47:25]

Description was changed from

==========
Added the metric to record the selected payment method of a Payment Request
transaction.

BUG=635006
==========

to

==========
Added the metric to record the selected payment method of a Payment Request
transaction.

BUG=635006

Committed: https://crrev.com/6b9ad19cdc0ac674121b1ba30b0c85e2119fcfd4
Cr-Commit-Position: refs/heads/master@{#410947}
==========

[commit-bot: I haz the power, 2016-08-10 03:47:26]

Patchset 3 (id:??) landed as
https://crrev.com/6b9ad19cdc0ac674121b1ba30b0c85e2119fcfd4
Cr-Commit-Position: refs/heads/master@{#410947}