[Suggestions] Add User action UMA for context menu actions

chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
index b3d078b387df8e40d788beedc8de93c0e1cbb58f..34f1dced433ae6a0f656e7e67b352585a8eb5185 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
@@ -13,6 +13,7 @@ import android.view.MenuItem;
 import android.view.MenuItem.OnMenuItemClickListener;
 import android.view.View;
 
+import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.suggestions.SuggestionsNavigationDelegate;
@@ -123,6 +124,9 @@ public class ContextMenuManager implements OnCloseContextMenuListener {
         // https://crbug.com/638555, https://crbug.com/636296).
         mTouchEnabledDelegate.setTouchEnabled(false);
         mContextMenuOpen = true;
+
+        // Record user action.

[dgn, 2017/04/20 18:10:24]

nit: remove this comment, it's not adding anything to explain the code. The
empty line is enough for separation purposes

[Galia, 2017/04/24 15:13:49]

Done.

+        RecordUserAction.record("NewTabPage_ContextMenuShown");

[Michael van Ouwerkerk, 2017/04/24 13:13:21]

What's the latest guidance on separators... underscores vs dots vs none? I find
dots easier on the eye, but maybe that is not recommended.

[Galia, 2017/04/24 15:13:49]

I'm not sure how to check what the latest guidance is, but in the file every
entry is different. I agree that dots are more clear, so I'll change to all use
dots instead of underscores.

     }
 
     @Override
@@ -184,18 +188,23 @@ public class ContextMenuManager implements OnCloseContextMenuListener {
             switch (item.getItemId()) {
                 case ID_OPEN_IN_NEW_WINDOW:
                     mDelegate.openItem(WindowOpenDisposition.NEW_WINDOW);
+                    RecordUserAction.record("NewTabPage_OpenItemInNewWindow");

[dgn, 2017/04/20 18:10:24]

I'd use ContentSuggestions.ContextMenu.OpenItemInNewWindow, and similar for
strings below.

bauerb@, mvanouwerkerk@, wdyt? the current UMA is all scoped to NewTabPage so
this is consistent, but it's unlikely that we will rename user actions. I would
prefer not to add new UMA with inaccurate names.

[Bernhard Bauer, 2017/04/24 13:06:35]

Hm, OTOH inconsistent naming isn't great either :-/ I don't suppose we can
define aliases for user actions?

[Michael van Ouwerkerk, 2017/04/24 13:13:21]

I think ContentSuggestions is also not quite correct for a Tile, is that
considered content? They're mostly top level site urls. How about
Suggestions.ContextMenu.OpenItemInNewWindow ?

[Galia, 2017/04/24 15:13:49]

It's true that "Suggestions" is more precise for this case, but this introduces
more inconsistency as everywhere in the other user actions is used
"_ContentSuggestions_smth" (underscores as well).
Should I name it "Suggestions. ..." anyway? @dgn, @bauerb?

[dgn, 2017/04/24 15:31:44]

Suggestions.foo lgtm. We have to start fixing our metrics somewhere.

                     return true;
                 case ID_OPEN_IN_NEW_TAB:
                     mDelegate.openItem(WindowOpenDisposition.NEW_BACKGROUND_TAB);
+                    RecordUserAction.record("NewTabPage_OpenItemInNewTab");

[Michael van Ouwerkerk, 2017/04/24 13:13:21]

It's a bit unfortunate that we cannot distinguish at all what kind of item we're
talking about here. A tile, a card? For what category? This is a good
improvement though, so I#m happy for this CL to go ahead as currently designed.

[dgn, 2017/04/24 14:06:37]

IMO distinguishing the type of content should be done at the level of the
implementer. So we could have something like
Suggestions.ContextMenu.OpenItemInNewWindow -> Suggestions.Downloads.Open and
Suggestions.Card.Tap -> Suggestions.Downloads.Open (made up names), allowing
also to have the breakdown of how items of specific categories are opened. 

So it could be done in a follow up CL looking more generally at all the
interactions in the suggestions UI, and this one can land as-is

                     return true;
                 case ID_OPEN_IN_INCOGNITO_TAB:
                     mDelegate.openItem(WindowOpenDisposition.OFF_THE_RECORD);
+                    RecordUserAction.record("NewTabPage_OpenItemInIncognitoTab");
                     return true;
                 case ID_SAVE_FOR_OFFLINE:
                     mDelegate.openItem(WindowOpenDisposition.SAVE_TO_DISK);
+                    RecordUserAction.record("NewTabPage_DownloadItem");
                     return true;
                 case ID_REMOVE:
                     mDelegate.removeItem();
+                    RecordUserAction.record("NewTabPage_RemoveItem");
                     return true;
                 default:
                     return false;