From 46313334904f8f121893346f826d08e29ed4eb96 Mon Sep 17 00:00:00 2001 From: fatadel Date: Thu, 13 Aug 2026 18:07:12 +0200 Subject: [PATCH 1/3] Describe CompositorScreenshot markers with a marker schema These markers had no schema, so both the tooltip and the string table lookup for their image special-cased this marker type. Declaring every field is also a prerequisite for storing marker payloads without repeating their keys. The schema uses two new field formats: screenshot-size, whose value is a { width, height } object, and screenshot-data-url, which renders a string table index as an image sized from a sibling screenshot-size field. The payload now has a single windowSize field instead of windowWidth and windowHeight. --- docs-developer/CHANGELOG-formats.md | 6 ++ src/app-logic/constants.ts | 2 +- src/components/timeline/TrackScreenshots.tsx | 24 ++--- src/components/tooltip/Marker.tsx | 91 +++++----------- src/profile-logic/import/chrome.ts | 3 +- src/profile-logic/marker-data.ts | 13 +-- src/profile-logic/marker-schema.ts | 51 +++++++-- src/profile-logic/process-profile.ts | 10 +- .../processed-profile-versioning.ts | 19 ++++ src/profile-logic/tracks.ts | 2 +- src/test/components/TooltipMarker.test.tsx | 7 +- .../__snapshots__/TooltipMarker.test.tsx.snap | 29 ++--- .../fixtures/profiles/processed-profile.ts | 7 +- src/test/fixtures/upgrades/processed-3.json | 40 ++++++- .../__snapshots__/profiler-edit.test.ts.snap | 8 +- .../__snapshots__/profile-view.test.ts.snap | 2 +- src/test/store/receive-profile.test.ts | 6 +- .../__snapshots__/marker-data.test.ts.snap | 6 +- .../__snapshots__/marker-schema.test.ts.snap | 27 +++++ .../profile-conversion.test.ts.snap | 36 +++---- .../profile-upgrading.test.ts.snap | 100 ++++++++++++------ src/test/unit/marker-data.test.ts | 8 +- src/test/unit/marker-schema.test.ts | 32 +++++- src/test/unit/merge-compare.test.ts | 6 +- src/types/markers.ts | 59 ++++++----- 25 files changed, 373 insertions(+), 221 deletions(-) diff --git a/docs-developer/CHANGELOG-formats.md b/docs-developer/CHANGELOG-formats.md index dcbcb8f484..d7f729cb8a 100644 --- a/docs-developer/CHANGELOG-formats.md +++ b/docs-developer/CHANGELOG-formats.md @@ -6,6 +6,12 @@ Note that this is not an exhaustive list. Processed profile format upgraders can ## Processed profile format +### Version 72 + +The `CompositorScreenshot` marker payload's `windowWidth` and `windowHeight` fields were replaced with a single `windowSize` field of the form `{ width, height }`. + +Two marker schema field formats were added to describe these markers: `screenshot-size`, whose value is a `{ width, height }` object, and `screenshot-data-url`, an object format `{ type: "screenshot-data-url", sizeFieldForAspectRatio }` whose value is a string table index holding an image data URL. Profiles containing `CompositorScreenshot` markers don't necessarily carry a schema for them, so the front end supplies one. + ### Version 71 The frame table (`profile.shared.frameTable`) representation changed in such a way that all its columns can now be typed arrays when using [JsonSlabs](https://github.com/mstange/json-slabs/) profiles. diff --git a/src/app-logic/constants.ts b/src/app-logic/constants.ts index e0f67bd2ff..034f51bdc5 100644 --- a/src/app-logic/constants.ts +++ b/src/app-logic/constants.ts @@ -12,7 +12,7 @@ export const GECKO_PROFILE_VERSION = 36; // The current version of the "processed" profile format. // Please don't forget to update the processed profile format changelog in // `docs-developer/CHANGELOG-formats.md`. -export const PROCESSED_PROFILE_VERSION = 71; +export const PROCESSED_PROFILE_VERSION = 72; // The following are the margin sizes for the left and right of the timeline. Independent // components need to share these values. diff --git a/src/components/timeline/TrackScreenshots.tsx b/src/components/timeline/TrackScreenshots.tsx index 4ec906de4e..282a55015d 100644 --- a/src/components/timeline/TrackScreenshots.tsx +++ b/src/components/timeline/TrackScreenshots.tsx @@ -244,19 +244,17 @@ class HoverPreview extends PureComponent { return null; } - if (payload.url === undefined) { + const { url, windowSize } = payload; + if (url === undefined || windowSize === undefined) { return null; } - const { url } = payload; - const maximumHoverSize = isMakingPreviewSelection ? MAXIMUM_HOVER_SIZE_WHEN_SELECTING_RANGE : MAXIMUM_HOVER_SIZE; - // Type guard: payload.url !== undefined means it has windowWidth and windowHeight const { width: hoverWidth, height: hoverHeight } = computeScreenshotSize( - payload as { windowWidth: number; windowHeight: number }, + windowSize, maximumHoverSize ); @@ -354,18 +352,12 @@ class ScreenshotStrip extends PureComponent { // Coerce the payload into a screenshot one. const payload: ScreenshotPayload = screenshots[screenshotIndex] .data as any; - if (payload.url === undefined) { + const { url: urlStringIndex, windowSize } = payload; + if (urlStringIndex === undefined || windowSize === undefined) { continue; } - const { - url: urlStringIndex, - windowWidth, - windowHeight, - } = payload as ScreenshotPayload & { - windowWidth: number; - windowHeight: number; - }; - const scaledImageWidth = (trackHeight * windowWidth) / windowHeight; + const scaledImageWidth = + (trackHeight * windowSize.width) / windowSize.height; images.push(
{ {/* The following image is centered and cropped by the outer container. */} { value, thread.stringTable, threadIdToNameMap, - processIdToNameMap + processIdToNameMap, + data )} ); @@ -352,67 +354,6 @@ class MarkerTooltipContents extends React.PureComponent { ); break; } - case 'CompositorScreenshot': { - if ( - data.url !== undefined && - 'windowWidth' in data && - 'windowHeight' in data - ) { - const { width, height } = computeScreenshotSize( - data, - MAXIMUM_IMAGE_SIZE - ); - details.push( - - - , - - <> - {data.windowWidth}px × {data.windowHeight}px - - , - - This marker spans the time between each composite of a window - and shows the window contents during that time. - , - - {data.windowID} - - ); - } else if (marker.name === 'CompositorScreenshotWindowDestroyed') { - details.push( - - This marker shows the moment a window has been destroyed. - , - - {data.windowID} - - ); - } - break; - } default: // Do nothing } @@ -587,7 +528,7 @@ const URL_REGEXP = /^(https?:\/\/)\S+$/; /** * This function may return structured markup for some types suchs as table, - * list, or urls. For other types this falls back to formatFromMarkerSchema + * list, urls, or images. For other types this falls back to formatFromMarkerSchema * above. */ export function renderMarkerFieldValue( @@ -596,7 +537,9 @@ export function renderMarkerFieldValue( value: any, stringTable: StringTable, threadIdToNameMap?: Map, - processIdToNameMap?: Map + processIdToNameMap?: Map, + // The payload the value comes from, for formats that refer to a sibling field. + payload?: MarkerPayload | null ): React.ReactElement | string { if (value === undefined || value === null) { console.warn(`Formatting ${value} for ${JSON.stringify(markerType)}`); @@ -653,7 +596,8 @@ export function renderMarkerFieldValue( cell, stringTable, threadIdToNameMap, - processIdToNameMap + processIdToNameMap, + payload )} ); @@ -665,6 +609,23 @@ export function renderMarkerFieldValue( ); } + case 'screenshot-data-url': { + const size = (payload as any)?.[format.sizeFieldForAspectRatio]; + return ( + + ); + } default: throw new Error( `Unknown format type ${JSON.stringify(format as never)}` diff --git a/src/profile-logic/import/chrome.ts b/src/profile-logic/import/chrome.ts index 06b723b68b..7aa7cfa938 100644 --- a/src/profile-logic/import/chrome.ts +++ b/src/profile-logic/import/chrome.ts @@ -897,8 +897,7 @@ async function extractScreenshots( type: 'CompositorScreenshot', url: stringTable.indexForString(urlString), windowID: 'id', - windowWidth: size.width, - windowHeight: size.height, + windowSize: { width: size.width, height: size.height }, }); markers.name.push(stringTable.indexForString('CompositorScreenshot')); markers.startTime.push(screenshot.ts / 1000); diff --git a/src/profile-logic/marker-data.ts b/src/profile-logic/marker-data.ts index 5c196abd14..658b0dbe00 100644 --- a/src/profile-logic/marker-data.ts +++ b/src/profile-logic/marker-data.ts @@ -614,7 +614,7 @@ export function deriveMarkersFromRawMarkerTable( return { ...startData, ...endData, - }; + } as MarkerPayload; } // We don't add a screenshot marker as we find it, because to know its @@ -734,7 +734,7 @@ export function deriveMarkersFromRawMarkerTable( // raw marker of the same type and the same window, we convert them to // Interval markers with a a start and end time. - const { windowID } = data; + const windowID = String(data.windowID); const previousScreenshotMarker = previousScreenshotMarkers.get(windowID); if (previousScreenshotMarker !== undefined) { @@ -1443,10 +1443,11 @@ export function groupScreenshotsById( const marker = getMarker(markerIndex); const { data } = marker; if (data && data.type === 'CompositorScreenshot') { - let markers = idToScreenshotMarkers.get(data.windowID); + const windowID = String(data.windowID); + let markers = idToScreenshotMarkers.get(windowID); if (markers === undefined) { markers = []; - idToScreenshotMarkers.set(data.windowID, markers); + idToScreenshotMarkers.set(windowID, markers); } markers.push(marker); @@ -1639,10 +1640,10 @@ export function filterMarkerByDisplayLocation( * Compute the Screenshot image's thumbnail size. */ export function computeScreenshotSize( - payload: { windowWidth: number; windowHeight: number }, + windowSize: { width: number; height: number }, maximumSize: number ): { readonly width: number; readonly height: number } { - const { windowWidth, windowHeight } = payload; + const { width: windowWidth, height: windowHeight } = windowSize; // Coefficient should be according to bigger side. const coefficient = diff --git a/src/profile-logic/marker-schema.ts b/src/profile-logic/marker-schema.ts index 90a81d4333..74d7ada8dd 100644 --- a/src/profile-logic/marker-schema.ts +++ b/src/profile-logic/marker-schema.ts @@ -97,6 +97,28 @@ export const markerSchemaFrontEndOnly: MarkerSchema[] = [ }, ], }, + { + // A profile can contain these markers without carrying a schema for them, + // so the front end defines one. + name: 'CompositorScreenshot', + display: ['marker-chart', 'marker-table'], + fields: [ + { + key: 'url', + label: 'Image', + format: { + type: 'screenshot-data-url', + sizeFieldForAspectRatio: 'windowSize', + }, + }, + { key: 'windowSize', label: 'Window Size', format: 'screenshot-size' }, + { key: 'windowID', label: 'Window ID', format: 'string' }, + ], + description: oneLine` + This marker spans the time between each composite of a window and shows + the window contents during that time. + `, + }, ]; /** @@ -566,9 +588,12 @@ export function formatFromMarkerSchema( rows.push(...cellRows); return rows.map((row) => `(${row.join(', ')})`).join(','); } + case 'screenshot-data-url': + // Don't expand a base64-encoded image into text output. + return '(screenshot)'; default: throw new Error( - `Unknown format type ${JSON.stringify(format.type as never)}` + `Unknown format type ${JSON.stringify(format as never)}` ); } } @@ -621,6 +646,8 @@ export function formatFromMarkerSchema( formatFromMarkerSchema(markerType, 'string', v, stringTable) ) .join(', '); + case 'screenshot-size': + return `${value.width}px × ${value.height}px`; default: console.warn( `A marker schema of type "${markerType}" had an unknown format ${JSON.stringify( @@ -654,7 +681,16 @@ export function markerPayloadMatchesSearch( continue; } - if (isStringIndexFormat(payloadField.format)) { + const { format } = payloadField; + if (typeof format === 'object' && format.type === 'screenshot-data-url') { + // Searching a base64-encoded image isn't useful. + continue; + } + if (format === 'screenshot-size') { + value = formatFromMarkerSchema(data.type, format, value, stringTable); + } + + if (isStringIndexFormat(format)) { if (typeof value !== 'number') { console.warn( `In marker ${marker.name}, the key ${payloadField.key} has an invalid value "${value}" as a unique string, it isn't a number.` @@ -685,6 +721,9 @@ export function markerPayloadMatchesSearch( export function isStringIndexFormat( format: MarkerFormatType | undefined ): boolean { + if (typeof format === 'object') { + return format.type === 'screenshot-data-url'; + } return ( format === 'unique-string' || format === 'flow-id' || @@ -696,17 +735,15 @@ export function isStringIndexFormat( * Returns a map of marker schema name -> array of field keys, listing any fields * that contain indexes into the string table. If a marker schema has no such * fields, then we don't put an entry for it in the returned map. + * The front-end-only schemas are always covered, whether or not they're in + * `markerSchemas`. */ export function computeStringIndexMarkerFieldsByDataType( markerSchemas: MarkerSchema[] ): Map { const stringIndexMarkerFieldsByDataType = new Map(); - // 'CompositorScreenshot' markers currently don't have a schema (#5303), - // hardcode the url field (which is a string index) until they do. - stringIndexMarkerFieldsByDataType.set('CompositorScreenshot', ['url']); - - for (const schema of markerSchemas) { + for (const schema of [...markerSchemas, ...markerSchemaFrontEndOnly]) { const { name, fields } = schema; const stringIndexFields = []; for (const field of fields) { diff --git a/src/profile-logic/process-profile.ts b/src/profile-logic/process-profile.ts index 8be48440cb..d324d3da45 100644 --- a/src/profile-logic/process-profile.ts +++ b/src/profile-logic/process-profile.ts @@ -955,12 +955,20 @@ function _processMarkerPayload( // here, and then to `MarkerPayload` as the return value for this function. // This doesn't provide type safety but it shows the intent of going from an // object without much type safety, to a specific type definition. - const data: MarkerPayload = payload as any; + let data: MarkerPayload = payload as any; if (!data.type) { return data; } + if (data.type === 'CompositorScreenshot') { + const { windowWidth, windowHeight, ...rest } = data as any; + data = + windowWidth === undefined || windowHeight === undefined + ? rest + : { ...rest, windowSize: { width: windowWidth, height: windowHeight } }; + } + const stringIndexMarkerFields = stringIndexMarkerFieldsByDataType.get( data.type ); diff --git a/src/profile-logic/processed-profile-versioning.ts b/src/profile-logic/processed-profile-versioning.ts index c009bd052d..b147b44e8a 100644 --- a/src/profile-logic/processed-profile-versioning.ts +++ b/src/profile-logic/processed-profile-versioning.ts @@ -3433,6 +3433,25 @@ const _upgraders: { frameTable.address = new Uint32Array(frameTable.address); } }, + [72]: (profile: any) => { + // The CompositorScreenshot marker payload's `windowWidth` and + // `windowHeight` fields were replaced with a single `windowSize` field. + for (const thread of profile.threads) { + const { markers } = thread; + for (let i = 0; i < markers.length; i++) { + const data = markers.data[i]; + if (!data || data.type !== 'CompositorScreenshot') { + continue; + } + const { windowWidth, windowHeight } = data; + if (windowWidth !== undefined && windowHeight !== undefined) { + data.windowSize = { width: windowWidth, height: windowHeight }; + } + delete data.windowWidth; + delete data.windowHeight; + } + } + }, // If you add a new upgrader here, please document the change in // `docs-developer/CHANGELOG-formats.md`. }; diff --git a/src/profile-logic/tracks.ts b/src/profile-logic/tracks.ts index a0c1489d5d..8abe5224b7 100644 --- a/src/profile-logic/tracks.ts +++ b/src/profile-logic/tracks.ts @@ -770,7 +770,7 @@ export function computeGlobalTracks( // Coerce the payload to a screenshot one. Don't do a runtime check that // this is correct. const data = markers.data[markerIndex] as ScreenshotPayload; - ids.add(data.windowID); + ids.add(String(data.windowID)); } } for (const id of ids) { diff --git a/src/test/components/TooltipMarker.test.tsx b/src/test/components/TooltipMarker.test.tsx index c80c87ae87..5f2ccc7c24 100644 --- a/src/test/components/TooltipMarker.test.tsx +++ b/src/test/components/TooltipMarker.test.tsx @@ -1180,8 +1180,7 @@ describe('TooltipMarker', function () { type: 'CompositorScreenshot', url: screenshotUrlIndex, windowID: 'XXX', - windowWidth: 600, - windowHeight: 300, + windowSize: { width: 600, height: 300 }, }, ], ]); @@ -1213,7 +1212,6 @@ describe('TooltipMarker', function () { { type: 'CompositorScreenshot', windowID: 'XXX', - url: undefined, }, ], ]); @@ -1300,8 +1298,7 @@ describe('TooltipMarker', function () { type: 'CompositorScreenshot', url: screenshotUrlIndex, windowID: 'XXX', - windowWidth: 600, - windowHeight: 300, + windowSize: { width: 600, height: 300 }, }, ], ] diff --git a/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap b/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap index 011f064231..19fbb30030 100644 --- a/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap +++ b/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap @@ -2037,7 +2037,11 @@ exports[`TooltipMarker renders the tooltip of CompositorScreenshotWindowDestroye Description :
- This marker shows the moment a window has been destroyed. +
+ This marker spans the time between each composite of a window and shows the window contents during that time. +
@@ -5189,6 +5193,17 @@ exports[`TooltipMarker shows image of CompositorScreenshot markers 1`] = `
+
+ Description + : +
+
+ This marker spans the time between each composite of a window and shows the window contents during that time. +
@@ -5206,17 +5221,7 @@ exports[`TooltipMarker shows image of CompositorScreenshot markers 1`] = ` Window Size :
- 600 - px × - 300 - px -
- Description - : -
- This marker spans the time between each composite of a window and shows the window contents during that time. + 600px × 300px
diff --git a/src/test/fixtures/profiles/processed-profile.ts b/src/test/fixtures/profiles/processed-profile.ts index 3b5dc077db..4acc71d729 100644 --- a/src/test/fixtures/profiles/processed-profile.ts +++ b/src/test/fixtures/profiles/processed-profile.ts @@ -378,8 +378,7 @@ export function makeCompositorScreenshot( type: 'CompositorScreenshot', url: 0, windowID: '', - windowWidth: 100, - windowHeight: 100, + windowSize: { width: 100, height: 100 }, }, }; } @@ -1388,8 +1387,7 @@ export function getScreenshotMarkersForWindowId( type: 'CompositorScreenshot', url: 0, // Some arbitrary string. windowID, - windowWidth: 300, - windowHeight: 150, + windowSize: { width: 300, height: 150 }, }, ]); } @@ -1406,7 +1404,6 @@ export function getScreenshotTrackProfile() { { type: 'CompositorScreenshot', windowID: '1', - url: undefined, }, ], ]); diff --git a/src/test/fixtures/upgrades/processed-3.json b/src/test/fixtures/upgrades/processed-3.json index df3eb93eaa..0e865bd71f 100644 --- a/src/test/fixtures/upgrades/processed-3.json +++ b/src/test/fixtures/upgrades/processed-3.json @@ -347,30 +347,36 @@ ] }, "markers": { - "length": 10, + "length": 13, "name": [ 4, 5, 10, 10, + 18, 5, 17, 8, 8, 9, - 14 + 18, + 14, + 19 ], "time": [ 0, 2, 4, 5, + 6, 8, 8, 9, 10, 11, - 13 + 12, + 13, + 14 ], "data": [ { @@ -396,6 +402,13 @@ "interval": "end", "type": "tracing" }, + { + "type": "CompositorScreenshot", + "url": 20, + "windowID": "0x136888400", + "windowWidth": 1280, + "windowHeight": 1000 + }, { "category": "Paint", "interval": "end", @@ -428,6 +441,13 @@ "startTime": 11, "endTime": 12 }, + { + "type": "CompositorScreenshot", + "url": 20, + "windowID": "0x136888400", + "windowWidth": 1280, + "windowHeight": 1000 + }, { "type": "Network", "startTime": 3, @@ -446,6 +466,10 @@ "requestStart": 10, "responseStart": 11, "responseEnd": 12 + }, + { + "type": "CompositorScreenshot", + "windowID": "0x136888400" } ], "category": [ @@ -455,10 +479,13 @@ 4, 4, 4, + 4, 5, 5, 1, - 7 + 4, + 7, + 4 ] }, "samples": { @@ -527,7 +554,10 @@ "Load 32: https://github.com/rustwasm/wasm-bindgen/issues/3", "DiskIO", "FileIO", - "TextureCacheFree" + "TextureCacheFree", + "CompositorScreenshot", + "CompositorScreenshotWindowDestroyed", + "data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAUD" ], "pid": "Unknown Process 1" }, diff --git a/src/test/integration/profiler-edit/__snapshots__/profiler-edit.test.ts.snap b/src/test/integration/profiler-edit/__snapshots__/profiler-edit.test.ts.snap index 27af272e13..cfec2157bc 100644 --- a/src/test/integration/profiler-edit/__snapshots__/profiler-edit.test.ts.snap +++ b/src/test/integration/profiler-edit/__snapshots__/profiler-edit.test.ts.snap @@ -87,7 +87,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "a.out", "sampleUnits": Object { @@ -1488,7 +1488,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "a.out", "sampleUnits": Object { @@ -2889,7 +2889,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "a.out", "sampleUnits": Object { @@ -4290,7 +4290,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "a.out", "sampleUnits": Object { diff --git a/src/test/store/__snapshots__/profile-view.test.ts.snap b/src/test/store/__snapshots__/profile-view.test.ts.snap index 92fd2e39d8..5bc3ba761c 100644 --- a/src/test/store/__snapshots__/profile-view.test.ts.snap +++ b/src/test/store/__snapshots__/profile-view.test.ts.snap @@ -428,7 +428,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "Firefox", "sourceURL": "", diff --git a/src/test/store/receive-profile.test.ts b/src/test/store/receive-profile.test.ts index afc2309460..a768f16605 100644 --- a/src/test/store/receive-profile.test.ts +++ b/src/test/store/receive-profile.test.ts @@ -1757,8 +1757,7 @@ describe('actions/receive-profile', function () { type: 'CompositorScreenshot', url: 0, // Some arbitrary string. windowID: '0', - windowWidth: 300, - windowHeight: 150, + windowSize: { width: 300, height: 150 }, }, ], ]); @@ -1771,8 +1770,7 @@ describe('actions/receive-profile', function () { type: 'CompositorScreenshot', url: 0, // Some arbitrary string. windowID: '1', - windowWidth: 300, - windowHeight: 150, + windowSize: { width: 300, height: 150 }, }, ], ]); diff --git a/src/test/unit/__snapshots__/marker-data.test.ts.snap b/src/test/unit/__snapshots__/marker-data.test.ts.snap index df135ea0a7..344a6dbae1 100644 --- a/src/test/unit/__snapshots__/marker-data.test.ts.snap +++ b/src/test/unit/__snapshots__/marker-data.test.ts.snap @@ -179,9 +179,11 @@ Array [ "data": Object { "type": "CompositorScreenshot", "url": 21, - "windowHeight": 1000, "windowID": "0x136888400", - "windowWidth": 1280, + "windowSize": Object { + "height": 1000, + "width": 1280, + }, }, "end": 25, "name": "CompositorScreenshot", diff --git a/src/test/unit/__snapshots__/marker-schema.test.ts.snap b/src/test/unit/__snapshots__/marker-schema.test.ts.snap index 9f60bce48e..50216e9a4f 100644 --- a/src/test/unit/__snapshots__/marker-schema.test.ts.snap +++ b/src/test/unit/__snapshots__/marker-schema.test.ts.snap @@ -278,6 +278,33 @@ Array [ , "a, b", ], + Array [ + "screenshot-size", + Object { + "height": 1000, + "width": 1280, + }, + "1280px × 1000px", + "1280px × 1000px", + ], + Array [ + Object { + "sizeFieldForAspectRatio": "windowSize", + "type": "screenshot-data-url", + }, + 0, + , + "(screenshot)", + ], ] `; diff --git a/src/test/unit/__snapshots__/profile-conversion.test.ts.snap b/src/test/unit/__snapshots__/profile-conversion.test.ts.snap index 46f5bf9b8e..3bffbb2edf 100644 --- a/src/test/unit/__snapshots__/profile-conversion.test.ts.snap +++ b/src/test/unit/__snapshots__/profile-conversion.test.ts.snap @@ -42,7 +42,7 @@ Object { "RefreshDriverTick", "Network", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "ART Trace (Android)", "symbolicated": true, "version": 36, @@ -1022,7 +1022,7 @@ Object { "RefreshDriverTick", "Network", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "ART Trace (Android)", "symbolicated": true, "version": 36, @@ -2305,7 +2305,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -2697,7 +2697,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3086,7 +3086,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3187,7 +3187,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3540,7 +3540,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3605,7 +3605,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3759,7 +3759,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3817,7 +3817,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4207,7 +4207,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4265,7 +4265,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4323,7 +4323,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4643,7 +4643,7 @@ Object { "importedFrom": "Simpleperf", "interval": 0, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "com.example.sampleapplication", "symbolicated": undefined, "version": 30, @@ -5019,7 +5019,7 @@ Object { "importedFrom": "Simpleperf", "interval": 0, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "com.example.sampleapplication", "symbolicated": undefined, "version": 30, @@ -5319,7 +5319,7 @@ Object { "importedFrom": "dhat", "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "target/debug/examples/work_log (dhat)", "symbolicated": true, "version": 36, @@ -5452,7 +5452,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Flamegraph", "symbolicated": true, "version": 36, @@ -5510,7 +5510,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "product": "Flamegraph", "symbolicated": true, "version": 36, diff --git a/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap b/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap index 4d039c9117..0a32ee9e50 100644 --- a/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap +++ b/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap @@ -40,7 +40,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -7834,7 +7834,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -9229,7 +9229,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -10794,7 +10794,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 71, + "preprocessedProfileVersion": 72, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -11143,15 +11143,15 @@ Object { null, ], "name": Array [ - 7, - 8, - 9, 10, 11, - 8, - 9, - 15, - 16, + 12, + 13, + 14, + 11, + 12, + 18, + 19, ], "originalLocation": Array [ null, @@ -11219,10 +11219,10 @@ Object { 1, ], "name": Array [ - 8, - 9, - 8, - 9, + 11, + 12, + 11, + 12, ], }, "resourceTable": Object { @@ -11233,9 +11233,9 @@ Object { ], "length": 3, "name": Array [ - 13, - 12, - 14, + 16, + 15, + 17, ], "type": Array [ 1, @@ -11254,7 +11254,7 @@ Object { null, ], "filename": Array [ - 12, + 15, ], "id": Array [ null, @@ -11333,10 +11333,13 @@ Object { "VsyncTimestamp", "Reflow", "Rasterize", + "CompositorScreenshot", + "data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAUD", "TextureCacheFree", "DOMEvent", "MinorGC", "Load 32: https://github.com/rustwasm/wasm-bindgen/issues/3", + "CompositorScreenshotWindowDestroyed", "(root)", "0x100000f84", "0x100001a45", @@ -11360,10 +11363,13 @@ Object { 4, 4, 4, + 4, 5, 5, 1, + 4, 7, + 4, ], "data": Array [ Object { @@ -11389,6 +11395,15 @@ Object { "interval": "end", "type": "tracing", }, + Object { + "type": "CompositorScreenshot", + "url": 4, + "windowID": "0x136888400", + "windowSize": Object { + "height": 1000, + "width": 1280, + }, + }, Object { "category": "Paint", "interval": "end", @@ -11416,6 +11431,15 @@ Object { "type": "DOMEvent", }, Object {}, + Object { + "type": "CompositorScreenshot", + "url": 4, + "windowID": "0x136888400", + "windowSize": Object { + "height": 1000, + "width": 1280, + }, + }, Object { "URI": "https://github.com/rustwasm/wasm-bindgen/issues/3", "connectEnd": 9, @@ -11435,55 +11459,71 @@ Object { "tcpConnectEnd": 7, "type": "Network", }, + Object { + "type": "CompositorScreenshot", + "windowID": "0x136888400", + }, ], "endTime": Array [ null, null, null, 5, + null, 8, null, null, 10, 12, + null, 13, + null, ], - "length": 10, + "length": 13, "name": Array [ 0, 1, 2, 2, - 1, 3, - 4, - 4, + 1, 5, 6, + 6, + 7, + 3, + 8, + 9, ], "phase": Array [ 0, 2, 2, 3, + 0, 3, 0, 2, 3, 1, + 0, 1, + 0, ], "startTime": Array [ 0, 2, 4, null, + 6, null, 8, 9, null, 11, + 12, 3, + 14, ], }, "name": "GeckoMain", @@ -11625,9 +11665,9 @@ Object { 2, 2, 1, - 4, - 4, - 5, + 6, + 6, + 7, ], "phase": Array [ 0, @@ -11811,10 +11851,10 @@ Object { 2, 2, 1, - 4, - 4, - 5, - 14, + 6, + 6, + 7, + 17, ], "phase": Array [ 0, diff --git a/src/test/unit/marker-data.test.ts b/src/test/unit/marker-data.test.ts index 9a5d06beff..1880882309 100644 --- a/src/test/unit/marker-data.test.ts +++ b/src/test/unit/marker-data.test.ts @@ -329,8 +329,7 @@ describe('Derive markers from Gecko phase markers', function () { const basePayload = { type: 'CompositorScreenshot' as const, url: 16, - windowWidth: 1280, - windowHeight: 1000, + windowSize: { width: 1280, height: 1000 }, }; const payloadsForWindowA: ScreenshotPayload[] = [ { @@ -339,7 +338,7 @@ describe('Derive markers from Gecko phase markers', function () { }, { ...basePayload, - windowWidth: 500, + windowSize: { width: 500, height: 1000 }, windowID: '0xAAAAAAAAA', }, ]; @@ -812,8 +811,7 @@ describe('deriveMarkersFromRawMarkerTable', function () { type: 'CompositorScreenshot', url: expect.anything(), windowID: '0x136888400', - windowWidth: 1280, - windowHeight: 1000, + windowSize: { width: 1280, height: 1000 }, }, name: 'CompositorScreenshot', start: 25, diff --git a/src/test/unit/marker-schema.test.ts b/src/test/unit/marker-schema.test.ts index 31f7ed8dd2..d53f23f747 100644 --- a/src/test/unit/marker-schema.test.ts +++ b/src/test/unit/marker-schema.test.ts @@ -6,6 +6,8 @@ import { formatFromMarkerSchema, parseLabel, markerSchemaFrontEndOnly, + isStringIndexFormat, + computeStringIndexMarkerFieldsByDataType, } from '../../profile-logic/marker-schema'; import { renderMarkerFieldValue } from 'firefox-profiler/components/tooltip/Marker'; import type { @@ -516,8 +518,17 @@ describe('marker schema formatting', function () { ], ['list', []], ['list', ['a', 'b']], + ['screenshot-size', { width: 1280, height: 1000 }], + // Without a payload there's no size field to look up, so the image falls + // back to a maximum size. + [ + { type: 'screenshot-data-url', sizeFieldForAspectRatio: 'windowSize' }, + 0, + ], ]; - const stringTable = StringTable.withBackingArray([]); + const stringTable = StringTable.withBackingArray([ + 'data:image/jpeg;base64,AAAA', + ]); expect( entries.map(([format, value]: [MarkerFormatType, any]): string[][] => [ format, @@ -529,6 +540,25 @@ describe('marker schema formatting', function () { }); }); +describe('computeStringIndexMarkerFieldsByDataType', function () { + it('treats the screenshot data URL as a string index', function () { + expect( + isStringIndexFormat({ + type: 'screenshot-data-url', + sizeFieldForAspectRatio: 'windowSize', + }) + ).toBe(true); + }); + + it('includes the front-end only schemas', function () { + // Callers may pass the profile's own schema list, which never contains the + // front-end only schemas. + expect(computeStringIndexMarkerFieldsByDataType([])).toEqual( + new Map([['CompositorScreenshot', ['url']]]) + ); + }); +}); + describe('getMarkerSchema', function () { it('combines front-end and Gecko marker schema', function () { const { profile } = getProfileFromTextSamples('A'); diff --git a/src/test/unit/merge-compare.test.ts b/src/test/unit/merge-compare.test.ts index f565e08bc2..13b55f396c 100644 --- a/src/test/unit/merge-compare.test.ts +++ b/src/test/unit/merge-compare.test.ts @@ -731,8 +731,7 @@ describe('mergeThreads function', function () { type: 'CompositorScreenshot', url: screenshot1UrlIndex, windowID: 'XXX', - windowWidth: 300, - windowHeight: 600, + windowSize: { width: 300, height: 600 }, }, ], ]); @@ -746,8 +745,7 @@ describe('mergeThreads function', function () { type: 'CompositorScreenshot', url: screenshot2UrlIndex, windowID: 'YYY', - windowWidth: 300, - windowHeight: 600, + windowSize: { width: 300, height: 600 }, }, ], ]); diff --git a/src/types/markers.ts b/src/types/markers.ts index 3dfcc65f10..b143779e1d 100644 --- a/src/types/markers.ts +++ b/src/types/markers.ts @@ -86,7 +86,14 @@ export type MarkerFormatType = | 'pid' | 'tid' | 'list' - | { type: 'table'; columns: TableColumnFormat[] }; + // The size of a window in pixels, as a { width, height } object. + // "Label: 1280px × 1000px" + | 'screenshot-size' + | { type: 'table'; columns: TableColumnFormat[] } + // An image data URL, stored as an index into the profile's string table. It is + // rendered at the aspect ratio of the 'screenshot-size' field named by + // `sizeFieldForAspectRatio`. + | { type: 'screenshot-data-url'; sizeFieldForAspectRatio: string }; type TableColumnFormat = { // type for formatting, default is string @@ -700,30 +707,30 @@ type VsyncTimestampPayload = { type: 'VsyncTimestamp'; }; -export type ScreenshotPayload = - | { - type: 'CompositorScreenshot'; - // This field represents the data url of the image. It is saved in the string table. - url: IndexIntoStringTable; - // A memory address that can uniquely identify a window. It has no meaning other than - // a way to identify a window. - windowID: string; - // The original dimensions of the window that was captured. The actual image that is - // stored in the string table will be scaled down from the original size. - windowWidth: number; - windowHeight: number; - } - // Markers that represent the closing of a window (name === 'CompositorScreenshotWindowDestroyed') - // only have a windowID data. - | { - type: 'CompositorScreenshot'; - // A memory address that can uniquely identify a window. It has no meaning other than - // a way to identify a window. - windowID: string; - // Having the property present but void makes it easier to deal with Flow in - // our flow version. - url: void; - }; +export type ScreenshotPayload = { + type: 'CompositorScreenshot'; + // A value that can uniquely identify a window. It has no meaning other than + // a way to identify a window. Both integers and hexadecimal strings like + // "0x136888400" occur, depending on the Firefox version that recorded the + // profile, so normalize it with String() before using it as a key. + windowID: number | string; + // This field represents the data url of the image. It is saved in the string table. + // The marker that only closes a window's last screenshot doesn't have one. + url?: IndexIntoStringTable; + // The original dimensions of the window that was captured. The actual image that is + // stored in the string table will be scaled down from the original size. + windowSize?: { width: number; height: number }; +}; + +export type ScreenshotPayload_Gecko = { + type: 'CompositorScreenshot'; + windowID: number | string; + url?: IndexIntoStringTable; + // Gecko writes the window dimensions as two separate fields. Profile + // processing folds them into a single `windowSize` field. + windowWidth?: number; + windowHeight?: number; +}; export type StyleMarkerPayload = { type: 'Styles'; @@ -896,7 +903,7 @@ export type MarkerPayload_Gecko = | GCMajorMarkerPayload_Gecko | GCSliceMarkerPayload_Gecko | VsyncTimestampPayload - | ScreenshotPayload + | ScreenshotPayload_Gecko | CcMarkerTracing | ArbitraryEventTracing | NavigationMarkerPayload From 64c791fce990b7fb7d57624426682f3a666f5f3d Mon Sep 17 00:00:00 2001 From: fatadel Date: Fri, 14 Aug 2026 13:45:47 +0200 Subject: [PATCH 2/3] Store screenshot markers as start and end marker pairs Screenshot markers arrived as instant markers and were turned into intervals by a special case in the marker derivation, which also had to track the last screenshot of every window separately in order to close it. Putting the window ID in the marker name lets the ordinary name-based pairing handle them, so that special case is gone. Each window now gets its own row in the marker chart. CompositorScreenshotWindowDestroyed is the end marker of that window's last screenshot rather than a marker of its own. A window that is never destroyed leaves its last screenshot open, so the derived marker is extended to the end of the thread and marked incomplete. --- docs-developer/CHANGELOG-formats.md | 2 + src/profile-logic/import/chrome.ts | 63 +++++++++--- src/profile-logic/marker-data.ts | 72 +------------- src/profile-logic/process-profile.ts | 97 ++++++++++++++++++- .../processed-profile-versioning.ts | 88 ++++++++++++++++- src/profile-logic/tracks.ts | 32 +++--- src/test/components/TooltipMarker.test.tsx | 16 --- src/test/components/TrackContextMenu.test.tsx | 6 +- src/test/components/TrackScreenshots.test.tsx | 15 ++- .../__snapshots__/TooltipMarker.test.tsx.snap | 70 +------------ .../TrackScreenshots.test.tsx.snap | 22 ++--- .../fixtures/profiles/processed-profile.ts | 82 ++++++++++------ src/test/store/profile-view.test.ts | 10 +- .../__snapshots__/marker-data.test.ts.snap | 3 +- .../profile-conversion.test.ts.snap | 8 +- .../profile-upgrading.test.ts.snap | 28 ++++-- src/test/unit/marker-data.test.ts | 82 +++++++++------- 17 files changed, 398 insertions(+), 298 deletions(-) diff --git a/docs-developer/CHANGELOG-formats.md b/docs-developer/CHANGELOG-formats.md index d7f729cb8a..f6bdd74299 100644 --- a/docs-developer/CHANGELOG-formats.md +++ b/docs-developer/CHANGELOG-formats.md @@ -10,6 +10,8 @@ Note that this is not an exhaustive list. Processed profile format upgraders can The `CompositorScreenshot` marker payload's `windowWidth` and `windowHeight` fields were replaced with a single `windowSize` field of the form `{ width, height }`. +These markers are now stored as pairs of start and end markers instead of instant markers. The window ID is part of the marker name (`CompositorScreenshot `), so starts and ends are matched by name like any other marker pair. The `CompositorScreenshotWindowDestroyed` marker is gone: it is now the end marker of that window's last screenshot. The last screenshot of a window that is never destroyed has no end marker, and is extended to the end of the thread. + Two marker schema field formats were added to describe these markers: `screenshot-size`, whose value is a `{ width, height }` object, and `screenshot-data-url`, an object format `{ type: "screenshot-data-url", sizeFieldForAspectRatio }` whose value is a string table index holding an image data URL. Profiles containing `CompositorScreenshot` markers don't necessarily carry a schema for them, so the front end supplies one. ### Version 71 diff --git a/src/profile-logic/import/chrome.ts b/src/profile-logic/import/chrome.ts index 7aa7cfa938..c87df9a94b 100644 --- a/src/profile-logic/import/chrome.ts +++ b/src/profile-logic/import/chrome.ts @@ -6,7 +6,10 @@ import type { Profile, RawThread, IndexIntoStackTable, + MarkerPhase, + Milliseconds, MixedObject, + ScreenshotPayload, } from 'firefox-profiler/types'; import { FrameFlag } from 'firefox-profiler/types'; @@ -30,6 +33,7 @@ import { } from 'firefox-profiler/app-logic/constants'; import { getTimeRangeForThread } from '../profile-data'; +import { getScreenshotMarkerName } from '../marker-data'; import { GlobalDataCollector } from '../global-data-collector'; // Chrome Tracing Event Spec: @@ -886,6 +890,29 @@ async function extractScreenshots( ); } + // Chrome's Screenshot events don't say which window they belong to, so they + // all share one made-up window ID. + const windowID = 'id'; + const nameIndex = stringTable.indexForString( + getScreenshotMarkerName(windowID) + ); + + function pushScreenshotMarker( + data: ScreenshotPayload, + startTime: Milliseconds | null, + endTime: Milliseconds | null, + phase: MarkerPhase + ) { + markers.data.push(data); + markers.name.push(nameIndex); + markers.startTime.push(startTime); + markers.endTime.push(endTime); + markers.phase.push(phase); + markers.category.push(graphicsIndex); + markers.length++; + } + + let hasOpenScreenshot = false; for (const screenshot of screenshots) { const urlString = 'data:image/jpg;base64,' + screenshot.args.snapshot; const size = await getImageSize(urlString); @@ -893,18 +920,30 @@ async function extractScreenshots( // The image could not be processed, do not add it. continue; } - markers.data.push({ - type: 'CompositorScreenshot', - url: stringTable.indexForString(urlString), - windowID: 'id', - windowSize: { width: size.width, height: size.height }, - }); - markers.name.push(stringTable.indexForString('CompositorScreenshot')); - markers.startTime.push(screenshot.ts / 1000); - markers.endTime.push(null); - markers.phase.push(INSTANT); - markers.category.push(graphicsIndex); - markers.length++; + const startTime = screenshot.ts / 1000; + + // Each screenshot is valid until the next one. + if (hasOpenScreenshot) { + pushScreenshotMarker( + { type: 'CompositorScreenshot', windowID }, + null, + startTime, + INTERVAL_END + ); + } + + pushScreenshotMarker( + { + type: 'CompositorScreenshot', + url: stringTable.indexForString(urlString), + windowID, + windowSize: { width: size.width, height: size.height }, + }, + startTime, + null, + INTERVAL_START + ); + hasOpenScreenshot = true; } } diff --git a/src/profile-logic/marker-data.ts b/src/profile-logic/marker-data.ts index 658b0dbe00..bdc882cdeb 100644 --- a/src/profile-logic/marker-data.ts +++ b/src/profile-logic/marker-data.ts @@ -559,7 +559,6 @@ export function correlateIPCMarkers( * the thread range. For the reverse situation, it's set to the start. * * There is also some special handling of different markers. - * - CompositorScreenshot - They are turned from Instant markers to Interval markers * - IPC - They are matched up. * - Network - They have different network phases. * @@ -617,10 +616,6 @@ export function deriveMarkersFromRawMarkerTable( } as MarkerPayload; } - // We don't add a screenshot marker as we find it, because to know its - // duration we need to wait until the next one or the end of the profile. So - // we keep it here. - const previousScreenshotMarkers: Map = new Map(); for ( let rawMarkerIndex = 0; rawMarkerIndex < rawMarkers.length; @@ -728,51 +723,6 @@ export function deriveMarkersFromRawMarkerTable( continue; } - case 'CompositorScreenshot': { - // Screenshot markers are already ordered. In the raw marker table, - // they're Instant markers, but since they're valid until the following - // raw marker of the same type and the same window, we convert them to - // Interval markers with a a start and end time. - - const windowID = String(data.windowID); - const previousScreenshotMarker = - previousScreenshotMarkers.get(windowID); - if (previousScreenshotMarker !== undefined) { - previousScreenshotMarkers.delete(windowID); - const previousStartTime = ensureExists( - rawMarkers.startTime[previousScreenshotMarker], - 'Expected to find a start time for a screenshot marker.' - ); - const thisStartTime = ensureExists( - maybeStartTime, - 'The CompositorScreenshot is assumed to have a start time.' - ); - const data = rawMarkers.data[previousScreenshotMarker]; - const markerThreadId = rawMarkers.threadId - ? rawMarkers.threadId[previousScreenshotMarker] - : null; - addMarker([previousScreenshotMarker], { - start: previousStartTime, - end: thisStartTime, - name: 'CompositorScreenshot', - category, - threadId: markerThreadId, - data, - }); - } - if (stringArray[name] === 'CompositorScreenshotWindowDestroyed') { - // This marker is added when a window is destroyed. In this case we - // don't want to store it as the start of the next compositor - // marker. But we do want to keep it, so we break out of the - // switch/case so that the standard processing happens. - break; - } else { - previousScreenshotMarkers.set(windowID, rawMarkerIndex); - } - - continue; - } - case 'IPC': { const sharedData = ipcCorrelations.get( // Older profiles don't have a tid, but they also don't have the IPC markers. @@ -990,24 +940,6 @@ export function deriveMarkersFromRawMarkerTable( }); } - // And we also need to add the "last screenshot markers". - for (const previousScreenshotMarker of previousScreenshotMarkers.values()) { - const start = ensureExists( - rawMarkers.startTime[previousScreenshotMarker], - 'Expected to find a CompositorScreenshot marker with a start time.' - ); - addMarker([previousScreenshotMarker], { - start, - end: Math.max(endOfThread, start), - name: 'CompositorScreenshot', - category: rawMarkers.category[previousScreenshotMarker], - threadId: rawMarkers.threadId - ? rawMarkers.threadId[previousScreenshotMarker] - : null, - data: rawMarkers.data[previousScreenshotMarker], - }); - } - return { markers, markerIndexToRawMarkerIndexes }; } @@ -1434,6 +1366,10 @@ export function getColorClassNameForMimeType( } } +export function getScreenshotMarkerName(windowID: number | string): string { + return `CompositorScreenshot ${windowID}`; +} + export function groupScreenshotsById( getMarker: (markerIndex: MarkerIndex) => Marker, markerIndexes: MarkerIndex[] diff --git a/src/profile-logic/process-profile.ts b/src/profile-logic/process-profile.ts index d324d3da45..8f8b0c30ac 100644 --- a/src/profile-logic/process-profile.ts +++ b/src/profile-logic/process-profile.ts @@ -42,6 +42,7 @@ import { isArtTraceFormat, convertArtTraceProfile } from './import/art-trace'; import { PROCESSED_PROFILE_VERSION, INTERVAL, + INTERVAL_START, INTERVAL_END, INSTANT, } from '../app-logic/constants'; @@ -50,6 +51,7 @@ import { nudgeReturnAddresses, subcategoriesNeedSixteenBits, } from '../profile-logic/profile-data'; +import { getScreenshotMarkerName } from './marker-data'; import { toInt32Array, toUint8Array, @@ -989,6 +991,91 @@ function _processMarkerPayload( return newData; } +/** + * Gecko emits one instant marker per composite of a window, plus a + * CompositorScreenshotWindowDestroyed marker when the window goes away. Each + * screenshot is valid until the next one for the same window, so rewrite them + * into start / end marker pairs. A window that is never destroyed keeps its last + * screenshot open, so that marker gets extended to the end of the thread. + */ +function _convertScreenshotMarkersToStartEnd( + markers: RawMarkerTable, + stringTable: StringTable +): RawMarkerTable { + const hasScreenshots = markers.data.some( + (data) => data !== null && data.type === 'CompositorScreenshot' + ); + if (!hasScreenshots) { + return markers; + } + + const newMarkers = getEmptyRawMarkerTable(); + const openMarkerPerWindow = new Map< + string, + { name: IndexIntoStringTable; index: number } + >(); + + function push( + name: IndexIntoStringTable, + startTime: Milliseconds | null, + endTime: Milliseconds | null, + phase: MarkerPhase, + sourceIndex: number, + data: MarkerPayload | null + ) { + newMarkers.name.push(name); + newMarkers.startTime.push(startTime); + newMarkers.endTime.push(endTime); + newMarkers.phase.push(phase); + newMarkers.category.push(markers.category[sourceIndex]); + newMarkers.data.push(data); + newMarkers.length++; + } + + for (let i = 0; i < markers.length; i++) { + const data = markers.data[i]; + if (data === null || data.type !== 'CompositorScreenshot') { + push( + markers.name[i], + markers.startTime[i], + markers.endTime[i], + markers.phase[i], + i, + data + ); + continue; + } + + const windowID = String(data.windowID); + const time = markers.startTime[i]; + const openMarker = openMarkerPerWindow.get(windowID); + if (openMarker !== undefined) { + openMarkerPerWindow.delete(windowID); + // The end marker only needs to identify the window: the start marker's + // payload is what the derived marker keeps. + push(openMarker.name, null, time, INTERVAL_END, openMarker.index, { + type: 'CompositorScreenshot', + windowID: data.windowID, + }); + } + + if ( + stringTable.getString(markers.name[i]) === + 'CompositorScreenshotWindowDestroyed' + ) { + continue; + } + + const name = stringTable.indexForString( + getScreenshotMarkerName(data.windowID) + ); + push(name, time, null, INTERVAL_START, i, data); + openMarkerPerWindow.set(windowID, { name, index: i }); + } + + return newMarkers; +} + function _timeColumnToCompactTimeDeltas(time: Milliseconds[]): Milliseconds[] { const NS_PER_MS = 1000000; @@ -1406,13 +1493,21 @@ function _processThread( frameIndexOffset ); - const { markers, jsAllocations, nativeAllocations } = _processMarkers( + const { + markers: processedMarkers, + jsAllocations, + nativeAllocations, + } = _processMarkers( geckoMarkers, thread.stringTable, stringIndexMarkerFieldsByDataType, globalDataCollector, stackIndexOffset ); + const markers = _convertScreenshotMarkersToStartEnd( + processedMarkers, + globalDataCollector.getStringTable() + ); const samples = _processSamples(geckoSamples, stackIndexOffset); // Compute usedInnerWindowIDs from the geckoFrameStruct and the thread markers. diff --git a/src/profile-logic/processed-profile-versioning.ts b/src/profile-logic/processed-profile-versioning.ts index b147b44e8a..0f59a84b42 100644 --- a/src/profile-logic/processed-profile-versioning.ts +++ b/src/profile-logic/processed-profile-versioning.ts @@ -3435,21 +3435,99 @@ const _upgraders: { }, [72]: (profile: any) => { // The CompositorScreenshot marker payload's `windowWidth` and - // `windowHeight` fields were replaced with a single `windowSize` field. + // `windowHeight` fields were replaced with a single `windowSize` field, and + // these markers are now stored as start / end marker pairs whose name + // carries the window ID, instead of instant markers. The + // CompositorScreenshotWindowDestroyed marker becomes the end marker of that + // window's last screenshot. + const INTERVAL_START = 2; + const INTERVAL_END = 3; + const stringTable = StringTable.withBackingArray( + profile.shared.stringArray + ); + for (const thread of profile.threads) { const { markers } = thread; + const newMarkers: any = { + data: [], + name: [], + startTime: [], + endTime: [], + phase: [], + category: [], + length: 0, + }; + const hasThreadId = Boolean(markers.threadId); + if (hasThreadId) { + newMarkers.threadId = []; + } + + const push = ( + name: number, + startTime: number | null, + endTime: number | null, + phase: number, + sourceIndex: number, + data: any + ) => { + newMarkers.name.push(name); + newMarkers.startTime.push(startTime); + newMarkers.endTime.push(endTime); + newMarkers.phase.push(phase); + newMarkers.category.push(markers.category[sourceIndex]); + newMarkers.data.push(data); + if (hasThreadId) { + newMarkers.threadId.push(markers.threadId[sourceIndex]); + } + newMarkers.length++; + }; + + const openMarkerPerWindow = new Map(); for (let i = 0; i < markers.length; i++) { const data = markers.data[i]; if (!data || data.type !== 'CompositorScreenshot') { + push( + markers.name[i], + markers.startTime[i], + markers.endTime[i], + markers.phase[i], + i, + data + ); continue; } - const { windowWidth, windowHeight } = data; + + const windowID = String(data.windowID); + const time = markers.startTime[i]; + const openMarker = openMarkerPerWindow.get(windowID); + if (openMarker !== undefined) { + openMarkerPerWindow.delete(windowID); + push(openMarker.name, null, time, INTERVAL_END, openMarker.index, { + type: 'CompositorScreenshot', + windowID: data.windowID, + }); + } + + if ( + stringTable.getString(markers.name[i]) === + 'CompositorScreenshotWindowDestroyed' + ) { + continue; + } + + const { windowWidth, windowHeight, ...startData } = data; if (windowWidth !== undefined && windowHeight !== undefined) { - data.windowSize = { width: windowWidth, height: windowHeight }; + startData.windowSize = { width: windowWidth, height: windowHeight }; } - delete data.windowWidth; - delete data.windowHeight; + + const name = stringTable.indexForString( + `CompositorScreenshot ${windowID}` + ); + push(name, time, null, INTERVAL_START, i, startData); + openMarkerPerWindow.set(windowID, { name, index: i }); } + + thread.markers = newMarkers; } }, // If you add a new upgrader here, please document the change in diff --git a/src/profile-logic/tracks.ts b/src/profile-logic/tracks.ts index 8abe5224b7..079ddd182a 100644 --- a/src/profile-logic/tracks.ts +++ b/src/profile-logic/tracks.ts @@ -2,7 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import type { - ScreenshotPayload, Profile, RawProfileSharedData, RawThread, @@ -29,7 +28,6 @@ import { getMarkerTypesForDisplay, } from './marker-data'; import { intersectSets, subtractSets } from '../utils/set'; -import { StringTable } from '../utils/string-table'; import { splitSearchString, stringsToRegExp } from '../utils/string'; import { ensureExists, assertExhaustiveCheck } from '../utils/types'; @@ -717,12 +715,6 @@ export function computeGlobalTracks( let globalTracks: GlobalTrack[] = []; // Create the global tracks. - const { stringArray } = profile.shared; - const stringTable = StringTable.withBackingArray(stringArray); - const screenshotNameIndex = stringTable.hasString('CompositorScreenshot') - ? stringTable.indexForString('CompositorScreenshot') - : null; - for ( let threadIndex = 0; threadIndex < profile.threads.length; @@ -762,20 +754,20 @@ export function computeGlobalTracks( } } - // Check for screenshots. + // Check for screenshots. Their marker name carries the window ID, so match + // on the payload type instead. Windows must keep being added in the order + // their first screenshot was taken: shared URLs refer to global tracks by + // index, so a different order would change what an existing URL selects. const ids: Set = new Set(); - if (screenshotNameIndex !== null) { - for (let markerIndex = 0; markerIndex < markers.length; markerIndex++) { - if (markers.name[markerIndex] === screenshotNameIndex) { - // Coerce the payload to a screenshot one. Don't do a runtime check that - // this is correct. - const data = markers.data[markerIndex] as ScreenshotPayload; - ids.add(String(data.windowID)); - } - } - for (const id of ids) { - globalTracks.push({ type: 'screenshots', id, threadIndex }); + for (let markerIndex = 0; markerIndex < markers.length; markerIndex++) { + const data = markers.data[markerIndex]; + if (data === null || data.type !== 'CompositorScreenshot') { + continue; } + ids.add(String(data.windowID)); + } + for (const id of ids) { + globalTracks.push({ type: 'screenshots', id, threadIndex }); } } diff --git a/src/test/components/TooltipMarker.test.tsx b/src/test/components/TooltipMarker.test.tsx index 5f2ccc7c24..9ac54c3ffc 100644 --- a/src/test/components/TooltipMarker.test.tsx +++ b/src/test/components/TooltipMarker.test.tsx @@ -1203,22 +1203,6 @@ describe('TooltipMarker', function () { expect(container.firstChild).toMatchSnapshot(); }); - it('renders the tooltip of CompositorScreenshotWindowDestroyed', () => { - setupWithPayload([ - [ - 'CompositorScreenshotWindowDestroyed', - 1, - 2, - { - type: 'CompositorScreenshot', - windowID: 'XXX', - }, - ], - ]); - - expect(document.body).toMatchSnapshot(); - }); - it('shows the source thread for markers from a merged thread', function () { // We construct a profile that has 2 threads from 2 different tabs. const tab1Domain = 'https://mozilla.org'; diff --git a/src/test/components/TrackContextMenu.test.tsx b/src/test/components/TrackContextMenu.test.tsx index 89807f1bdc..813a5bd347 100644 --- a/src/test/components/TrackContextMenu.test.tsx +++ b/src/test/components/TrackContextMenu.test.tsx @@ -36,7 +36,7 @@ import { getScreenshotTrackProfile, getNetworkTrackProfile, addIPCMarkerPairToThreads, - getThreadWithMarkers, + getThreadWithRawMarkers, getScreenshotMarkersForWindowId, } from '../fixtures/profiles/processed-profile'; @@ -1229,14 +1229,14 @@ describe('timeline/TrackContextMenu', function () { // add a couple of global screenshots tracks profile.threads.push({ - ...getThreadWithMarkers( + ...getThreadWithRawMarkers( profile.shared, getScreenshotMarkersForWindowId('0', 5) ), tid: profile.threads.length, }); profile.threads.push({ - ...getThreadWithMarkers( + ...getThreadWithRawMarkers( profile.shared, getScreenshotMarkersForWindowId('1', 5) ), diff --git a/src/test/components/TrackScreenshots.test.tsx b/src/test/components/TrackScreenshots.test.tsx index 43ccf90be8..8cfa6bdee1 100644 --- a/src/test/components/TrackScreenshots.test.tsx +++ b/src/test/components/TrackScreenshots.test.tsx @@ -204,8 +204,10 @@ describe('timeline/TrackScreenshots', function () { const profile = getScreenshotTrackProfile(); const { shared, threads } = profile; const [thread] = threads; - const markerIndexA = thread.markers.length - 3; - const markerIndexB = thread.markers.length - 2; + // Screenshots are stored as start / end pairs, so these are the start + // markers of the second to last and third to last screenshots. + const markerIndexA = thread.markers.length - 5; + const markerIndexB = thread.markers.length - 3; // We keep the last marker so that the profile's root range is correct. _setScreenshotMarkersToUnknown(thread, shared, markerIndexA, markerIndexB); @@ -232,8 +234,9 @@ describe('timeline/TrackScreenshots', function () { const { shared, threads } = profile; const [thread] = threads; + // The start markers of the first two screenshots. const markerIndexA = 0; - const markerIndexB = 1; + const markerIndexB = 2; _setScreenshotMarkersToUnknown(thread, shared, markerIndexA, markerIndexB); @@ -396,15 +399,11 @@ function _setScreenshotMarkersToUnknown( shared: RawProfileSharedData, ...markerIndexes: IndexIntoRawMarkerTable[] ) { - // Remove off the last few screenshot markers const stringTable = StringTable.withBackingArray(shared.stringArray); const unknownStringIndex = stringTable.indexForString('Unknown'); - const screenshotStringIndex = stringTable.indexForString( - 'CompositorScreenshot' - ); for (const markerIndex of markerIndexes) { // Double check that we've actually got screenshot markers: - if (thread.markers.name[markerIndex] !== screenshotStringIndex) { + if (thread.markers.data[markerIndex]?.type !== 'CompositorScreenshot') { throw new Error('This is not a screenshot marker.'); } thread.markers.name[markerIndex] = unknownStringIndex; diff --git a/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap b/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap index 19fbb30030..f5fda6d905 100644 --- a/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap +++ b/src/test/components/__snapshots__/TooltipMarker.test.tsx.snap @@ -1994,74 +1994,6 @@ exports[`TooltipMarker renders properly redirect network markers without additio
`; -exports[`TooltipMarker renders the tooltip of CompositorScreenshotWindowDestroyed 1`] = ` - -
-
-
-
-
- 1ms -
-
- - CompositorScreenshotWindowDestroyed - -
-
-
-
-
- Description - : -
-
- This marker spans the time between each composite of a window and shows the window contents during that time. -
-
- Window ID - : -
- XXX -
- Track - : -
- Empty -
-
-
- -`; - exports[`TooltipMarker renders tooltips for various markers: Bailout_ShapeGuard after getelem on line 3666 of resource://foo.js -> resource://bar.js:3662-10 1`] = `
- 2ms + 1ms
@@ -24,7 +24,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` >
@@ -34,7 +34,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` >
@@ -44,7 +44,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > @@ -54,7 +54,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > @@ -64,7 +64,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > @@ -74,7 +74,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > @@ -84,7 +84,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > @@ -94,7 +94,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > @@ -104,7 +104,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > @@ -114,7 +114,7 @@ exports[`timeline/TrackScreenshots matches the component snapshot 1`] = ` > diff --git a/src/test/fixtures/profiles/processed-profile.ts b/src/test/fixtures/profiles/processed-profile.ts index 4acc71d729..0576df7d8e 100644 --- a/src/test/fixtures/profiles/processed-profile.ts +++ b/src/test/fixtures/profiles/processed-profile.ts @@ -65,6 +65,7 @@ import type { import { FrameFlag } from 'firefox-profiler/types'; import { deriveMarkersFromRawMarkerTable, + getScreenshotMarkerName, IPCMarkerCorrelations, } from '../../../profile-logic/marker-data'; import { @@ -370,19 +371,33 @@ export function makeIntervalMarker( * A utility to make TestDefinedRawMarker */ export function makeCompositorScreenshot( - startTime: Milliseconds + startTime: Milliseconds, + windowID: string = '0' ): TestDefinedRawMarker { return { - ...makeInstantMarker('CompositorScreenshot', startTime), + ...makeStartMarker(getScreenshotMarkerName(windowID), startTime), data: { type: 'CompositorScreenshot', url: 0, - windowID: '', - windowSize: { width: 100, height: 100 }, + windowID, + windowSize: { width: 300, height: 150 }, }, }; } +/** + * A utility to make TestDefinedRawMarker + */ +export function makeCompositorScreenshotEnd( + endTime: Milliseconds, + windowID: string = '0' +): TestDefinedRawMarker { + return { + ...makeEndMarker(getScreenshotMarkerName(windowID), endTime), + data: { type: 'CompositorScreenshot', windowID }, + }; +} + export function getUserTiming( name: string, startTime: Milliseconds, @@ -421,6 +436,19 @@ export function getProfileWithMarkers( return profile; } +export function getProfileWithRawMarkers( + ...markersPerThread: TestDefinedRawMarker[][] +): Profile { + const profile = getEmptyProfile(); + profile.meta.markerSchema = markerSchemaForTests; + + profile.threads = markersPerThread.map((markers, i) => ({ + ...getThreadWithRawMarkers(profile.shared, markers), + tid: i, + })); + return profile; +} + /** * This profile is useful for marker table tests. The markers were taken from * real-world values. @@ -1373,39 +1401,33 @@ export function getIPCTrackProfile() { return getProfileWithMarkers(arrayOfIPCMarkers); } +/** + * One screenshot per millisecond, starting at 0. Each one ends where the next + * one starts; the last one is left open unless `destroyTime` is given. + */ export function getScreenshotMarkersForWindowId( windowID: string, - count: number -): TestDefinedMarker[] { - return Array(count) - .fill(undefined) - .map((_, i) => [ - 'CompositorScreenshot', - i, - null, - { - type: 'CompositorScreenshot', - url: 0, // Some arbitrary string. - windowID, - windowSize: { width: 300, height: 150 }, - }, - ]); + count: number, + destroyTime: Milliseconds | null = null +): TestDefinedRawMarker[] { + const markers: TestDefinedRawMarker[] = []; + for (let i = 0; i < count; i++) { + if (i > 0) { + markers.push(makeCompositorScreenshotEnd(i, windowID)); + } + markers.push(makeCompositorScreenshot(i, windowID)); + } + if (destroyTime !== null) { + markers.push(makeCompositorScreenshotEnd(destroyTime, windowID)); + } + return markers; } export function getScreenshotTrackProfile() { - return getProfileWithMarkers([ + return getProfileWithRawMarkers([ ...getScreenshotMarkersForWindowId('0', 5), // This window isn't closed, so we should repeat the last screenshot - ...getScreenshotMarkersForWindowId('1', 5), // This window is closed after screenshot 6. + ...getScreenshotMarkersForWindowId('1', 5, 6), // This window is closed after screenshot 6. ...getScreenshotMarkersForWindowId('2', 10), // This window isn't closed and define the profile length - [ - 'CompositorScreenshotWindowDestroyed', - 6, - null, - { - type: 'CompositorScreenshot', - windowID: '1', - }, - ], ]); } diff --git a/src/test/store/profile-view.test.ts b/src/test/store/profile-view.test.ts index 127d1bca69..b22f5006d8 100644 --- a/src/test/store/profile-view.test.ts +++ b/src/test/store/profile-view.test.ts @@ -2079,9 +2079,9 @@ describe('actions/ProfileView', function () { const screenshots = screenshotMarkersById.get('0'); expect(screenshots?.length).toEqual(5); for (const screenshot of screenshots ?? []) { - expect(screenshot.name).toEqual('CompositorScreenshot'); + expect(screenshot.name).toEqual('CompositorScreenshot 0'); } - expect(screenshotMarkersById.get('1')?.length).toEqual(6); + expect(screenshotMarkersById.get('1')?.length).toEqual(5); expect(screenshotMarkersById.get('2')?.length).toEqual(10); }); @@ -2090,9 +2090,9 @@ describe('actions/ProfileView', function () { const [{ markers }] = profile.threads; const { dispatch, getState } = storeWithProfile(profile); - // Double check that there are 21 markers in the test data, and commit a - // subsection of that range. - expect(markers.length).toBe(21); + // Double check that there are 38 raw markers in the test data, and commit + // a subsection of that range. + expect(markers.length).toBe(38); dispatch(ProfileView.commitRange(3.1, 7.5)); // Get out the markers. diff --git a/src/test/unit/__snapshots__/marker-data.test.ts.snap b/src/test/unit/__snapshots__/marker-data.test.ts.snap index 344a6dbae1..47bf7283f3 100644 --- a/src/test/unit/__snapshots__/marker-data.test.ts.snap +++ b/src/test/unit/__snapshots__/marker-data.test.ts.snap @@ -186,7 +186,8 @@ Array [ }, }, "end": 25, - "name": "CompositorScreenshot", + "incomplete": true, + "name": "CompositorScreenshot 0x136888400", "start": 25, "threadId": null, }, diff --git a/src/test/unit/__snapshots__/profile-conversion.test.ts.snap b/src/test/unit/__snapshots__/profile-conversion.test.ts.snap index 3bffbb2edf..4a0e5b346c 100644 --- a/src/test/unit/__snapshots__/profile-conversion.test.ts.snap +++ b/src/test/unit/__snapshots__/profile-conversion.test.ts.snap @@ -2323,11 +2323,11 @@ Object { Object { "isMainThread": true, "jsAllocationCount": 0, - "markerCount": 483, + "markerCount": 512, "markerNamesTop": Array [ "MessageLoop::RunTask (376)", "BrowserCrApplication::sendEvent (74)", - "CompositorScreenshot (30)", + "CompositorScreenshot id (59)", "LatencyInfo.Flow (2)", "TracingStartedInBrowser (1)", ], @@ -2715,11 +2715,11 @@ Object { Object { "isMainThread": true, "jsAllocationCount": 0, - "markerCount": 483, + "markerCount": 512, "markerNamesTop": Array [ "MessageLoop::RunTask (376)", "BrowserCrApplication::sendEvent (74)", - "CompositorScreenshot (30)", + "CompositorScreenshot id (59)", "LatencyInfo.Flow (2)", "TracingStartedInBrowser (1)", ], diff --git a/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap b/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap index 0a32ee9e50..7bb9b4e2df 100644 --- a/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap +++ b/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap @@ -11350,6 +11350,7 @@ Object { "firefox-webcontent", "setTimeout handler", "Element.getBoundingClientRect", + "CompositorScreenshot 0x136888400", ], }, "threads": Array [ @@ -11368,6 +11369,7 @@ Object { 5, 1, 4, + 4, 7, 4, ], @@ -11431,6 +11433,10 @@ Object { "type": "DOMEvent", }, Object {}, + Object { + "type": "CompositorScreenshot", + "windowID": "0x136888400", + }, Object { "type": "CompositorScreenshot", "url": 4, @@ -11475,40 +11481,43 @@ Object { null, 10, 12, + 12, null, 13, - null, + 14, ], - "length": 13, + "length": 14, "name": Array [ 0, 1, 2, 2, - 3, + 20, 1, 5, 6, 6, 7, - 3, + 20, + 20, 8, - 9, + 20, ], "phase": Array [ 0, 2, 2, 3, - 0, + 2, 3, 0, 2, 3, 1, - 0, + 3, + 2, 1, - 0, + 3, ], "startTime": Array [ 0, @@ -11521,9 +11530,10 @@ Object { 9, null, 11, + null, 12, 3, - 14, + null, ], }, "name": "GeckoMain", diff --git a/src/test/unit/marker-data.test.ts b/src/test/unit/marker-data.test.ts index 1880882309..c2f5687b53 100644 --- a/src/test/unit/marker-data.test.ts +++ b/src/test/unit/marker-data.test.ts @@ -31,6 +31,7 @@ import { makeIntervalMarker, makeInstantMarker, makeCompositorScreenshot, + makeCompositorScreenshotEnd, makeStartMarker, makeEndMarker, } from '../fixtures/profiles/processed-profile'; @@ -325,7 +326,7 @@ describe('Derive markers from Gecko phase markers', function () { ]); }); - it('has special handling for CompositorScreenshot', function () { + it('turns the instant CompositorScreenshot markers into intervals', function () { const basePayload = { type: 'CompositorScreenshot' as const, url: 16, @@ -350,34 +351,33 @@ describe('Derive markers from Gecko phase markers', function () { const startTimesForWindowA = [2, 5]; const startTimesForWindowB = [3, 6]; + const windowBDestroyedTime = 8; + + const screenshot = ( + startTime: number, + data: ScreenshotPayload + ): TestDefinedGeckoMarker => ({ + name: 'CompositorScreenshot', + startTime, + endTime: null, + phase: INSTANT, + data, + }); + const { markers, getState } = setupWithTestDefinedMarkers([ + screenshot(startTimesForWindowA[0], payloadsForWindowA[0]), + screenshot(startTimesForWindowB[0], payloadsForWindowB[0]), + screenshot(startTimesForWindowA[1], payloadsForWindowA[1]), + screenshot(startTimesForWindowB[1], payloadsForWindowB[1]), { - name: 'CompositorScreenshot', - startTime: startTimesForWindowA[0], - endTime: null, - phase: INTERVAL_START, - data: payloadsForWindowA[0], - }, - { - name: 'CompositorScreenshot', - startTime: startTimesForWindowB[0], - endTime: null, - phase: INTERVAL_START, - data: payloadsForWindowB[0], - }, - { - name: 'CompositorScreenshot', - startTime: startTimesForWindowA[1], + name: 'CompositorScreenshotWindowDestroyed', + startTime: windowBDestroyedTime, endTime: null, - phase: INTERVAL_START, - data: payloadsForWindowA[1], - }, - { - name: 'CompositorScreenshot', - startTime: startTimesForWindowB[1], - endTime: null, - phase: INTERVAL_START, - data: payloadsForWindowB[1], + phase: INSTANT, + data: { + type: 'CompositorScreenshot', + windowID: payloadsForWindowB[0].windowID, + }, }, ]); @@ -387,7 +387,7 @@ describe('Derive markers from Gecko phase markers', function () { // The two firsts have a duration from the first screenshot to the next in // the same window. { - name: 'CompositorScreenshot', + name: 'CompositorScreenshot 0xAAAAAAAAA', data: { ...payloadsForWindowA[0], url: expect.anything(), @@ -398,7 +398,7 @@ describe('Derive markers from Gecko phase markers', function () { threadId: null, }, { - name: 'CompositorScreenshot', + name: 'CompositorScreenshot 0xBBBBBBBBB', data: { ...payloadsForWindowB[0], url: expect.anything(), @@ -409,9 +409,10 @@ describe('Derive markers from Gecko phase markers', function () { threadId: null, }, - // The 2 lasts have a duration until the end of the thread range. + // Window A is still open, so its last screenshot is extended to the end + // of the thread range. { - name: 'CompositorScreenshot', + name: 'CompositorScreenshot 0xAAAAAAAAA', data: { ...payloadsForWindowA[1], url: expect.anything(), @@ -420,15 +421,18 @@ describe('Derive markers from Gecko phase markers', function () { end: threadRange.end, category: 0, threadId: null, + incomplete: true, }, + + // Window B was destroyed, so its last screenshot ends there. { - name: 'CompositorScreenshot', + name: 'CompositorScreenshot 0xBBBBBBBBB', data: { ...payloadsForWindowB[1], url: expect.anything(), }, start: startTimesForWindowB[1], - end: threadRange.end, + end: windowBDestroyedTime, category: 0, threadId: null, }, @@ -500,7 +504,7 @@ describe('deriveMarkersFromRawMarkerTable', function () { 'tracing:ArbitraryName', 'Network:Load 32: https://github.com/rustwasm/wasm-bindgen/issues/5', 'FileIO:FileIO', - 'CompositorScreenshot:CompositorScreenshot', + 'CompositorScreenshot:CompositorScreenshot 0x136888400', 'PreferenceRead:PreferenceRead', 'Text:RefreshDriverTick', 'NoPayloadUserData:Navigation::Start', @@ -813,9 +817,10 @@ describe('deriveMarkersFromRawMarkerTable', function () { windowID: '0x136888400', windowSize: { width: 1280, height: 1000 }, }, - name: 'CompositorScreenshot', + name: 'CompositorScreenshot 0x136888400', start: 25, end: 25, + incomplete: true, }); }); }); @@ -953,12 +958,17 @@ describe('filterRawMarkerTableToRange', () => { end: 5.6, markers: [ makeCompositorScreenshot(0), + makeCompositorScreenshotEnd(3), makeCompositorScreenshot(3), + makeCompositorScreenshotEnd(7), makeCompositorScreenshot(7), ], }); - expect(rawMarkerTable.startTime).toEqual([0, 3]); + // Both markers of a screenshot's pair are kept, and the screenshot taken + // after the range is dropped. + expect(rawMarkerTable.startTime).toEqual([0, null, 3, null]); + expect(rawMarkerTable.endTime).toEqual([null, 3, null, 7]); }); it('keeps a screenshot markers happening before the range if there is no other marker', () => { @@ -971,7 +981,7 @@ describe('filterRawMarkerTableToRange', () => { makeInstantMarker('EndMarkerOutOfRange', 8), ], }); - expect(processedMarkerNames).toEqual(['CompositorScreenshot']); + expect(processedMarkerNames).toEqual(['CompositorScreenshot 0']); }); it('filters network markers', () => { From 5804096065377ddc50dd68d889844e10f5ad89e8 Mon Sep 17 00:00:00 2001 From: fatadel Date: Thu, 27 Aug 2026 15:22:46 +0200 Subject: [PATCH 3/3] Create screenshot tracks from marker schema The CompositorScreenshot schema now declares a timeline-screenshots display location. computeGlobalTracks uses the combined front-end and profile schema list to identify marker types for screenshot tracks instead of naming the payload type directly. Window IDs remain in first-screenshot order so global track indexes referenced by shared URLs stay stable. --- docs-developer/CHANGELOG-formats.md | 2 ++ src/profile-logic/marker-schema.ts | 2 +- src/profile-logic/tracks.ts | 21 ++++++++++++++++----- src/test/store/tracks.test.ts | 23 +++++++++++++++++++++++ src/types/markers.ts | 2 ++ 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/docs-developer/CHANGELOG-formats.md b/docs-developer/CHANGELOG-formats.md index f6bdd74299..c02a3890f9 100644 --- a/docs-developer/CHANGELOG-formats.md +++ b/docs-developer/CHANGELOG-formats.md @@ -14,6 +14,8 @@ These markers are now stored as pairs of start and end markers instead of instan Two marker schema field formats were added to describe these markers: `screenshot-size`, whose value is a `{ width, height }` object, and `screenshot-data-url`, an object format `{ type: "screenshot-data-url", sizeFieldForAspectRatio }` whose value is a string table index holding an image data URL. Profiles containing `CompositorScreenshot` markers don't necessarily carry a schema for them, so the front end supplies one. +A new marker schema display location, `timeline-screenshots`, was added. Markers in this location create Screenshot tracks and must carry a string or number `windowID` field. + ### Version 71 The frame table (`profile.shared.frameTable`) representation changed in such a way that all its columns can now be typed arrays when using [JsonSlabs](https://github.com/mstange/json-slabs/) profiles. diff --git a/src/profile-logic/marker-schema.ts b/src/profile-logic/marker-schema.ts index 74d7ada8dd..790b5ec9da 100644 --- a/src/profile-logic/marker-schema.ts +++ b/src/profile-logic/marker-schema.ts @@ -101,7 +101,7 @@ export const markerSchemaFrontEndOnly: MarkerSchema[] = [ // A profile can contain these markers without carrying a schema for them, // so the front end defines one. name: 'CompositorScreenshot', - display: ['marker-chart', 'marker-table'], + display: ['marker-chart', 'marker-table', 'timeline-screenshots'], fields: [ { key: 'url', diff --git a/src/profile-logic/tracks.ts b/src/profile-logic/tracks.ts index 079ddd182a..747bba6efd 100644 --- a/src/profile-logic/tracks.ts +++ b/src/profile-logic/tracks.ts @@ -713,6 +713,13 @@ export function computeGlobalTracks( }; const globalTracksByPid: Map = new Map(); let globalTracks: GlobalTrack[] = []; + const markerSchema = computeCombinedMarkerSchemaList( + profile.meta.markerSchema || [] + ); + const screenshotTimelineMarkerTypes = getMarkerTypesForDisplay( + markerSchema, + 'timeline-screenshots' + ); // Create the global tracks. for ( @@ -754,14 +761,18 @@ export function computeGlobalTracks( } } - // Check for screenshots. Their marker name carries the window ID, so match - // on the payload type instead. Windows must keep being added in the order - // their first screenshot was taken: shared URLs refer to global tracks by - // index, so a different order would change what an existing URL selects. + // Windows must keep being added in the order their first screenshot was + // taken: shared URLs refer to global tracks by index, so a different order + // would change what an existing URL selects. const ids: Set = new Set(); for (let markerIndex = 0; markerIndex < markers.length; markerIndex++) { const data = markers.data[markerIndex]; - if (data === null || data.type !== 'CompositorScreenshot') { + if ( + data === null || + !screenshotTimelineMarkerTypes.has(data.type) || + !('windowID' in data) || + (typeof data.windowID !== 'string' && typeof data.windowID !== 'number') + ) { continue; } ids.add(String(data.windowID)); diff --git a/src/test/store/tracks.test.ts b/src/test/store/tracks.test.ts index 4a128f35be..80d0984101 100644 --- a/src/test/store/tracks.test.ts +++ b/src/test/store/tracks.test.ts @@ -345,6 +345,29 @@ describe('ordering and hiding', function () { ]); }); + it('creates screenshot tracks from the schema display location', function () { + const customScreenshot = { + type: 'Url' as const, + url: '', + windowID: 42, + }; + const profile = getProfileWithMarkers([ + ['CustomScreenshot', 0, null, customScreenshot], + ]); + profile.meta.markerSchema.push({ + name: 'Url', + display: ['marker-chart', 'timeline-screenshots'], + fields: [], + }); + + const { getState } = storeWithProfile(profile); + expect(ProfileViewSelectors.getGlobalTracks(getState())).toContainEqual({ + type: 'screenshots', + id: '42', + threadIndex: 0, + }); + }); + describe('sorting of track types to ensure proper URL backwards compatibility', function () { const stableIndexOrder = [ 'process', diff --git a/src/types/markers.ts b/src/types/markers.ts index b143779e1d..595a7ce63f 100644 --- a/src/types/markers.ts +++ b/src/types/markers.ts @@ -125,6 +125,8 @@ export type MarkerDisplayLocation = | 'timeline-fileio' // This adds markers to the Network track. | 'timeline-network' + // This adds markers to the Screenshots track. + | 'timeline-screenshots' // TODO - This is not supported yet. | 'stack-chart';