[Payments] Add CanMakePayment metrics.

[Payments] Add CanMakePayment metrics.

BUG=683990
Review-Url: https://codereview.chromium.org/2642213005
Cr-Commit-Position: refs/heads/master@{#446093}
Committed: https://chromium.googlesource.com/chromium/src/+/3f634c72e1f91fe2a142e922691867546362ea1f

[sebsg, 2017-01-23 18:06:35]

Description was changed from

==========
WIP [Payments] Add CanMakePayment metrics.

BUG=
==========

to

==========
[Payments] Add CanMakePayment metrics.

BUG=683990
==========

[sebsg, 2017-01-23 18:06:45]

Patchset #1 (id:1) has been deleted

[sebsg, 2017-01-23 18:13:50]

Patchset #1 (id:20001) has been deleted

[sebsg, 2017-01-23 18:15:01]

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

[sebsg, 2017-01-23 18:15:02]

Hi Roulsan could you please take a look? Thanks!

Hi Jwd, could you please take a look at histograms.xml? Thanks!

[please use gerrit instead, 2017-01-23 21:04:33]

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java
(right):

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:169:
? EFFECT_ON_COMPLETION_ABORTED
This can easily be confused with PaymentRequest.abort(). Add a note that this
matches such histograms as "UserAborted", for example.

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:176:
int sample = 0;
s/sample/effectOnPayment/g

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:178:
if (mCouldMakePayment) sample += 2;
Boy, this makes my head hurt. Can this be simplified in some way? (It is
correct, though.)

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:203:
}
I cannot, however, vouch for the validity of the larger if-then-else :-(. Any
way to simplify it?

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

https://codereview.chromium.org/2642213005/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:100243: +  <int value="0" label="False
and not shown"/>
Nit: would it be too much to write "canMakePayment() returned false and show()
was not called"?

[sebsg, 2017-01-23 22:57:06]

Patchset #2 (id:60001) has been deleted

[sebsg, 2017-01-23 22:57:23]

Thanks Rouslan! Another look?

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java
(right):

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:169:
? EFFECT_ON_COMPLETION_ABORTED
On 2017/01/23 21:04:32, rouslan wrote:
> This can easily be confused with PaymentRequest.abort(). Add a note that this
> matches such histograms as "UserAborted", for example.

Done.

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:176:
int sample = 0;
On 2017/01/23 21:04:32, rouslan wrote:
> s/sample/effectOnPayment/g

Done.

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:178:
if (mCouldMakePayment) sample += 2;
On 2017/01/23 21:04:32, rouslan wrote:
> Boy, this makes my head hurt. Can this be simplified in some way? (It is
> correct, though.)

Done.

https://codereview.chromium.org/2642213005/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLogger.java:203:
}
On 2017/01/23 21:04:33, rouslan wrote:
> I cannot, however, vouch for the validity of the larger if-then-else :-(. Any
> way to simplify it?

Done.

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

https://codereview.chromium.org/2642213005/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:100243: +  <int value="0" label="False
and not shown"/>
On 2017/01/23 21:04:33, rouslan wrote:
> Nit: would it be too much to write "canMakePayment() returned false and show()
> was not called"?

Done.

[please use gerrit instead, 2017-01-24 17:56:50]

lgtm

[jwd, 2017-01-24 21:59:59]

lgtm

[sebsg, 2017-01-25 03:30:43]

The CQ bit was checked by sebsg@chromium.org

[commit-bot: I haz the power, 2017-01-25 03:31:09]

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, 2017-01-25 03:33:11]

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

[commit-bot: I haz the power, 2017-01-25 03:33:12]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[sebsg, 2017-01-25 18:32:39]

The CQ bit was checked by sebsg@chromium.org

[sebsg, 2017-01-25 18:32:40]

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

[commit-bot: I haz the power, 2017-01-25 18:33:25]

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, 2017-01-25 19:44:23]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485369159314140,
"parent_rev": "e9f69602736a3f71bb6251fdd5f08c90ca210c31", "commit_rev":
"3f634c72e1f91fe2a142e922691867546362ea1f"}

[commit-bot: I haz the power, 2017-01-25 19:44:58]

Description was changed from

==========
[Payments] Add CanMakePayment metrics.

BUG=683990
==========

to

==========
[Payments] Add CanMakePayment metrics.

BUG=683990

Review-Url: https://codereview.chromium.org/2642213005
Cr-Commit-Position: refs/heads/master@{#446093}
Committed:
https://chromium.googlesource.com/chromium/src/+/3f634c72e1f91fe2a142e9226918...
==========

[commit-bot: I haz the power, 2017-01-25 19:44:59]

Committed patchset #3 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/3f634c72e1f91fe2a142e9226918...