From 324c86a306e32dc96709c7dc2f0fa84209b694ef Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Mon, 22 Apr 2024 15:22:39 +0100 Subject: [PATCH] RPP: extend backendNodeId gathering This CL adds the ability for the new engine to gather backendNodeIds from Paint, PaintImage and ScrollLayer events. It also updates the nodeId type on Paint to be optional, as per the comment in TimelineModel (which was landed in 2021), making it clear that we might not always have nodeIds in Paint events. Bug: 336237398 Change-Id: I0f76c28337fd189695a51cf6e2d2b8eca9a3953e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5472243 Commit-Queue: Jack Franklin Auto-Submit: Jack Franklin Reviewed-by: Andres Olivares --- .../models/timeline_model/TimelineModel.ts | 7 ---- .../models/trace/extras/FetchNodes.test.ts | 28 +++++++++++++++ front_end/models/trace/extras/FetchNodes.ts | 7 ++++ front_end/models/trace/types/TraceEvents.ts | 34 ++++++++++++++++++- 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/front_end/models/timeline_model/TimelineModel.ts b/front_end/models/timeline_model/TimelineModel.ts index 731fa625e1..610f462c9f 100644 --- a/front_end/models/timeline_model/TimelineModel.ts +++ b/front_end/models/timeline_model/TimelineModel.ts @@ -546,8 +546,6 @@ export class TimelineModelImpl { } case RecordType.Paint: { - // With CompositeAfterPaint enabled, paint events are no longer - // associated with a Node, and nodeId will not be present. if ('nodeId' in eventData) { timelineData.backendNodeIds.push(eventData['nodeId']); } @@ -560,11 +558,6 @@ export class TimelineModelImpl { break; } - case RecordType.ScrollLayer: { - timelineData.backendNodeIds.push(eventData['nodeId']); - break; - } - case RecordType.PaintImage: { timelineData.backendNodeIds.push(eventData['nodeId']); timelineData.url = eventData['url']; diff --git a/front_end/models/trace/extras/FetchNodes.test.ts b/front_end/models/trace/extras/FetchNodes.test.ts index d38a0217ad..549658d16d 100644 --- a/front_end/models/trace/extras/FetchNodes.test.ts +++ b/front_end/models/trace/extras/FetchNodes.test.ts @@ -135,6 +135,34 @@ describeWithMockConnection('FetchNodes', function() { 188, ]); }); + + it('identifies node ids for a Paint event', async function() { + const traceData = await TraceLoader.traceEngine(this, 'web-dev-initial-url.json.gz'); + const paintEvent = traceData.Renderer.allTraceEntries.find(TraceEngine.Types.TraceEvents.isTraceEventPaint); + assert.isOk(paintEvent); + const nodeIds = TraceEngine.Extras.FetchNodes.nodeIdsForEvent(traceData, paintEvent); + assert.deepEqual(Array.from(nodeIds), [75]); + }); + + it('identifies node ids for a PaintImage event', async function() { + const traceData = await TraceLoader.traceEngine(this, 'web-dev-initial-url.json.gz'); + const paintImageEvent = + traceData.Renderer.allTraceEntries.find(TraceEngine.Types.TraceEvents.isTraceEventPaintImage); + assert.isOk(paintImageEvent); + const nodeIds = TraceEngine.Extras.FetchNodes.nodeIdsForEvent(traceData, paintImageEvent); + assert.deepEqual(Array.from(nodeIds), [107]); + }); + + it('identifies node ids for a ScrollLayer event', async function() { + // This trace chosen as it happens to have ScrollLayer events, unlike the + // web-dev traces used in tests above. + const traceData = await TraceLoader.traceEngine(this, 'extension-tracks-and-marks.json.gz'); + const scrollLayerEvent = + traceData.Renderer.allTraceEntries.find(TraceEngine.Types.TraceEvents.isTraceEventScrollLayer); + assert.isOk(scrollLayerEvent); + const nodeIds = TraceEngine.Extras.FetchNodes.nodeIdsForEvent(traceData, scrollLayerEvent); + assert.deepEqual(Array.from(nodeIds), [4]); + }); }); describe('LayoutShifts', () => { diff --git a/front_end/models/trace/extras/FetchNodes.ts b/front_end/models/trace/extras/FetchNodes.ts index b9d5ac087c..61d32d3906 100644 --- a/front_end/models/trace/extras/FetchNodes.ts +++ b/front_end/models/trace/extras/FetchNodes.ts @@ -73,7 +73,14 @@ export function nodeIdsForEvent( event.args.endData.layoutRoots.forEach(root => foundIds.add(root.nodeId)); } else if (Types.TraceEvents.isSyntheticLayoutShift(event) && event.args.data?.impacted_nodes) { event.args.data.impacted_nodes.forEach(node => foundIds.add(node.node_id)); + } else if (Types.TraceEvents.isTraceEventPaint(event) && typeof event.args.data.nodeId !== 'undefined') { + foundIds.add(event.args.data.nodeId); + } else if (Types.TraceEvents.isTraceEventPaintImage(event) && typeof event.args.data.nodeId !== 'undefined') { + foundIds.add(event.args.data.nodeId); + } else if (Types.TraceEvents.isTraceEventScrollLayer(event) && typeof event.args.data.nodeId !== 'undefined') { + foundIds.add(event.args.data.nodeId); } + nodeIdsForEventCache.set(event, foundIds); return foundIds; } diff --git a/front_end/models/trace/types/TraceEvents.ts b/front_end/models/trace/types/TraceEvents.ts index 252c8f4a43..d2aa09917c 100644 --- a/front_end/models/trace/types/TraceEvents.ts +++ b/front_end/models/trace/types/TraceEvents.ts @@ -1867,7 +1867,9 @@ export interface TraceEventPaint extends TraceEventComplete { clip: number[], frame: string, layerId: number, - nodeId: number, + // With CompositeAfterPaint enabled, paint events are no longer + // associated with a Node, and nodeId will not be present. + nodeId?: Protocol.DOM.BackendNodeId, }, }; } @@ -1876,6 +1878,36 @@ export function isTraceEventPaint(event: TraceEventData): event is TraceEventPai return event.name === KnownEventName.Paint; } +export interface TraceEventPaintImage extends TraceEventComplete { + name: KnownEventName.PaintImage; + args: TraceEventArgs&{ + data: TraceEventData & { + height: number, + width: number, + x: number, + y: number, + url?: string, srcHeight: number, srcWidth: number, + nodeId?: Protocol.DOM.BackendNodeId, + }, + }; +} +export function isTraceEventPaintImage(event: TraceEventData): event is TraceEventPaintImage { + return event.name === KnownEventName.PaintImage; +} + +export interface TraceEventScrollLayer extends TraceEventComplete { + name: KnownEventName.ScrollLayer; + args: TraceEventArgs&{ + data: TraceEventData & { + frame: string, + nodeId?: Protocol.DOM.BackendNodeId, + }, + }; +} +export function isTraceEventScrollLayer(event: TraceEventData): event is TraceEventScrollLayer { + return event.name === KnownEventName.ScrollLayer; +} + export interface TraceEventSetLayerTreeId extends TraceEventInstant { name: KnownEventName.SetLayerTreeId; args: TraceEventArgs&{