Most visited iframe now postMessage to signal the iframing page that the link has been displayed

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

Index: chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
diff --git a/chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc b/chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
index 4fca77a3986d5207c47ae3ed9cf06110e70cc944..875d90e2206d451a21540ae1990311892590edf2 100644
--- a/chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
+++ b/chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
@@ -54,39 +54,43 @@ NTPUserDataLogger* NTPUserDataLogger::GetOrCreateFromWebContents(
   return logger;
 }
 
-void NTPUserDataLogger::EmitThumbnailErrorRate() {
-  DCHECK_LE(number_of_thumbnail_errors_, number_of_thumbnail_attempts_);
-  if (number_of_thumbnail_attempts_ != 0) {
-    UMA_HISTOGRAM_PERCENTAGE(
-      "NewTabPage.ThumbnailErrorRate",
-      GetPercentError(number_of_thumbnail_errors_,
-                      number_of_thumbnail_attempts_));
-  }
-  DCHECK_LE(number_of_fallback_thumbnails_used_,
-            number_of_fallback_thumbnails_requested_);
-  if (number_of_fallback_thumbnails_requested_ != 0) {
-    UMA_HISTOGRAM_PERCENTAGE(
-        "NewTabPage.ThumbnailFallbackRate",
-        GetPercentError(number_of_fallback_thumbnails_used_,
-                        number_of_fallback_thumbnails_requested_));
-  }
-  number_of_thumbnail_attempts_ = 0;
-  number_of_thumbnail_errors_ = 0;
-  number_of_fallback_thumbnails_requested_ = 0;
-  number_of_fallback_thumbnails_used_ = 0;
-}
-
 void NTPUserDataLogger::EmitNtpStatistics() {
   UMA_HISTOGRAM_COUNTS("NewTabPage.NumberOfMouseOvers", number_of_mouseovers_);
   number_of_mouseovers_ = 0;
-  UMA_HISTOGRAM_COUNTS("NewTabPage.NumberOfExternalTiles",
-                       number_of_external_tiles_);
-  number_of_external_tiles_ = 0;
-  UMA_HISTOGRAM_ENUMERATION(
-      "NewTabPage.SuggestionsType",
-      server_side_suggestions_ ? SERVER_SIDE : CLIENT_SIDE,
-      SUGGESTIONS_TYPE_COUNT);
-  server_side_suggestions_ = false;
+
+  // Only log the following statistics if there is we recorded at least one

[Mathieu, 2014/01/09 18:37:18]

*if at least one tile was recorded

[beaudoin, 2014/01/15 23:39:56]

Done.

+  // tile. This check is required because the statistics are emitted whenever
+  // the user changes tab away from the NTP. However, if he comes back to that

[Mathieu, 2014/01/09 18:37:18]

he -> the user

[beaudoin, 2014/01/15 23:39:56]

Done.

+  // NTP tab later the statistics are not regenerated (ie. they are all 0). If

[Jered, 2014/01/10 15:12:36]

nit: NTP tab -> NTP, ie. -> i.e.

[beaudoin, 2014/01/15 23:39:56]

Done.

+  // we log them again we get a strong bias.
+  if (number_of_tiles_ > 0) {
+    UMA_HISTOGRAM_CUSTOM_COUNTS("NewTabPage.NumberOfTiles",
+                                number_of_tiles_, 0, 8, 9 );

[Alexei Svitkine (slow), 2014/01/09 18:54:41]

Perhaps its worth defining your own macro for this to avoid repeating the later
params?

e.g.

#define UMA_HISTOGRAM_NTP_TILES(name, value) \
    UMA_HISTOGRAM_CUSTOM_COUNTS(name, value, 0, 8, 9)

and changing these macros calls to:

UMA_HISTOGRAM_NTP_TILES("NewTabPage.NumberOfTiles", number_of_tiles_);

Also, the values 0, 8, 9 should be explained somewhere in a comment.

[beaudoin, 2014/01/15 23:39:56]

Done.

+    number_of_tiles_ = 0;
+    UMA_HISTOGRAM_CUSTOM_COUNTS("NewTabPage.NumberOfThumbnailAttempts",
+                                number_of_thumbnail_attempts_, 0, 8, 9 );
+    number_of_thumbnail_attempts_ = 0;
+    UMA_HISTOGRAM_CUSTOM_COUNTS("NewTabPage.NumberOfThumbnailErrors",
+                                number_of_thumbnail_errors_, 0, 8, 9 );
+    number_of_thumbnail_errors_ = 0;
+    UMA_HISTOGRAM_CUSTOM_COUNTS("NewTabPage.NumberOfGrayTileFallbacks",
+                                number_of_gray_tile_fallbacks_, 0, 8, 9 );
+    number_of_gray_tile_fallbacks_ = 0;
+    UMA_HISTOGRAM_CUSTOM_COUNTS("NewTabPage.NumberOfExternalFallbacks",
+                                number_of_external_fallbacks_, 0, 8, 9 );
+    number_of_external_fallbacks_ = 0;
+    UMA_HISTOGRAM_CUSTOM_COUNTS("NewTabPage.NumberOfExternalTiles",
+                                number_of_external_tiles_, 0, 8, 9 );
+    number_of_external_tiles_ = 0;
+    UMA_HISTOGRAM_CUSTOM_COUNTS("NewTabPage.NumberOfGrayTiles",
+                                number_of_gray_tiles_, 0, 8, 9 );

[Mathieu, 2014/01/09 18:37:18]

you tested this does the right thing at boundary cases by checking
chrome://histograms?

[beaudoin, 2014/01/15 23:39:56]

Yes. (It didn't, that's why I has to use CUSTOM_COUNTS :)) The unit test
exercises the boundary cases, but I'm not sure it's indicative of what show s up
in chrome://histograms. In all cases, I've tested it manually.

Ack.

+    number_of_gray_tiles_ = 0;
+    UMA_HISTOGRAM_ENUMERATION(
+        "NewTabPage.SuggestionsType",
+        server_side_suggestions_ ? SERVER_SIDE : CLIENT_SIDE,
+        SUGGESTIONS_TYPE_COUNT);
+    server_side_suggestions_ = false;
+  }
 }
 
 void NTPUserDataLogger::LogEvent(NTPLoggingEventType event) {
@@ -100,12 +104,6 @@ void NTPUserDataLogger::LogEvent(NTPLoggingEventType event) {
     case NTP_THUMBNAIL_ERROR:
       number_of_thumbnail_errors_++;
       break;
-    case NTP_FALLBACK_THUMBNAIL_REQUESTED:
-      number_of_fallback_thumbnails_requested_++;
-      break;
-    case NTP_FALLBACK_THUMBNAIL_USED:
-      number_of_fallback_thumbnails_used_++;
-      break;
     case NTP_SERVER_SIDE_SUGGESTION:
       server_side_suggestions_ = true;
       break;
@@ -118,6 +116,18 @@ void NTPUserDataLogger::LogEvent(NTPLoggingEventType event) {
     case NTP_EXTERNAL_TILE:
       number_of_external_tiles_++;
       break;
+    case NTP_TILE:
+      number_of_tiles_++;
+      break;
+    case NTP_GRAY_TILE:
+      number_of_gray_tiles_++;
+      break;
+    case NTP_GRAY_TILE_FALLBACK:
+      number_of_gray_tile_fallbacks_++;
+      break;
+    case NTP_EXTERNAL_FALLBACK:
+      number_of_external_fallbacks_++;
+      break;
     default:
       NOTREACHED();
   }
@@ -144,24 +154,18 @@ void NTPUserDataLogger::NavigationEntryCommitted(
 
   if (search::MatchesOriginAndPath(ntp_url_, load_details.previous_url)) {
     EmitNtpStatistics();
-    // Only log thumbnail error rates for Instant NTP pages, as we do not have
-    // this data for non-Instant NTPs.
-    if (ntp_url_ != GURL(chrome::kChromeUINewTabURL))
-      EmitThumbnailErrorRate();
   }
 }
 
 NTPUserDataLogger::NTPUserDataLogger(content::WebContents* contents)
     : content::WebContentsObserver(contents),
       number_of_mouseovers_(0),
+      number_of_tiles_(0),
       number_of_thumbnail_attempts_(0),
       number_of_thumbnail_errors_(0),
-      number_of_fallback_thumbnails_requested_(0),
-      number_of_fallback_thumbnails_used_(0),
+      number_of_gray_tile_fallbacks_(0),
+      number_of_external_fallbacks_(0),
       number_of_external_tiles_(0),
+      number_of_gray_tiles_(0),
       server_side_suggestions_(false) {
 }
-
-size_t NTPUserDataLogger::GetPercentError(size_t errors, size_t events) const {
-  return (100 * errors) / events;
-}