core(main-resource): find last matching document request

[adamraine]

Noticed this on https://umich.edu

If a page does a JS refresh then we will use the wrong network request as the main resource because we match on url. We should be looking for the last request.

[brendankenny]

Our most bullet-proof method for this is probably:

lighthouse/core/audits/redirects.js

Lines 38 to 78 in e5d1b9c

	/**
	* This method generates the document request chain including client-side and server-side redirects.
	*
	* Example:
	* GET /requestedUrl => 302 /firstRedirect
	* GET /firstRedirect => 200 /firstRedirect, window.location = '/secondRedirect'
	* GET /secondRedirect => 302 /thirdRedirect
	* GET /thirdRedirect => 302 /mainDocumentUrl
	* GET /mainDocumentUrl => 200 /mainDocumentUrl
	*
	* Returns network records [/requestedUrl, /firstRedirect, /secondRedirect, /thirdRedirect, /mainDocumentUrl]
	*
	* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
	* @param {LH.Artifacts.ProcessedTrace} processedTrace
	* @return {Array<LH.Artifacts.NetworkRequest>}
	*/
	static getDocumentRequestChain(networkRecords, processedTrace) {
	/** @type {Array<LH.Artifacts.NetworkRequest>} */
	const documentRequests = [];
	// Find all the document requests by examining navigation events and their redirects
	for (const event of processedTrace.processEvents) {
	if (event.name !== 'navigationStart') continue;
	const data = event.args.data || {};
	if (!data.documentLoaderURL || !data.isLoadingMainFrame) continue;
	let networkRecord = networkRecords.find(record => record.requestId === data.navigationId);
	while (networkRecord) {
	documentRequests.push(networkRecord);
	// HTTP redirects won't have separate navStarts, so find through the redirect chain.
	networkRecord = networkRecord.redirectDestination;
	}
	}
	if (!documentRequests.length) {
	throw new Error('No navigation requests found');
	}
	return documentRequests;
	}

checks frame (don't want to match iframes from pages embedding themselves), matches requests by requestId not URL, follows JS and HTTP redirects, etc.

It requires the processed trace, but that's already in page-dependency-graph, and it's not great but not the worst to compute it at the end of the navigation.

[adamraine]

It requires the processed trace, but that's already in page-dependency-graph, and it's not great but not the worst to compute it at the end of the navigation.

The error I saw was in main-document-content which does not have the processed trace or page dependency graph.

[connorjclark]

fyi #15001

[brendankenny]

fyi #15001

Yeah, I brought up the redirects method because I was thinking back then we should switch more infrastructure over to using request IDs.

The error I saw was in main-document-content which does not have the processed trace

Any reason it couldn't? Anything expecting to find a main resource should have observed it in the trace, too.

[adamraine]

Any reason it couldn't? Anything expecting to find a main resource should have observed it in the trace, too.

Sorry I should have said the reason for the error in main-document-content comes from the MainResources computed artifact getting the wrong resource. Presumably we would want to correct this behavior everywhere. It would be a lot of work to add the processed trace to that computed artifact and then update all it's call sites.

[connorjclark]

totally forgot I did this: #14520

[connorjclark]

we going with above pr