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..a46b1fa55a8e36fadcc35036a95f01bb3b285ce2 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,15 @@
 #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 Sample = base::HistogramBase::Sample;
+using Samples = std::vector<Sample>;
+
 class TestNTPUserDataLogger : public NTPUserDataLogger {
  public:
   TestNTPUserDataLogger() : NTPUserDataLogger(NULL) {}
@@ -27,24 +32,58 @@ base::HistogramBase::Count GetTotalCount(const std::string& histogram_name) {
 }
 
 base::HistogramBase::Count GetBinCount(const std::string& histogram_name,
-                                       base::HistogramBase::Sample value) {
+                                       Sample value) {
   base::HistogramBase* histogram = base::StatisticsRecorder::FindHistogram(
       histogram_name);
   // Return 0 if history is uninitialized.
   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 {
   content::TestBrowserThreadBundle thread_bundle_;
 };
 
-TEST_F(NTPUserDataLoggerTest, TestLogging) {
+TEST_F(NTPUserDataLoggerTest, TestNumberOfTiles) {
   base::StatisticsRecorder::Initialize();
 
   // Enusure non-zero statistics.
   TestNTPUserDataLogger logger;
+  logger.ntp_url_ = GURL("chrome://newtab/");
 
   base::TimeDelta delta = base::TimeDelta::FromMilliseconds(0);
 
@@ -60,84 +99,112 @@ TEST_F(NTPUserDataLoggerTest, TestLogging) {
   // Statistics should be reset to 0, so we should not log anything else.
   logger.LogEvent(NTP_ALL_TILES_LOADED, delta);
   EXPECT_EQ(1, GetTotalCount("NewTabPage.NumberOfTiles"));
+
+  // Navigating away and back resets stats.
+  logger.NavigatedFromURLToURL(GURL("chrome://newtab/"),
+                               GURL("http://chromium.org"));
+  logger.NavigatedFromURLToURL(GURL("http://chromium.org"),
+                               GURL("chrome://newtab/"));
+  logger.LogEvent(NTP_ALL_TILES_LOADED, delta);
+  EXPECT_EQ(2, GetTotalCount("NewTabPage.NumberOfTiles"));
 }
 
 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;
+  logger.ntp_url_ = GURL("chrome://newtab/");
 
+  // Impressions increment the associated bins.
+
+  logger.LogMostVisitedImpression(1, NTPLoggingTileSource::SERVER);
+  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(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(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));
+
+  // 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));
+
+  // Impressions are silently ignored for tiles >= 8.
+  logger.LogMostVisitedImpression(8, NTPLoggingTileSource::SERVER);
+  logger.LogMostVisitedImpression(9, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalImpressions({8, 9}), ElementsAre(0, 0));
+  EXPECT_THAT(ServerImpressions({8, 9}), ElementsAre(0, 0));
+  EXPECT_THAT(ClientImpressions({8, 9}), ElementsAre(0, 0));
+
+  // Navigating away from the NTP and back resets stats.
+  logger.NavigatedFromURLToURL(GURL("chrome://newtab/"),
+                               GURL("http://chromium.org"));
+  logger.NavigatedFromURLToURL(GURL("http://chromium.org"),
+                               GURL("chrome://newtab/"));
   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));
-
-  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(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(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));
+  logger.LogMostVisitedImpression(2, NTPLoggingTileSource::CLIENT);
+  logger.LogMostVisitedImpression(3, NTPLoggingTileSource::SERVER);
+  logger.LogMostVisitedImpression(4, NTPLoggingTileSource::CLIENT);
+  EXPECT_THAT(TotalImpressions({1, 2, 3, 4}), ElementsAre(2, 2, 2, 2));
+  EXPECT_THAT(ServerImpressions({1, 2, 3, 4}), ElementsAre(2, 1, 1, 0));
+  EXPECT_THAT(ClientImpressions({1, 2, 3, 4}), ElementsAre(0, 1, 1, 2));
 }
 
 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));
 }