diff --git a/front_end/models/trace/root-causes/LayoutShift.test.ts b/front_end/models/trace/root-causes/LayoutShift.test.ts index 93344068bf6..eafc40b10cb 100644 --- a/front_end/models/trace/root-causes/LayoutShift.test.ts +++ b/front_end/models/trace/root-causes/LayoutShift.test.ts @@ -9,6 +9,8 @@ import { import {getBaseTraceParseModelData} from '../../../testing/TraceHelpers.js'; import * as TraceEngine from '../trace.js'; +import * as RootCauses from './RootCauses.js'; + type TraceParseData = TraceEngine.Handlers.Types.TraceParseData; type TraceParseDataMutable = TraceEngine.Handlers.Types.TraceParseDataMutable; @@ -47,7 +49,7 @@ describeWithMockConnection('LayoutShift root causes', () => { * required. */ describe('assigns root causes to layout shifts', () => { - let rootCausesEngine: TraceEngine.RootCauses.RootCauses.RootCauses; + let layoutShifts: RootCauses.LayoutShiftRootCauses; let prePaintEvents: TraceEngine.Types.TraceEvents.TraceEventPrePaint[]; let resizeEvents: TraceEngine.Types.TraceEvents.TraceEventLayoutInvalidationTracking[]; let injectedIframeEvents: TraceEngine.Types.TraceEvents.TraceEventLayoutInvalidationTracking[]; @@ -60,6 +62,7 @@ describeWithMockConnection('LayoutShift root causes', () => { let iframesNodeIds: number[]; let shifts: TraceEngine.Types.TraceEvents.SyntheticLayoutShift[]; let matchedStylesMock: Omit; + let protocolInterface: RootCauses.RootCauseProtocolInterface; let computedStylesMock: Protocol.CSS.CSSComputedStyleProperty[]; let fontFaceMock: Protocol.CSS.FontFace; const fontSource = 'mock-source.woff'; @@ -109,6 +112,7 @@ describeWithMockConnection('LayoutShift root causes', () => { data: { nodeId: i + 1 as Protocol.DOM.BackendNodeId, reason: TraceEngine.Types.TraceEvents.LayoutInvalidationReason.SIZE_CHANGED, + nodeName: 'IMG', frame: 'frame-id-123', }, }; @@ -118,6 +122,7 @@ describeWithMockConnection('LayoutShift root causes', () => { data: { nodeId: i + 11 as Protocol.DOM.BackendNodeId, reason: TraceEngine.Types.TraceEvents.LayoutInvalidationReason.ADDED_TO_LAYOUT, + nodeName: 'IFRAME', frame: 'frame-id-123', }, }; @@ -127,6 +132,7 @@ describeWithMockConnection('LayoutShift root causes', () => { data: { nodeId: i + 21 as Protocol.DOM.BackendNodeId, reason: TraceEngine.Types.TraceEvents.LayoutInvalidationReason.FONTS_CHANGED, + nodeName: 'DIV', frame: 'frame-id-123', }, }; @@ -136,6 +142,7 @@ describeWithMockConnection('LayoutShift root causes', () => { data: { nodeId: i + 31 as Protocol.DOM.BackendNodeId, reason: TraceEngine.Types.TraceEvents.LayoutInvalidationReason.UNKNOWN, + nodeName: 'DIV', frame: 'frame-id-123', }, }; @@ -158,14 +165,12 @@ describeWithMockConnection('LayoutShift root causes', () => { for (let i = 0 as Protocol.DOM.BackendNodeId; i < layoutInvalidationEvents.length; i++) { const backendNodeId = layoutInvalidationEvents[i].args.data.nodeId; const nodeId = i as unknown as Protocol.DOM.NodeId; + const nodeName = layoutInvalidationEvents[i].args.data.nodeName || 'DIV'; const fakeNode = { backendNodeId, nodeId, - localName: 'img', - nodeName: layoutInvalidationEvents[i].args.data.reason === - TraceEngine.Types.TraceEvents.LayoutInvalidationReason.ADDED_TO_LAYOUT ? - 'IFRAME' : - 'IMG', + localName: nodeName.toLowerCase(), + nodeName, attributes: [], nodeType: Node.ELEMENT_NODE, } as unknown as Protocol.DOM.Node; @@ -196,7 +201,7 @@ describeWithMockConnection('LayoutShift root causes', () => { computedStylesMock = []; matchedStylesMock = {}; - rootCausesEngine = new TraceEngine.RootCauses.RootCauses.RootCauses({ + protocolInterface = { getInitiatorForRequest(_: string): Protocol.Network.Initiator | null { return null; @@ -234,14 +239,32 @@ describeWithMockConnection('LayoutShift root causes', () => { } return; }, + }; + + layoutShifts = new RootCauses.LayoutShiftRootCauses(protocolInterface, {enableIframeRootCauses: true}); + }); + + it('uses cached node details', async () => { + // Use duplicate node ids for invalidation events that use `getNode` + resizeEvents.forEach(e => { + e.args.data.nodeId = 1 as Protocol.DOM.BackendNodeId; }); + injectedIframeEvents.forEach(e => { + e.args.data.nodeId = 11 as Protocol.DOM.BackendNodeId; + }); + + const getNodeSpy = sinon.spy(protocolInterface, 'getNode'); + + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); + assertArrayHasNoNulls(rootCauses); + + assert.strictEqual(getNodeSpy.callCount, 2); }); describe('Unsized media', () => { it('marks unsized media node in LayoutInvalidation events as a potential root cause to layout shifts correctly', async () => { - const rootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); assertArrayHasNoNulls(rootCauses); const shiftCausesNodeIds = rootCauses.map(cause => { @@ -272,7 +295,7 @@ describeWithMockConnection('LayoutShift root causes', () => { inlineStyle: createMockStyle([{name: 'height', value: '20px'}]), matchedCSSRules: createMockMatchedRules([{name: 'height', value: '10px'}]), }; - const rootCause = await rootCausesEngine.layoutShifts.rootCausesForEvent(model, shifts[0]); + const rootCause = await layoutShifts.rootCausesForEvent(model, shifts[0]); const authoredDimensions = rootCause?.unsizedMedia[0].authoredDimensions; if (!authoredDimensions) { @@ -293,7 +316,7 @@ describeWithMockConnection('LayoutShift root causes', () => { matchedCSSRules: createMockMatchedRules([{name: 'height', value: '30px'}]), }; - const rootCause = await rootCausesEngine.layoutShifts.rootCausesForEvent(model, shifts[1]); + const rootCause = await layoutShifts.rootCausesForEvent(model, shifts[1]); const authoredDimensions = rootCause?.unsizedMedia[0].authoredDimensions; if (!authoredDimensions) { assert.fail('Expected defined authored dimensions'); @@ -311,7 +334,7 @@ describeWithMockConnection('LayoutShift root causes', () => { {name: 'width', value: width}, ]; - const rootCause = await rootCausesEngine.layoutShifts.rootCausesForEvent(model, shifts[1]); + const rootCause = await layoutShifts.rootCausesForEvent(model, shifts[1]); const computedDimensions = rootCause?.unsizedMedia[0].computedDimensions; if (!computedDimensions) { assert.fail('Expected defined computed dimensions'); @@ -324,7 +347,7 @@ describeWithMockConnection('LayoutShift root causes', () => { async function assertAmountOfBlamedLayoutInvalidations(amount: number) { const allShiftsRootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); const nodesFromLayoutInvalidations = new Set(); for (const currentShiftRootCauses of allShiftsRootCauses) { @@ -405,7 +428,7 @@ describeWithMockConnection('LayoutShift root causes', () => { modelMut.LayoutShifts.clusters = clusters; assert.doesNotThrow(async () => { - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); }); }); }); @@ -413,8 +436,7 @@ describeWithMockConnection('LayoutShift root causes', () => { describe('Injected iframes', () => { it('marks injected iframes in LayoutInvalidation events as a potential root cause to layout shifts correctly', async () => { - const rootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); assertArrayHasNoNulls(rootCauses); const shiftCausesNodeIds = rootCauses.map(cause => { return cause.iframes.map(node => Number(node.iframe.backendNodeId)); @@ -427,6 +449,24 @@ describeWithMockConnection('LayoutShift root causes', () => { assert.strictEqual(shiftCausesNodeIds[4].length, 1); assert.strictEqual(shiftCausesNodeIds[4][0], iframesNodeIds[1]); }); + + it('ignores injected iframes if disabled', async () => { + layoutShifts = new RootCauses.LayoutShiftRootCauses(protocolInterface, {enableIframeRootCauses: false}); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); + assertArrayHasNoNulls(rootCauses); + assert(rootCauses.every(cause => cause.iframes.length === 0), 'contained iframe root causes'); + }); + + it('ignores events that could not add or resize an iframe', async () => { + injectedIframeEvents.forEach(e => { + e.args.data.nodeName = 'DIV'; + e.args.data.reason = TraceEngine.Types.TraceEvents.LayoutInvalidationReason.SIZE_CHANGED; + }); + + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); + assertArrayHasNoNulls(rootCauses); + assert(rootCauses.every(cause => cause.iframes.length === 0), 'contained iframe root causes'); + }); }); describe('Font changes', () => { @@ -459,8 +499,7 @@ describeWithMockConnection('LayoutShift root causes', () => { async () => { modelMut.NetworkRequests.byTime = fontRequests; - const rootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); assertArrayHasNoNulls(rootCauses); const shiftCausesNodeIds = rootCauses.map(cause => { return cause.fontChanges; @@ -489,8 +528,7 @@ describeWithMockConnection('LayoutShift root causes', () => { }] as unknown as TraceEngine.Types.TraceEvents.SyntheticNetworkRequest[]; modelMut.NetworkRequests.byTime = optionalFontRequests; fontFaceMock = {fontFamily: 'Roboto', src: fontSource, fontDisplay: 'optional'} as Protocol.CSS.FontFace; - const rootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); assertArrayHasNoNulls(rootCauses); const shiftCausesNodeIds = rootCauses.map(cause => { return cause.fontChanges; @@ -516,8 +554,7 @@ describeWithMockConnection('LayoutShift root causes', () => { }] as unknown as TraceEngine.Types.TraceEvents.SyntheticNetworkRequest[]; modelMut.NetworkRequests.byTime = optionalFontRequests; fontFaceMock = {fontFamily: 'Roboto', src: fontSource, fontDisplay: 'swap'} as Protocol.CSS.FontFace; - const rootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); assertArrayHasNoNulls(rootCauses); const shiftCausesNodeIds = rootCauses.map(cause => { return cause.fontChanges; @@ -561,8 +598,7 @@ describeWithMockConnection('LayoutShift root causes', () => { async () => { modelMut.NetworkRequests.byTime = RenderBlockingRequest; - const rootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); assertArrayHasNoNulls(rootCauses); const shiftCausesNodeIds = rootCauses.map(cause => { return cause.renderBlockingRequests; @@ -617,8 +653,7 @@ describeWithMockConnection('LayoutShift root causes', () => { } as TraceEngine.Types.TraceEvents.TraceEventData); // Verify the Layout initiator's stack trace is added to the last shift. - const rootCauses = - await Promise.all(shifts.map(shift => rootCausesEngine.layoutShifts.rootCausesForEvent(model, shift))); + const rootCauses = await Promise.all(shifts.map(shift => layoutShifts.rootCausesForEvent(model, shift))); assertArrayHasNoNulls(rootCauses); const rootCauseStackTraces = rootCauses.map(cause => { return cause.scriptStackTrace; diff --git a/front_end/models/trace/root-causes/LayoutShift.ts b/front_end/models/trace/root-causes/LayoutShift.ts index e740d030d02..3e8f1b192af 100644 --- a/front_end/models/trace/root-causes/LayoutShift.ts +++ b/front_end/models/trace/root-causes/LayoutShift.ts @@ -84,13 +84,20 @@ function networkRequestIsRenderBlockingInFrame( return isRenderBlocking && event.args.data.frame === frameId; } +interface Options { + /** Checking iframe root causes can be an expensive operation, so it is disabled by default. */ + enableIframeRootCauses?: boolean; +} + export class LayoutShiftRootCauses { #protocolInterface: RootCauseProtocolInterface; #rootCauseCacheMap = new Map(); #nodeDetailsCache = new Map(); + #iframeRootCausesEnabled: boolean; - constructor(protocolInterface: RootCauseProtocolInterface) { + constructor(protocolInterface: RootCauseProtocolInterface, options?: Options) { this.#protocolInterface = protocolInterface; + this.#iframeRootCausesEnabled = options?.enableIframeRootCauses ?? false; } /** @@ -323,7 +330,11 @@ export class LayoutShiftRootCauses { async getIframeRootCause( layoutInvalidation: Types.TraceEvents.TraceEventLayoutInvalidationTracking, layoutInvalidationNodeId: Protocol.DOM.NodeId): Promise { - if (layoutInvalidation.args.data.nodeName?.startsWith('IFRAME') && + if (!this.#iframeRootCausesEnabled) { + return null; + } + + if (!layoutInvalidation.args.data.nodeName?.startsWith('IFRAME') && layoutInvalidation.args.data.reason !== Types.TraceEvents.LayoutInvalidationReason.STYLE_CHANGED && layoutInvalidation.args.data.reason !== Types.TraceEvents.LayoutInvalidationReason.ADDED_TO_LAYOUT) { return null; diff --git a/front_end/models/trace/root-causes/RootCauses.ts b/front_end/models/trace/root-causes/RootCauses.ts index 1fa0730aa3a..2a6d04c8ca0 100644 --- a/front_end/models/trace/root-causes/RootCauses.ts +++ b/front_end/models/trace/root-causes/RootCauses.ts @@ -22,3 +22,5 @@ export class RootCauses { this.layoutShifts = new LayoutShiftRootCauses(protocolInterface); } } + +export {LayoutShiftRootCauses} from './LayoutShift.js';