Record impressions/navigations only once per tile.

chrome/browser/ui/webui/ntp/ntp_user_data_logger.h

Index: chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
diff --git a/chrome/browser/ui/webui/ntp/ntp_user_data_logger.h b/chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
index bb96075e7331873169a20da3c905616ab4cc7247..24be4da6d42ad77b97972ed4b3c6f7e697811edc 100644
--- a/chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
+++ b/chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
@@ -7,10 +7,12 @@
 
 #include <stddef.h>
 
+#include <bitset>
 #include "base/gtest_prod_util.h"

[Marc Treib, 2016/07/15 10:04:46]

nit: empty line between <> and "" includes

[sfiera, 2016/07/15 10:26:00]

Done.

 #include "base/macros.h"
 #include "base/time/time.h"
 #include "chrome/common/search/ntp_logging_events.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
 namespace content {
@@ -19,10 +21,14 @@ class WebContents;
 
 // Helper class for logging data from the NTP. Attached to each NTP instance.
 class NTPUserDataLogger
-    : public content::WebContentsUserData<NTPUserDataLogger> {
+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<NTPUserDataLogger> {
  public:
   ~NTPUserDataLogger() override;
 
+  // Gets the associated NTPUserDataLogger, creating it if necessary.
+  //
+  // MUST be called only when the NTP is active.
   static NTPUserDataLogger* GetOrCreateFromWebContents(
       content::WebContents* content);
 
@@ -50,11 +56,25 @@ class NTPUserDataLogger
   FRIEND_TEST_ALL_PREFIXES(NTPUserDataLoggerTest,
                            TestLogging);
 
+  // content::WebContentsObserver override
+  void NavigationEntryCommitted(
+      const content::LoadCommittedDetails& load_details) override;
+
   // Logs a number of statistics regarding the NTP. Called when an NTP tab is
   // about to be deactivated (be it by switching tabs, losing focus or closing
   // the tab/shutting down Chrome), or when the user navigates to a URL.
   void EmitNtpStatistics(base::TimeDelta load_time);
 
+  // Records whether we have yet logged an impression for the tile at a given
+  // index. A typical NTP will log 8 impressions, but could record fewer for new
+  // users that haven't built up a history yet.
+  //
+  // If something happens that causes the NTP to pull tiles from different
+  // sources, such as logging in (switching from client to server tiles), then

[Marc Treib, 2016/07/15 10:04:46]

nit: s/logging/signing (that's the usual nomenclature)

[sfiera, 2016/07/15 10:26:00]

Done.

+  // only the impressions for the first source will be logged, leaving the
+  // number of impressions for a source slightly out-of-sync with navigations.
+  std::bitset<8> impression_was_logged_;

[Marc Treib, 2016/07/15 10:04:46]

Please use the kNumMostVisited constant instead of a magic number (promote
kNumMostVisited to a private static member)

[sfiera, 2016/07/15 10:26:00]

Done.

+
   // True if at least one iframe came from a server-side suggestion.
   bool has_server_side_suggestions_;