[Prerender] first contentful paint histograms.

chrome/browser/prerender/prerender_histograms.h

Index: chrome/browser/prerender/prerender_histograms.h
diff --git a/chrome/browser/prerender/prerender_histograms.h b/chrome/browser/prerender/prerender_histograms.h
index 7fdc187567504a42fc24806be46898132a984016..1901c669580d9f9dfd84d05147aaf3a4cc903ab7 100644
--- a/chrome/browser/prerender/prerender_histograms.h
+++ b/chrome/browser/prerender/prerender_histograms.h
@@ -52,6 +52,17 @@ class PrerenderHistograms {
                                    NavigationType navigation_type,
                                    const GURL& url);
 
+  // Record perceived first contentful paint time for prerendered pages,
+  // analogous to the page load time above.

[pasko, 2016/11/17 19:52:16]

let's copy the relevant parts of the comment from the above because the above
function is likely to go away soon, and when it happens this comment won't be
updated.

[mattcary, 2016/11/18 09:21:11]

Done.

+  void RecordPerceivedFirstContentfulPaintTime(
+      Origin origin,
+      base::TimeDelta perceived_first_contentful_paint);
+
+  // Record that a first contentful paint occured, and whether we were able to
+  // successfuly record the perceived FCP.
+  void RecordPerceivedFirstContentfulPaintStatus(Origin origin,
+                                                 bool successful);
+
   // Records, in a histogram, the percentage of the page load time that had
   // elapsed by the time it is swapped in.  Values outside of [0, 1.0] are
   // invalid and ignored.
@@ -121,26 +132,34 @@ class PrerenderHistograms {
                                    bool is_main_resource,
                                    int redirect_count) const;
 
-  // Records the time to first contentful paint.
-  // Must not be called for prefetch loads (which are never rendered anyway).
+  // Records the time to first contentful paint in the context of NoState
+  // Prefetch.
+
+  // Must be called for an actual page load, and not be called for prefetch
+  // loads (which are never rendered anyway).
   // |is_no_store| must be true if the main resource has a "no-store" cache
   // control HTTP header.
   // |prefetch_age| must be zero if the page was not prefetched.
-  void RecordFirstContentfulPaint(Origin origin,
-                                  bool is_no_store,
-                                  base::TimeDelta time,
-                                  base::TimeDelta prefetch_age);
+  void RecordNoStateFirstContentfulPaint(Origin origin,
+                                         bool is_no_store,
+                                         base::TimeDelta time,
+                                         base::TimeDelta prefetch_age);
 
   // Returns the name of the histogram used to record the time to first
-  // contentful paint.
+  // contentful paint for NoState Prefetch.
   // Exposed for testing.
-  static std::string GetFirstContentfulPaintHistogramName(
+  static std::string GetNoStateFirstContentfulPaintHistogramName(

[pasko, 2016/11/17 19:52:16]

It would be better to make this method private and expand the full histogram
names in tests. This would also protect from accidentally changing what the
function does with all tests unaffected.

(may be better as a separate change)

[mattcary, 2016/11/18 09:21:11]

What I've done here is consistent with how the other tests work, so this is best
left to another change.

       Origin origin,
       bool is_wash,
       bool is_no_store,
       base::TimeDelta prefetch_age);
 
  private:
+  // Returns the name of the histogram used to record the time to first
+  // contentful paint for Prerender.
+  std::string GetPerceivedFirstContentfulPaintHistogramName(Origin origin,
+                                                            bool is_wash);
+
   base::TimeTicks GetCurrentTimeTicks() const;
 
   // Returns the time elapsed since the last prerender happened.