[Android] Add UMA histograms to measure context menu actions.

[Android] Add UMA histograms to measure context menu actions.

This adds four related histograms to measure how users interact with
context menus. The histograms are ContextMenu.Link, ContextMenu.Image,
ContextMenu.ImageLink, and ContextMenu.Video, and the values are the
various actions that can be taken on each context menu, e.g. "Open in
new tab" or "Save image".

For example, if a user long presses an image and selects "Copy image",
we'll record the "Copy image" event in the ContextMenu.Image histogram.

ContextMenu.ImageLink is used when the user long presses an image which
is also a link. This case is treated specially because its context menu
is currently so long that it needs extra love and investigation.

BUG=483685
Committed: https://crrev.com/a60a2e9061334a5726ae30166ef004209719f4f9
Cr-Commit-Position: refs/heads/master@{#343283}

[newt (away), 2015-08-04 01:19:39]

newt@chromium.org changed reviewers:
+ asvitkine@chromium.org, dtrainor@chromium.org, edwardjung@chromium.org

[newt (away), 2015-08-04 01:19:40]

dtrainor: ChromeContextMenuPopulator.java
asvitkine: histograms.xml

[newt (away), 2015-08-04 01:20:15]

edwardjung: Could you chime in on whether the histograms I've created will work
for desktop too?

[David Trainor- moved to gerrit, 2015-08-04 20:08:06]

lgtm

https://chromiumcodereview.appspot.com/1267213002/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
(right):

https://chromiumcodereview.appspot.com/1267213002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:52:
static void record(ContextMenuParams params, int action) {
assert action is valid (< NUM_ACTIONS)?

[Alexei Svitkine (slow), 2015-08-04 21:29:13]

https://codereview.chromium.org/1267213002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1267213002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:4120: +<histogram
name="ContextMenu.Image" enum="ContextMenuAction">
Since these are Android specific, perhaps they should be named
Android.ContextMenu.*? Or are you planning to add these for other platforms as
well?

(One reason I'm asking is that we try to avoid adding many new top-level
prefixes, since this clutters the top-level list in some of our dashboard UIs.
So finding existing top level prefixes to nest under is preferable.)

https://codereview.chromium.org/1267213002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:4143: +<histogram
name="ContextMenu.Video" enum="ContextMenuAction">
Since you have a number of suffixes that split the metrics depending on what
you're context menu-ing on, I suggest using the histogram_suffixes feature of
the XML format to make this more concise. See comment at the top of the file for
an example.

[edwardjung, 2015-08-10 10:32:55]

On 2015/08/04 01:20:15, newt wrote:
> edwardjung: Could you chime in on whether the histograms I've created will
work
> for desktop too?

These work for desktop but there are a few more contexts than the four, the main
one being just the page itself. I know on Android this brings up just the text
selection tools / context search. So maybe follow the Android.ContextMenu
suggestion from asvitkine.

[newt (away), 2015-08-10 20:24:01]

On 2015/08/10 10:32:55, edwardjung wrote:
> On 2015/08/04 01:20:15, newt wrote:
> > edwardjung: Could you chime in on whether the histograms I've created will
> work
> > for desktop too?
> 
> These work for desktop but there are a few more contexts than the four, the
main
> one being just the page itself. I know on Android this brings up just the text
> selection tools / context search. So maybe follow the Android.ContextMenu
> suggestion from asvitkine.

We have three options for sharing histograms between Android and desktop:
 1. Use the same histograms (ContextMenu.Video, ContextMenu.Link, etc).
ContextMenuAction will have all the various actions for all the context menus.
Desktop will have some additional Desktop-only histograms (e.g.
ContextMenu.Page).
 2. Use different histograms (Android.ContextMenu.Video,
Android.ContextMenu.Video, etc), but the same ContextMenuAction enum.
 3. Don't share any histogram pieces

Seems like 1 would work, and it shares the most, so that's what I'd prefer.
edwardjung@, you seem to be suggesting 2 or 3. Which one and why?

[edwardjung, 2015-08-11 10:38:43]

On 2015/08/10 20:24:01, newt wrote:
> On 2015/08/10 10:32:55, edwardjung wrote:
> > On 2015/08/04 01:20:15, newt wrote:
> > > edwardjung: Could you chime in on whether the histograms I've created will
> > work
> > > for desktop too?
> > 
> > These work for desktop but there are a few more contexts than the four, the
> main
> > one being just the page itself. I know on Android this brings up just the
text
> > selection tools / context search. So maybe follow the Android.ContextMenu
> > suggestion from asvitkine.
> 
> We have three options for sharing histograms between Android and desktop:
>  1. Use the same histograms (ContextMenu.Video, ContextMenu.Link, etc).
> ContextMenuAction will have all the various actions for all the context menus.
> Desktop will have some additional Desktop-only histograms (e.g.
> ContextMenu.Page).
>  2. Use different histograms (Android.ContextMenu.Video,
> Android.ContextMenu.Video, etc), but the same ContextMenuAction enum.
>  3. Don't share any histogram pieces
> 
> Seems like 1 would work, and it shares the most, so that's what I'd prefer.
> edwardjung@, you seem to be suggesting 2 or 3. Which one and why?

Sure option 1 would work, I can add the extra contexts when I come to
implementing these metrics for desktop. I just wasn't sure whether there was
merit in pooling these metrics since I'm not sure when we would need aggregate
metrics since the platforms are so different. For the desktop context menu
analysis I'll need to exclude the Android metrics anyway, for Android Rebecca
would exclude desktop.

@asvitkine what do you suggest would be best?

[Alexei Svitkine (slow), 2015-08-11 15:43:10]

So, from dashboard use point of view, it's fine to have different enums be
logged by different platform. When you do an analysis - you just have to be
careful to not analyze "Everything" but instead choose a platform in the version
selector (e.g. "All-A").

I'm okay with 1. assuming you are indeed planning to add a number of histograms
under the new prefix. (If it's only a couple, I usually try to steer folks to
adopting an existing prefix).

[newt (away), 2015-08-12 23:43:20]

https://codereview.chromium.org/1267213002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
(right):

https://codereview.chromium.org/1267213002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java:52:
static void record(ContextMenuParams params, int action) {
On 2015/08/04 20:08:06, David Trainor wrote:
> assert action is valid (< NUM_ACTIONS)?

Done.

https://codereview.chromium.org/1267213002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1267213002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:4143: +<histogram
name="ContextMenu.Video" enum="ContextMenuAction">
On 2015/08/04 21:29:13, Alexei Svitkine wrote:
> Since you have a number of suffixes that split the metrics depending on what
> you're context menu-ing on, I suggest using the histogram_suffixes feature of
> the XML format to make this more concise. See comment at the top of the file
for
> an example.

Done.

[Alexei Svitkine (slow), 2015-08-13 15:35:40]

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

https://codereview.chromium.org/1267213002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:4159: +<histogram name="ContextMenu"
enum="ContextMenuAction">
I think this should be named ContextMenu.SelectedOption or something similar.
"ContextMenu" doesn't provide enough context (heh) - even with the suffixes
(e.g. ContextMenu.Link).

[newt (away), 2015-08-13 17:32:03]

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

https://codereview.chromium.org/1267213002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:4159: +<histogram name="ContextMenu"
enum="ContextMenuAction">
On 2015/08/13 15:35:40, Alexei Svitkine (OOO Aug15-24) wrote:
> I think this should be named ContextMenu.SelectedOption or something similar.
> "ContextMenu" doesn't provide enough context (heh) - even with the suffixes
> (e.g. ContextMenu.Link).

Done. I agree that it's clearer now :)

[Alexei Svitkine (slow), 2015-08-13 17:32:59]

lgtm

[newt (away), 2015-08-13 17:37:27]

edwardjung: Does this all look good to you?

[edwardjung, 2015-08-13 17:53:49]

lgtm

[newt (away), 2015-08-13 20:41:21]

The CQ bit was checked by newt@chromium.org

[newt (away), 2015-08-13 20:41:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/1267213002/#ps40001
(title: "renamed to ContextMenu.SelectedOption")

[commit-bot: I haz the power, 2015-08-13 20:42:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1267213002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1267213002/40001

[commit-bot: I haz the power, 2015-08-13 22:01:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-13 22:02:15]

Patchset 3 (id:??) landed as
https://crrev.com/a60a2e9061334a5726ae30166ef004209719f4f9
Cr-Commit-Position: refs/heads/master@{#343283}