ntp_tiles::metrics: Add rappor metrics for impression URLs per icon type.

components/ntp_tiles/metrics.cc

Index: components/ntp_tiles/metrics.cc
diff --git a/components/ntp_tiles/metrics.cc b/components/ntp_tiles/metrics.cc
index 1b9a64fced2270fe4c4c98d9d102f7852232409a..485cd1b181ad099edce52b265f4584441b60dcf6 100644
--- a/components/ntp_tiles/metrics.cc
+++ b/components/ntp_tiles/metrics.cc
@@ -10,6 +10,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/stringprintf.h"
+#include "components/rappor/public/rappor_utils.h"
 
 namespace ntp_tiles {
 namespace metrics {
@@ -55,15 +56,16 @@ std::string GetSourceHistogramName(NTPTileSource source) {
 
 }  // namespace
 
-void RecordPageImpression(
-    const std::vector<std::pair<NTPTileSource, MostVisitedTileType>>& tiles) {
+void RecordPageImpression(const std::vector<TileImpression>& tiles,
+                          rappor::RapporService* rappor_service) {
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.NumberOfTiles", tiles.size());
 
   int counts_per_type[NUM_RECORDED_TILE_TYPES] = {0};
   bool have_tile_types = false;
   for (int index = 0; index < static_cast<int>(tiles.size()); index++) {
-    NTPTileSource source = tiles[index].first;
-    MostVisitedTileType tile_type = tiles[index].second;
+    NTPTileSource source = tiles[index].source;
+    MostVisitedTileType tile_type = tiles[index].type;
+    const GURL& url = tiles[index].url;
 
     UMA_HISTOGRAM_ENUMERATION("NewTabPage.SuggestionsImpression", index,
                               kMaxNumTiles);
@@ -86,6 +88,29 @@ void RecordPageImpression(
     std::string tile_type_histogram =
         base::StringPrintf("NewTabPage.TileType.%s", source_name.c_str());
     LogHistogramEvent(tile_type_histogram, tile_type, NUM_RECORDED_TILE_TYPES);
+
+    switch (tile_type) {
+      case NONE:
+        break;

[Marc Treib, 2016/12/08 13:52:59]

Can this case happen?

[mastiz, 2016/12/08 14:02:51]

Yes, NONE is 0 and thus < NUM_RECORDED_TILE_TYPES.

In practice, UMA metrics like MostVisitedTileType also reflect a small but
non-zero value for bucket 0.

+      case ICON_COLOR:
+        rappor::SampleDomainAndRegistryFromGURL(
+            rappor_service, "NTP.SuggestionsImpressions.IconsColor", url);
+        break;
+      case ICON_DEFAULT:
+        rappor::SampleDomainAndRegistryFromGURL(
+            rappor_service, "NTP.SuggestionsImpressions.IconsGray", url);
+        break;
+      case ICON_REAL:
+        rappor::SampleDomainAndRegistryFromGURL(
+            rappor_service, "NTP.SuggestionsImpressions.IconsReal", url);
+        break;
+      case NUM_RECORDED_TILE_TYPES:
+        NOTREACHED();
+      case THUMBNAIL:
+        NOTREACHED();
+      case UNKNOWN_TILE_TYPE:

[Marc Treib, 2016/12/08 13:52:59]

IMO this is one of the rare instances where fallthrough is okay, i.e. only a
single NOTREACHED() after the 3 cases.

[mastiz, 2016/12/08 14:02:51]

Done.

+        NOTREACHED();
+    }
   }
 
   if (have_tile_types) {