Track age and score of snippets that are clicked (along with position).

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
index 54f9e9d5ddcd7c45f7fd59457a5509fe7ae2c273..810847eb02e9b18b9658778f192c559d434ae0dd 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
@@ -30,6 +30,7 @@ import org.chromium.chrome.browser.ntp.cards.NewTabPageViewHolder;
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.concurrent.TimeUnit;
 
 /**
  * A class that represents the view for a single card snippet.
@@ -38,6 +39,7 @@ public class SnippetArticleViewHolder extends NewTabPageViewHolder implements Vi
     private static final String TAG = "NtpSnippets";
     private static final String PUBLISHER_FORMAT_STRING = "%s - %s";
     private static final int FADE_IN_ANIMATION_TIME_MS = 300;
+    private static final int[] HISTOGRAM_FOR_POSITIONS = {0, 2, 4};
 
     private final NewTabPageManager mNewTabPageManager;
     private final TextView mHeadlineTextView;
@@ -46,6 +48,8 @@ public class SnippetArticleViewHolder extends NewTabPageViewHolder implements Vi
     private final ImageView mThumbnailView;
 
     private FetchImageCallback mImageCallback;
+    private long mPublishTimestampMilliseconds;
+    private float mScore;
 
     public String mUrl;
     public int mPosition;
@@ -95,6 +99,18 @@ public class SnippetArticleViewHolder extends NewTabPageViewHolder implements Vi
         RecordHistogram.recordSparseSlowlyHistogram("NewTabPage.Snippets.CardClicked", mPosition);
         NewTabPageUma.recordSnippetAction(NewTabPageUma.SNIPPETS_ACTION_CLICKED);
         NewTabPageUma.recordAction(NewTabPageUma.ACTION_OPENED_SNIPPET);
+
+        // track how the (approx.) position relates to age / score of the snippet that is clicked
+        long age = System.currentTimeMillis() - mPublishTimestampMilliseconds;

[Ted C, 2016/05/19 16:52:50]

FWIW, currentTimeMillis states to not use it for time deltas.

"""
This method shouldn't be used for measuring timeouts or other elapsed time
measurements, as changing the system time can affect the results. Use nanoTime()
for that.
"""

I think you are probably ok, but you might want to use a Math.max(age, 1) to
ensure you're getting something sane (and to handle any wonky user behavior). 
Unless recordCustomTimesHistogram rejects numbers outside of its valid range (no
idea honestly).

[jkrcal, 2016/05/20 12:23:19]

Heh, I am not used yet to look at android documentation. 
This line is missing in Oracle's documentation :)

Anyway, nanoTime() is no help here as I am comparing to an externally provided
timestamp.
Good point anyway. I will not record negative values as they provide no insight.

+        recordAge("NewTabPage.Snippets.CardClickedAge", age);
+        recordScore("NewTabPage.Snippets.CardClickedScore", mScore);
+        for (int position : HISTOGRAM_FOR_POSITIONS) {
+            if (mPosition <= position) {
+                String suffix = "_" + position;

[Marc Treib, 2016/05/19 16:07:10]

I think it should be "." rather than "_"?

[jkrcal, 2016/05/19 16:40:52]

histograms.xml:41 supports my version. The documentation may be outdated,
though. I need to test it properly.

[Alexei Svitkine (slow), 2016/05/19 16:42:43]

This is correct, but you can also use "." if you prefer by specifying
separator="." in the xml.

[jkrcal, 2016/05/20 12:23:19]

Thanks for the explanation!
 (I am fine with "_")

+                recordAge("NewTabPage.Snippets.CardClickedAge" + suffix, age);
+                recordScore("NewTabPage.Snippets.CardClickedScore" + suffix, mScore);

[Ted C, 2016/05/19 16:52:50]

do you want to record it for all buckets or just the narrowest range?  i.e.
should you have a break here?

So if you click item 0, then as written you'll record entries in

NewTabPage.Snippets.CardClickedAge_0
NewTabPage.Snippets.CardClickedAge_2
NewTabPage.Snippets.CardClickedAge_4

Not sure what exactly that gains you or what you're able to tell.  I would have
expected that you'd want to see how the age and score affect the position
buckets.

[jkrcal, 2016/05/20 12:23:19]

Well, you are probably right that your suggested way makes it easier to
interpret the data. I've changed it.

+            }
+        }
     }
 
     @Override
@@ -109,6 +125,8 @@ public class SnippetArticleViewHolder extends NewTabPageViewHolder implements Vi
         mArticleSnippetTextView.setText(item.mPreviewText);
         mUrl = item.mUrl;
         mPosition = item.mPosition;
+        mPublishTimestampMilliseconds = item.mTimestamp;
+        mScore = item.mScore;
 
         // If there's still a pending thumbnail fetch, cancel it.
         cancelImageFetch();
@@ -146,6 +164,18 @@ public class SnippetArticleViewHolder extends NewTabPageViewHolder implements Vi
         }
     }
 
+    private static void recordAge(String histogramName, long age) {
+        long maxAge = TimeUnit.HOURS.toMillis(72);
+        age = Math.min(age, maxAge);
+        RecordHistogram.recordCustomTimesHistogram(
+                histogramName, age, 1, maxAge, TimeUnit.MILLISECONDS, 50);
+    }
+
+    private static void recordScore(String histogramName, float score) {
+        int recordedScore = Math.min((int) Math.ceil(score), 1000);
+        RecordHistogram.recordCount1000Histogram(histogramName, recordedScore);
+    }
+
     private void cancelImageFetch() {
         if (mImageCallback != null) {
             mImageCallback.cancel();