[Prerender] first contentful paint histograms.

chrome/browser/prerender/prerender_histograms.cc

 // Copyright (c) 2012 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 "chrome/browser/prerender/prerender_histograms.h"
 
 #include <string>
 
 #include "base/format_macros.h"
 #include "base/logging.h"
@@ skipping 95 matching lines @@
 
   // Dummy return value to make the compiler happy.
   NOTREACHED();
   return ComposeHistogramName("wash", name);
 }
 
 bool OriginIsOmnibox(Origin origin) {
   return origin == ORIGIN_OMNIBOX;
 }
 
+const char* FirstContentfulPaintHiddenName(bool was_hidden) {
+  return was_hidden ? ".Hidden" : ".Visible";
+}
+
 }  // namespace
 
 // Helper macros for origin-based histogram reporting. All HISTOGRAM arguments
 // must be UMA_HISTOGRAM... macros that contain an argument "name" which these
 // macros will eventually substitute for the actual name used.
 #define PREFIXED_HISTOGRAM(histogram_name, origin, HISTOGRAM)           \
   PREFIXED_HISTOGRAM_INTERNAL(origin, IsOriginWash(), HISTOGRAM, histogram_name)
 
 #define PREFIXED_HISTOGRAM_ORIGIN_EXPERIMENT(histogram_name, origin, \
                                              HISTOGRAM) \
@@ skipping 168 matching lines @@
                      perceived_page_load_time);
         } else if (recorded_non_overlapping) {
           RECORD_PLT("PerceivedPLTFirstAfterMissNonOverlappingOnly",
                      perceived_page_load_time);
         }
       }
     }
   }
 }
 
+void PrerenderHistograms::RecordPerceivedFirstContentfulPaintStatus(
+    Origin origin,
+    bool successful,
+    bool was_hidden) {
+  RecordHistogramEnum(
+      GetHistogramName(origin, IsOriginWash(), "PerceivedTTFCPRecorded") +
+          FirstContentfulPaintHiddenName(was_hidden),
+      successful ? 1 : 0, 2);

[Ilya Sherman, 2016/12/27 22:46:06]

nit: Please use UmaHistogramBoolean from //base/histogram_functions.h.  (In a
separate CL, you might want to switch other uses of RecordHistogramEnum over to
these shared functions as well.)

[mattcary, 2016/12/28 09:52:33]

Done; created crbug/677257 so I don't drop switching over the other uses.

+}
+
 void PrerenderHistograms::RecordPageLoadTimeNotSwappedIn(
     Origin origin,
     base::TimeDelta page_load_time,
     const GURL& url) const {
   // If the URL to be prerendered is not a http[s] URL, or is a Google URL,
   // do not record.
   if (!url.SchemeIsHTTPOrHTTPS() || IsGoogleDomain(url))
     return;
   RECORD_PLT("PrerenderNotSwappedInPLT", page_load_time);
 }
@@ skipping 122 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
 
   const int kMaxRedirectCount = 10;
   std::string histogram_base_name = base::StringPrintf(
       "NoStatePrefetch%sResourceRedirects", is_main_resource ? "Main" : "Sub");
   std::string histogram_name =
       GetHistogramName(origin, IsOriginWash(), histogram_base_name);
   RecordHistogramEnum(histogram_name, redirect_count, kMaxRedirectCount);
 }
 
-void PrerenderHistograms::RecordFirstContentfulPaint(
+void PrerenderHistograms::RecordPrefetchFirstContentfulPaintTime(
     Origin origin,
     bool is_no_store,
+    bool was_hidden,
     base::TimeDelta time,
     base::TimeDelta prefetch_age) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!prefetch_age.is_zero()) {
     DCHECK_NE(origin, ORIGIN_NONE);
-    RecordHistogramTime(GetHistogramName(origin, IsOriginWash(),
-                                         "Prerender.NoStatePrefetchAge"),
-                        base::TimeDelta::FromMilliseconds(10),
-                        base::TimeDelta::FromMinutes(30), prefetch_age, 50);
+    RecordHistogramTime(
+        GetHistogramName(origin, IsOriginWash(), "Prerender.PrefetchAge"),

[Ilya Sherman, 2016/12/27 22:46:06]

Why did you drop "NoState" from this histogram name?  I don't see a
corresponding update in histograms.xml.

[mattcary, 2016/12/28 09:52:33]

Oops, thanks. Updated histograms.xml to obsolete the old one and add this one.

This histogram is no recording more than just NoState prefetch ages; it will
also record effective prefetch age for prerender and non-prefetched loads as
well (depending on the experiment branch).

+        base::TimeDelta::FromMilliseconds(10), base::TimeDelta::FromMinutes(30),
+        prefetch_age, 50);
   }
 
-  RecordHistogramTime(GetFirstContentfulPaintHistogramName(
-                          origin, IsOriginWash(), is_no_store, prefetch_age),
-                      base::TimeDelta::FromMilliseconds(10),
+  std::string histogram_base_name;
+  if (prefetch_age.is_zero()) {
+    histogram_base_name = "PrefetchTTFCP.Reference";
+  } else {
+    histogram_base_name = prefetch_age < base::TimeDelta::FromMinutes(
+                                             net::HttpCache::kPrefetchReuseMins)
+                              ? "PrefetchTTFCP.Warm"
+                              : "PrefetchTTFCP.Cold";
+  }
+
+  histogram_base_name += is_no_store ? ".NoStore" : ".Cacheable";
+  histogram_base_name += FirstContentfulPaintHiddenName(was_hidden);
+  std::string histogram_name =
+      GetHistogramName(origin, IsOriginWash(), histogram_base_name);
+
+  RecordHistogramTime(histogram_name, base::TimeDelta::FromMilliseconds(10),
                       base::TimeDelta::FromMinutes(2), time, 50);
 }
 
 bool PrerenderHistograms::IsOriginWash() const {
   if (!WithinWindow())
     return false;
   return origin_wash_;
 }
 
-// static
-std::string PrerenderHistograms::GetFirstContentfulPaintHistogramName(
-    Origin origin,
-    bool is_wash,
-    bool is_no_store,
-    base::TimeDelta prefetch_age) {
-  std::string histogram_base_name;
-  if (prefetch_age.is_zero()) {
-    histogram_base_name = "NoStatePrefetchTTFCP.Reference";
-  } else {
-    histogram_base_name = prefetch_age < base::TimeDelta::FromMinutes(
-                                             net::HttpCache::kPrefetchReuseMins)
-                              ? "NoStatePrefetchTTFCP.Warm"
-                              : "NoStatePrefetchTTFCP.Cold";

[Ilya Sherman, 2016/12/27 22:46:06]

It looks like you dropped "NoState" from this name.  Could you please mark the
corresponding histograms in histograms.xml as <obsolete>?

[mattcary, 2016/12/28 09:52:33]

Done.

I have left the obsolete histogram (both this and NoStatePrefetchAge) in
histogram_suffixes, is that appropriate?

-  }
-
-  histogram_base_name += is_no_store ? ".NoStore" : ".Cacheable";
-  return GetHistogramName(origin, is_wash, histogram_base_name);
-}
-
 }  // namespace prerender