[NoStatePrefetch] Add performance histograms.

chrome/browser/prerender/prerender_histograms.cc

Index: chrome/browser/prerender/prerender_histograms.cc
diff --git a/chrome/browser/prerender/prerender_histograms.cc b/chrome/browser/prerender/prerender_histograms.cc
index 28c43c0968095a78f4c072d41a676220737994bf..26be5072bb642091b9cefea7a4c310e739899659 100644
--- a/chrome/browser/prerender/prerender_histograms.cc
+++ b/chrome/browser/prerender/prerender_histograms.cc
@@ -13,6 +13,7 @@
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_util.h"
+#include "net/http/http_cache.h"
 
 namespace prerender {
 
@@ -47,6 +48,18 @@ void RecordHistogramEnum(std::string histogram_name,
   histogram_pointer->Add(sample);
 }
 
+// Similar to UMA_HISTOGRAM_CUSTOM_TIMES but allows a dynamic histogram name.
+void RecordHistogramTime(std::string histogram_name,

[Alexei Svitkine (slow), 2016/09/15 16:53:28]

Pass by const std::string&

[droger, 2016/09/16 12:21:50]

Good catch. This mistake comes from a copy-paste, so I'm fixing the code I
copied from too.

+                         base::TimeDelta time_min,
+                         base::TimeDelta time_max,
+                         base::TimeDelta sample,
+                         base::HistogramBase::Sample bucket_count) {
+  base::HistogramBase* histogram_pointer = base::Histogram::FactoryTimeGet(
+      histogram_name, time_min, time_max, bucket_count,
+      base::HistogramBase::kUmaTargetedHistogramFlag);
+  histogram_pointer->AddTime(sample);
+}
+
 // Time window for which we will record windowed PLTs from the last observed
 // link rel=prefetch tag. This is not intended to be the same as the prerender
 // ttl, it's just intended to be a window during which a prerender has likely
@@ -439,6 +452,38 @@ void PrerenderHistograms::RecordPrefetchRedirectCount(
   RecordHistogramEnum(histogram_name, redirect_count, kMaxRedirectCount);
 }
 
+void PrerenderHistograms::RecordFirstContentfulPaint(
+    Origin origin,
+    bool is_no_store,
+    base::TimeDelta time,
+    base::TimeDelta prefetch_age) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  std::string histogram_base_name;
+  if (prefetch_age.is_zero()) {
+    histogram_base_name = "NoStatePrefetchTTFCPReference";
+  } else {
+    DCHECK_NE(origin, ORIGIN_NONE);
+
+    RecordHistogramTime(GetHistogramName(origin, IsOriginWash(),
+                                         "Prerender.NoStatePrefetchAge"),
+                        base::TimeDelta::FromMilliseconds(10),
+                        base::TimeDelta::FromMinutes(30), prefetch_age, 50);
+
+    histogram_base_name = prefetch_age < base::TimeDelta::FromMinutes(
+                                             net::HttpCache::kPrefetchReuseMins)
+                              ? "NoStatePrefetchTTFCPWarm"
+                              : "NoStatePrefetchTTFCPCold";
+  }
+
+  histogram_base_name += is_no_store ? "NoStore" : "Cacheable";
+
+  RecordHistogramTime(
+      GetHistogramName(origin, IsOriginWash(), histogram_base_name),
+      base::TimeDelta::FromMilliseconds(10), base::TimeDelta::FromMinutes(2),
+      time, 100);

[Alexei Svitkine (slow), 2016/09/15 16:53:28]

We don't recommend using 100 bucket histograms unless you have evidence that 50
bucket isn't enough. 100 bucket histograms end up using much more data than 50
as usually more unique buckets are used and there's less aggregation.

[droger, 2016/09/16 12:21:49]

Done.

I did this because all other timing histograms in this file also use 100 buckets
(see line 238 above). Should I change them all to 50?

[Alexei Svitkine (slow), 2016/09/16 14:16:21]

Changing the other histograms' buckets is a breaking change and requires
renaming them. So it's probably OK to leave them as-is but use 50 for all new
ones.

+}
+
 bool PrerenderHistograms::IsOriginWash() const {
   if (!WithinWindow())
     return false;