[Prerender] first contentful paint histograms.

chrome/browser/prerender/prerender_tab_helper.cc

Index: chrome/browser/prerender/prerender_tab_helper.cc
diff --git a/chrome/browser/prerender/prerender_tab_helper.cc b/chrome/browser/prerender/prerender_tab_helper.cc
index 096705dc8ab5c57a1f271d56271684e192e30a71..54294075cf1c5d9903b266e94481f908ef586441 100644
--- a/chrome/browser/prerender/prerender_tab_helper.cc
+++ b/chrome/browser/prerender/prerender_tab_helper.cc
@@ -26,16 +26,7 @@ PrerenderTabHelper::PrerenderTabHelper(content::WebContents* web_contents)
       origin_(ORIGIN_NONE),
       next_load_is_control_prerender_(false),
       next_load_origin_(ORIGIN_NONE),
-      weak_factory_(this) {
-  // Determine if this is a prerender.
-  PrerenderManager* prerender_manager = MaybeGetPrerenderManager();
-  if (prerender_manager &&
-      prerender_manager->IsWebContentsPrerendering(web_contents, &origin_)) {
-    navigation_type_ = NAVIGATION_TYPE_PRERENDERED;
-  } else {
-    navigation_type_ = NAVIGATION_TYPE_NORMAL;
-  }
-}
+      weak_factory_(this) {}
 
 PrerenderTabHelper::~PrerenderTabHelper() {
 }
@@ -106,6 +97,15 @@ void PrerenderTabHelper::DidStartProvisionalLoadForFrame(
     const GURL& validated_url,
     bool is_error_page,
     bool is_iframe_srcdoc) {
+  // Determine the navigation type.
+  PrerenderManager* prerender_manager = MaybeGetPrerenderManager();
+  if (prerender_manager &&
+      prerender_manager->IsWebContentsPrerendering(web_contents(), &origin_)) {
+    navigation_type_ = NAVIGATION_TYPE_PRERENDERED;
+  } else {
+    navigation_type_ = NAVIGATION_TYPE_NORMAL;
+  }
+
   if (render_frame_host->GetParent())
     return;
 
@@ -156,10 +156,10 @@ void PrerenderTabHelper::PrerenderSwappedIn() {
   // Ensure we are not prerendering any more.
   DCHECK_EQ(NAVIGATION_TYPE_PRERENDERED, navigation_type_);
   DCHECK(!IsPrerendering());
+  last_swap_ = GetTimeTicksFromPrerenderManager();
   if (pplt_load_start_.is_null()) {
     // If we have already finished loading, report a 0 PPLT.
     RecordPerceivedPageLoadTime(base::TimeDelta(), 1.0);
-    DCHECK_EQ(NAVIGATION_TYPE_NORMAL, navigation_type_);

[pasko, 2016/11/21 14:44:36]

we should not call PrerenderSwappedIn() before
DidStartProvisionalLoadForFrame(), so the fact that determining navigation time
moved to the latter method, should not affect this DCHECK. Should we still have
this DCHECK then?

[mattcary, 2016/11/21 16:13:37]

No. The FCP can occur after the page load, and we can't reset the navigation
type else we lose the histograms.

   } else {
     // If we have not finished loading yet, record the actual load start, and
     // rebase the start time to now.
@@ -188,10 +188,6 @@ void PrerenderTabHelper::RecordPerceivedPageLoadTime(
   prerender_manager->RecordPerceivedPageLoadTime(
       origin_, navigation_type_, perceived_page_load_time,
       fraction_plt_elapsed_at_swap_in, url_);
-
-  // Reset state for the next navigation.
-  navigation_type_ = NAVIGATION_TYPE_NORMAL;

[pasko, 2016/11/21 14:44:36]

why removing this?

[mattcary, 2016/11/21 16:13:37]

It's moved up to provisional start. This is the confusing state I mentioned that
prerender_tab_helper has to keep track of; it was a bad idea for it to
originally set it in the constructor.

-  origin_ = ORIGIN_NONE;
 }
 
 }  // namespace prerender