Log creation for many dialog box types.

Log creation of many dialog box types.

Expand the logging introduced in codereview.chromium.org/2790723004
to include more dialog boxes that are of interest for DPM.

BUG=705331
Review-Url: https://codereview.chromium.org/2802703004
Cr-Commit-Position: refs/heads/master@{#465816}
Committed: https://chromium.googlesource.com/chromium/src/+/a8a9ea03f2476826e9b3a6ad6e9c08b465ac1535

[pdyson, 2017-04-06 00:49:54]

pdyson@chromium.org changed reviewers:
+ jwd@chromium.org

[pdyson, 2017-04-06 02:38:11]

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

[commit-bot: I haz the power, 2017-04-06 02:38:38]

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-06 04:40:27]

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

[commit-bot: I haz the power, 2017-04-06 04:40:27]

Dry run: This issue passed the CQ dry run.

[pdyson, 2017-04-06 05:04:56]

Jesse -- can you check that the names I've chosen for ENUMs are appropriate?

[pdyson, 2017-04-06 05:05:51]

pdyson@chromium.org changed reviewers:
+ msw@chromium.org

[pdyson, 2017-04-06 05:05:59]

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

[commit-bot: I haz the power, 2017-04-06 05:06: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-06 07:48:08]

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

[commit-bot: I haz the power, 2017-04-06 07:48:09]

Dry run: This issue passed the CQ dry run.

[msw, 2017-04-07 10:31:09]

Area owners and/or someone more invested in these histograms should review the
naming. If it's possible, it might be nice to include some additional context
alongside the histogram labels (is there a parallel description field or
similar?)

https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/confirm_bubble_views.cc (right):

https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/confirm_bubble_views.cc:46:
chrome::DialogIdentifier::ASK_GOOGLE_FOR_SUGGESTIONS);
Hmm, I know this is (probably?) only used for the spelling suggestions bubble,
but conceptually, ConfirmBubbleViews was meant to be a general-purpose bubble.
Is there a better home for recording this metric, or can we guarantee
([D]CHECK?) that this is indeed a spelling suggestions bubble instance?

https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/critical_notification_bubble_view.cc (right):

https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/critical_notification_bubble_view.cc:49:
chrome::DialogIdentifier::CRITICAL_UPGRADE_INSTALLED);
I vaguely recall this bubble being used for more than upgrade installations;
maybe rename the constant and histogram label to CRITICAL_NOTIFICATION /
"Critical Notification"?

https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
(right):

https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc:221:
chrome::RecordDialogCreation(chrome::DialogIdentifier::SCREENSHARE);
Hmm, a name closer to the class might be more appropriate; I'd potentially ping
owners for specific bubbles, especially if you deviate from the class name.

[jwd, 2017-04-07 22:36:06]

On 2017/04/07 10:31:09, msw wrote:
> Area owners and/or someone more invested in these histograms should review the
> naming. If it's possible, it might be nice to include some additional context
> alongside the histogram labels (is there a parallel description field or
> similar?)
You can include a more description in the body of the int tag of the enum. This
is surfaced in the dashboards when you hover over the enum label. It's not the
most visible thing on that UI, but it certainly would be helpful in the xml
file. Like msw commented in some of the files, it might be good to have the
class names included, either in the label, or in the body of the enum value.
> 
>
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> File chrome/browser/ui/views/confirm_bubble_views.cc (right):
> 
>
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> chrome/browser/ui/views/confirm_bubble_views.cc:46:
> chrome::DialogIdentifier::ASK_GOOGLE_FOR_SUGGESTIONS);
> Hmm, I know this is (probably?) only used for the spelling suggestions bubble,
> but conceptually, ConfirmBubbleViews was meant to be a general-purpose bubble.
> Is there a better home for recording this metric, or can we guarantee
> ([D]CHECK?) that this is indeed a spelling suggestions bubble instance?
> 
>
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> File chrome/browser/ui/views/critical_notification_bubble_view.cc (right):
> 
>
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> chrome/browser/ui/views/critical_notification_bubble_view.cc:49:
> chrome::DialogIdentifier::CRITICAL_UPGRADE_INSTALLED);
> I vaguely recall this bubble being used for more than upgrade installations;
> maybe rename the constant and histogram label to CRITICAL_NOTIFICATION /
> "Critical Notification"?
> 
>
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> File chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
> (right):
> 
>
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc:221:
> chrome::RecordDialogCreation(chrome::DialogIdentifier::SCREENSHARE);
> Hmm, a name closer to the class might be more appropriate; I'd potentially
ping
> owners for specific bubbles, especially if you deviate from the class name.

