RPP: support new format screenshot events

Once crrev.com/c/6197645 lands the screenshot trace event will be
completely changed in terms of shape. This is to enable us to accurately
relate screenshots to animation frames. This CL does not do that work,
but instead does the work to update the trace engine types and the code
to be able to handle both screenshot events which are of different
shapes.

Bug: 390127829
Change-Id: Iad9e563977b772ed185ea98554ce5c72392f61ea
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6216264
Reviewed-by: Andres Olivares <[email protected]>
Auto-Submit: Jack Franklin <[email protected]>
Commit-Queue: Andres Olivares <[email protected]>
Commit-Queue: Jack Franklin <[email protected]>

Author: jackfranklin

front_end/models/trace/extras/FilmStrip.test.ts

@@ -7,10 +7,55 @@ import {TraceLoader} from '../../../testing/TraceLoader.js';
 import * as Trace from '../trace.js';
 
 describeWithEnvironment('FilmStrip', function() {
-  it('identifies the frames from a trace', async function() {
-    const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
-    const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
-    assert.lengthOf(filmStrip.frames, 5);
+  /**
+   * See `models/trace/types/TraceEvents.ts` for the type of `LegacyScreenshot`
+   * and the comment explaining the differences in the "old" and "new" format.
+   */
+  describe('using the old screenshot format', () => {
+    it('identifies the frames from a trace', async function() {
+      const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
+      const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
+      assert.lengthOf(filmStrip.frames, 5);
+    });
+
+    it('exposes the snapshot string for each frame', async function() {
+      const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
+      const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
+      assert.isTrue(filmStrip.frames.every(frame => {
+        return Trace.Types.Events.isLegacySyntheticScreenshot(frame.screenshotEvent) &&
+            typeof frame.screenshotEvent.args.dataUri === 'string' && frame.screenshotEvent.args.dataUri.length > 0;
+      }));
+    });
+
+    it('can use a custom zero time to filter out screenshots', async function() {
+      const {screenshotImageDataUri} = Trace.Handlers.ModelHandlers.Screenshots;
+      const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
+      const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
+      // Set a custom zero time after the first screenshot and ensure that we now only have four events.
+      const newCustomZeroTime = Trace.Types.Timing.Micro(filmStrip.frames[0].screenshotEvent.ts + 1000);
+      const newFilmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace, newCustomZeroTime);
+      // Check that the new film strip is all the frames other than the first, now we have set a custom time.
+      assert.deepEqual(newFilmStrip.frames.map(f => screenshotImageDataUri(f.screenshotEvent)), [
+        screenshotImageDataUri(filmStrip.frames[1].screenshotEvent),
+        screenshotImageDataUri(filmStrip.frames[2].screenshotEvent),
+        screenshotImageDataUri(filmStrip.frames[3].screenshotEvent),
+        screenshotImageDataUri(filmStrip.frames[4].screenshotEvent),
+      ]);
+    });
+  });
+
+  describe('using the new screenshot format', () => {
+    it('identifies the frames from a trace', async function() {
+      const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-screenshot-source-ids.json.gz');
+      const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
+      assert.lengthOf(filmStrip.frames, 20);
+    });
+
+    it('has an array of new screenshot events', async function() {
+      const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-screenshot-source-ids.json.gz');
+      const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
+      assert.isTrue(filmStrip.frames.every(f => Trace.Types.Events.isScreenshot(f.screenshotEvent)));
+    });
   });
 
   it('caches the film strip based on the trace data and the zero time', async function() {
@@ -26,29 +71,6 @@ describeWithEnvironment('FilmStrip', function() {
     assert.notStrictEqual(filmStrip3, filmStrip4);
   });
 
-  it('exposes the snapshot string for each frame', async function() {
-    const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
-    const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
-    assert.isTrue(filmStrip.frames.every(frame => {
-      return typeof frame.screenshotEvent.args.dataUri === 'string' && frame.screenshotEvent.args.dataUri.length > 0;
-    }));
-  });
-
-  it('can use a custom zero time to filter out screenshots', async function() {
-    const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
-    const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);
-    // Set a custom zero time after the first screenshot and ensure that we now only have four events.
-    const newCustomZeroTime = Trace.Types.Timing.Micro(filmStrip.frames[0].screenshotEvent.ts + 1000);
-    const newFilmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace, newCustomZeroTime);
-    // Check that the new film strip is all the frames other than the first, now we have set a custom time.
-    assert.deepEqual(newFilmStrip.frames.map(f => f.screenshotEvent.args.dataUri), [
-      filmStrip.frames[1].screenshotEvent.args.dataUri,
-      filmStrip.frames[2].screenshotEvent.args.dataUri,
-      filmStrip.frames[3].screenshotEvent.args.dataUri,
-      filmStrip.frames[4].screenshotEvent.args.dataUri,
-    ]);
-  });
-
   it('can return the frame closest to a given timestamp', async function() {
     const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
     const filmStrip = Trace.Extras.FilmStrip.fromParsedTrace(parsedTrace);

front_end/models/trace/extras/FilmStrip.ts

@@ -13,7 +13,7 @@ export interface Data {
 }
 
 export interface Frame {
-  screenshotEvent: Types.Events.SyntheticScreenshot;
+  screenshotEvent: Types.Events.LegacySyntheticScreenshot|Types.Events.Screenshot;
   index: number;
 }
 
@@ -42,7 +42,9 @@ export function fromParsedTrace(parsedTrace: HandlerDataWithScreenshots, customZ
     return fromCache;
   }
 
-  for (const screenshotEvent of parsedTrace.Screenshots.all) {
+  const screenshots = parsedTrace.Screenshots.screenshots ?? parsedTrace.Screenshots.legacySyntheticScreenshots ?? [];
+
+  for (const screenshotEvent of screenshots) {
     if (screenshotEvent.ts < zeroTime) {
       continue;
     }

front_end/models/trace/handlers/LayoutShiftsHandler.ts

@@ -183,10 +183,19 @@ function updateTraceWindowMax(traceWindow: Types.Timing.TraceWindowMicro, newMax
 }
 
 function findScreenshots(timestamp: Types.Timing.Micro): Types.Events.LayoutShiftParsedData['screenshots'] {
-  const screenshots = screenshotsHandlerData().all;
-  const before = Helpers.Trace.findPreviousEventBeforeTimestamp(screenshots, timestamp);
-  const after = before ? screenshots[screenshots.indexOf(before) + 1] : null;
-  return {before, after};
+  const data = screenshotsHandlerData();
+  if (data.screenshots) {
+    const before = Helpers.Trace.findPreviousEventBeforeTimestamp(data.screenshots, timestamp);
+    const after = before ? data.screenshots[data.screenshots.indexOf(before) + 1] : null;
+    return {before, after};
+  }
+  if (data.legacySyntheticScreenshots) {
+    const before = Helpers.Trace.findPreviousEventBeforeTimestamp(data.legacySyntheticScreenshots, timestamp);
+    const after = before ? data.legacySyntheticScreenshots[data.legacySyntheticScreenshots.indexOf(before) + 1] : null;
+    return {before, after};
+  }
+  // No screenshots
+  return {before: null, after: null};
 }
 
 function buildScoreRecords(): void {

front_end/models/trace/handlers/ScreenshotsHandler.test.ts

@@ -2,66 +2,53 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import {defaultTraceEvent} from '../../../testing/TraceHelpers.js';
 import {TraceLoader} from '../../../testing/TraceLoader.js';
 import * as Trace from '../trace.js';
 
 describe('ScreenshotsHandler', function() {
-  const baseEvent = {
-    ...defaultTraceEvent,
-    name: 'Screenshot',
-    // Ensure that the screenshots are held against the pid & tid values
-    // that match the Browser process and CrBrowserMain in defaultTraceEvents.
-    pid: Trace.Types.Events.ProcessID(8017),
-    tid: Trace.Types.Events.ThreadID(775),
-    ts: Trace.Types.Timing.Micro(0),
-    args: {},
-    cat: 'test',
-    ph: Trace.Types.Events.Phase.OBJECT_SNAPSHOT,
-  };
-
-  let baseEvents: readonly Trace.Types.Events.Event[];
-
   beforeEach(async function() {
-    // The screenshot handler requires the meta handler because it needs
-    // to know the browser process and thread IDs. Here, then, we reset
-    // and later we will pass events to the meta handler, otherwise the
-    // screenshots handler will fail.
     Trace.Handlers.ModelHandlers.Meta.reset();
-
     Trace.Handlers.ModelHandlers.Screenshots.reset();
   });
+  describe('supporting old and new screenshot formats', () => {
+    async function runHandler(events: readonly Trace.Types.Events.Event[]) {
+      Trace.Helpers.SyntheticEvents.SyntheticEventsManager.createAndActivate(events);
 
-  describe('frames', () => {
-    it('obtains them if present', async function() {
-      const defaultTraceEvents = await TraceLoader.rawEvents(this, 'basic.json.gz');
-
-      baseEvents = [
-        ...defaultTraceEvents,
-        {...baseEvent, ts: Trace.Types.Timing.Micro(100)},
-        {...baseEvent, ts: Trace.Types.Timing.Micro(200)},
-      ];
-      Trace.Helpers.SyntheticEvents.SyntheticEventsManager.createAndActivate(baseEvents);
-
-      for (const event of baseEvents) {
+      for (const event of events) {
         Trace.Handlers.ModelHandlers.Meta.handleEvent(event);
         Trace.Handlers.ModelHandlers.Screenshots.handleEvent(event);
       }
 
       await Trace.Handlers.ModelHandlers.Meta.finalize();
       await Trace.Handlers.ModelHandlers.Screenshots.finalize();
+    }
+
+    it('finds the screenshots in traces using the OBJECT_SNAPSHOT screenshot format', async function() {
+      const events = await TraceLoader.rawEvents(this, 'web-dev-with-commit.json.gz');
+      await runHandler(events);
+      const data = Trace.Handlers.ModelHandlers.Screenshots.data();
+      assert.isOk(data.legacySyntheticScreenshots);
+      assert.isNull(data.screenshots);
+      assert.lengthOf(data.legacySyntheticScreenshots, 18);
+    });
 
-      const data = Trace.Handlers.ModelHandlers.Screenshots.data().all;
-      assert.lengthOf(data, 2);
+    // TODO: leave explainer comment here
+    it('finds the screenshots in traces using the new instant event screenshot format', async function() {
+      const events = await TraceLoader.rawEvents(this, 'web-dev-screenshot-source-ids.json.gz');
+      await runHandler(events);
+      const data = Trace.Handlers.ModelHandlers.Screenshots.data();
+      assert.isOk(data.screenshots);
+      assert.isNull(data.legacySyntheticScreenshots);
+      assert.lengthOf(data.screenshots, 20);
     });
   });
 
   describe('presentation timestamps', () => {
     function getMsDifferences(
-        syntheticScreenshots: Trace.Types.Events.SyntheticScreenshot[],
-        originalScreenshotEvents: Trace.Types.Events.Screenshot[]): number[] {
+        syntheticScreenshots: Trace.Types.Events.LegacySyntheticScreenshot[],
+        originalScreenshotEvents: Trace.Types.Events.LegacyScreenshot[]): number[] {
       return syntheticScreenshots.map((synEvent, i) => {
-        const origEvent = originalScreenshotEvents.at(i) as Trace.Types.Events.Screenshot;
+        const origEvent = originalScreenshotEvents.at(i) as Trace.Types.Events.LegacyScreenshot;
         const msDifference = (synEvent.ts - origEvent.ts) / 1000;
         return msDifference;
       });
@@ -77,8 +64,9 @@ describe('ScreenshotsHandler', function() {
       await Trace.Handlers.ModelHandlers.Meta.finalize();
       await Trace.Handlers.ModelHandlers.Screenshots.finalize();
 
-      const syntheticScreenshots = Trace.Handlers.ModelHandlers.Screenshots.data().all;
-      const originalScreenshotEvents = events.filter(Trace.Types.Events.isScreenshot);
+      const syntheticScreenshots = Trace.Handlers.ModelHandlers.Screenshots.data().legacySyntheticScreenshots;
+      assert.isOk(syntheticScreenshots);
+      const originalScreenshotEvents = events.filter(Trace.Types.Events.isLegacyScreenshot);
       assert.strictEqual(syntheticScreenshots.length, originalScreenshotEvents.length);
 
       for (const oEvent of originalScreenshotEvents) {
@@ -103,8 +91,9 @@ describe('ScreenshotsHandler', function() {
       await Trace.Handlers.ModelHandlers.Meta.finalize();
       await Trace.Handlers.ModelHandlers.Screenshots.finalize();
 
-      const syntheticScreenshots = Trace.Handlers.ModelHandlers.Screenshots.data().all;
-      const originalScreenshotEvents = events.filter(Trace.Types.Events.isScreenshot);
+      const syntheticScreenshots = Trace.Handlers.ModelHandlers.Screenshots.data().legacySyntheticScreenshots;
+      const originalScreenshotEvents = events.filter(Trace.Types.Events.isLegacyScreenshot);
+      assert.isOk(syntheticScreenshots);
       assert.strictEqual(syntheticScreenshots.length, originalScreenshotEvents.length);
 
       for (const oEvent of originalScreenshotEvents) {

front_end/models/trace/handlers/ScreenshotsHandler.ts

@@ -11,20 +11,24 @@ import type {HandlerName} from './types.js';
 // used to store the event in the correct process thread entry below.
 const unpairedAsyncEvents: Types.Events.PipelineReporter[] = [];
 
-const snapshotEvents: Types.Events.Screenshot[] = [];
-const syntheticScreenshots: Types.Events.SyntheticScreenshot[] = [];
+const legacyScreenshotEvents: Types.Events.LegacyScreenshot[] = [];
+const modernScreenshotEvents: Types.Events.Screenshot[] = [];
+const syntheticScreenshots: Types.Events.LegacySyntheticScreenshot[] = [];
 let frameSequenceToTs: Record<string, Types.Timing.Micro> = {};
 
 export function reset(): void {
   unpairedAsyncEvents.length = 0;
-  snapshotEvents.length = 0;
+  legacyScreenshotEvents.length = 0;
   syntheticScreenshots.length = 0;
+  modernScreenshotEvents.length = 0;
   frameSequenceToTs = {};
 }
 
 export function handleEvent(event: Types.Events.Event): void {
-  if (Types.Events.isScreenshot(event)) {
-    snapshotEvents.push(event);
+  if (Types.Events.isLegacyScreenshot(event)) {
+    legacyScreenshotEvents.push(event);
+  } else if (Types.Events.isScreenshot(event)) {
+    modernScreenshotEvents.push(event);
   } else if (Types.Events.isPipelineReporter(event)) {
     unpairedAsyncEvents.push(event);
   }
@@ -39,10 +43,10 @@ export async function finalize(): Promise<void> {
     return [frameSequenceId, presentationTs];
   }));
 
-  for (const snapshotEvent of snapshotEvents) {
+  for (const snapshotEvent of legacyScreenshotEvents) {
     const {cat, name, ph, pid, tid} = snapshotEvent;
     const syntheticEvent = Helpers.SyntheticEvents.SyntheticEventsManager.registerSyntheticEvent<
-        Types.Events.SyntheticScreenshot>({
+        Types.Events.LegacySyntheticScreenshot>({
       rawSourceEvent: snapshotEvent,
       cat,
       name,
@@ -60,14 +64,21 @@ export async function finalize(): Promise<void> {
   }
 }
 
+export function screenshotImageDataUri(event: Types.Events.LegacySyntheticScreenshot|Types.Events.Screenshot): string {
+  if (Types.Events.isLegacySyntheticScreenshot(event)) {
+    return event.args.dataUri;
+  }
+  return `data:image/jpg;base64,${event.args.snapshot}`;
+}
+
 /**
  * Correct the screenshot timestamps
  * The screenshot 'snapshot object' trace event has the "frame sequence number" attached as an ID.
  * We match that up with the "PipelineReporter" trace events as they terminate at presentation.
  * Presentation == when the pixels hit the screen. AKA Swap on the GPU
  */
 // eslint-disable-next-line @typescript-eslint/no-unused-vars
-function getPresentationTimestamp(screenshotEvent: Types.Events.Screenshot): Types.Timing.Micro {
+function getPresentationTimestamp(screenshotEvent: Types.Events.LegacyScreenshot): Types.Timing.Micro {
   const frameSequence = parseInt(screenshotEvent.id, 16);
   // If it's 1, then it's an old trace (before https://crrev.com/c/4957973) and cannot be corrected.
   if (frameSequence === 1) {
@@ -85,9 +96,23 @@ function getPresentationTimestamp(screenshotEvent: Types.Events.Screenshot): Typ
   return updatedTs ?? screenshotEvent.ts;
 }
 
+export interface Data {
+  // These are nullable because in January 2025 a CL in Chromium
+  // crrev.com/c/6197645 landed which changed the format of screenshots. For a
+  // given trace, it can have either "legacy" screenshot events, or "modern"
+  // screenshot events, but no trace can ever contain both.
+  // So, if either of these arrays are empty, we instead return `null`. This forces consumers to check the presence of the array.
+  // Traces can have no screenshots if the trace category is not enabled, so it
+  // is possible for a trace to return null for both of these arrays.
+  legacySyntheticScreenshots: Types.Events.LegacySyntheticScreenshot[]|null;
+  screenshots: Types.Events.Screenshot[]|null;
+}
 // TODO(crbug/41484172): should be readonly
-export function data(): ({all: Types.Events.SyntheticScreenshot[]}) {
-  return {all: syntheticScreenshots};
+export function data(): Data {
+  return {
+    legacySyntheticScreenshots: syntheticScreenshots.length ? syntheticScreenshots : null,
+    screenshots: modernScreenshotEvents.length ? modernScreenshotEvents : null,
+  };
 }
 
 export function deps(): HandlerName[] {

front_end/models/trace/helpers/SyntheticEvents.test.ts

@@ -45,7 +45,7 @@ describe('SyntheticEvents', function() {
          const allSyntheticEvents = [
            ...parsedTrace.Animations.animations,
            ...parsedTrace.NetworkRequests.byTime,
-           ...parsedTrace.Screenshots.all,
+           ...parsedTrace.Screenshots.legacySyntheticScreenshots ?? [],
          ];
          const syntheticEventsManager = Trace.Helpers.SyntheticEvents.SyntheticEventsManager.getActiveManager();
          for (const syntheticEvent of allSyntheticEvents) {

front_end/models/trace/helpers/Trace.test.ts

@@ -732,7 +732,7 @@ describeWithEnvironment('Trace helpers', function() {
   describe('findNextEventAfterTimestamp', () => {
     it('gets the first screenshot after a trace', async function() {
       const {parsedTrace} = await TraceLoader.traceEngine(this, 'cls-multiple-frames.json.gz');
-      const screenshots = parsedTrace.Screenshots.all;
+      const screenshots = parsedTrace.Screenshots.legacySyntheticScreenshots ?? [];
       const {clusters} = parsedTrace.LayoutShifts;
       const shifts = clusters.flatMap(cluster => cluster.events);
       assert.isAtLeast(shifts.length, 10);