[Prerender] first contentful paint histograms.

chrome/browser/prerender/prerender_manager.cc

Index: chrome/browser/prerender/prerender_manager.cc
diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
index 48300bbae5e5b419ef3ea964014765b13faef654..645a9317ba14f531504312bf2a97f3dbbc8efd53 100644
--- a/chrome/browser/prerender/prerender_manager.cc
+++ b/chrome/browser/prerender/prerender_manager.cc
@@ -150,6 +150,8 @@ class PrerenderManager::OnCloseWebContentsDeleter
   DISALLOW_COPY_AND_ASSIGN(OnCloseWebContentsDeleter);
 };
 
+PrerenderManagerObserver::~PrerenderManagerObserver() {}
+
 // static
 int PrerenderManager::prerenders_per_session_count_ = 0;
 
@@ -175,6 +177,7 @@ PrerenderManager::PrerenderManager(Profile* profile)
       last_recorded_profile_network_bytes_(0),
       clock_(new base::DefaultClock()),
       tick_clock_(new base::DefaultTickClock()),
+      page_load_metric_observer_disabled_(false),
       weak_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -566,31 +569,52 @@ void PrerenderManager::RecordPrefetchRedirectCount(Origin origin,
                                            redirect_count);
 }
 
-void PrerenderManager::RecordFirstContentfulPaint(const GURL& url,
-                                                  bool is_no_store,
-                                                  base::TimeDelta time) {
-  CleanUpOldNavigations(&prefetches_, base::TimeDelta::FromMinutes(30));
-
-  // Compute the prefetch age.
+void PrerenderManager::RecordNoStateFirstContentfulPaint(const GURL& url,
+                                                         bool is_no_store,
+                                                         bool was_hidden,
+                                                         base::TimeDelta time) {
   base::TimeDelta prefetch_age;
-  Origin origin = ORIGIN_NONE;
-  for (auto it = prefetches_.crbegin(); it != prefetches_.crend(); ++it) {
-    if (it->url == url) {
-      prefetch_age = GetCurrentTimeTicks() - it->time;
-      origin = it->origin;
-      break;
-    }
+  Origin origin;
+  GetPrefetchInformation(url, &prefetch_age, &origin);
+
+  histograms_->RecordPrefetchFirstContentfulPaintTime(
+      origin, is_no_store, was_hidden, time, prefetch_age);
+
+  for (auto& observer : observers_) {
+    observer->OnFirstContentfulPaint();
   }
+}
 
-  histograms_->RecordFirstContentfulPaint(origin, is_no_store, time,
-                                          prefetch_age);
+void PrerenderManager::RecordPrerenderFirstContentfulPaint(
+    const GURL& url,
+    content::WebContents* web_contents,
+    bool is_no_store,
+    bool was_hidden,
+    base::TimeTicks first_contentful_paint) {
+  PrerenderTabHelper* tab_helper =
+      PrerenderTabHelper::FromWebContents(web_contents);
+  DCHECK(tab_helper);
 
-  // Loading a prefetched URL resets the revalidation bypass. Remove the url
-  // from the prefetch list for more accurate metrics.
-  prefetches_.erase(
-      std::remove_if(prefetches_.begin(), prefetches_.end(),
-                     [url](const NavigationRecord& r) { return r.url == url; }),
-      prefetches_.end());
+  base::TimeDelta prefetch_age;
+  // The prefetch origin may not be relevant to the prerender.

[pasko, 2016/12/21 18:39:09]

s/may not be/is not/

[mattcary, 2016/12/22 10:49:07]

No, may not be. Sometimes it is the same.

[pasko, 2016/12/22 18:22:00]

Then extracting this bit of information from the code and the comment makes the
comment not worth it IMO. It could be that I asked to put a clarifying comment
to explain a little about |unused_origin|. Apologies in this case, it did not
work well.

[mattcary, 2016/12/23 09:58:15]

Oh, I see. Done.

+  Origin unused_origin;
+  GetPrefetchInformation(url, &prefetch_age, &unused_origin);
+
+  base::TimeTicks last_swap = tab_helper->last_swap();
+  if (!last_swap.is_null() && !first_contentful_paint.is_null()) {
+    histograms_->RecordPrefetchFirstContentfulPaintTime(
+        tab_helper->origin(), is_no_store, was_hidden,
+        first_contentful_paint - last_swap, prefetch_age);
+    histograms_->RecordPerceivedFirstContentfulPaintStatus(tab_helper->origin(),
+                                                           true, was_hidden);
+  } else {
+    histograms_->RecordPerceivedFirstContentfulPaintStatus(tab_helper->origin(),
+                                                           false, was_hidden);

[pasko, 2016/12/21 18:39:09]

this repetition with changing one magic boolean can be explained better like
this:

  base::TimeTicks last_swap = tab_helper->last_swap();
  bool perceived_fcp_recorded = false;
  if (!last_swap.is_null() && !first_contentful_paint.is_null()) {
    perceived_fcp_recorded = true;
    histograms_->RecordPrefetchFirstContentfulPaintTime(
        tab_helper->origin(), is_no_store, was_hidden,
        first_contentful_paint - last_swap, prefetch_age);
  }
  histograms_->RecordPerceivedFirstContentfulPaintStatus(
      tab_helper->origin(), perceived_fcp_recorded, was_hidden);

[mattcary, 2016/12/22 10:49:07]

Done.

+  }
+
+  for (auto& observer : observers_) {
+    observer->OnFirstContentfulPaint();
+  }
 }
 
 // static
@@ -1039,6 +1063,8 @@ void PrerenderManager::PeriodicCleanup() {
 
   to_delete_prerenders_.clear();
 
+  CleanUpOldNavigations(&prefetches_, base::TimeDelta::FromMinutes(30));
+
   // Measure how long a the various cleanup tasks took. http://crbug.com/305419.
   UMA_HISTOGRAM_TIMES("Prerender.PeriodicCleanupDeleteContentsTime",
                       cleanup_timer.Elapsed());
@@ -1092,6 +1118,11 @@ void PrerenderManager::SetTickClockForTesting(
   tick_clock_ = std::move(tick_clock);
 }
 
+void PrerenderManager::AddObserver(
+    std::unique_ptr<PrerenderManagerObserver> observer) {
+  observers_.push_back(std::move(observer));
+}
+
 std::unique_ptr<PrerenderContents> PrerenderManager::CreatePrerenderContents(
     const GURL& url,
     const content::Referrer& referrer,
@@ -1154,6 +1185,31 @@ void PrerenderManager::DeleteOldWebContents() {
   old_web_contents_list_.clear();
 }
 
+void PrerenderManager::GetPrefetchInformation(const GURL& url,
+                                              base::TimeDelta* prefetch_age,
+                                              Origin* origin) {
+  DCHECK(prefetch_age);
+  DCHECK(origin);
+  CleanUpOldNavigations(&prefetches_, base::TimeDelta::FromMinutes(30));
+
+  *prefetch_age = base::TimeDelta();
+  *origin = ORIGIN_NONE;
+  for (auto it = prefetches_.crbegin(); it != prefetches_.crend(); ++it) {
+    if (it->url == url) {
+      *prefetch_age = GetCurrentTimeTicks() - it->time;
+      *origin = it->origin;
+      break;
+    }
+  }
+
+  // Loading a prefetched URL resets the revalidation bypass. Remove all
+  // matching urls from the prefetch list for more accurate metrics.
+  prefetches_.erase(
+      std::remove_if(prefetches_.begin(), prefetches_.end(),
+                     [url](const NavigationRecord& r) { return r.url == url; }),
+      prefetches_.end());
+}
+
 void PrerenderManager::CleanUpOldNavigations(
     std::vector<NavigationRecord>* navigations,
     base::TimeDelta max_age) {