Prerender: Add FCP histograms to nostate browsertests.

chrome/browser/prerender/prerender_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/prerender/prerender_manager.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <functional>
@@ skipping 597 matching lines @@
 
   histograms_->RecordNoStateFirstContentfulPaint(origin, is_no_store, time,
                                                  prefetch_age);
 
   // 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());
+
+  for (auto& observer : observers_) {
+    observer->OnFirstContentfulPaint();

[pasko, 2016/11/18 15:56:56]

can we notify observers just after recording histograms? The manipulations of
prefetches_ above might be potentially non-instant, so in case observers ever
get sensitive to precision, we would do our best here. I think this does not
matter in practice, but looks safer and raises less questions.

[mattcary, 2016/11/18 16:13:27]

SGTM. I moved it to the end of the function for a similar reasons, but your
suggestion is more precise.

Note that it now occurs to me that what we are really using these observers for
is to wait for the FCP histograms, not actually the FCP. That only affects
tests, though, and it seems like it might be confusing to emphasize that detail.

+  }
 }
 
 void PrerenderManager::RecordPerceivedFirstContentfulPaint(
     content::WebContents* web_contents,
     base::TimeDelta first_contentful_paint) {
-  for (auto& observer : observers_) {
-    observer->OnFirstContentfulPaint();
-  }
-
   PrerenderTabHelper* tab_helper =
       PrerenderTabHelper::FromWebContents(web_contents);
   DCHECK(tab_helper);
 
   base::TimeDelta perceived_delta;
   if (tab_helper->LoadToPerceivedDelta(&perceived_delta)) {
     histograms_->RecordPerceivedFirstContentfulPaintTime(
         tab_helper->origin(), first_contentful_paint - perceived_delta);
     histograms_->RecordPerceivedFirstContentfulPaintStatus(tab_helper->origin(),
                                                            true);
   } else {
     histograms_->RecordPerceivedFirstContentfulPaintStatus(tab_helper->origin(),
                                                            false);
   }
+
+  for (auto& observer : observers_) {

[pasko, 2016/11/18 15:56:56]

this moved too, does anything depend on firing it later?

[mattcary, 2016/11/18 16:13:27]

I moved this to be consistent with the other FCP (ie, at the end). At any rate,
it happily matches your other suggestion.

+    observer->OnFirstContentfulPaint();
+  }
 }
 
 // static
 PrerenderManager::PrerenderManagerMode PrerenderManager::GetMode() {
   return mode_;
 }
 
 // static
 void PrerenderManager::SetMode(PrerenderManagerMode mode) {
   mode_ = mode;
@@ skipping 775 matching lines @@
   DCHECK_EQ(1u, erased);
 }
 
 void PrerenderManager::SetPrerenderContentsFactoryForTest(
     PrerenderContents::Factory* prerender_contents_factory) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   prerender_contents_factory_.reset(prerender_contents_factory);
 }
 
 }  // namespace prerender