Added UMA_HISTOGRAM_ENUMERATION for RenderViewContextMenu.

chrome/browser/tab_contents/render_view_context_menu.cc

Index: chrome/browser/tab_contents/render_view_context_menu.cc
diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index 8a46c6e7631df2d894938b7ee2f824f9c0b2a5e4..21d58caab224e2bcdf169a1c8ccb94b32c5b001b 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -117,6 +117,97 @@ using extensions::MenuManager;
 
 namespace {
 
+// Maps UMA enumeration to IDC. IDC could be changed so we can't use
+// just them and |UMA_HISTOGRAM_CUSTOM_ENUMERATION|.
+static const struct {

[Ilya Sherman, 2013/04/10 02:22:42]

nit: No need for static.  You're already in an anonymous namespace.

[Vitaly Buka (NO REVIEWS), 2013/04/10 03:34:55]

Done.

+  int enum_id;
+  int command_id;
+} kUmaEnumToCommand[] = {
+  {  0, IDC_CONTENT_CONTEXT_CUSTOM_FIRST },
+  {  1, IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST },
+  {  2, IDC_CONTENT_CONTEXT_PROTOCOL_HANDLER_FIRST },
+  {  3, IDC_CONTENT_CONTEXT_OPENLINKNEWTAB },
+  {  4, IDC_CONTENT_CONTEXT_OPENLINKNEWWINDOW },
+  {  5, IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD },
+  {  6, IDC_CONTENT_CONTEXT_SAVELINKAS },
+  {  7, IDC_CONTENT_CONTEXT_SAVEAVAS },
+  {  8, IDC_CONTENT_CONTEXT_SAVEIMAGEAS },
+  {  9, IDC_CONTENT_CONTEXT_COPYLINKLOCATION },
+  { 10, IDC_CONTENT_CONTEXT_COPYIMAGELOCATION },
+  { 11, IDC_CONTENT_CONTEXT_COPYAVLOCATION },
+  { 12, IDC_CONTENT_CONTEXT_COPYIMAGE },
+  { 13, IDC_CONTENT_CONTEXT_OPENIMAGENEWTAB },
+  { 14, IDC_CONTENT_CONTEXT_OPENAVNEWTAB },
+  { 15, IDC_CONTENT_CONTEXT_PLAYPAUSE },
+  { 16, IDC_CONTENT_CONTEXT_MUTE },
+  { 17, IDC_CONTENT_CONTEXT_LOOP },
+  { 18, IDC_CONTENT_CONTEXT_CONTROLS },
+  { 19, IDC_CONTENT_CONTEXT_ROTATECW },
+  { 20, IDC_CONTENT_CONTEXT_ROTATECCW },
+  { 21, IDC_BACK },
+  { 22, IDC_FORWARD },
+  { 23, IDC_SAVE_PAGE },
+  { 24, IDC_RELOAD },
+  { 25, IDC_CONTENT_CONTEXT_RELOAD_PACKAGED_APP },
+  { 26, IDC_CONTENT_CONTEXT_RESTART_PACKAGED_APP },
+  { 27, IDC_PRINT },
+  { 28, IDC_VIEW_SOURCE },
+  { 29, IDC_CONTENT_CONTEXT_INSPECTELEMENT },
+  { 30, IDC_CONTENT_CONTEXT_INSPECTBACKGROUNDPAGE },
+  { 31, IDC_CONTENT_CONTEXT_VIEWPAGEINFO },
+  { 32, IDC_CONTENT_CONTEXT_TRANSLATE },
+  { 33, IDC_CONTENT_CONTEXT_RELOADFRAME },
+  { 34, IDC_CONTENT_CONTEXT_VIEWFRAMESOURCE },
+  { 35, IDC_CONTENT_CONTEXT_VIEWFRAMEINFO },
+  { 36, IDC_CONTENT_CONTEXT_UNDO },
+  { 37, IDC_CONTENT_CONTEXT_REDO },
+  { 38, IDC_CONTENT_CONTEXT_CUT },
+  { 39, IDC_CONTENT_CONTEXT_COPY },
+  { 40, IDC_CONTENT_CONTEXT_PASTE },

[Vitaly Buka (NO REVIEWS), 2013/04/10 02:50:00]

Ilya, I assume it's OK to extend enum in new versions?. We don't need to reserve
some large boundary value?

[Ilya Sherman, 2013/04/10 02:51:43]

Yes, it's safe to extend the enum in new versions as long as you ensure that the
histogram's boundary value is one more than any legal value.

+  { 41, IDC_CONTENT_CONTEXT_PASTE_AND_MATCH_STYLE },
+  { 42, IDC_CONTENT_CONTEXT_DELETE },
+  { 43, IDC_CONTENT_CONTEXT_SELECTALL },
+  { 44, IDC_CONTENT_CONTEXT_SEARCHWEBFOR },
+  { 45, IDC_CONTENT_CONTEXT_GOTOURL },
+  { 46, IDC_CONTENT_CONTEXT_LANGUAGE_SETTINGS },
+  { 47, IDC_CONTENT_CONTEXT_PROTOCOL_HANDLER_SETTINGS },
+  { 48, IDC_CONTENT_CONTEXT_ADDSEARCHENGINE },
+  { 49, IDC_CONTENT_CONTEXT_SPEECH_INPUT_FILTER_PROFANITIES },
+  { 50, IDC_CONTENT_CONTEXT_SPEECH_INPUT_ABOUT },
+};
+
+int GetEnumerationBoundaryValue() {

[Ilya Sherman, 2013/04/10 02:22:42]

nit: Docs.

[Lei Zhang, 2013/04/10 02:49:57]

If you never delete from |kUmaEnumToCommand|, then you can just return
arraysize() here.

If you keep the code as is, make boundary_value static and initialize it to, say
-1. If it's not -1, then just return the value instead of recalculating every
time.

Also, bounadary_value is misspelled.

[Vitaly Buka (NO REVIEWS), 2013/04/10 03:34:55]

IDC_ constants can be deleted in future. Otherwise I'd be using array of int,
not structures.
That's not necessary. UMA_HISTOGRAM_ENUMERATION implemented  such way, so it
calls GetEnumerationBoundaryValue twice (I still expected once :-) )
GetEnumerationBoundaryValue is just protection from cases when someone puts
largest value in the middle.

Probably to protective. Comment should be enough.

+  int bounadary_value = 0;
+  for (size_t i = 0; i < arraysize(kUmaEnumToCommand); ++i) {
+    bounadary_value = std::max(bounadary_value, kUmaEnumToCommand[i].enum_id);
+  }
+  return bounadary_value + 1;
+}
+
+void RecordExecutedContextItem(int id) {

[Ilya Sherman, 2013/04/10 02:22:42]

nit: Docs.

[Vitaly Buka (NO REVIEWS), 2013/04/10 03:34:55]

Done.

+  // Collapse large ranges of ids.
+  if (id >= IDC_CONTENT_CONTEXT_CUSTOM_FIRST &&
+      id <= IDC_CONTENT_CONTEXT_CUSTOM_LAST) {
+    id = IDC_CONTENT_CONTEXT_CUSTOM_FIRST;
+  } else if (id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST &&
+             id <= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST) {
+    id = IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST;
+  } else if (id >= IDC_CONTENT_CONTEXT_PROTOCOL_HANDLER_FIRST &&
+             id <= IDC_CONTENT_CONTEXT_PROTOCOL_HANDLER_LAST) {
+    id = IDC_CONTENT_CONTEXT_PROTOCOL_HANDLER_FIRST;
+  }
+
+  for (size_t i = 0; i < arraysize(kUmaEnumToCommand); ++i) {
+    if (kUmaEnumToCommand[i].command_id == id) {
+      UMA_HISTOGRAM_ENUMERATION("RenderViewContextMenu.ExecuteCommand",
+                                kUmaEnumToCommand[i].enum_id,
+                                GetEnumerationBoundaryValue());
+      return;
+    }
+  }
+  NOTREACHED();
+}
+
 // Usually a new tab is expected where this function is used,
 // however users should be able to open a tab in background
 // or in a new window.
@@ -1337,6 +1428,8 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
       return observer->ExecuteCommand(id);
   }
 
+  RecordExecutedContextItem(id);
+
   RenderViewHost* rvh = source_web_contents_->GetRenderViewHost();
 
   // Process custom actions range.