[pdyson, 2017-04-19 04:39:16]

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

[commit-bot: I haz the power, 2017-04-19 04:39:31]

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-19 06:13:06]

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

[commit-bot: I haz the power, 2017-04-19 06:13:07]

Dry run: This issue passed the CQ dry run.

[pdyson, 2017-04-19 06:16:43]

On 2017/04/07 at 22:36:06, jwd wrote:
> On 2017/04/07 10:31:09, msw wrote:
> > Area owners and/or someone more invested in these histograms should review
the
> > naming. If it's possible, it might be nice to include some additional
context
> > alongside the histogram labels (is there a parallel description field or
> > similar?)
> You can include a more description in the body of the int tag of the enum.
This is surfaced in the dashboards when you hover over the enum label. It's not
the most visible thing on that UI, but it certainly would be helpful in the xml
file. Like msw commented in some of the files, it might be good to have the
class names included, either in the label, or in the body of the enum value.
> > 
> >
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> > File chrome/browser/ui/views/confirm_bubble_views.cc (right):
> > 
> >
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> > chrome/browser/ui/views/confirm_bubble_views.cc:46:
> > chrome::DialogIdentifier::ASK_GOOGLE_FOR_SUGGESTIONS);
> > Hmm, I know this is (probably?) only used for the spelling suggestions
bubble,
> > but conceptually, ConfirmBubbleViews was meant to be a general-purpose
bubble.
> > Is there a better home for recording this metric, or can we guarantee
> > ([D]CHECK?) that this is indeed a spelling suggestions bubble instance?
> > 
> >
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> > File chrome/browser/ui/views/critical_notification_bubble_view.cc (right):
> > 
> >
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> > chrome/browser/ui/views/critical_notification_bubble_view.cc:49:
> > chrome::DialogIdentifier::CRITICAL_UPGRADE_INSTALLED);
> > I vaguely recall this bubble being used for more than upgrade installations;
> > maybe rename the constant and histogram label to CRITICAL_NOTIFICATION /
> > "Critical Notification"?
> > 
> >
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> > File chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
> > (right):
> > 
> >
https://codereview.chromium.org/2802703004/diff/20001/chrome/browser/ui/views...
> > chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc:221:
> > chrome::RecordDialogCreation(chrome::DialogIdentifier::SCREENSHARE);
> > Hmm, a name closer to the class might be more appropriate; I'd potentially
ping
> > owners for specific bubbles, especially if you deviate from the class name.


I've discussed this with the team and we have decided to stick closer to
the class names for these enums, as suggested. So I've made those changes
and put some of the more specific information in parentheses in the histogram
labels.

[msw, 2017-04-19 18:27:58]

lgtm

[jwd, 2017-04-19 19:02:10]

lgtm

[pdyson, 2017-04-19 23:54:49]

The CQ bit was checked by pdyson@chromium.org

[commit-bot: I haz the power, 2017-04-19 23:55:21]

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-20 00:00:19]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492646089432280,
"parent_rev": "a6618dc84a89e04cad9c0badf3097b8af91c7795", "commit_rev":
"a8a9ea03f2476826e9b3a6ad6e9c08b465ac1535"}

[commit-bot: I haz the power, 2017-04-20 00:00:29]

Description was changed from

==========
Log creation of many dialog box types.

Expand the logging introduced in codereview.chromium.org/2790723004
to include more dialog boxes that are of interest for DPM.

BUG=705331
==========

to

==========
Log creation of many dialog box types.

Expand the logging introduced in codereview.chromium.org/2790723004
to include more dialog boxes that are of interest for DPM.

BUG=705331

Review-Url: https://codereview.chromium.org/2802703004
Cr-Commit-Position: refs/heads/master@{#465816}
Committed:
https://chromium.googlesource.com/chromium/src/+/a8a9ea03f2476826e9b3a6ad6e9c...
==========

[commit-bot: I haz the power, 2017-04-20 00:00:30]

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