[Prerender] first contentful paint histograms.

chrome/browser/prerender/prerender_browsertest.cc

Index: chrome/browser/prerender/prerender_browsertest.cc
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc
index 9956202fd38c3e2a1faba9a100a6c2a5ea7114da..86cc1215797a6d505173d7114194b645d5bed4af 100644
--- a/chrome/browser/prerender/prerender_browsertest.cc
+++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -26,6 +26,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "base/test/test_timeouts.h"
 #include "base/values.h"
 #include "build/build_config.h"
@@ -40,6 +41,8 @@
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/external_protocol/external_protocol_handler.h"
 #include "chrome/browser/net/prediction_options.h"
+#include "chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h"
+#include "chrome/browser/page_load_metrics/observers/prerender_page_load_metrics_observer.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor_factory.h"
@@ -50,6 +53,7 @@
 #include "chrome/browser/prerender/prerender_link_manager_factory.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
+#include "chrome/browser/prerender/prerender_tab_helper.h"
 #include "chrome/browser/prerender/prerender_test_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
@@ -534,6 +538,16 @@ base::FilePath GetTestPath(const std::string& file_name) {
       base::FilePath().AppendASCII(file_name));
 }
 
+page_load_metrics::PageLoadExtraInfo GenericPageLoadExtraInfo(
+    const GURL& dest_url) {
+  return page_load_metrics::PageLoadExtraInfo(
+      base::TimeDelta(), base::TimeDelta(), false,
+      page_load_metrics::UserInitiatedInfo::BrowserInitiated(), dest_url,
+      dest_url, page_load_metrics::ABORT_NONE,
+      page_load_metrics::UserInitiatedInfo::NotUserInitiated(),
+      base::TimeDelta(), 0, 0, page_load_metrics::PageLoadMetadata());
+}
+
 }  // namespace
 
 class PrerenderBrowserTest : public test_utils::PrerenderInProcessBrowserTest {
@@ -721,6 +735,8 @@ class PrerenderBrowserTest : public test_utils::PrerenderInProcessBrowserTest {
     call_javascript_ = false;
   }
 
+  void EnableJavascriptCalls() { call_javascript_ = true; }
+
   void DisableLoadEventCheck() {
     check_load_events_ = false;
   }
@@ -875,6 +891,26 @@ class PrerenderBrowserTest : public test_utils::PrerenderInProcessBrowserTest {
         base::ASCIIToUTF16(javascript));
   }
 
+  base::SimpleTestTickClock* OverridePrerenderManagerTimeTicks() {
+    auto clock = base::MakeUnique<base::SimpleTestTickClock>();
+    auto clock_ptr = clock.get();
+    // The default zero time causes the prerender manager to do strange things.
+    clock->Advance(base::TimeDelta::FromSeconds(1));
+    GetPrerenderManager()->SetTickClockForTesting(std::move(clock));
+    return clock_ptr;
+  }
+
+  void SetMidLoadClockAdvance(base::SimpleTestTickClock* clock,

[pasko, 2016/12/20 19:33:34]

This is used only in one test, which is not a good excuse to increase the amount
of state in the fixture. If Advance absolutely needs to be done inside
PrerenderTestURLImpl (which does not seem obviously necessary), then something
like SetAfterPrerenderLoadCallback() would do it and keep the state controlled
enough in the test.

[mattcary, 2016/12/21 17:08:58]

This is a text fixture only used in this file (ie, not in prerender_test_utils).
Callbacks seem to have unnecessary generality and are much more difficult to
read, which especially for tests is the wrong way to go.

We need to be able to control the clock while in the middle of
PrerenderTestURLImpl in order to time out the prerender, as the timeout occurs
during the periodic cleanup that happens in the WaitForStop; otherwise the
prerender will continue and we'll get a first contentful paint which we don't
want for this test.

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

Yes, but the class is also about 500 lines of code, which makes it preferable to
push the state out of it into the individual tests that need it. The
PrerenderTestURLImpl() is already a bit too complex, hard to always keep in mind
all the things it does. 
I agree that callbacks are not good here, I was hoping that it was not necessary
to advance time in the middle.
Ah, this was to be able to timeout without FCP! Makes sense. Thanks!

Hm, is the ultimate goal to test with FINAL_STATUS_TIMED_OUT? If the goal is to
test what happens if a prerender does not reach FCP, then other final statuses
should work without complications of intercepting the clock in the middle of the
test, like: FINAL_STATUS_UNSUPPORTED_SCHEME.

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

yes, exactly, hence the test name.

It seems that the case of a page starting a prefetch, and then timing out due to
the main resource or redirects or whatever stalling on a flakey mobile network,
is practical and likely, and thus a test for this case specifically is a good
idea.

+                              base::TimeDelta delta) {
+    mid_load_clock_ = clock;
+    mid_load_clock_tick_advance_ = delta;
+  }
+
+  void ClearMidLoadClock() {
+    mid_load_clock_tick_advance_ = base::TimeDelta();
+    mid_load_clock_ = nullptr;
+  }
+
  private:
   // TODO(davidben): Remove this altogether so the tests don't globally assume
   // only one prerender.
