[Prerender] first contentful paint histograms.

chrome/browser/prerender/prerender_manager.h

Index: chrome/browser/prerender/prerender_manager.h
diff --git a/chrome/browser/prerender/prerender_manager.h b/chrome/browser/prerender/prerender_manager.h
index 428c3a7b733ae870bbfdfd5b432a002b9cb3ca65..627a1d862d4e9f6706d213231eee966ac01672a3 100644
--- a/chrome/browser/prerender/prerender_manager.h
+++ b/chrome/browser/prerender/prerender_manager.h
@@ -65,6 +65,13 @@ class PrerenderInProcessBrowserTest;
 class PrerenderHandle;
 class PrerenderHistory;
 
+// Observer interface for PrerenderManager events.
+class PrerenderManagerObserver {
+ public:
+  // Called from the UI thread.
+  virtual void OnFirstContentfulPaint() = 0;

[droger, 2016/11/29 15:54:39]

Leak: Add a empty virtual destructor (or pure).

[mattcary, 2016/11/29 16:39:45]

Done.

+};
+
 // PrerenderManager is responsible for initiating and keeping prerendered
 // views of web pages. All methods must be called on the UI thread unless
 // indicated otherwise.
@@ -206,13 +213,31 @@ class PrerenderManager : public content::NotificationObserver,
                                    bool is_main_resource,
                                    int redirect_count);
 
-  // Records the time to first contentful paint.
-  // Must 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.
-  void RecordFirstContentfulPaint(const GURL& url,
-                                  bool is_no_store,
-                                  base::TimeDelta time);
+  // Called when a NoStatePrefetch first contentful paint has fired.
+  void RecordPrefetchFirstContentfulPaint(Origin origin,
+                                          bool is_no_store,
+                                          bool was_hidden,
+                                          base::TimeDelta time,
+                                          base::TimeDelta prefetch_age);
+
+  // Records the time to first contentful paint for loads that previously had a
+  // no state prefetch load.  Must not be called for prefetch loads themselves
+  // (which are never rendered anyway).  |is_no_store| must be true if the main
+  // resource has a "no-store" cache control HTTP header.
+  void RecordNoStateFirstContentfulPaint(const GURL& url,
+                                         bool is_no_store,
+                                         bool was_hidden,
+                                         base::TimeDelta time);
+
+  // Records the perceived first contentful paint time for a prerendered page,
+  // analogous to |RecordPerceivedPageLoadTime|. The FCP ticks is in absolute
+  // time; this has the disadvantage that the histogram will mix browser and
+  // renderer ticks, but there seems to be no way around that.
+  void RecordPrerenderFirstContentfulPaint(const GURL& url,
+                                           content::WebContents* web_contents,
+                                           bool is_no_store,
+                                           bool was_hidden,
+                                           base::TimeTicks ticks);
 
   static PrerenderManagerMode GetMode();
   static void SetMode(PrerenderManagerMode mode);
@@ -314,6 +339,16 @@ class PrerenderManager : public content::NotificationObserver,
   void SetTickClockForTesting(
       std::unique_ptr<base::SimpleTestTickClock> tick_clock);
 
+  void DisablePageLoadMetricsObserverForTesting() {
+    page_load_metric_observer_disabled_ = true;
+  }
+
+  bool PageLoadMetricsObserverDisabledForTesting() const {
+    return page_load_metric_observer_disabled_;
+  }
+
+  void AddObserver(std::unique_ptr<PrerenderManagerObserver> observer);
+
   // Notification that a prerender has completed and its bytes should be
   // recorded.
   void RecordNetworkBytes(Origin origin, bool used, int64_t prerender_bytes);
@@ -502,6 +537,11 @@ class PrerenderManager : public content::NotificationObserver,
   // so cannot immediately be deleted.
   void DeleteOldWebContents();
 
+  // Get infromation associated with a possible prefetch of |url|.
+  void GetPrefetchInformation(const GURL& url,
+                              base::TimeDelta* prefetch_age,
+                              Origin* origin);
+
   // Cleans up old NavigationRecord's.
   void CleanUpOldNavigations(std::vector<NavigationRecord>* navigations,
                              base::TimeDelta max_age);
@@ -597,6 +637,10 @@ class PrerenderManager : public content::NotificationObserver,
   std::unique_ptr<base::Clock> clock_;
   std::unique_ptr<base::TickClock> tick_clock_;
 
+  bool page_load_metric_observer_disabled_;
+
+  std::vector<std::unique_ptr<PrerenderManagerObserver>> observers_;

[droger, 2016/11/29 15:54:40]

Nit: consider using base::ObserverList.

Note it has a slightly different behavior because it does not take ownership
though.

[mattcary, 2016/11/29 16:39:45]

Yeah, the ownership issue seemed more convenient to me to do this way. If
there's a hard rule for observer lists, thought, I can work around it.

[droger, 2016/11/29 16:46:18]

I don't think there is a hard rule. ObserverList has just some nice features
like Observers being able to remove themselves from within an observer call and
such.

This is not needed here (for now at least), I'm fine with the current code.

[pasko, 2016/11/29 16:51:17]

I think it is good to not use the ObserverList here, because it makes it clear
observers do *not* remove themselves. The tricky lifetime issues are .1% less
tricky?

[mattcary, 2016/11/29 16:52:19]

Good point, sgtm

+
   base::WeakPtrFactory<PrerenderManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PrerenderManager);