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

components/ntp_tiles/metrics.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/ntp_tiles/metrics.h"
 
 #include <string>
 
 #include "base/metrics/histogram.h"
 #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 {
 
 namespace {
 
 // Maximum number of tiles to record in histograms.
 const int kMaxNumTiles = 12;
 
 // Identifiers for the various tile sources.
@@ skipping 25 matching lines @@
       return kHistogramWhitelistName;
     case NTPTileSource::SUGGESTIONS_SERVICE:
       return kHistogramServerName;
   }
   NOTREACHED();
   return std::string();
 }
 
 }  // 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);
 
     std::string source_name = GetSourceHistogramName(source);
     std::string impression_histogram = base::StringPrintf(
         "NewTabPage.SuggestionsImpression.%s", source_name.c_str());
     LogHistogramEvent(impression_histogram, index, kMaxNumTiles);
 
     if (tile_type >= NUM_RECORDED_TILE_TYPES) {
       continue;
     }
 
     have_tile_types = true;
     ++counts_per_type[tile_type];
 
     UMA_HISTOGRAM_ENUMERATION("NewTabPage.TileType", tile_type,
                               NUM_RECORDED_TILE_TYPES);
 
     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 ICON_COLOR:
+        rappor::SampleDomainAndRegistryFromGURL(
+            rappor_service, "NTP.SuggestionsImpressions.IconsColor", url);

[Marc Treib, 2016/12/08 12:58:22]

It's a bit unfortunate that these use a different naming convention vs the
histograms ("NTP" vs "NewTabPage"). But I guess staying consistent with the
existing Rappor metrics is more important...

[mastiz, 2016/12/08 13:47:07]

Acknowledged, that was my rationale, although I don't have a strong preference.

+        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;
+      default:

[Marc Treib, 2016/12/08 12:58:22]

Explicitly list the uninteresting cases? Then we'll get a compile error if a new
one is added that's not handled here.
Or alternatively, add a comment about why it's okay to ignore all other cases.
(And maybe add a NOTREACHED()? Due to the "if (tile_type >=
NUM_RECORDED_TILE_TYPES)" above we shouldn't even get here, right?)

[mastiz, 2016/12/08 13:47:07]

Done.

+        break;
+    }
   }
 
   if (have_tile_types) {
     UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.IconsReal",
                                 counts_per_type[ICON_REAL]);
     UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.IconsColor",
                                 counts_per_type[ICON_COLOR]);
     UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.IconsGray",
                                 counts_per_type[ICON_DEFAULT]);
   }
@@ skipping 14 matching lines @@
 
     std::string histogram =
         base::StringPrintf("NewTabPage.TileTypeClicked.%s",
                            GetSourceHistogramName(source).c_str());
     LogHistogramEvent(histogram, tile_type, NUM_RECORDED_TILE_TYPES);
   }
 }
 
 }  // namespace metrics
 }  // namespace ntp_tiles