[Prerender] first contentful paint histograms.

chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5141920c07781eaa03eba73e5cae9206ada6e168
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.cc
@@ -0,0 +1,41 @@
+// Copyright 2016 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/page_load_metrics/observers/prerender_page_load_metrics_observer.h"
+
+#include "base/memory/ptr_util.h"
+#include "chrome/browser/prerender/prerender_manager.h"
+#include "chrome/browser/prerender/prerender_manager_factory.h"
+#include "content/public/browser/web_contents.h"
+
+// static
+std::unique_ptr<page_load_metrics::PageLoadMetricsObserver>
+PrerenderPageLoadMetricsObserver::CreateIfNeeded(
+    content::WebContents* web_contents) {
+  prerender::PrerenderManager* manager =
+      prerender::PrerenderManagerFactory::GetForBrowserContext(
+          web_contents->GetBrowserContext());
+  if (!manager)
+    return nullptr;
+
+  if (manager->PageLoadMetricsObserverDisabledForTesting())
+    return nullptr;
+
+  return base::WrapUnique<page_load_metrics::PageLoadMetricsObserver>(
+      new PrerenderPageLoadMetricsObserver(manager, web_contents));
+}
+
+PrerenderPageLoadMetricsObserver::PrerenderPageLoadMetricsObserver(
+    prerender::PrerenderManager* manager,
+    content::WebContents* web_contents)
+    : prerender_manager_(manager), web_contents_(web_contents) {
+  DCHECK(prerender_manager_);
+}
+
+void PrerenderPageLoadMetricsObserver::OnFirstContentfulPaint(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  prerender_manager_->RecordPerceivedFirstContentfulPaint(

[Bryan McQuade, 2016/11/18 15:23:28]

one thing we do for paint-based timings is only track metrics for pages that are
foregrounded for the entire duration from nav_start to the given paint timing.
The reason for this is that the rendering pipeline is disabled when a page is
backgrounded, so if you include timings for pages that spent some time in the
background, what you may end up measuring is the time until the user
foregrounded the tab, a long time after it had been ready to paint in the
background.

So I'd suggest one of a few options:
1. only log metrics for page loads that spend all of their time in the
foreground
2. log metrics that spend time between start and FCP in the background in a
separate histogram with a '.Background' suffix - this is what we do elsewhere,
though in practice these histograms haven't been very useful other than to have
a sense for the % of page loads that get backgrounded before FCP.

Your implementation is made slightly more challenging by the fact that
prerenders do start in the background. I think you want to track whether the
page load spends any time from the time the prerender is 'used' to the given
paint event in the background. To do this you probably need to implement the
PLMO OnHidden callback, to track if you end up being hidden after the prerender
is used but before FCP fires. One simple thing you could do is just return
STOP_OBSERVING in your OnHidden impl, the first time it gets called after the
prerender is used. Else you can have a boolean member or a timestamp for
first_background_after_use_ that allows you to know if you were backgrounded
before a given metric event.

[mattcary, 2016/11/18 15:36:14]

Interesting. I designed this to be as parallel as possible to PerceivedPLT, but
as we're getting rid of PLT I guess that may not matter much.

In case it wasn't clear from the other parts of this CL, what we're doing is
reporting the time from prerender swap in until FCP (by keeping track of load
start time, prerender swap time, and subtracting appropriately).

Okay, with that out of the way, let me see if I understand your suggestion.
You're concerned about when the (pre)renderer gets hidden between swap and FCP.
This can be detected via the PLMO OnHidden callback, and you suggest just
splitting our histogram in two depending on whether it was backgrounded or not,
but otherwise not adjusting any of the timings. 

If I return STOP_OBSERVING from OnHidden, I won't fire on the FCP, and the
effect will be to not record any backgrounded pages (and so I just keep the
single histogram that I currently have).

This seems pretty reasonable to me, I'll see if I can make that work. Let me
know if I misinterpreted anything.

+      web_contents_, *timing.first_contentful_paint);

[Bryan McQuade, 2016/11/18 14:52:37]

why is the * needed here when passing timing.first_contentful_paint?

[mattcary, 2016/11/18 15:14:14]

timing.first_contentful_paint is a base::Optional<base::TimeDelta>. The *
burrows to the inside of the option. Someone in base likes C++ overloading :)

+}