I cannot reproduce this.

I modified LogDialogDismissalCause as so:

void JavaScriptDialogTabHelper::LogDialogDismissalCause(
    JavaScriptDialogTabHelper::DismissalCause cause) {
  LOG(ERROR) << "dialog dismissed; cause " << (int)cause;  // << -- modified line
  switch (dialog_type_) {

Clicking OK or Cancel yields
[62417:1295:0810/132458.385374:ERROR:javascript_dialog_tab_helper.cc(472)] dialog dismissed; cause 6
which is DIALOG_BUTTON_CLICKED.

Closing the tab yields
[62417:1295:0810/132603.439726:ERROR:javascript_dialog_tab_helper.cc(472)] dialog dismissed; cause 3
which is CANCEL_DIALOGS_CALLED.

Please clarify what you are seeing and how to achieve the misclassification.

Also, re your original post, how do you match up instances of the UMA recording with your actions? UMA stats take a little while to kick in.

I highly recommend that you try to reproduce with logging statements rather than looking at the UMA histograms afterwards.

Filed Issue 873236 to track fixing JS dialog metrics on Android

FYI, https://chromium-review.googlesource.com/c/chromium/src/+/1171462 shows this is an issue on every platform but Mac. :(

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a84f590ccefe258375248ced877e213dd86999be

commit a84f590ccefe258375248ced877e213dd86999be
Author: Yao Xiao <yaoxia@chromium.org>
Date: Thu Aug 16 23:41:04 2018

Fix the JS dialog dismissal cause

Previously on a non-Mac platform when JS dialog is dismissed by user
closing the tab, |DIALOG_BUTTON_CLICKED| is logged. This change
separates the case of the user replying to the dialog with the buttons
vs rejecting it.

Bug: 872795
Change-Id: I4b8781daeb3cead5ee4e975a67f79f94b7fa84e5
Reviewed-on: https://chromium-review.googlesource.com/1164647
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583885}
[modify] https://crrev.com/a84f590ccefe258375248ced877e213dd86999be/chrome/browser/ui/javascript_dialogs/javascript_dialog_browsertest.cc
[modify] https://crrev.com/a84f590ccefe258375248ced877e213dd86999be/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/a84f590ccefe258375248ced877e213dd86999be/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[modify] https://crrev.com/a84f590ccefe258375248ced877e213dd86999be/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.cc
[modify] https://crrev.com/a84f590ccefe258375248ced877e213dd86999be/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.h
[modify] https://crrev.com/a84f590ccefe258375248ced877e213dd86999be/tools/metrics/histograms/enums.xml

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe

commit c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Fri Aug 17 09:50:08 2018

Revert "Fix the JS dialog dismissal cause"

This reverts commit a84f590ccefe258375248ced877e213dd86999be.

Reason for revert: the introduced test JavaScriptDialogTest.DismissalCausePromptTabClosedByUser fails more often than it passes (e.g. https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29/4745)
This test seems to be fairly essential given that this CL mainly affects the log. If it's not, feel free to disable on Mac and reland.

Original change's description:
> Fix the JS dialog dismissal cause
> 
> Previously on a non-Mac platform when JS dialog is dismissed by user
> closing the tab, |DIALOG_BUTTON_CLICKED| is logged. This change
> separates the case of the user replying to the dialog with the buttons
> vs rejecting it.
> 
> Bug: 872795
> Change-Id: I4b8781daeb3cead5ee4e975a67f79f94b7fa84e5
> Reviewed-on: https://chromium-review.googlesource.com/1164647
> Reviewed-by: Charlie Harrison <csharrison@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Yao Xiao <yaoxia@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583885}

TBR=avi@chromium.org,csharrison@chromium.org,huayinz@chromium.org,yaoxia@chromium.org

Change-Id: I0de2f8e0ded623c121325a80975d131681aaa493
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 872795
Reviewed-on: https://chromium-review.googlesource.com/1179701
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584004}
[modify] https://crrev.com/c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe/chrome/browser/ui/javascript_dialogs/javascript_dialog_browsertest.cc
[modify] https://crrev.com/c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[modify] https://crrev.com/c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.cc
[modify] https://crrev.com/c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.h
[modify] https://crrev.com/c4b2c0cbbd67f79dcaa29646368a9a5997cd29fe/tools/metrics/histograms/enums.xml

The flake linked is interesting. JavaScriptDialogTest.DismissalCausePromptTabClosedByUser says kCancelDialogsCalled for Mac, kDialogClosed for other platforms. I wonder if on the Mac it's a race and that perhaps sometimes we get kDialogClosed.

Hmmmm...

Maybe we can use HistogramTester to test that there was one sample (ExpectTotalCount) and we can use GetBucketCount to check that the counts for the two buckets add to 1 (meaning that it's in either of those buckets).

Avi: That's a possible cause. Have you ever seen the test is flaky on Mac platform? I'm just thinking how to verify that. If we are not going to trace down the race right now,  it'd still be good to verify your assumption and fix the test in the way you suggested.

That's a wild guess on my part. I don't know for sure. A way to verify it is to add logging for the exact number and then run trybots, I guess. It definitely should be verified.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee6dac2e080228132192bdd4f95cff3921375c65

commit ee6dac2e080228132192bdd4f95cff3921375c65
Author: Yao Xiao <yaoxia@chromium.org>
Date: Wed Aug 22 17:18:25 2018

Fix JS dialog dismissal cause - reland

To reland 1164647 which was reverted in 1179701 where one test
fails on MacOS 10.13. After manual testing on MacOS 10.13, it
consistently logs |kDialogClosed| rather than the expected
|kCancelDialogsCalled|. It never appears as flaky since for
all builds between 4738 and 4746
(https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29?limit=200),
where the reverted change was in place, the test
"browser_tests on (none) GPU" never succeeded.

Thus, we need to relax the check in this test for MacOS to cover
the behavior difference for different versions.


Bug: 872795
Change-Id: Ie2c5ccd4881f5c8bbbf37bd7aae4be4fc5591c59
Reviewed-on: https://chromium-review.googlesource.com/1181245
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585117}
[modify] https://crrev.com/ee6dac2e080228132192bdd4f95cff3921375c65/chrome/browser/ui/javascript_dialogs/javascript_dialog_browsertest.cc
[modify] https://crrev.com/ee6dac2e080228132192bdd4f95cff3921375c65/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/ee6dac2e080228132192bdd4f95cff3921375c65/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[modify] https://crrev.com/ee6dac2e080228132192bdd4f95cff3921375c65/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.cc
[modify] https://crrev.com/ee6dac2e080228132192bdd4f95cff3921375c65/chrome/browser/ui/javascript_dialogs/javascript_dialog_views.h
[modify] https://crrev.com/ee6dac2e080228132192bdd4f95cff3921375c65/tools/metrics/histograms/enums.xml