From 85b059d03890dc520ce841119db35e15f6a6b61f Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 7 Jun 2024 10:44:33 -0700 Subject: [PATCH 1/9] core(lantern): remove usage of Lighthouse's ProcessedNavigation --- core/lib/lantern/metric.js | 4 +- .../lantern/metrics/first-contentful-paint.js | 4 +- .../metrics/largest-contentful-paint.js | 4 +- core/lib/lantern/types/lantern.d.ts | 10 ++++- .../lib/lantern/metrics/metric-test-utils.js | 41 ++++++++++++++++--- 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/core/lib/lantern/metric.js b/core/lib/lantern/metric.js index 8909543e151e..f799c98e2d4b 100644 --- a/core/lib/lantern/metric.js +++ b/core/lib/lantern/metric.js @@ -64,7 +64,7 @@ class Metric { /** * @param {Node} dependencyGraph - * @param {LH.Artifacts.ProcessedNavigation} processedNavigation + * @param {Lantern.Simulation.ProcessedNavigation} processedNavigation * @return {Node} */ static getOptimisticGraph(dependencyGraph, processedNavigation) { // eslint-disable-line no-unused-vars @@ -73,7 +73,7 @@ class Metric { /** * @param {Node} dependencyGraph - * @param {LH.Artifacts.ProcessedNavigation} processedNavigation + * @param {Lantern.Simulation.ProcessedNavigation} processedNavigation * @return {Node} */ static getPessimisticGraph(dependencyGraph, processedNavigation) { // eslint-disable-line no-unused-vars diff --git a/core/lib/lantern/metrics/first-contentful-paint.js b/core/lib/lantern/metrics/first-contentful-paint.js index accecd55752a..ef1c6a8e968a 100644 --- a/core/lib/lantern/metrics/first-contentful-paint.js +++ b/core/lib/lantern/metrics/first-contentful-paint.js @@ -170,7 +170,7 @@ class FirstContentfulPaint extends Metric { /** * @param {Node} dependencyGraph - * @param {LH.Artifacts.ProcessedNavigation} processedNavigation + * @param {Lantern.Simulation.ProcessedNavigation} processedNavigation * @return {Node} */ static getOptimisticGraph(dependencyGraph, processedNavigation) { @@ -186,7 +186,7 @@ class FirstContentfulPaint extends Metric { /** * @param {Node} dependencyGraph - * @param {LH.Artifacts.ProcessedNavigation} processedNavigation + * @param {Lantern.Simulation.ProcessedNavigation} processedNavigation * @return {Node} */ static getPessimisticGraph(dependencyGraph, processedNavigation) { diff --git a/core/lib/lantern/metrics/largest-contentful-paint.js b/core/lib/lantern/metrics/largest-contentful-paint.js index 135ccbabf435..65bb3eb403f7 100644 --- a/core/lib/lantern/metrics/largest-contentful-paint.js +++ b/core/lib/lantern/metrics/largest-contentful-paint.js @@ -39,7 +39,7 @@ class LargestContentfulPaint extends Metric { /** * @param {Node} dependencyGraph - * @param {LH.Artifacts.ProcessedNavigation} processedNavigation + * @param {Lantern.Simulation.ProcessedNavigation} processedNavigation * @return {Node} */ static getOptimisticGraph(dependencyGraph, processedNavigation) { @@ -56,7 +56,7 @@ class LargestContentfulPaint extends Metric { /** * @param {Node} dependencyGraph - * @param {LH.Artifacts.ProcessedNavigation} processedNavigation + * @param {Lantern.Simulation.ProcessedNavigation} processedNavigation * @return {Node} */ static getPessimisticGraph(dependencyGraph, processedNavigation) { diff --git a/core/lib/lantern/types/lantern.d.ts b/core/lib/lantern/types/lantern.d.ts index e69afef87592..8658edc75a88 100644 --- a/core/lib/lantern/types/lantern.d.ts +++ b/core/lib/lantern/types/lantern.d.ts @@ -169,9 +169,17 @@ export namespace Simulation { nodeTimings: Map, NodeTiming>; } + interface ProcessedNavigation { + timestamps: { + firstContentfulPaint: number; + largestContentfulPaint?: number; + firstMeaningfulPaint?: number; + }; + } + interface MetricComputationDataInput { simulator: Simulator; graph: GraphNode; - processedNavigation: LH.Artifacts.ProcessedNavigation; + processedNavigation: ProcessedNavigation; } } diff --git a/core/test/lib/lantern/metrics/metric-test-utils.js b/core/test/lib/lantern/metrics/metric-test-utils.js index 4cdab0982d40..bfd40583fb5a 100644 --- a/core/test/lib/lantern/metrics/metric-test-utils.js +++ b/core/test/lib/lantern/metrics/metric-test-utils.js @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import {ProcessedNavigation} from '../../../../computed/processed-navigation.js'; import {ProcessedTrace} from '../../../../computed/processed-trace.js'; import {TraceEngineResult} from '../../../../computed/trace-engine-result.js'; import {PageDependencyGraph} from '../../../../lib/lantern/page-dependency-graph.js'; @@ -14,21 +13,52 @@ import * as Lantern from '../../../../lib/lantern/types/lantern.js'; import {getURLArtifactFromDevtoolsLog} from '../../../test-utils.js'; /** @typedef {Lantern.NetworkRequest} NetworkRequest */ +/** @typedef {import('@paulirish/trace_engine/models/trace/handlers/PageLoadMetricsHandler.js').MetricName} MetricName */ +/** @typedef {import('@paulirish/trace_engine/models/trace/handlers/PageLoadMetricsHandler.js').MetricScore} MetricScore */ // TODO(15841): remove usage of Lighthouse code to create test data /** + * @param {LH.Artifacts.TraceEngineResult} traceEngineResult * @param {LH.Artifacts.URL} theURL * @param {LH.Trace} trace * @param {LH.Artifacts.ComputedContext} context */ -async function createGraph(theURL, trace, context) { +async function createGraph(traceEngineResult, theURL, trace, context) { const {mainThreadEvents} = await ProcessedTrace.request(trace, context); - const traceEngineResult = await TraceEngineResult.request({trace}, context); return PageDependencyGraph.createGraphFromTrace( mainThreadEvents, trace, traceEngineResult, theURL); } +/** + * @param {LH.Artifacts.TraceEngineResult} traceEngineResult + * @return {Lantern.Simulation.ProcessedNavigation} + */ +function createProcessedNavigation(traceEngineResult) { + const Meta = traceEngineResult.data.Meta; + const frameId = Meta.mainFrameId; + const navigationId = Meta.mainFrameNavigations[0].args.data?.navigationId || ''; + const scores = traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigationId); + /** @param {MetricScore=} metricScore */ + const getTimestampOrUndefined = metricScore => metricScore?.event ? metricScore.event.ts : undefined; + /** @param {MetricScore=} metricScore */ + const getTimestamp = metricScore => { + if (!metricScore?.event) { + throw new Error('missing metric'); + } + return metricScore.event.ts; + }; + // TODO: should use `MetricName.LCP`, but it is a const enum. + const FCP = /** @type {MetricName} */('FCP'); + const LCP = /** @type {MetricName} */('LCP'); + return { + timestamps: { + firstContentfulPaint: getTimestamp(scores?.get(FCP)), + largestContentfulPaint: getTimestampOrUndefined(scores?.get(LCP)), + }, + }; +} + /** * @param {{trace: LH.Trace, devtoolsLog: LH.DevtoolsLog, settings?: LH.Config.Settings, URL?: LH.Artifacts.URL}} opts */ @@ -38,8 +68,9 @@ async function getComputationDataFromFixture({trace, devtoolsLog, settings, URL} if (!URL) URL = getURLArtifactFromDevtoolsLog(devtoolsLog); const context = {settings, computedCache: new Map()}; - const {graph, records} = await createGraph(URL, trace, context); - const processedNavigation = await ProcessedNavigation.request(trace, context); + const traceEngineResult = await TraceEngineResult.request({trace}, context); + const {graph, records} = await createGraph(traceEngineResult, URL, trace, context); + const processedNavigation = createProcessedNavigation(traceEngineResult); const networkAnalysis = NetworkAnalyzer.analyze(records); const simulator = Simulator.createSimulator({...settings, networkAnalysis}); From 6e795c19e4b532b6b16240c19d3cf2729bf55e9b Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 6 Jun 2024 15:58:33 -0700 Subject: [PATCH 2/9] refactor a bit --- core/computed/metrics/lantern-metric.js | 5 +++ core/lib/lantern/lantern.js | 38 +++++++++++++++++++ .../lib/lantern/metrics/metric-test-utils.js | 32 +--------------- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/core/computed/metrics/lantern-metric.js b/core/computed/metrics/lantern-metric.js index fc8c56f9b6c1..c22f3009e248 100644 --- a/core/computed/metrics/lantern-metric.js +++ b/core/computed/metrics/lantern-metric.js @@ -9,6 +9,8 @@ import {LighthouseError} from '../../lib/lh-error.js'; import {LoadSimulator} from '../load-simulator.js'; import {ProcessedNavigation} from '../processed-navigation.js'; import {PageDependencyGraph} from '../page-dependency-graph.js'; +import {TraceEngineResult} from '../trace-engine-result.js'; +import {createProcessedNavigation} from '../../lib/lantern/lantern.js'; /** * @param {LH.Artifacts.MetricComputationDataInput} data @@ -37,6 +39,9 @@ async function getComputationDataParamsFromTrace(data, context) { const graph = await PageDependencyGraph.request({...data, fromTrace: true}, context); const processedNavigation = await ProcessedNavigation.request(data.trace, context); + // TODO(15841): investigate failures + // const processedNavigation = createProcessedNavigation( + // await TraceEngineResult.request(data, context)); const simulator = data.simulator || (await LoadSimulator.request(data, context)); return {simulator, graph, processedNavigation}; diff --git a/core/lib/lantern/lantern.js b/core/lib/lantern/lantern.js index 63312ed5cdec..88a6b71bcec0 100644 --- a/core/lib/lantern/lantern.js +++ b/core/lib/lantern/lantern.js @@ -4,6 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as Lantern from './types/lantern.js'; + +/** @typedef {import('@paulirish/trace_engine/models/trace/handlers/PageLoadMetricsHandler.js').MetricName} MetricName */ +/** @typedef {import('@paulirish/trace_engine/models/trace/handlers/PageLoadMetricsHandler.js').MetricScore} MetricScore */ + /** @type {LH.Util.SelfMap} */ const NetworkRequestTypes = { XHR: 'XHR', @@ -26,6 +31,39 @@ const NetworkRequestTypes = { Prefetch: 'Prefetch', }; +/** + * @param {LH.Artifacts.TraceEngineResult} traceEngineResult + * @return {Lantern.Simulation.ProcessedNavigation} + */ +function createProcessedNavigation(traceEngineResult) { + const Meta = traceEngineResult.data.Meta; + const frameId = Meta.mainFrameId; + const navigationId = Meta.mainFrameNavigations[0].args.data?.navigationId || ''; + const scores = traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigationId); + /** @param {MetricName} metric */ + const getTimestampOrUndefined = metric => { + const metricScore = scores?.get(metric); + if (!metricScore?.event) return; + return metricScore.event.ts; + }; + /** @param {MetricName} metric */ + const getTimestamp = metric => { + const metricScore = scores?.get(metric); + if (!metricScore?.event) throw new Error(`missing metric: ${metric}`); + return metricScore.event.ts; + }; + // TODO: should use `MetricName.LCP`, but it is a const enum. + const FCP = /** @type {MetricName} */('FCP'); + const LCP = /** @type {MetricName} */('LCP'); + return { + timestamps: { + firstContentfulPaint: getTimestamp(FCP), + largestContentfulPaint: getTimestampOrUndefined(LCP), + }, + }; +} + export { NetworkRequestTypes, + createProcessedNavigation, }; diff --git a/core/test/lib/lantern/metrics/metric-test-utils.js b/core/test/lib/lantern/metrics/metric-test-utils.js index bfd40583fb5a..4ae467dd10fc 100644 --- a/core/test/lib/lantern/metrics/metric-test-utils.js +++ b/core/test/lib/lantern/metrics/metric-test-utils.js @@ -6,6 +6,7 @@ import {ProcessedTrace} from '../../../../computed/processed-trace.js'; import {TraceEngineResult} from '../../../../computed/trace-engine-result.js'; +import {createProcessedNavigation} from '../../../../lib/lantern/lantern.js'; import {PageDependencyGraph} from '../../../../lib/lantern/page-dependency-graph.js'; import {NetworkAnalyzer} from '../../../../lib/lantern/simulator/network-analyzer.js'; import {Simulator} from '../../../../lib/lantern/simulator/simulator.js'; @@ -13,8 +14,6 @@ import * as Lantern from '../../../../lib/lantern/types/lantern.js'; import {getURLArtifactFromDevtoolsLog} from '../../../test-utils.js'; /** @typedef {Lantern.NetworkRequest} NetworkRequest */ -/** @typedef {import('@paulirish/trace_engine/models/trace/handlers/PageLoadMetricsHandler.js').MetricName} MetricName */ -/** @typedef {import('@paulirish/trace_engine/models/trace/handlers/PageLoadMetricsHandler.js').MetricScore} MetricScore */ // TODO(15841): remove usage of Lighthouse code to create test data @@ -30,35 +29,6 @@ async function createGraph(traceEngineResult, theURL, trace, context) { mainThreadEvents, trace, traceEngineResult, theURL); } -/** - * @param {LH.Artifacts.TraceEngineResult} traceEngineResult - * @return {Lantern.Simulation.ProcessedNavigation} - */ -function createProcessedNavigation(traceEngineResult) { - const Meta = traceEngineResult.data.Meta; - const frameId = Meta.mainFrameId; - const navigationId = Meta.mainFrameNavigations[0].args.data?.navigationId || ''; - const scores = traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigationId); - /** @param {MetricScore=} metricScore */ - const getTimestampOrUndefined = metricScore => metricScore?.event ? metricScore.event.ts : undefined; - /** @param {MetricScore=} metricScore */ - const getTimestamp = metricScore => { - if (!metricScore?.event) { - throw new Error('missing metric'); - } - return metricScore.event.ts; - }; - // TODO: should use `MetricName.LCP`, but it is a const enum. - const FCP = /** @type {MetricName} */('FCP'); - const LCP = /** @type {MetricName} */('LCP'); - return { - timestamps: { - firstContentfulPaint: getTimestamp(scores?.get(FCP)), - largestContentfulPaint: getTimestampOrUndefined(scores?.get(LCP)), - }, - }; -} - /** * @param {{trace: LH.Trace, devtoolsLog: LH.DevtoolsLog, settings?: LH.Config.Settings, URL?: LH.Artifacts.URL}} opts */ From 2348559c42378a25aeaf4c8f24221419f6f27efa Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 6 Jun 2024 16:00:12 -0700 Subject: [PATCH 3/9] lint --- core/computed/metrics/lantern-metric.js | 4 ++-- core/lib/lantern/lantern.js | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/computed/metrics/lantern-metric.js b/core/computed/metrics/lantern-metric.js index c22f3009e248..f796791bb3a8 100644 --- a/core/computed/metrics/lantern-metric.js +++ b/core/computed/metrics/lantern-metric.js @@ -9,8 +9,8 @@ import {LighthouseError} from '../../lib/lh-error.js'; import {LoadSimulator} from '../load-simulator.js'; import {ProcessedNavigation} from '../processed-navigation.js'; import {PageDependencyGraph} from '../page-dependency-graph.js'; -import {TraceEngineResult} from '../trace-engine-result.js'; -import {createProcessedNavigation} from '../../lib/lantern/lantern.js'; +// import {TraceEngineResult} from '../trace-engine-result.js'; +// import {createProcessedNavigation} from '../../lib/lantern/lantern.js'; /** * @param {LH.Artifacts.MetricComputationDataInput} data diff --git a/core/lib/lantern/lantern.js b/core/lib/lantern/lantern.js index 88a6b71bcec0..183f7633ba7c 100644 --- a/core/lib/lantern/lantern.js +++ b/core/lib/lantern/lantern.js @@ -39,7 +39,8 @@ function createProcessedNavigation(traceEngineResult) { const Meta = traceEngineResult.data.Meta; const frameId = Meta.mainFrameId; const navigationId = Meta.mainFrameNavigations[0].args.data?.navigationId || ''; - const scores = traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigationId); + const scores = + traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigationId); /** @param {MetricName} metric */ const getTimestampOrUndefined = metric => { const metricScore = scores?.get(metric); From bbf768be5cab3454b96d89866964ddb3f81a4f78 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 7 Jun 2024 10:44:49 -0700 Subject: [PATCH 4/9] avoiding ProcessedNavigation in experimental lantern mode too --- core/audits/redirects.js | 3 +-- core/computed/metrics/lantern-metric.js | 10 ++++------ core/test/audits/redirects-test.js | 4 ++++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/core/audits/redirects.js b/core/audits/redirects.js index d5742e20e0bd..6201544c9de5 100644 --- a/core/audits/redirects.js +++ b/core/audits/redirects.js @@ -90,6 +90,7 @@ class Redirects extends Audit { const processedTrace = await ProcessedTrace.request(trace, context); const networkRecords = await NetworkRecords.request(devtoolsLog, context); + const documentRequests = Redirects.getDocumentRequestChain(networkRecords, processedTrace); const metricComputationData = {trace, devtoolsLog, gatherContext, settings, URL: artifacts.URL}; const metricResult = await LanternInteractive.request(metricComputationData, context); @@ -102,8 +103,6 @@ class Redirects extends Audit { } } - const documentRequests = Redirects.getDocumentRequestChain(networkRecords, processedTrace); - let totalWastedMs = 0; const tableRows = []; diff --git a/core/computed/metrics/lantern-metric.js b/core/computed/metrics/lantern-metric.js index f796791bb3a8..f431d3523674 100644 --- a/core/computed/metrics/lantern-metric.js +++ b/core/computed/metrics/lantern-metric.js @@ -9,8 +9,8 @@ import {LighthouseError} from '../../lib/lh-error.js'; import {LoadSimulator} from '../load-simulator.js'; import {ProcessedNavigation} from '../processed-navigation.js'; import {PageDependencyGraph} from '../page-dependency-graph.js'; -// import {TraceEngineResult} from '../trace-engine-result.js'; -// import {createProcessedNavigation} from '../../lib/lantern/lantern.js'; +import {TraceEngineResult} from '../trace-engine-result.js'; +import {createProcessedNavigation} from '../../lib/lantern/lantern.js'; /** * @param {LH.Artifacts.MetricComputationDataInput} data @@ -38,10 +38,8 @@ async function getComputationDataParamsFromTrace(data, context) { } const graph = await PageDependencyGraph.request({...data, fromTrace: true}, context); - const processedNavigation = await ProcessedNavigation.request(data.trace, context); - // TODO(15841): investigate failures - // const processedNavigation = createProcessedNavigation( - // await TraceEngineResult.request(data, context)); + const traceEngineResult = await TraceEngineResult.request(data, context); + const processedNavigation = createProcessedNavigation(traceEngineResult); const simulator = data.simulator || (await LoadSimulator.request(data, context)); return {simulator, graph, processedNavigation}; diff --git a/core/test/audits/redirects-test.js b/core/test/audits/redirects-test.js index fa8aecb92f0b..8db1c8865c01 100644 --- a/core/test/audits/redirects-test.js +++ b/core/test/audits/redirects-test.js @@ -137,6 +137,10 @@ describe('Performance: Redirects audit', () => { }); const navStart = trace.traceEvents.find(e => e.name === 'navigationStart'); navStart.args.data.navigationId = '1'; + const fcp = trace.traceEvents.find(e => e.name === 'firstContentfulPaint'); + fcp.args.data.navigationId = '1'; + const lcp = trace.traceEvents.find(e => e.name === 'largestContentfulPaint::Candidate'); + lcp.args.data.navigationId = '1'; return { GatherContext: {gatherMode: 'navigation'}, From 105636e209bd009bb0efa0af345ae82648ca40ae Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 6 Jun 2024 17:23:03 -0700 Subject: [PATCH 5/9] grab latest main frame nav with scores --- core/lib/lantern/lantern.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/core/lib/lantern/lantern.js b/core/lib/lantern/lantern.js index 183f7633ba7c..3018122f4ab9 100644 --- a/core/lib/lantern/lantern.js +++ b/core/lib/lantern/lantern.js @@ -38,9 +38,24 @@ const NetworkRequestTypes = { function createProcessedNavigation(traceEngineResult) { const Meta = traceEngineResult.data.Meta; const frameId = Meta.mainFrameId; - const navigationId = Meta.mainFrameNavigations[0].args.data?.navigationId || ''; - const scores = - traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigationId); + const scoresByNav = traceEngineResult.data.PageLoadMetrics.metricScoresByFrameId.get(frameId); + if (!scoresByNav) { + throw new Error('missing metric scores for main frame'); + } + + // Grab the latest navigation with scores. + let scores; + for (const navigation of Meta.mainFrameNavigations.reverse()) { + const navigationId = navigation.args.data?.navigationId; + if (!navigationId) continue; + + scores = scoresByNav.get(navigationId); + if (scores) break; + } + if (!scores) { + throw new Error('no metric scores found for main frame navigation'); + } + /** @param {MetricName} metric */ const getTimestampOrUndefined = metric => { const metricScore = scores?.get(metric); From 035a521916bcc5a389a4b9a6356a2dd7a6a6c556 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 6 Jun 2024 19:02:26 -0700 Subject: [PATCH 6/9] typescript --- core/lib/lantern/lantern.js | 1 + 1 file changed, 1 insertion(+) diff --git a/core/lib/lantern/lantern.js b/core/lib/lantern/lantern.js index 3018122f4ab9..f2535058b3c0 100644 --- a/core/lib/lantern/lantern.js +++ b/core/lib/lantern/lantern.js @@ -44,6 +44,7 @@ function createProcessedNavigation(traceEngineResult) { } // Grab the latest navigation with scores. + /** @type {Map=} */ let scores; for (const navigation of Meta.mainFrameNavigations.reverse()) { const navigationId = navigation.args.data?.navigationId; From 4f2d74ab0a54df0ce7e61ad74273b4a59c7f8948 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 7 Jun 2024 10:45:20 -0700 Subject: [PATCH 7/9] rm fmp from type --- core/lib/lantern/types/lantern.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/core/lib/lantern/types/lantern.d.ts b/core/lib/lantern/types/lantern.d.ts index 8658edc75a88..f2279891e838 100644 --- a/core/lib/lantern/types/lantern.d.ts +++ b/core/lib/lantern/types/lantern.d.ts @@ -173,7 +173,6 @@ export namespace Simulation { timestamps: { firstContentfulPaint: number; largestContentfulPaint?: number; - firstMeaningfulPaint?: number; }; } From 5fba3ae1b891f1640af012f67a55e182a0729b2e Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 7 Jun 2024 11:31:19 -0700 Subject: [PATCH 8/9] fix busted synthetic test trace redirects --- core/lib/lantern/lantern.js | 14 +++----------- core/test/audits/redirects-test.js | 31 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/core/lib/lantern/lantern.js b/core/lib/lantern/lantern.js index f2535058b3c0..f4694e598271 100644 --- a/core/lib/lantern/lantern.js +++ b/core/lib/lantern/lantern.js @@ -43,18 +43,10 @@ function createProcessedNavigation(traceEngineResult) { throw new Error('missing metric scores for main frame'); } - // Grab the latest navigation with scores. - /** @type {Map=} */ - let scores; - for (const navigation of Meta.mainFrameNavigations.reverse()) { - const navigationId = navigation.args.data?.navigationId; - if (!navigationId) continue; - - scores = scoresByNav.get(navigationId); - if (scores) break; - } + const lastNavigationId = Meta.mainFrameNavigations.at(-1)?.args.data?.navigationId ?? ''; + const scores = scoresByNav.get(lastNavigationId); if (!scores) { - throw new Error('no metric scores found for main frame navigation'); + throw new Error('missing metric scores for specified navigation'); } /** @param {MetricName} metric */ diff --git a/core/test/audits/redirects-test.js b/core/test/audits/redirects-test.js index 8db1c8865c01..7c33488c8c5c 100644 --- a/core/test/audits/redirects-test.js +++ b/core/test/audits/redirects-test.js @@ -160,6 +160,9 @@ describe('Performance: Redirects audit', () => { const traceEvents = artifacts.traces.defaultPass.traceEvents; const navStart = traceEvents.find(e => e.name === 'navigationStart'); + const fcp = traceEvents.find(e => e.name === 'firstContentfulPaint'); + const lcp = traceEvents.find(e => e.name === 'largestContentfulPaint::Candidate'); + const secondNavStart = JSON.parse(JSON.stringify(navStart)); traceEvents.push(secondNavStart); navStart.args.data.isLoadingMainFrame = true; @@ -169,6 +172,16 @@ describe('Performance: Redirects audit', () => { secondNavStart.args.data.documentLoaderURL = 'https://www.lisairish.com/'; secondNavStart.args.data.navigationId = '2'; + const secondFcp = JSON.parse(JSON.stringify(fcp)); + traceEvents.push(secondFcp); + secondFcp.args.data.navigationId = '2'; + secondFcp.ts += 2; + + const secondLcp = JSON.parse(JSON.stringify(lcp)); + traceEvents.push(secondLcp); + secondLcp.args.data.navigationId = '2'; + secondFcp.ts += 2; + const output = await RedirectsAudit.audit(artifacts, context); expect(output.details.items).toHaveLength(3); expect(Math.round(output.score * 100) / 100).toMatchInlineSnapshot(`0`); @@ -255,15 +268,33 @@ describe('Performance: Redirects audit', () => { const traceEvents = artifacts.traces.defaultPass.traceEvents; const navStart = traceEvents.find(e => e.name === 'navigationStart'); + const fcp = traceEvents.find(e => e.name === 'firstContentfulPaint'); + const lcp = traceEvents.find(e => e.name === 'largestContentfulPaint::Candidate'); const secondNavStart = JSON.parse(JSON.stringify(navStart)); traceEvents.push(secondNavStart); secondNavStart.args.data.navigationId = '2'; + const secondFcp = JSON.parse(JSON.stringify(fcp)); + traceEvents.push(secondFcp); + secondFcp.args.data.navigationId = '2'; + + const secondLcp = JSON.parse(JSON.stringify(lcp)); + traceEvents.push(secondLcp); + secondLcp.args.data.navigationId = '2'; + const thirdNavStart = JSON.parse(JSON.stringify(navStart)); traceEvents.push(thirdNavStart); thirdNavStart.args.data.navigationId = '3'; + const thirdFcp = JSON.parse(JSON.stringify(fcp)); + traceEvents.push(thirdFcp); + thirdFcp.args.data.navigationId = '3'; + + const thirdLcp = JSON.parse(JSON.stringify(lcp)); + traceEvents.push(thirdLcp); + thirdLcp.args.data.navigationId = '3'; + const output = await RedirectsAudit.audit(artifacts, context); expect(output).toMatchObject({ score: 0, From d55da597540d32f7393f827000ce2b73aa304830 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 7 Jun 2024 12:34:59 -0700 Subject: [PATCH 9/9] minor ts improvement --- core/lib/lantern/lantern.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/lib/lantern/lantern.js b/core/lib/lantern/lantern.js index f4694e598271..c4004be6324a 100644 --- a/core/lib/lantern/lantern.js +++ b/core/lib/lantern/lantern.js @@ -43,21 +43,21 @@ function createProcessedNavigation(traceEngineResult) { throw new Error('missing metric scores for main frame'); } - const lastNavigationId = Meta.mainFrameNavigations.at(-1)?.args.data?.navigationId ?? ''; - const scores = scoresByNav.get(lastNavigationId); + const lastNavigationId = Meta.mainFrameNavigations.at(-1)?.args.data?.navigationId; + const scores = lastNavigationId && scoresByNav.get(lastNavigationId); if (!scores) { throw new Error('missing metric scores for specified navigation'); } /** @param {MetricName} metric */ const getTimestampOrUndefined = metric => { - const metricScore = scores?.get(metric); + const metricScore = scores.get(metric); if (!metricScore?.event) return; return metricScore.event.ts; }; /** @param {MetricName} metric */ const getTimestamp = metric => { - const metricScore = scores?.get(metric); + const metricScore = scores.get(metric); if (!metricScore?.event) throw new Error(`missing metric: ${metric}`); return metricScore.event.ts; };