diff --git a/core/computed/metrics/cumulative-layout-shift.js b/core/computed/metrics/cumulative-layout-shift.js index 48e9d6cfbbe8..122417a39e61 100644 --- a/core/computed/metrics/cumulative-layout-shift.js +++ b/core/computed/metrics/cumulative-layout-shift.js @@ -7,7 +7,7 @@ import {makeComputedArtifact} from '../computed-artifact.js'; import {ProcessedTrace} from '../processed-trace.js'; -/** @typedef {{ts: number, isMainFrame: boolean, weightedScore: number}} LayoutShiftEvent */ +/** @typedef {{ts: number, isMainFrame: boolean, weightedScore: number, impactedNodes?: LH.Artifacts.TraceImpactedNode[]}} LayoutShiftEvent */ const RECENT_INPUT_WINDOW = 500; @@ -65,6 +65,7 @@ class CumulativeLayoutShift { ts: event.ts, isMainFrame: event.args.data.is_main_frame, weightedScore: event.args.data.weighted_score_delta, + impactedNodes: event.args.data.impacted_nodes, }); } diff --git a/core/gather/gatherers/trace-elements.js b/core/gather/gatherers/trace-elements.js index df1833ba3134..884d4cf8b866 100644 --- a/core/gather/gatherers/trace-elements.js +++ b/core/gather/gatherers/trace-elements.js @@ -22,6 +22,7 @@ import {ProcessedTrace} from '../../computed/processed-trace.js'; import {ProcessedNavigation} from '../../computed/processed-navigation.js'; import {LighthouseError} from '../../lib/lh-error.js'; import {Responsiveness} from '../../computed/metrics/responsiveness.js'; +import {CumulativeLayoutShift} from '../../computed/metrics/cumulative-layout-shift.js'; /** @typedef {{nodeId: number, score?: number, animations?: {name?: string, failureReasonsMask?: number, unsupportedProperties?: string[]}[], type?: string}} TraceElementData */ @@ -80,34 +81,24 @@ class TraceElements extends FRGatherer { * We calculate the score per element by taking the 'score' of each layout shift event and * distributing it between all the nodes that were shifted, proportianal to the impact region of * each shifted element. - * @param {Array} mainThreadEvents + * @param {LH.Artifacts.ProcessedTrace} processedTrace * @return {Array} */ - static getTopLayoutShiftElements(mainThreadEvents) { + static getTopLayoutShiftElements(processedTrace) { /** @type {Map} */ const clsPerNode = new Map(); - const shiftEvents = mainThreadEvents - .filter(e => e.name === 'LayoutShift') - .map(e => e.args?.data); - const indexFirstEventWithoutInput = - shiftEvents.findIndex(event => event && !event.had_recent_input); - - shiftEvents.forEach((event, index) => { - if (!event || !event.impacted_nodes || !event.score) { - return; - } + const shiftEvents = CumulativeLayoutShift.getLayoutShiftEvents(processedTrace); - // Ignore events with input, unless it's one of the initial events. - // See comment in computed/metrics/cumulative-layout-shift.js. - if (indexFirstEventWithoutInput !== -1 && index >= indexFirstEventWithoutInput) { - if (event.had_recent_input) return; + shiftEvents.forEach((event) => { + if (!event || !event.impactedNodes) { + return; } let totalAreaOfImpact = 0; /** @type {Map} */ const pixelsMovedPerNode = new Map(); - event.impacted_nodes.forEach(node => { + event.impactedNodes.forEach(node => { if (!node.node_id || !node.old_rect || !node.new_rect) { return; } @@ -124,7 +115,7 @@ class TraceElements extends FRGatherer { for (const [nodeId, pixelsMoved] of pixelsMovedPerNode.entries()) { let clsContribution = clsPerNode.get(nodeId) || 0; - clsContribution += (pixelsMoved / totalAreaOfImpact) * event.score; + clsContribution += (pixelsMoved / totalAreaOfImpact) * event.weightedScore; clsPerNode.set(nodeId, clsContribution); } }); @@ -272,7 +263,7 @@ class TraceElements extends FRGatherer { const {mainThreadEvents} = processedTrace; const lcpNodeData = await TraceElements.getLcpElement(trace, context); - const clsNodeData = TraceElements.getTopLayoutShiftElements(mainThreadEvents); + const clsNodeData = TraceElements.getTopLayoutShiftElements(processedTrace); const animatedElementData = await this.getAnimatedElements(mainThreadEvents); const responsivenessElementData = await TraceElements.getResponsivenessElement(trace, context); diff --git a/core/test/gather/gatherers/trace-elements-test.js b/core/test/gather/gatherers/trace-elements-test.js index 475d643e2018..be0ae1418613 100644 --- a/core/test/gather/gatherers/trace-elements-test.js +++ b/core/test/gather/gatherers/trace-elements-test.js @@ -10,6 +10,7 @@ import {Connection} from '../../../legacy/gather/connections/connection.js'; import {createTestTrace, rootFrame} from '../../create-test-trace.js'; import {createMockSendCommandFn, createMockOnFn} from '../mock-commands.js'; import {flushAllTimersAndMicrotasks, fnAny, readJson, timers} from '../../test-utils.js'; +import {ProcessedTrace} from '../../../computed/processed-trace.js'; const animationTrace = readJson('../../fixtures/artifacts/animation/trace.json', import.meta); @@ -23,11 +24,13 @@ function makeLayoutShiftTraceEvent(score, impactedNodes, had_recent_input = fals ts: 1200, args: { data: { + is_main_frame: true, had_recent_input, // eslint-disable-line camelcase impacted_nodes: impactedNodes, score: score, + weighted_score_delta: score, }, - frame: '3C4CBF06AF1ED5B9EAA59BECA70111F4', + frame: 'ROOT_FRAME', }, }; } @@ -87,8 +90,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { expect(diff).toBeLessThanOrEqual(Number.EPSILON); } - it('returns layout shift data sorted by impact area', () => { - const traceEvents = [ + it('returns layout shift data sorted by impact area', async () => { + const trace = createTestTrace({}); + trace.traceEvents.push( makeLayoutShiftTraceEvent(1, [ { new_rect: [0, 0, 200, 200], @@ -100,10 +104,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { node_id: 25, old_rect: [0, 100, 200, 100], }, - ]), - ]; + ]) + ); + const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()}); - const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents); + const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace); expect(result).toEqual([ {nodeId: 25, score: 0.6}, {nodeId: 60, score: 0.4}, @@ -112,8 +117,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { expectEqualFloat(total, 1.0); }); - it('does not ignore initial trace events with input', () => { - const traceEvents = [ + it('does not ignore initial trace events with input', async () => { + const trace = createTestTrace({}); + trace.traceEvents.push( makeLayoutShiftTraceEvent(1, [ { new_rect: [0, 0, 200, 200], @@ -127,18 +133,20 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { node_id: 2, old_rect: [0, 0, 200, 100], }, - ], true), - ]; + ], true) + ); + const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()}); - const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents); + const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace); expect(result).toEqual([ {nodeId: 1, score: 1}, {nodeId: 2, score: 1}, ]); }); - it('does ignore later trace events with input', () => { - const traceEvents = [ + it('does ignore later trace events with input', async () => { + const trace = createTestTrace({}); + trace.traceEvents.push( makeLayoutShiftTraceEvent(1, [ { new_rect: [0, 0, 200, 200], @@ -152,17 +160,19 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { node_id: 2, old_rect: [0, 0, 200, 100], }, - ], true), - ]; + ], true) + ); + const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()}); - const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents); + const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace); expect(result).toEqual([ {nodeId: 1, score: 1}, ]); }); - it('correctly ignores trace events with input (complex)', () => { - const traceEvents = [ + it('correctly ignores trace events with input (complex)', async () => { + const trace = createTestTrace({}); + trace.traceEvents.push( makeLayoutShiftTraceEvent(1, [ { new_rect: [0, 0, 200, 200], @@ -211,10 +221,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { node_id: 7, old_rect: [0, 0, 200, 100], }, - ]), - ]; + ]) + ); + const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()}); - const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents); + const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace); expect(result).toEqual([ {nodeId: 1, score: 1}, {nodeId: 2, score: 1}, @@ -224,8 +235,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { ]); }); - it('combines scores for the same nodeId accross multiple shift events', () => { - const traceEvents = [ + it('combines scores for the same nodeId accross multiple shift events', async () => { + const trace = createTestTrace({}); + trace.traceEvents.push( makeLayoutShiftTraceEvent(1, [ { new_rect: [0, 0, 200, 200], @@ -244,10 +256,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { node_id: 60, old_rect: [0, 0, 200, 200], }, - ]), - ]; + ]) + ); + const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()}); - const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents); + const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace); expect(result).toEqual([ {nodeId: 60, score: 0.7}, {nodeId: 25, score: 0.6}, @@ -256,8 +269,9 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { expectEqualFloat(total, 1.3); }); - it('returns only the top five values', () => { - const traceEvents = [ + it('returns only the top five values', async () => { + const trace = createTestTrace({}); + trace.traceEvents.push( makeLayoutShiftTraceEvent(1, [ { new_rect: [0, 100, 100, 100], @@ -298,10 +312,11 @@ describe('Trace Elements gatherer - GetTopLayoutShiftElements', () => { node_id: 7, old_rect: [0, 0, 100, 100], }, - ]), - ]; + ]) + ); + const processedTrace = await ProcessedTrace.request(trace, {computedCache: new Map()}); - const result = TraceElementsGatherer.getTopLayoutShiftElements(traceEvents); + const result = TraceElementsGatherer.getTopLayoutShiftElements(processedTrace); expect(result).toEqual([ {nodeId: 3, score: 1.0}, {nodeId: 1, score: 0.5}, diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 5119ab1c54c1..d240bf4d6ab4 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -948,6 +948,12 @@ declare module Artifacts { // Convenience methods. isFirstParty: (url: string) => boolean; } + + interface TraceImpactedNode { + node_id: number; + old_rect?: Array; + new_rect?: Array; + } } export interface Trace { @@ -1017,11 +1023,7 @@ export interface TraceEvent { nodeId?: number; DOMNodeId?: number; imageUrl?: string; - impacted_nodes?: Array<{ - node_id: number, - old_rect?: Array, - new_rect?: Array, - }>; + impacted_nodes?: Artifacts.TraceImpactedNode[]; score?: number; weighted_score_delta?: number; had_recent_input?: boolean;