Web payments metric for multiple show() calls.

Web payments metric for multiple show() calls.

It's valid for multiple renderers to call PaymentRequest.show() at the
same time, because they are not coordinating. Chrome allows only one
renderer at a time to call PaymentRequest.show(). When the show() calls
of the other renderers are being rejected, the recorded metric should be
NO_SHOW_CONCURRENT_REQUESTS instead of
ABORT_REASON_INVALID_DATA_FROM_RENDERER.

BUG=711674
Review-Url: https://codereview.chromium.org/2816513004
Cr-Commit-Position: refs/heads/master@{#465110}
Committed: https://chromium.googlesource.com/chromium/src/+/8dbd95ed5e1531af5df3d342edaf0e8b31ef8144

[please use gerrit instead, 2017-04-11 22:55:16]

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

[please use gerrit instead, 2017-04-11 22:55:44]

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

[please use gerrit instead, 2017-04-11 22:55:44]

Seb, ptal.

[commit-bot: I haz the power, 2017-04-11 22:56: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, 2017-04-11 23:39:20]

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

[commit-bot: I haz the power, 2017-04-11 23:39:21]

Dry run: This issue passed the CQ dry run.

[sebsg, 2017-04-12 16:54:24]

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

https://codereview.chromium.org/2816513004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:565:
recordAbortReasonHistogram(PaymentRequestMetrics.ABORT_REASON_OTHER);
Could you add another entry like ABORT_REASON_CONCURRENT_REQUESTS or something
like that? Thanks!

[please use gerrit instead, 2017-04-14 15:51:35]

Description was changed from

==========
Web payments metric for multiple show () calls.

It's valid for multiple renderers to call PaymentRequest.show() at the
same time, because they are not coordinating. Chrome allows only one
renderer at a time to call PaymentRequest.show(). When the show() calls
of the other renderers are being rejected, the recorded metric should be
ABORT_REASON_OTHER instead of ABORT_REASON_INVALID_DATA_FROM_RENDERER.

BUG=NONE
==========

to

==========
Web payments metric for multiple show () calls.

It's valid for multiple renderers to call PaymentRequest.show() at the
same time, because they are not coordinating. Chrome allows only one
renderer at a time to call PaymentRequest.show(). When the show() calls
of the other renderers are being rejected, the recorded metric should be
ABORT_REASON_OTHER instead of ABORT_REASON_INVALID_DATA_FROM_RENDERER.

BUG=711674
==========

[please use gerrit instead, 2017-04-14 17:10:42]

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

[commit-bot: I haz the power, 2017-04-14 17:10:55]

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

[please use gerrit instead, 2017-04-14 17:38:06]

Description was changed from

==========
Web payments metric for multiple show () calls.

It's valid for multiple renderers to call PaymentRequest.show() at the
same time, because they are not coordinating. Chrome allows only one
renderer at a time to call PaymentRequest.show(). When the show() calls
of the other renderers are being rejected, the recorded metric should be
ABORT_REASON_OTHER instead of ABORT_REASON_INVALID_DATA_FROM_RENDERER.

BUG=711674
==========

to

==========
Web payments metric for multiple show() calls.

It's valid for multiple renderers to call PaymentRequest.show() at the
same time, because they are not coordinating. Chrome allows only one
renderer at a time to call PaymentRequest.show(). When the show() calls
of the other renderers are being rejected, the recorded metric should be
NO_SHOW_CONCURRENT_REQUESTS instead of
ABORT_REASON_INVALID_DATA_FROM_RENDERER.

BUG=711674
==========

[please use gerrit instead, 2017-04-14 17:42:53]

Seb: PTAL patch 3.

[please use gerrit instead, 2017-04-14 17:43:02]

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

[please use gerrit instead, 2017-04-14 17:43:07]

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

https://codereview.chromium.org/2816513004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:565:
recordAbortReasonHistogram(PaymentRequestMetrics.ABORT_REASON_OTHER);
On 2017/04/12 16:54:24, sebsg wrote:
> Could you add another entry like ABORT_REASON_CONCURRENT_REQUESTS or something
> like that? Thanks!

Done.

[commit-bot: I haz the power, 2017-04-14 17:43:17]

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, 2017-04-14 18:46:12]

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