@@ -899,6 +935,11 @@ class PrerenderBrowserTest : public test_utils::PrerenderInProcessBrowserTest {
         NavigateWithPrerenders(loader_url, expected_final_status_queue);
     prerenders[0]->WaitForLoads(expected_number_of_loads);
 
+    if (!mid_load_clock_tick_advance_.is_zero()) {
+      EXPECT_TRUE(mid_load_clock_);
+      mid_load_clock_->Advance(mid_load_clock_tick_advance_);
+    }
+
     FinalStatus expected_final_status = expected_final_status_queue.front();
     if (ShouldAbortPrerenderBeforeSwap(expected_final_status)) {
       // The prerender will abort on its own. Assert it does so correctly.
@@ -991,12 +1032,17 @@ class PrerenderBrowserTest : public test_utils::PrerenderInProcessBrowserTest {
   std::string loader_path_;
   std::string loader_query_;
   base::test::ScopedFeatureList feature_list_;
+  base::TimeDelta mid_load_clock_tick_advance_;
+  base::SimpleTestTickClock* mid_load_clock_;
 };
 
 // Checks that a page is correctly prerendered in the case of a
 // <link rel=prerender> tag and then loaded into a tab in response to a
 // navigation.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPage) {
+  test_utils::FirstContentfulPaintManagerWaiter* fcp_waiter =
+      test_utils::FirstContentfulPaintManagerWaiter::Create(
+          GetPrerenderManager());
   PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
   EXPECT_EQ(1, GetPrerenderDomContentLoadedEventCountForLinkNumber(0));
   histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
@@ -1011,18 +1057,58 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPage) {
       GetActiveWebContents()->GetRenderProcessHost());
   NavigateToDestURL();
   channel_close_watcher.WaitForChannelClose();
+  fcp_waiter->Wait();
 
   histogram_tester().ExpectTotalCount("Prerender.websame_PerceivedPLT", 1);
   histogram_tester().ExpectTotalCount("Prerender.websame_PerceivedPLTMatched",
                                       1);
   histogram_tester().ExpectTotalCount(
       "Prerender.websame_PerceivedPLTMatchedComplete", 1);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PerceivedTTFCPRecorded.Visible", 1);
 
   ASSERT_TRUE(IsEmptyPrerenderLinkManager());
 }
 
+// Checks that the correct page load metrics observers are produced with and
+// without a prerender.
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PageLoadMetrics) {
+  // Simple load. The prefetch page is used as a simple page with a nonempty
+  // layout; no prefetching is performed.
+  test_utils::FirstContentfulPaintManagerWaiter* simple_fcp_waiter =
+      test_utils::FirstContentfulPaintManagerWaiter::Create(
+          GetPrerenderManager());
+  ui_test_utils::NavigateToURL(
+      current_browser(), src_server()->GetURL("/prerender/prefetch_page.html"));
+  simple_fcp_waiter->Wait();
+
+  // Prerendered load.

[pasko, 2016/12/20 19:33:34]

The Simple/Prerender tests have little in common, any reason not to make it two
separate tests? maybe it tries to cover some tricky state during the transition
and renderer reuse? Would be nice to add a comment in such case.

[mattcary, 2016/12/21 17:08:58]

Done.

+  test_utils::FirstContentfulPaintManagerWaiter* prerender_fcp_waiter =
+      test_utils::FirstContentfulPaintManagerWaiter::Create(
+          GetPrerenderManager());
+  PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
+  NavigateToDestURL();
+  prerender_fcp_waiter->Wait();
+
+  // Histogram only emitted during the simple load.
+  histogram_tester().ExpectTotalCount(
+      "Prerender.none_PrefetchTTFCP.Reference.Cacheable.Visible", 1);
+  // Histogram only emitted during prerender.
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
+  // Histogram only emitted during the two simple loads: the NavigateToURL call,
+  // and the page containing the link rel=prerender.
+  histogram_tester().ExpectTotalCount(
+      "PageLoad.DocumentTiming.NavigationToFirstLayout", 2);
+}
+
 // Checks that cross-domain prerenders emit the correct histograms.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageCrossDomain) {
