Record impressions/navigations only once per tile.

chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc

Index: chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc
diff --git a/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc b/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc
index 43d9cbc71740667f1f454333e559c0b770c0875d..5401c7af15e8b4b5ab2f283a8f67d08ce111ba1c 100644
--- a/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc
+++ b/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc
@@ -9,10 +9,14 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/common/search/ntp_logging_events.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
+using testing::ElementsAre;
+using Samples = std::vector<base::HistogramBase::Sample>;

[Marc Treib, 2016/07/14 14:40:52]

nit: Also add "using base::HistogramBase::Sample" for consistency?

[sfiera, 2016/07/15 09:55:10]

Done.

+
 class TestNTPUserDataLogger : public NTPUserDataLogger {
  public:
   TestNTPUserDataLogger() : NTPUserDataLogger(NULL) {}
@@ -34,6 +38,39 @@ base::HistogramBase::Count GetBinCount(const std::string& histogram_name,
   return histogram ? histogram->SnapshotSamples()->GetCount(value) : 0;
 }
 
+std::vector<int> GetBinCounts(const std::string& histogram_name,
+                              const Samples& values) {
+  std::vector<int> results;
+  for (const auto& value : values) {
+    results.push_back(GetBinCount(histogram_name, value));
+  }
+  return results;
+}
+
+std::vector<int> TotalImpressions(const Samples& values) {
+  return GetBinCounts("NewTabPage.SuggestionsImpression", values);
+}
+
+std::vector<int> ServerImpressions(const Samples& values) {
+  return GetBinCounts("NewTabPage.SuggestionsImpression.server", values);
+}
+
+std::vector<int> ClientImpressions(const Samples& values) {
+  return GetBinCounts("NewTabPage.SuggestionsImpression.client", values);
+}
+
+std::vector<int> TotalNavigations(const Samples& values) {
+  return GetBinCounts("NewTabPage.MostVisited", values);
+}
+
+std::vector<int> ServerNavigations(const Samples& values) {
+  return GetBinCounts("NewTabPage.MostVisited.server", values);
+}
+
+std::vector<int> ClientNavigations(const Samples& values) {
+  return GetBinCounts("NewTabPage.MostVisited.client", values);
+}
+
 }  // namespace
 
 class NTPUserDataLoggerTest : public testing::Test {
@@ -64,80 +101,79 @@ TEST_F(NTPUserDataLoggerTest, TestLogging) {
 
 TEST_F(NTPUserDataLoggerTest, TestLogMostVisitedImpression) {
   base::StatisticsRecorder::Initialize();
-
-  EXPECT_EQ(0, GetTotalCount("NewTabPage.SuggestionsImpression"));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.server", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.server", 5));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.client", 5));
+  EXPECT_THAT(TotalImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
+  EXPECT_THAT(ServerImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
+  EXPECT_THAT(ClientImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
 
   TestNTPUserDataLogger logger;
 
+  // Impressions increment the associated bins.
+
   logger.LogMostVisitedImpression(1, NTPLoggingTileSource::SERVER);
-  EXPECT_EQ(1, GetTotalCount("NewTabPage.SuggestionsImpression"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.server", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.server", 5));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.client", 5));
+  EXPECT_THAT(TotalImpressions({1, 2, 3, 4}), ElementsAre(1, 0, 0, 0));
+  EXPECT_THAT(ServerImpressions({1, 2, 3, 4}), ElementsAre(1, 0, 0, 0));
+  EXPECT_THAT(ClientImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
+
+  logger.LogMostVisitedImpression(2, NTPLoggingTileSource::SERVER);
+  EXPECT_THAT(TotalImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ServerImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ClientImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
 
-  logger.LogMostVisitedImpression(5, NTPLoggingTileSource::SERVER);
-  EXPECT_EQ(2, GetTotalCount("NewTabPage.SuggestionsImpression"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.server", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.server", 5));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.client", 5));
+  logger.LogMostVisitedImpression(3, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 1, 0));
+  EXPECT_THAT(ServerImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ClientImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 1, 0));
 
-  logger.LogMostVisitedImpression(1, NTPLoggingTileSource::CLIENT);
-  EXPECT_EQ(3, GetTotalCount("NewTabPage.SuggestionsImpression"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.server", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.server", 5));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.SuggestionsImpression.client", 5));
+  logger.LogMostVisitedImpression(4, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 1, 1));
+  EXPECT_THAT(ServerImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ClientImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 1, 1));
 
-  logger.LogMostVisitedImpression(5, NTPLoggingTileSource::CLIENT);
-  EXPECT_EQ(4, GetTotalCount("NewTabPage.SuggestionsImpression"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.server", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.server", 5));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.client", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.SuggestionsImpression.client", 5));
+  // But once incremented, they don't increase again unless reset.
+  logger.LogMostVisitedImpression(1, NTPLoggingTileSource::SERVER);
+  logger.LogMostVisitedImpression(2, NTPLoggingTileSource::CLIENT);
+  logger.LogMostVisitedImpression(3, NTPLoggingTileSource::SERVER);
+  logger.LogMostVisitedImpression(4, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 1, 1));
+  EXPECT_THAT(ServerImpressions({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ClientImpressions({1, 2, 3, 4}), ElementsAre(0, 0, 1, 1));
 }
 
 TEST_F(NTPUserDataLoggerTest, TestLogMostVisitedNavigation) {
   base::StatisticsRecorder::Initialize();
 
-  EXPECT_EQ(0, GetTotalCount("NewTabPage.MostVisited"));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.server", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.server", 5));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.client", 5));
+  EXPECT_THAT(TotalNavigations({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
+  EXPECT_THAT(ServerNavigations({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
+  EXPECT_THAT(ClientNavigations({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
 
   TestNTPUserDataLogger logger;
 
   logger.LogMostVisitedNavigation(1, NTPLoggingTileSource::SERVER);
-  EXPECT_EQ(1, GetTotalCount("NewTabPage.MostVisited"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.server", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.server", 5));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.client", 5));
+  EXPECT_THAT(TotalNavigations({1, 2, 3, 4}), ElementsAre(1, 0, 0, 0));
+  EXPECT_THAT(ServerNavigations({1, 2, 3, 4}), ElementsAre(1, 0, 0, 0));
+  EXPECT_THAT(ClientNavigations({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
 
-  logger.LogMostVisitedNavigation(5, NTPLoggingTileSource::SERVER);
-  EXPECT_EQ(2, GetTotalCount("NewTabPage.MostVisited"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.server", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.server", 5));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.client", 5));
+  logger.LogMostVisitedNavigation(2, NTPLoggingTileSource::SERVER);
+  EXPECT_THAT(TotalNavigations({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ServerNavigations({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ClientNavigations({1, 2, 3, 4}), ElementsAre(0, 0, 0, 0));
 
-  logger.LogMostVisitedNavigation(1, NTPLoggingTileSource::CLIENT);
-  EXPECT_EQ(3, GetTotalCount("NewTabPage.MostVisited"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.server", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.server", 5));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.client", 1));
-  EXPECT_EQ(0, GetBinCount("NewTabPage.MostVisited.client", 5));
+  logger.LogMostVisitedNavigation(3, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalNavigations({1, 2, 3, 4}), ElementsAre(1, 1, 1, 0));
+  EXPECT_THAT(ServerNavigations({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ClientNavigations({1, 2, 3, 4}), ElementsAre(0, 0, 1, 0));
 
-  logger.LogMostVisitedNavigation(5, NTPLoggingTileSource::CLIENT);
-  EXPECT_EQ(4, GetTotalCount("NewTabPage.MostVisited"));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.server", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.server", 5));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.client", 1));
-  EXPECT_EQ(1, GetBinCount("NewTabPage.MostVisited.client", 5));
+  logger.LogMostVisitedNavigation(4, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalNavigations({1, 2, 3, 4}), ElementsAre(1, 1, 1, 1));
+  EXPECT_THAT(ServerNavigations({1, 2, 3, 4}), ElementsAre(1, 1, 0, 0));
+  EXPECT_THAT(ClientNavigations({1, 2, 3, 4}), ElementsAre(0, 0, 1, 1));
+
+  // Navigations always increase.
+  logger.LogMostVisitedNavigation(1, NTPLoggingTileSource::SERVER);
+  logger.LogMostVisitedNavigation(2, NTPLoggingTileSource::CLIENT);
+  logger.LogMostVisitedNavigation(3, NTPLoggingTileSource::SERVER);
+  logger.LogMostVisitedNavigation(4, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalNavigations({1, 2, 3, 4}), ElementsAre(2, 2, 2, 2));
+  EXPECT_THAT(ServerNavigations({1, 2, 3, 4}), ElementsAre(2, 1, 1, 0));
+  EXPECT_THAT(ClientNavigations({1, 2, 3, 4}), ElementsAre(0, 1, 1, 2));
 }