[commit-bot: I haz the power, 2017-04-14 18:46:12]

Dry run: This issue passed the CQ dry run.

[sebsg, 2017-04-14 20:37:58]

LGTM % small comments

Thanks for making the metrics more robust!

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

https://codereview.chromium.org/2816513004/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:562:
if (mUI != null) {
Can you add a comment describing in what case this would trigger.

https://codereview.chromium.org/2816513004/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:569:
if (getIsAnyPaymentRequestShowing()) {
Can you describe here what you explained to me offline, about the different
tabs? Thanks!

[please use gerrit instead, 2017-04-14 21:08:04]

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

[please use gerrit instead, 2017-04-14 21:08:15]

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

[please use gerrit instead, 2017-04-14 21:08:15]

haraken: ptal histograms.xml.

[commit-bot: I haz the power, 2017-04-14 21:08:32]

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

[please use gerrit instead, 2017-04-14 21:09:02]

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

https://codereview.chromium.org/2816513004/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:562:
if (mUI != null) {
On 2017/04/14 20:37:58, sebsg wrote:
> Can you add a comment describing in what case this would trigger.

Done.

https://codereview.chromium.org/2816513004/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:569:
if (getIsAnyPaymentRequestShowing()) {
On 2017/04/14 20:37:57, sebsg wrote:
> Can you describe here what you explained to me offline, about the different
> tabs? Thanks!

Done.

[commit-bot: I haz the power, 2017-04-14 22:12:52]

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

[commit-bot: I haz the power, 2017-04-14 22:12:53]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-04-15 03:15:32]

haraken@chromium.org changed reviewers:
+ isherman@chromium.org

[haraken, 2017-04-15 03:15:32]

+isherman

(I'm only eligible to review UseCounter-only changes.)

[please use gerrit instead, 2017-04-16 22:43:13]

rouslan@chromium.org changed reviewers:
- haraken@chromium.org

[Ilya Sherman, 2017-04-18 00:11:37]

histograms.xml lgtm

[please use gerrit instead, 2017-04-18 00:19:06]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2017-04-18 00:19:06]

The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org
Link to the patchset: https://codereview.chromium.org/2816513004/#ps60001
(title: "Comments")

[commit-bot: I haz the power, 2017-04-18 00:19:37]

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-04-18 02:37:12]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492474746195610,
"parent_rev": "0f6dc1c0458928b9e3abe4927ee99018e6945737", "commit_rev":
"8dbd95ed5e1531af5df3d342edaf0e8b31ef8144"}

[commit-bot: I haz the power, 2017-04-18 02:37:22]

Description was changed from

==========
Web payments metric for multiple show() calls.

It's valid for multiple renderers to call PaymentRequest.show() at the
same time, because they are not coordinating. Chrome allows only one
renderer at a time to call PaymentRequest.show(). When the show() calls
of the other renderers are being rejected, the recorded metric should be
NO_SHOW_CONCURRENT_REQUESTS instead of
ABORT_REASON_INVALID_DATA_FROM_RENDERER.

BUG=711674
==========

to

==========
Web payments metric for multiple show() calls.

It's valid for multiple renderers to call PaymentRequest.show() at the
same time, because they are not coordinating. Chrome allows only one
renderer at a time to call PaymentRequest.show(). When the show() calls
of the other renderers are being rejected, the recorded metric should be
NO_SHOW_CONCURRENT_REQUESTS instead of
ABORT_REASON_INVALID_DATA_FROM_RENDERER.

BUG=711674

Review-Url: https://codereview.chromium.org/2816513004
Cr-Commit-Position: refs/heads/master@{#465110}
Committed:
https://chromium.googlesource.com/chromium/src/+/8dbd95ed5e1531af5df3d342edaf...
==========

[commit-bot: I haz the power, 2017-04-18 02:37:23]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/8dbd95ed5e1531af5df3d342edaf...