+  test_utils::FirstContentfulPaintManagerWaiter* fcp_waiter =
+      test_utils::FirstContentfulPaintManagerWaiter::Create(
+          GetPrerenderManager());
   PrerenderTestURL(GetCrossDomainTestUrl("prerender/prerender_page.html"),
                    FINAL_STATUS_USED, 1);
   histogram_tester().ExpectTotalCount("Prerender.none_PerceivedPLT", 1);
@@ -1033,11 +1119,15 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderPageCrossDomain) {
       "Prerender.webcross_PrerenderNotSwappedInPLT", 1);
 
   NavigateToDestURL();
+  fcp_waiter->Wait();
+
   histogram_tester().ExpectTotalCount("Prerender.webcross_PerceivedPLT", 1);
   histogram_tester().ExpectTotalCount("Prerender.webcross_PerceivedPLTMatched",
                                       1);
   histogram_tester().ExpectTotalCount(
       "Prerender.webcross_PerceivedPLTMatchedComplete", 1);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.webcross_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
 }
 
 // Checks that pending prerenders launch and receive proper event treatment.
@@ -3184,6 +3274,276 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, AutosigninInPrerenderer) {
   EXPECT_EQ(0, done_counter.count());
 }
 
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, FirstContentfulPaintTimingSimple) {
+  GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting();
+  base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks();
+  PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
+
+  base::TimeTicks load_start = clock->NowTicks();
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  NavigateToDestURL();
+
+  test_utils::TestPrerenderPageLoadMetricsObserver observer(
+      GetPrerenderManager(), GetActiveWebContents());
+  observer.SetNavigationStartTo(load_start);
+
+  page_load_metrics::PageLoadTiming timing;
+  timing.navigation_start = base::Time::FromDoubleT(1);

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

this number does not contribute to any of the expectations in the test, right?
It was not obvious, and I was wondering of the reasons to initialize it from
such a distinctive floating point value. Maybe make it clear that it is
something like kUnusedTime?

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

Actually, it's just to initialize it to a non-null time. Added comment.

+  timing.first_contentful_paint = base::TimeDelta::FromMilliseconds(2654);
+  page_load_metrics::PageLoadMetricsObserverTestHarness::
+      PopulateRequiredTimingFields(&timing);
+  observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url()));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1654, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, FirstContentfulPaintTimingReuse) {
+  GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting();
+  base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks();
+
+  GURL url = embedded_test_server()->GetURL("/prerender/prerender_page.html");
+  DisableJavascriptCalls();

[pasko, 2016/12/20 19:33:34]

We are not expecting swap to succeed on this prerender, so it is not obvious why
checking DidPrerenderPass() should be disabled. Please add a comment to explain
it.

[mattcary, 2016/12/21 17:08:58]

Done, here and below. It's not the swap bit, it's the fact that we don't wait
for the load.

+  PrerenderTestURL(url, FINAL_STATUS_CANCELLED, 0);
+
+  // This prerender cancels and reuses the first.
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  base::TimeTicks load_start = clock->NowTicks();
+  EnableJavascriptCalls();
+  PrerenderTestURL(url, FINAL_STATUS_USED, 1);
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+
+  NavigateToDestURL();
+  test_utils::TestPrerenderPageLoadMetricsObserver observer(
+      GetPrerenderManager(), GetActiveWebContents());
+  observer.SetNavigationStartTo(load_start);
+
+  page_load_metrics::PageLoadTiming timing;
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.first_contentful_paint = base::TimeDelta::FromMilliseconds(2361);
+  page_load_metrics::PageLoadMetricsObserverTestHarness::
+      PopulateRequiredTimingFields(&timing);
+  observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url()));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
+  // If the first prerender had been used, the perceived TTFCP would have been
+  // under a second: 2362ms - 2 sec worth of Advance().
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1361, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       FirstContentfulPaintTimingTimeout) {
+  GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting();
+  base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks();
+
+  // Make the first prerender time out.
+  base::TimeDelta time_out_delta =
+      GetPrerenderManager()->config().time_to_live +
+      base::TimeDelta::FromSeconds(10);
+  SetMidLoadClockAdvance(clock, time_out_delta);
+
+  GURL url = embedded_test_server()->GetURL("/prerender/prerender_page.html");
+  PrerenderTestURL(url, FINAL_STATUS_TIMED_OUT, 1);
+
+  ClearMidLoadClock();
+  base::TimeTicks load_start = clock->NowTicks();
+  PrerenderTestURL(url, FINAL_STATUS_USED, 1);
+
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  NavigateToDestURL();
+
+  test_utils::TestPrerenderPageLoadMetricsObserver observer(
+      GetPrerenderManager(), GetActiveWebContents());
+  observer.SetNavigationStartTo(load_start);
+
+  page_load_metrics::PageLoadTiming timing;

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

this part seems to repeat what the previous test did. The name of the test
suggests that we are testing here that timed out prerenders would not record the
histograms. But we cannot check it because we are not setting the observer.

Then idea #2: we might be testing that histograms are OK on after-timeout
prerenders. Is there a reason to suspect that the mechanics would be
significantly different from the ones for CANCELLED?

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

Yes. The point is that parts of the page load timing are set in different stages
of the prerenderer lifetime, and we want to make sure that we correctly detect &
ignore those partial loads. Timing out and cancelling are different mechanisms.

+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.first_contentful_paint = base::TimeDelta::FromMilliseconds(2361);
+  page_load_metrics::PageLoadMetricsObserverTestHarness::
+      PopulateRequiredTimingFields(&timing);
+  observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url()));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
+  // If the timed out had been used, the perceived TTFCP would have been
+  // negative.
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1361, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       FirstContentfulPaintTimingNoCommit) {
+  GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting();
+  base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks();
+
+  GURL url = embedded_test_server()->GetURL("/prerender/prerender_page.html");
+  base::FilePath url_file = ui_test_utils::GetTestFilePath(
+      base::FilePath(),
+      base::FilePath(FILE_PATH_LITERAL("prerender/prerender_page.html")));
+
+  base::RunLoop prerender_start_loop;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&CreateHangingFirstRequestInterceptorOnIO, url, url_file,
+                 prerender_start_loop.QuitClosure()));
+  DisableJavascriptCalls();
+  PrerenderTestURL(url, FINAL_STATUS_NAVIGATION_UNCOMMITTED, 0);
+  prerender_start_loop.Run();
+
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  NavigateToDestURLWithDisposition(WindowOpenDisposition::CURRENT_TAB, false);
+
+  test_utils::TestPrerenderPageLoadMetricsObserver observer(
+      GetPrerenderManager(), GetActiveWebContents());
+  observer.SetNavigationStartTo(clock->NowTicks());
+
+  page_load_metrics::PageLoadTiming timing;
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.first_contentful_paint = base::TimeDelta::FromMilliseconds(2362);
+  page_load_metrics::PageLoadMetricsObserverTestHarness::
+      PopulateRequiredTimingFields(&timing);
+  observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url()));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.none_PrefetchTTFCP.Warm.Cacheable.Visible", 0);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.none_PerceivedTTFCPRecorded.Visible", 1);
+
+  // Check that the prerender didn't happen with a defined origin.
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 0);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PerceivedTTFCPRecorded.Visible", 0);
+
+  // A FCP is fired by the observer, but the manager should detect that the
+  // perceived time is not set and so update the following histogram.

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

