From 97e713a249e3127ee8ebc22c40a190f27e1181a0 Mon Sep 17 00:00:00 2001 From: fatadel Date: Thu, 13 Aug 2026 18:07:12 +0200 Subject: [PATCH 1/2] 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 6d1ed2b5c8..98adcb5c83 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 70 + +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 69 A new marker schema display location, `timeline-network`, was added. A marker schema can list `timeline-network` in its `display` array to have markers of that type surfaced in the Network track. diff --git a/src/app-logic/constants.ts b/src/app-logic/constants.ts index 1f6b47f2d5..f8a4d98330 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 = 69; +export const PROCESSED_PROFILE_VERSION = 70; // 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 0a448666e0..2959e4b341 100644 --- a/src/profile-logic/import/chrome.ts +++ b/src/profile-logic/import/chrome.ts @@ -889,8 +889,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 bb56860d7d..8c726f1d6a 100644 --- a/src/profile-logic/marker-schema.ts +++ b/src/profile-logic/marker-schema.ts @@ -85,6 +85,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. + `, + }, ]; /** @@ -554,9 +576,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)}` ); } } @@ -609,6 +634,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( @@ -642,7 +669,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.` @@ -673,6 +709,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' || @@ -684,17 +723,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 69c4712e7a..9eada0b863 100644 --- a/src/profile-logic/process-profile.ts +++ b/src/profile-logic/process-profile.ts @@ -907,12 +907,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 0999f5c078..cd37af63c8 100644 --- a/src/profile-logic/processed-profile-versioning.ts +++ b/src/profile-logic/processed-profile-versioning.ts @@ -3310,6 +3310,25 @@ const _upgraders: { } } }, + [70]: (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 4d1fe7ff09..108a6ce27d 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 938a5bc361..07d0708574 100644 --- a/src/test/components/TooltipMarker.test.tsx +++ b/src/test/components/TooltipMarker.test.tsx @@ -1156,8 +1156,7 @@ describe('TooltipMarker', function () { type: 'CompositorScreenshot', url: screenshotUrlIndex, windowID: 'XXX', - windowWidth: 600, - windowHeight: 300, + windowSize: { width: 600, height: 300 }, }, ], ]); @@ -1189,7 +1188,6 @@ describe('TooltipMarker', function () { { type: 'CompositorScreenshot', windowID: 'XXX', - url: undefined, }, ], ]); @@ -1276,8 +1274,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 dc886ee205..3117ba0f10 100644 --- a/src/test/fixtures/profiles/processed-profile.ts +++ b/src/test/fixtures/profiles/processed-profile.ts @@ -377,8 +377,7 @@ export function makeCompositorScreenshot( type: 'CompositorScreenshot', url: 0, windowID: '', - windowWidth: 100, - windowHeight: 100, + windowSize: { width: 100, height: 100 }, }, }; } @@ -1349,8 +1348,7 @@ export function getScreenshotMarkersForWindowId( type: 'CompositorScreenshot', url: 0, // Some arbitrary string. windowID, - windowWidth: 300, - windowHeight: 150, + windowSize: { width: 300, height: 150 }, }, ]); } @@ -1367,7 +1365,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 8df0ad49ac..69a5ffdc23 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": 69, + "preprocessedProfileVersion": 70, "processType": 0, "product": "a.out", "sampleUnits": Object { @@ -1452,7 +1452,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "processType": 0, "product": "a.out", "sampleUnits": Object { @@ -2817,7 +2817,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "processType": 0, "product": "a.out", "sampleUnits": Object { @@ -4182,7 +4182,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "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 c16fdb16e5..afb8dd010c 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": 69, + "preprocessedProfileVersion": 70, "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 a3831da47a..907fa724ce 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": 69, + "preprocessedProfileVersion": 70, "product": "ART Trace (Android)", "symbolicated": true, "version": 36, @@ -1022,7 +1022,7 @@ Object { "RefreshDriverTick", "Network", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "ART Trace (Android)", "symbolicated": true, "version": 36, @@ -2305,7 +2305,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -2697,7 +2697,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3086,7 +3086,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3187,7 +3187,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3540,7 +3540,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3605,7 +3605,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3759,7 +3759,7 @@ Object { "markerSchemaNames": Array [ "EventDispatch", ], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Chrome Trace", "symbolicated": true, "version": 36, @@ -3817,7 +3817,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4207,7 +4207,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4265,7 +4265,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4323,7 +4323,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Firefox", "symbolicated": true, "version": 36, @@ -4643,7 +4643,7 @@ Object { "importedFrom": "Simpleperf", "interval": 0, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "com.example.sampleapplication", "symbolicated": undefined, "version": 30, @@ -5019,7 +5019,7 @@ Object { "importedFrom": "Simpleperf", "interval": 0, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "com.example.sampleapplication", "symbolicated": undefined, "version": 30, @@ -5319,7 +5319,7 @@ Object { "importedFrom": "dhat", "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "target/debug/examples/work_log (dhat)", "symbolicated": true, "version": 36, @@ -5452,7 +5452,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "product": "Flamegraph", "symbolicated": true, "version": 36, @@ -5510,7 +5510,7 @@ Object { "importedFrom": undefined, "interval": 1, "markerSchemaNames": Array [], - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "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 394eaa6d2e..7605f513b0 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": 69, + "preprocessedProfileVersion": 70, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -7644,7 +7644,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -9021,7 +9021,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -10566,7 +10566,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 69, + "preprocessedProfileVersion": 70, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -10890,15 +10890,15 @@ Object { null, ], "name": Array [ - 7, - 8, - 9, 10, 11, - 8, - 9, - 15, - 16, + 12, + 13, + 14, + 11, + 12, + 18, + 19, ], "originalLocation": Array [ null, @@ -10966,10 +10966,10 @@ Object { 1, ], "name": Array [ - 8, - 9, - 8, - 9, + 11, + 12, + 11, + 12, ], }, "resourceTable": Object { @@ -10985,9 +10985,9 @@ Object { 1, ], "name": Array [ - 13, - 12, - 14, + 16, + 15, + 17, ], "type": Array [ 1, @@ -11006,7 +11006,7 @@ Object { null, ], "filename": Array [ - 12, + 15, ], "id": Array [ null, @@ -11085,10 +11085,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", @@ -11112,10 +11115,13 @@ Object { 4, 4, 4, + 4, 5, 5, 1, + 4, 7, + 4, ], "data": Array [ Object { @@ -11141,6 +11147,15 @@ Object { "interval": "end", "type": "tracing", }, + Object { + "type": "CompositorScreenshot", + "url": 4, + "windowID": "0x136888400", + "windowSize": Object { + "height": 1000, + "width": 1280, + }, + }, Object { "category": "Paint", "interval": "end", @@ -11168,6 +11183,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, @@ -11187,55 +11211,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", @@ -11377,9 +11417,9 @@ Object { 2, 2, 1, - 4, - 4, - 5, + 6, + 6, + 7, ], "phase": Array [ 0, @@ -11563,10 +11603,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 c064168786..85d1135d1b 100644 --- a/src/test/unit/merge-compare.test.ts +++ b/src/test/unit/merge-compare.test.ts @@ -687,8 +687,7 @@ describe('mergeThreads function', function () { type: 'CompositorScreenshot', url: screenshot1UrlIndex, windowID: 'XXX', - windowWidth: 300, - windowHeight: 600, + windowSize: { width: 300, height: 600 }, }, ], ]); @@ -702,8 +701,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 25cac35a6a..e8a49720fd 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 @@ -691,30 +698,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'; @@ -887,7 +894,7 @@ export type MarkerPayload_Gecko = | GCMajorMarkerPayload_Gecko | GCSliceMarkerPayload_Gecko | VsyncTimestampPayload - | ScreenshotPayload + | ScreenshotPayload_Gecko | CcMarkerTracing | ArbitraryEventTracing | NavigationMarkerPayload From a0d2a9335a991ad48f098ea51635c5e6caed9547 Mon Sep 17 00:00:00 2001 From: fatadel Date: Fri, 14 Aug 2026 13:45:47 +0200 Subject: [PATCH 2/2] 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 98adcb5c83..01de815fb3 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 69 diff --git a/src/profile-logic/import/chrome.ts b/src/profile-logic/import/chrome.ts index 2959e4b341..2c1d5ee2f4 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 { @@ -29,6 +32,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: @@ -878,6 +882,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); @@ -885,18 +912,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 9eada0b863..e41b02d9b4 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'; @@ -49,6 +50,7 @@ import { getFriendlyThreadName, nudgeReturnAddresses, } from '../profile-logic/profile-data'; +import { getScreenshotMarkerName } from './marker-data'; import { toInt32Array, toUint8Array, @@ -941,6 +943,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; @@ -1357,13 +1444,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 cd37af63c8..f660f2e843 100644 --- a/src/profile-logic/processed-profile-versioning.ts +++ b/src/profile-logic/processed-profile-versioning.ts @@ -3312,21 +3312,99 @@ const _upgraders: { }, [70]: (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 108a6ce27d..eb304ddfe3 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 07d0708574..05518e6d64 100644 --- a/src/test/components/TooltipMarker.test.tsx +++ b/src/test/components/TooltipMarker.test.tsx @@ -1179,22 +1179,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 3117ba0f10..5c9a368e4e 100644 --- a/src/test/fixtures/profiles/processed-profile.ts +++ b/src/test/fixtures/profiles/processed-profile.ts @@ -64,6 +64,7 @@ import type { } from 'firefox-profiler/types'; import { deriveMarkersFromRawMarkerTable, + getScreenshotMarkerName, IPCMarkerCorrelations, } from '../../../profile-logic/marker-data'; import { @@ -369,19 +370,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, @@ -420,6 +435,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. @@ -1334,39 +1362,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 907fa724ce..892fc08ddb 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 7605f513b0..99c7339e87 100644 --- a/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap +++ b/src/test/unit/__snapshots__/profile-upgrading.test.ts.snap @@ -11102,6 +11102,7 @@ Object { "firefox-webcontent", "setTimeout handler", "Element.getBoundingClientRect", + "CompositorScreenshot 0x136888400", ], }, "threads": Array [ @@ -11120,6 +11121,7 @@ Object { 5, 1, 4, + 4, 7, 4, ], @@ -11183,6 +11185,10 @@ Object { "type": "DOMEvent", }, Object {}, + Object { + "type": "CompositorScreenshot", + "windowID": "0x136888400", + }, Object { "type": "CompositorScreenshot", "url": 4, @@ -11227,40 +11233,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, @@ -11273,9 +11282,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', () => {