Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2199813003:
[Payments] Add metrics for Payment Request aborts. (Closed)
Description[Payments] Add metrics for Payment Request aborts.
BUG=631013
Committed: https://crrev.com/901a7d18d952ced7ed3ac901dca4860580f5a4c9
Cr-Commit-Position: refs/heads/master@{#410069}
Patch Set 1 #
Total comments: 26
Patch Set 2 : Addressed comments and added missing files #Patch Set 3 : Rebase #Patch Set 4 : Moved metrics enum to PaymentRequestMetrics.java #
Total comments: 4
Patch Set 5 : Addressed comments #
Total comments: 16
Patch Set 6 : Addressed bauerb's comments #
Total comments: 2
Patch Set 7 : Nit #Patch Set 8 : Rebase #Messages
Total messages: 38 (20 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
sebsg@chromium.org changed reviewers: + rouslan@chromium.org
Hi Rouslan, here is my first draft for the abort metrics. Two issues with this draft: 1- The dependency I introduce for PaymentRequestImpl in PaymentRequestUI. I know it's bad but it shows it works for now. Thinking about alternative solutions. 2- Closing a tab leads to a ABORT_REASON_MOJO_RENDERER_CLOSING log even though we expect it to be a ABORT_REASON_ABORTED_BY_USER. I wonder if there are any legitimate reasons we would get a ABORT_REASON_MOJO_RENDERER_CLOSING in a real use case. Otherwise we could just replace it with ABORT_REASON_ABORTED_BY_USER. I'm also looking into a way to figure out that the tab was closed by the user so we can log the correct metric. Thanks!
On 2016/08/01 20:39:18, sebsg wrote: > 1- The dependency I introduce for PaymentRequestImpl in PaymentRequestUI. I know > it's bad but it shows it works for now. Thinking about alternative solutions. Let's not introduce this dependency. Integration tests should not call methods in PaymentRequestImpl directly. It's better to remove those tests than messing with dependencies. > 2- Closing a tab leads to a ABORT_REASON_MOJO_RENDERER_CLOSING log even though > we expect it to be a ABORT_REASON_ABORTED_BY_USER. RENDERER_CLOSING is also OK for tab closing. ABORTED_BY_USER is something that should be recorder when user clicks R.id.close_button or R.id.button_secondary. This triggers PaymentRequestImpl.onDismiss(). https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:105: public static final int ABORT_REASON_ABORTED_BY_USER = 0; "public static final" should be above "private static final". https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:175: No need for newline between these variables. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:176: private boolean mHasRecordedAbortReason = false; It's false by default, so no need to write " = false" https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:256: recordAbortReasonHistogram(ABORT_REASON_OTHER); This is invalid data from renderer. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:263: recordAbortReasonHistogram(ABORT_REASON_OTHER); This is invalid data from renderer. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:462: recordAbortReasonHistogram(ABORT_REASON_OTHER); Invalid data from renderer. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:879: recordAbortReasonHistogram(ABORT_REASON_ABORTED_BY_MERCHANT); Move up 2 lines. If mPaymentAppRunning is true, the abort does not happen. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1060: recordAbortReasonHistogram(ABORT_REASON_INSTRUMENT_DETAILS_ERROR); This does not abort payment. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1107: * Adds an entry to the aborted Payment Request histogram in the bucket corresponding to the +1 indent for all * https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1111: private void recordAbortReasonHistogram(final int abortReason) { No need for final here. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:1163: ((PaymentRequestImpl) mClient).close(); No need to test. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:1168: ((PaymentRequestImpl) mClient).onConnectionError(null); No need to test. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:1173: ((PaymentRequestImpl) mClient).onInstrumentDetailsError(); 1) This is called when CVC unmask is cancelled: https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromiu... 2) Cancelling the unmask dialog does not cancel payment. UI is still showing. User still can select a different credit card to pay. There's no need to record abort metrics for this case.
Patchset #4 (id:100001) has been deleted
Patchset #3 (id:80001) has been deleted
Patchset #3 (id:120001) has been deleted
Patchset #3 (id:140001) has been deleted
Patchset #2 (id:60001) has been deleted
Patchset #4 (id:200001) has been deleted
Patchset #5 (id:240001) has been deleted
Patchset #4 (id:220001) has been deleted
Hi Rouslan, another look? Thanks! https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:105: public static final int ABORT_REASON_ABORTED_BY_USER = 0; On 2016/08/01 21:12:09, rouslan wrote: > "public static final" should be above "private static final". Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:175: On 2016/08/01 21:12:09, rouslan wrote: > No need for newline between these variables. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:176: private boolean mHasRecordedAbortReason = false; On 2016/08/01 21:12:09, rouslan wrote: > It's false by default, so no need to write " = false" Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:256: recordAbortReasonHistogram(ABORT_REASON_OTHER); On 2016/08/01 21:12:09, rouslan wrote: > This is invalid data from renderer. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:263: recordAbortReasonHistogram(ABORT_REASON_OTHER); On 2016/08/01 21:12:09, rouslan wrote: > This is invalid data from renderer. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:462: recordAbortReasonHistogram(ABORT_REASON_OTHER); On 2016/08/01 21:12:09, rouslan wrote: > Invalid data from renderer. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:879: recordAbortReasonHistogram(ABORT_REASON_ABORTED_BY_MERCHANT); On 2016/08/01 21:12:09, rouslan wrote: > Move up 2 lines. If mPaymentAppRunning is true, the abort does not happen. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1060: recordAbortReasonHistogram(ABORT_REASON_INSTRUMENT_DETAILS_ERROR); On 2016/08/01 21:12:09, rouslan wrote: > This does not abort payment. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1107: * Adds an entry to the aborted Payment Request histogram in the bucket corresponding to the On 2016/08/01 21:12:09, rouslan wrote: > +1 indent for all * Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1111: private void recordAbortReasonHistogram(final int abortReason) { On 2016/08/01 21:12:09, rouslan wrote: > No need for final here. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:1163: ((PaymentRequestImpl) mClient).close(); On 2016/08/01 21:12:09, rouslan wrote: > No need to test. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:1168: ((PaymentRequestImpl) mClient).onConnectionError(null); On 2016/08/01 21:12:09, rouslan wrote: > No need to test. Done. https://codereview.chromium.org/2199813003/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:1173: ((PaymentRequestImpl) mClient).onInstrumentDetailsError(); On 2016/08/01 21:12:09, rouslan wrote: > 1) This is called when CVC unmask is cancelled: > > https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromiu... > > 2) Cancelling the unmask dialog does not cancel payment. UI is still showing. > User still can select a different credit card to pay. There's no need to record > abort metrics for this case. Done.
lgtm % comments https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:90: * Launches the PaymentRequest UI which accepts only and unsupported payment s/and/an/ https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... File chrome/test/data/android/payments/payment_request_metrics_test.html (right): https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... chrome/test/data/android/payments/payment_request_metrics_test.html:15: <button onclick="buy()" id="buy">Metrics Test</button><br> Are made huge in style.css, so that the test code does not miss them. Placing several buttons next to each other will cause some of them to be offscreen, especially on smaller screens that are used on some test bots. This will prevent your tests from running. I recommend using the workaround from abort.js in the same directory: Remove style.css from line 12.
sebsg@chromium.org changed reviewers: + bauerb@chromium.org, jwd@chromium.org
sebsg@chromium.org changed reviewers: + bauerb@chromium.org, jwd@chromium.org
bauerb@chromium.org: Could you please review changes in chrome/android/java/* and chrome/android/javatests/* jwd@chromium.org: Could you please review histograms.xml? Thanks! https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:90: * Launches the PaymentRequest UI which accepts only and unsupported payment On 2016/08/03 18:05:23, rouslan wrote: > s/and/an/ Done. https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... File chrome/test/data/android/payments/payment_request_metrics_test.html (right): https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... chrome/test/data/android/payments/payment_request_metrics_test.html:15: <button onclick="buy()" id="buy">Metrics Test</button><br> On 2016/08/03 18:05:23, rouslan wrote: > Are made huge in style.css, so that the test code does not miss them. Placing > several buttons next to each other will cause some of them to be offscreen, > especially on smaller screens that are used on some test bots. This will prevent > your tests from running. I recommend using the workaround from abort.js in the > same directory: Remove style.css from line 12. Done.
bauerb@chromium.org: Could you please review changes in chrome/android/java/* and chrome/android/javatests/* jwd@chromium.org: Could you please review histograms.xml? Thanks! https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:90: * Launches the PaymentRequest UI which accepts only and unsupported payment On 2016/08/03 18:05:23, rouslan wrote: > s/and/an/ Done. https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... File chrome/test/data/android/payments/payment_request_metrics_test.html (right): https://codereview.chromium.org/2199813003/diff/260001/chrome/test/data/andro... chrome/test/data/android/payments/payment_request_metrics_test.html:15: <button onclick="buy()" id="buy">Metrics Test</button><br> On 2016/08/03 18:05:23, rouslan wrote: > Are made huge in style.css, so that the test code does not miss them. Placing > several buttons next to each other will cause some of them to be offscreen, > especially on smaller screens that are used on some test bots. This will prevent > your tests from running. I recommend using the workaround from abort.js in the > same directory: Remove style.css from line 12. Done.
https://codereview.chromium.org/2199813003/diff/280001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1113: if (!mHasRecordedAbortReason) { You could return early here. https://codereview.chromium.org/2199813003/diff/280001/chrome/android/javates... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/android/javates... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java:97: for (int i = 0; i < PaymentRequestMetrics.ABORT_REASON_MAX; ++i) { It might be worth extracting this into a method that verifies that exactly one histogram value is increased. https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:13: function buy() { // eslint-disable-line no-unused-vars Out of curiosity: Where do we run a JS linting step? https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:28: resp.complete('success') If you return this from the callback, you can chain the then() calls, avoiding the "leaning tower of callbacks" as well as the need for multiple .catch() calls (rejected promises will simply propagate). https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:45: print(error.message); Maybe for a future CL: You could try to use window.onerror to install a global handler for uncaught exceptions (I haven't tried that myself, but it would remove the need to wrap every single call in a try/catch). https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:67: .then(function(resp) { Would a `=>`-style function work here as well? https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:134: request.abort().then(() => { Is this method actually called? What does |request| refer to? I see a |request| above, but both instances should be in a closure. (Also, you could try to see whether adding "use strict" to the top of the file would have caught this.)
https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:67: .then(function(resp) { On 2016/08/04 08:14:38, Bernhard Bauer wrote: > Would a `=>`-style function work here as well? These test files will eventually be used on iOS, which does not support ()=>{} style functions yet. If we use ()=>{} today, then we'd have to change it back later. So I advise against it, but you may use your own judgement.
https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:13: function buy() { // eslint-disable-line no-unused-vars On 2016/08/04 08:14:38, Bernhard Bauer wrote: > Out of curiosity: Where do we run a JS linting step? I run this manually in my editor. You can also run this locally after install eslint and the Google config (https://github.com/google/eslint-config-google): $ eslint -c google --env browser .
Patchset #6 (id:300001) has been deleted
Thanks for the nice suggestions! Another look? https://codereview.chromium.org/2199813003/diff/280001/chrome/android/java/sr... File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/android/java/sr... chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1113: if (!mHasRecordedAbortReason) { On 2016/08/04 08:14:38, Bernhard Bauer wrote: > You could return early here. Done. https://codereview.chromium.org/2199813003/diff/280001/chrome/android/javates... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/android/javates... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java:97: for (int i = 0; i < PaymentRequestMetrics.ABORT_REASON_MAX; ++i) { On 2016/08/04 08:14:38, Bernhard Bauer wrote: > It might be worth extracting this into a method that verifies that exactly one > histogram value is increased. Done. https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:28: resp.complete('success') On 2016/08/04 08:14:38, Bernhard Bauer wrote: > If you return this from the callback, you can chain the then() calls, avoiding > the "leaning tower of callbacks" as well as the need for multiple .catch() calls > (rejected promises will simply propagate). Nice! I'll fix that for the other js files too: crbug.com/634457 https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:45: print(error.message); On 2016/08/04 08:14:38, Bernhard Bauer wrote: > Maybe for a future CL: You could try to use window.onerror to install a global > handler for uncaught exceptions (I haven't tried that myself, but it would > remove the need to wrap every single call in a try/catch). I'll try that: crbug.com/634463 https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:67: .then(function(resp) { On 2016/08/04 17:16:49, rouslan wrote: > On 2016/08/04 08:14:38, Bernhard Bauer wrote: > > Would a `=>`-style function work here as well? > > These test files will eventually be used on iOS, which does not support ()=>{} > style functions yet. If we use ()=>{} today, then we'd have to change it back > later. So I advise against it, but you may use your own judgement. Removed the => completely. https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:134: request.abort().then(() => { On 2016/08/04 08:14:38, Bernhard Bauer wrote: > Is this method actually called? What does |request| refer to? I see a |request| > above, but both instances should be in a closure. > > (Also, you could try to see whether adding "use strict" to the top of the file > would have caught this.) I tried adding "use strict" but I didn't get any warning or error. Is there a special flag to set to GN or something like that? For the rest, it seems I was dumb and actually didn't test it... Fixed it now thanks!
LGTM! https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... File chrome/test/data/android/payments/metrics.js (right): https://codereview.chromium.org/2199813003/diff/280001/chrome/test/data/andro... chrome/test/data/android/payments/metrics.js:134: request.abort().then(() => { On 2016/08/04 20:47:49, sebsg wrote: > On 2016/08/04 08:14:38, Bernhard Bauer wrote: > > Is this method actually called? What does |request| refer to? I see a > |request| > > above, but both instances should be in a closure. > > > > (Also, you could try to see whether adding "use strict" to the top of the file > > would have caught this.) > > I tried adding "use strict" but I didn't get any warning or error. Is there a > special flag to set to GN or something like that? Oh right, I think even strict mode would only catch this at runtime, because it only knows at runtime whether that property on the window object exists or not... <sigh> Javascript. > For the rest, it seems I was dumb and actually didn't test it... Fixed it now > thanks! https://codereview.chromium.org/2199813003/diff/320001/chrome/android/javates... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java (right): https://codereview.chromium.org/2199813003/diff/320001/chrome/android/javates... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java:209: /* Nit: Javadoc-style comment.
Thanks Bernhard! https://codereview.chromium.org/2199813003/diff/320001/chrome/android/javates... File chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java (right): https://codereview.chromium.org/2199813003/diff/320001/chrome/android/javates... chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java:209: /* On 2016/08/05 08:56:12, Bernhard Bauer wrote: > Nit: Javadoc-style comment. Done.
lgtm
The CQ bit was checked by sebsg@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org, bauerb@chromium.org Link to the patchset: https://codereview.chromium.org/2199813003/#ps340001 (title: "Nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by sebsg@chromium.org
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/2199813003/#ps360001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #8 (id:360001)
Message was sent while issue was closed.
Description was changed from ========== [Payments] Add metrics for Payment Request aborts. BUG=631013 ========== to ========== [Payments] Add metrics for Payment Request aborts. BUG=631013 Committed: https://crrev.com/901a7d18d952ced7ed3ac901dca4860580f5a4c9 Cr-Commit-Position: refs/heads/master@{#410069} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/901a7d18d952ced7ed3ac901dca4860580f5a4c9 Cr-Commit-Position: refs/heads/master@{#410069} |