I was confused by this comment. The notion of perceived FCP is internal to
PrerenderManager, while it receives request to record FCP from outside. So it
should not 'detect' that 'perceived time' is set or not, it records it or avoids
recording it depending on the situation, right?

I think it would be clearer to say that the PrerenderManager observes that FCP
is recorded without webcontents swap (since it is done by observer), so it
records that it did not record perceived time.

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

Done.

+  histogram_tester().ExpectUniqueSample(
+      "Prerender.none_PerceivedTTFCPRecorded.Visible", 0, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       FirstContentfulPaintTimingTwoPages) {
+  GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting();
+  base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks();
+
+  DisableJavascriptCalls();
+  // First prerender a different page from the usual target.
+  PrerenderTestURL("/prerender/prefetch_page.html", FINAL_STATUS_CANCELLED, 0);
+
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  base::TimeTicks load_start = clock->NowTicks();
+  EnableJavascriptCalls();
+  PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
+
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  NavigateToDestURL();
+
+  test_utils::TestPrerenderPageLoadMetricsObserver observer(
+      GetPrerenderManager(), GetActiveWebContents());
+  observer.SetNavigationStartTo(load_start);
+
+  page_load_metrics::PageLoadTiming timing;
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  // The FCP time should end up on the edge of the bucket.
+  timing.first_contentful_paint = base::TimeDelta::FromMilliseconds(2654);
+  page_load_metrics::PageLoadMetricsObserverTestHarness::
+      PopulateRequiredTimingFields(&timing);
+  observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url()));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1);
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Visible", 1654, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, FirstContentfulPaintHidden) {
+  GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting();
+  base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks();
+  base::TimeTicks load_start = clock->NowTicks();
+  PrerenderTestURL("/prerender/prerender_page.html", FINAL_STATUS_USED, 1);
+
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  NavigateToDestURL();
+
+  test_utils::TestPrerenderPageLoadMetricsObserver observer(
+      GetPrerenderManager(), GetActiveWebContents());
+  observer.SetNavigationStartTo(load_start);
+
+  EXPECT_EQ(page_load_metrics::PageLoadMetricsObserver::CONTINUE_OBSERVING,
+            observer.OnHidden(page_load_metrics::PageLoadTiming(),
+                              GenericPageLoadExtraInfo(dest_url())));
+
+  page_load_metrics::PageLoadTiming timing;
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.first_contentful_paint = base::TimeDelta::FromMilliseconds(2654);
+  page_load_metrics::PageLoadMetricsObserverTestHarness::
+      PopulateRequiredTimingFields(&timing);
+  observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url()));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Hidden", 1);
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Hidden", 1654, 1);
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       FirstContentfulPaintHiddenNoCommit) {
+  GetPrerenderManager()->DisablePageLoadMetricsObserverForTesting();
+  base::SimpleTestTickClock* clock = OverridePrerenderManagerTimeTicks();
+
+  GURL url = embedded_test_server()->GetURL("/prerender/prerender_page.html");
+  base::FilePath url_file = ui_test_utils::GetTestFilePath(
+      base::FilePath(),
+      base::FilePath(FILE_PATH_LITERAL("prerender/prerender_page.html")));
+
+  base::RunLoop prerender_start_loop;
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&CreateHangingFirstRequestInterceptorOnIO, url, url_file,
+                 prerender_start_loop.QuitClosure()));
+  DisableJavascriptCalls();
+  PrerenderTestURL(url, FINAL_STATUS_NAVIGATION_UNCOMMITTED, 0);
+  prerender_start_loop.Run();
+
+  clock->Advance(base::TimeDelta::FromSeconds(1));
+  NavigateToDestURLWithDisposition(WindowOpenDisposition::CURRENT_TAB, false);
+
+  test_utils::TestPrerenderPageLoadMetricsObserver observer(
+      GetPrerenderManager(), GetActiveWebContents());
+  observer.SetNavigationStartTo(clock->NowTicks());
+
+  EXPECT_EQ(page_load_metrics::PageLoadMetricsObserver::CONTINUE_OBSERVING,
+            observer.OnHidden(page_load_metrics::PageLoadTiming(),
+                              GenericPageLoadExtraInfo(dest_url())));
+
+  page_load_metrics::PageLoadTiming timing;
+  timing.navigation_start = base::Time::FromDoubleT(1);
+  timing.first_contentful_paint = base::TimeDelta::FromMilliseconds(2362);
+  page_load_metrics::PageLoadMetricsObserverTestHarness::
+      PopulateRequiredTimingFields(&timing);
+  observer.OnFirstContentfulPaint(timing, GenericPageLoadExtraInfo(dest_url()));
+
+  histogram_tester().ExpectTotalCount(
+      "Prerender.none_PrefetchTTFCP.Warm.Cacheable.Hidden", 0);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.none_PerceivedTTFCPRecorded.Hidden", 1);
+
+  // Check that the prerender didn't happen with a defined origin.
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PrefetchTTFCP.Warm.Cacheable.Hidden", 0);
+  histogram_tester().ExpectTotalCount(
+      "Prerender.websame_PerceivedTTFCPRecorded.Hidden", 0);
+
+  // A FCP is fired by the observer, but the manager should detect that the
+  // perceived time is not set and so update the following histogram.
+  histogram_tester().ExpectUniqueSample(
+      "Prerender.none_PerceivedTTFCPRecorded.Hidden", 0, 1);
+}
+
 class PrerenderIncognitoBrowserTest : public PrerenderBrowserTest {
  public:
   void SetUpOnMainThread() override {