From 70f085bc1417eb415a3e9230ee2093c4ecd44f0e Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 1 Aug 2023 15:48:33 -0700 Subject: [PATCH] core(main-resource): find last matching request --- cli/test/fixtures/redirects-refresh.html | 18 +++++++++++ cli/test/smokehouse/config/exclusions.js | 1 + cli/test/smokehouse/core-tests.js | 2 ++ .../test-definitions/redirects-refresh.js | 30 +++++++++++++++++++ core/computed/main-resource.js | 2 +- core/computed/page-dependency-graph.js | 3 +- .../simulator/network-analyzer.js | 16 +++++++++- core/lib/navigation-error.js | 2 +- 8 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 cli/test/fixtures/redirects-refresh.html create mode 100644 cli/test/smokehouse/test-definitions/redirects-refresh.js diff --git a/cli/test/fixtures/redirects-refresh.html b/cli/test/fixtures/redirects-refresh.html new file mode 100644 index 000000000000..56ff389b31aa --- /dev/null +++ b/cli/test/fixtures/redirects-refresh.html @@ -0,0 +1,18 @@ + + + + + + + +

This page refreshes itself one time.

+ + + diff --git a/cli/test/smokehouse/config/exclusions.js b/cli/test/smokehouse/config/exclusions.js index 2c994314d908..8d3d5a9e4ac4 100644 --- a/cli/test/smokehouse/config/exclusions.js +++ b/cli/test/smokehouse/config/exclusions.js @@ -20,6 +20,7 @@ const exclusions = { 'redirects-client-paint-server', 'redirects-multiple-server', 'redirects-single-server', 'redirects-single-client', 'redirects-history-push-state', 'redirects-scripts', + 'redirects-refresh', // Disabled because these tests use settings that cannot be fully configured in // DevTools (e.g. throttling method "provided"). 'metrics-tricky-tti', 'metrics-tricky-tti-late-fcp', 'screenshot', diff --git a/cli/test/smokehouse/core-tests.js b/cli/test/smokehouse/core-tests.js index c6f501103355..c66b62a2eefc 100644 --- a/cli/test/smokehouse/core-tests.js +++ b/cli/test/smokehouse/core-tests.js @@ -53,6 +53,7 @@ import pwaSvgomg from './test-definitions/pwa-svgomg.js'; import redirectsClientPaintServer from './test-definitions/redirects-client-paint-server.js'; import redirectsHistoryPushState from './test-definitions/redirects-history-push-state.js'; import redirectsMultipleServer from './test-definitions/redirects-multiple-server.js'; +import redirectsRefresh from './test-definitions/redirects-refresh.js'; import redirectsScripts from './test-definitions/redirects-scripts.js'; import redirectsSelf from './test-definitions/redirects-self.js'; import redirectsSingleClient from './test-definitions/redirects-single-client.js'; @@ -116,6 +117,7 @@ const smokeTests = [ redirectsClientPaintServer, redirectsHistoryPushState, redirectsMultipleServer, + redirectsRefresh, redirectsScripts, redirectsSelf, redirectsSingleClient, diff --git a/cli/test/smokehouse/test-definitions/redirects-refresh.js b/cli/test/smokehouse/test-definitions/redirects-refresh.js new file mode 100644 index 000000000000..3c1560fee9d2 --- /dev/null +++ b/cli/test/smokehouse/test-definitions/redirects-refresh.js @@ -0,0 +1,30 @@ +/** + * @license Copyright 2022 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ + +/** + * @type {Smokehouse.ExpectedRunnerResult} + */ +const expectations = { + artifacts: { + MainDocumentContent: /This page refreshes itself one time/, + URL: { + requestedUrl: 'http://localhost:10200/redirects-refresh.html', + mainDocumentUrl: 'http://localhost:10200/redirects-refresh.html', + finalDisplayedUrl: 'http://localhost:10200/redirects-refresh.html', + }, + }, + lhr: { + requestedUrl: 'http://localhost:10200/redirects-refresh.html', + finalDisplayedUrl: 'http://localhost:10200/redirects-refresh.html', + audits: {}, + }, +}; + +export default { + id: 'redirects-refresh', + expectations, +}; + diff --git a/core/computed/main-resource.js b/core/computed/main-resource.js index c0f69453582a..b3d6e4797d2b 100644 --- a/core/computed/main-resource.js +++ b/core/computed/main-resource.js @@ -22,7 +22,7 @@ class MainResource { const {mainDocumentUrl} = data.URL; if (!mainDocumentUrl) throw new Error('mainDocumentUrl must exist to get the main resource'); const requests = await NetworkRecords.request(data.devtoolsLog, context); - const mainResource = NetworkAnalyzer.findResourceForUrl(requests, mainDocumentUrl); + const mainResource = NetworkAnalyzer.findLastResourceForUrl(requests, mainDocumentUrl); if (!mainResource) { throw new Error('Unable to identify the main resource'); } diff --git a/core/computed/page-dependency-graph.js b/core/computed/page-dependency-graph.js index 5741076ed1ad..f17de574bce2 100644 --- a/core/computed/page-dependency-graph.js +++ b/core/computed/page-dependency-graph.js @@ -412,7 +412,8 @@ class PageDependencyGraph { const rootNode = networkNodeOutput.idToNodeMap.get(rootRequest.requestId); if (!rootNode) throw new Error('rootNode not found'); - const mainDocumentRequest = NetworkAnalyzer.findResourceForUrl(networkRecords, mainDocumentUrl); + const mainDocumentRequest = + NetworkAnalyzer.findLastResourceForUrl(networkRecords, mainDocumentUrl); if (!mainDocumentRequest) throw new Error('mainDocumentRequest not found'); const mainDocumentNode = networkNodeOutput.idToNodeMap.get(mainDocumentRequest.requestId); if (!mainDocumentNode) throw new Error('mainDocumentNode not found'); diff --git a/core/lib/dependency-graph/simulator/network-analyzer.js b/core/lib/dependency-graph/simulator/network-analyzer.js index 9962dab6070c..f98b2b20a66e 100644 --- a/core/lib/dependency-graph/simulator/network-analyzer.js +++ b/core/lib/dependency-graph/simulator/network-analyzer.js @@ -506,9 +506,23 @@ class NetworkAnalyzer { ); } + /** + * @param {Array} records + * @param {string} resourceUrl + * @return {LH.Artifacts.NetworkRequest|undefined} + */ + static findLastResourceForUrl(records, resourceUrl) { + // equalWithExcludedFragments is expensive, so check that the resourceUrl starts with the request url first + const matchingRequests = records.filter(request => + resourceUrl.startsWith(request.url) && + UrlUtils.equalWithExcludedFragments(request.url, resourceUrl) + ); + return matchingRequests[matchingRequests.length - 1]; + } + /** * Resolves redirect chain given a main document. - * See: {@link NetworkAnalyzer.findResourceForUrl}) for how to retrieve main document. + * See: {@link NetworkAnalyzer.findLastResourceForUrl}) for how to retrieve main document. * * @param {LH.Artifacts.NetworkRequest} request * @return {LH.Artifacts.NetworkRequest} diff --git a/core/lib/navigation-error.js b/core/lib/navigation-error.js index ea155442df37..1b52f71a0876 100644 --- a/core/lib/navigation-error.js +++ b/core/lib/navigation-error.js @@ -119,7 +119,7 @@ function getNonHtmlError(finalRecord) { function getPageLoadError(navigationError, context) { const {url, loadFailureMode, networkRecords} = context; /** @type {LH.Artifacts.NetworkRequest|undefined} */ - let mainRecord = NetworkAnalyzer.findResourceForUrl(networkRecords, url); + let mainRecord = NetworkAnalyzer.findLastResourceForUrl(networkRecords, url); // If the url doesn't give us a network request, it's possible we landed on a chrome-error:// page // In this case, just get the first document request.