ResourcePrefetchPredictor has some logic that depends on whether a given resource was processed before vs after FCP.

One challenge here is that reporting of FCP from render to browser process is buffered for up to 1 second, to reduce the number of IPCs that get dispatched.

So there can be cases where an FCP has happened, but the browser process hasn't observed it yet. It seems like some of the ResourcePrefetchPredictor code may be susceptible to this issue.

In particular, the SubresourceFcpOrder test, and the associated logic it is testing, can break if the FCP event has happened but is still being buffered at the time the before_first_contentful_paint logic is evaluated.

The particular test has some logic to work around this:
  ResourceSummary* image = AddResource(
      GetURL(kImagePath), content::RESOURCE_TYPE_IMAGE, net::LOWEST);
  // Delay HTTP response to ensure enough time to receive notice of
  // firstContentfulPaint.
  image->delay = base::TimeDelta::FromMilliseconds(1500);
  image->request.before_first_contentful_paint = false;

and while this works to mitigate the effects of the 1s FCP buffering in the render process, a 1s delay won't always be present for real world pages, and thus it seems like this logic may not always work correctly in production.

I don't understand the code especially well so I may be misunderstanding - but I think the browser process can't safely depend on having received the FCP notification by the time the before_first_contentful_paint attribute is populated, which seems to happen here:
https://cs.chromium.org/chromium/src/chrome/browser/predictors/resource_prefetch_predictor.cc?q=resource_prefetch_predictor.cc&dr&l=882

and IIUC is triggered when the page fires the onload event. There are no guarantees that FCP will be flushed to the browser process by the time onload fires.

In general the page load timing code is intended to be used to discover and log when events happened, but makes no guarantees about the occurrence of those events being dispatched aggressively to the browser process.

We should try to figure out how to address the goals of the before_first_contentful_paint logic w/o being susceptible to the buffering issues.


More context: 

This email is prompted by a change I'm working on to record the first contentful paint across all frames in a page, which has been how FCP worked historically, but due to some recent refactoring / bug fixing in blink, has required changes in the page load metrics code.

My change adds an additional 1s of buffering for FCP reporting in the browser process. This additional buffering causes the SubresourceFcpOrder test to fail, since the 1.5s delay is no longer enough.

I believe the root issue here is that we can never safely assume that we'll have received the FCP update by the time the onload event fires in production. For this to be the case we'd need to aggressively flush the FCP time to the browser process, which we've decided not to do so far to minimize the number of IPCs we send for page load timing updates.

In the interim, I'm inclined to increase the timeout in the test to accommodate the increased buffering time, so this change can land and we can address a bug in the current page load metrics code (https://bugs.chromium.org/p/chromium/issues/detail?id=726387).

As we think about fixing this, a few questions: what's the goal / intent of this before_first_contentful_paint check? How does this affect the predictor's behavior? Might there be other ways we could accomplish the same goal?