[RPP] Begin using ParsedTraceFile as definitive way to pass around trace

Trace.TraceModel.Model and TraceLoader contained internal objects that
wrapped a trace, it's parsed handler data, insights, and metadata. But
this wasn't available to outside callers. Turns out, we want to pass
around all of these things together pretty often, so let's start doing
that.

This CL should have no functional changes, and is a first pass at
refactoring this trace data into a simpler interface.

Bug: 358583420
Change-Id: I115577695b8a58f36ee3c2957d0b518ceea82192
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6931525
Reviewed-by: Paul Irish <[email protected]>
Auto-Submit: Connor Clark <[email protected]>
Commit-Queue: Connor Clark <[email protected]>

Author: Connor Clark

front_end/models/ai_assistance/agents/PerformanceAgent.test.ts

@@ -269,7 +269,8 @@ const FAKE_INP_MODEL = {
 const FAKE_PARSED_TRACE = {
   Meta: {traceBounds: {min: 0, max: 10}, mainFrameURL: 'https://www.example.com'},
 } as unknown as Trace.Handlers.Types.ParsedTrace;
-const FAKE_INSIGHTS = new Map() as unknown as Trace.Insights.Types.TraceInsightSets;
+const FAKE_INSIGHTS = new Map([['', {model: {LCPBreakdown: FAKE_LCP_MODEL, INPBreakdown: FAKE_INP_MODEL}}]]) as
+    unknown as Trace.Insights.Types.TraceInsightSets;
 const FAKE_METADATA = {} as unknown as Trace.Types.File.MetaData;
 
 function createAgentForInsightConversation(opts: {aidaClient?: Host.AidaClient.AidaClient} = {}) {

front_end/models/trace/ModelImpl.ts

@@ -100,14 +100,8 @@ export class Model extends EventTarget {
 
     this.#processor.addEventListener(TraceParseProgressEvent.eventName, onTraceUpdate);
 
-    // Create a parsed trace file.  It will be populated with data from the processor.
-    const file: ParsedTraceFile = {
-      traceEvents,
-      metadata,
-      parsedTrace: null,
-      traceInsights: null,
-      syntheticEventsManager: Helpers.SyntheticEvents.SyntheticEventsManager.createAndActivate(traceEvents),
-    };
+    // TODO(cjamcl): this.#processor.parse needs this to work. So it should either take it as input, or create it itself.
+    const syntheticEventsManager = Helpers.SyntheticEvents.SyntheticEventsManager.createAndActivate(traceEvents);
 
     try {
       // Wait for all outstanding promises before finishing the async execution,
@@ -119,7 +113,11 @@ export class Model extends EventTarget {
         resolveSourceMap: config?.resolveSourceMap,
       };
       await this.#processor.parse(traceEvents, parseConfig);
-      this.#storeParsedFileData(file, this.#processor.parsedTrace, this.#processor.insights);
+      if (!this.#processor.parsedTrace) {
+        throw new Error('processor did not parse trace');
+      }
+      const file = this.#storeAndCreateParsedTraceFile(
+          syntheticEventsManager, traceEvents, metadata, this.#processor.parsedTrace, this.#processor.insights);
       // We only push the file onto this.#traces here once we know it's valid
       // and there's been no errors in the parsing.
       this.#traces.push(file);
@@ -133,39 +131,51 @@ export class Model extends EventTarget {
     }
   }
 
-  #storeParsedFileData(
-      file: ParsedTraceFile, data: Handlers.Types.ParsedTrace|null,
-      insights: Insights.Types.TraceInsightSets|null): void {
-    file.parsedTrace = data;
-    file.traceInsights = insights;
+  #storeAndCreateParsedTraceFile(
+      syntheticEventsManager: Helpers.SyntheticEvents.SyntheticEventsManager,
+      traceEvents: readonly Types.Events.Event[], metadata: Types.File.MetaData, data: Handlers.Types.ParsedTrace,
+      traceInsights: Insights.Types.TraceInsightSets|null): ParsedTraceFile {
     this.#lastRecordingIndex++;
     let recordingName = `Trace ${this.#lastRecordingIndex}`;
-    let origin: string|null = null;
-    if (file.parsedTrace) {
-      origin = Helpers.Trace.extractOriginFromTrace(file.parsedTrace.Meta.mainFrameURL);
-      if (origin) {
-        const nextSequenceForDomain = Platform.MapUtilities.getWithDefault(this.#nextNumberByDomain, origin, () => 1);
-        recordingName = `${origin} (${nextSequenceForDomain})`;
-        this.#nextNumberByDomain.set(origin, nextSequenceForDomain + 1);
-      }
+    const origin = Helpers.Trace.extractOriginFromTrace(data.Meta.mainFrameURL);
+    if (origin) {
+      const nextSequenceForDomain = Platform.MapUtilities.getWithDefault(this.#nextNumberByDomain, origin, () => 1);
+      recordingName = `${origin} (${nextSequenceForDomain})`;
+      this.#nextNumberByDomain.set(origin, nextSequenceForDomain + 1);
     }
     this.#recordingsAvailable.push(recordingName);
+
+    return {
+      traceEvents,
+      metadata,
+      parsedTrace: data,
+      insights: traceInsights,
+      syntheticEventsManager,
+    };
   }
 
   lastTraceIndex(): number {
     return this.size() - 1;
   }
 
+  findIndexForParsedTrace(parsedTrace: Handlers.Types.ParsedTrace): number {
+    return this.#traces.findIndex(file => file.parsedTrace === parsedTrace);
+  }
+
   /**
    * Returns the parsed trace data indexed by the order in which it was stored.
    * If no index is given, the last stored parsed data is returned.
    */
+  parsedTraceFile(index: number = this.#traces.length - 1): ParsedTraceFile|null {
+    return this.#traces.at(index) ?? null;
+  }
+
   parsedTrace(index: number = this.#traces.length - 1): Handlers.Types.ParsedTrace|null {
     return this.#traces.at(index)?.parsedTrace ?? null;
   }
 
   traceInsights(index: number = this.#traces.length - 1): Insights.Types.TraceInsightSets|null {
-    return this.#traces.at(index)?.traceInsights ?? null;
+    return this.#traces.at(index)?.insights ?? null;
   }
 
   metadata(index: number = this.#traces.length - 1): Types.File.MetaData|null {
@@ -211,8 +221,9 @@ export class Model extends EventTarget {
  * essentially the TraceFile plus whatever the model has parsed from it.
  */
 export type ParsedTraceFile = Types.File.TraceFile&{
-  parsedTrace: Handlers.Types.ParsedTrace | null,
-  traceInsights: Insights.Types.TraceInsightSets | null,
+  parsedTrace: Handlers.Types.ParsedTrace,
+  /** Is null for CPU profiles. */
+  insights: Insights.Types.TraceInsightSets | null,
   syntheticEventsManager: Helpers.SyntheticEvents.SyntheticEventsManager,
 };
 

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

@@ -44,13 +44,13 @@ describeWithEnvironment('Handler Threads helper', function() {
         Trace.Types.Events.ThreadID(1),
     );
 
-    const {parsedTrace} = await TraceLoader.executeTraceEngineOnFileContents(contents);
+    const {parsedTraceFile} = await TraceLoader.executeTraceEngineOnFileContents(contents);
 
     // Check that we did indeed parse this properly as a CPU Profile.
-    assert.strictEqual(parsedTrace.Renderer.processes.size, 0);
-    assert.strictEqual(parsedTrace.Samples.profilesInProcess.size, 1);
+    assert.strictEqual(parsedTraceFile.parsedTrace.Renderer.processes.size, 0);
+    assert.strictEqual(parsedTraceFile.parsedTrace.Samples.profilesInProcess.size, 1);
 
-    const threads = Trace.Handlers.Threads.threadsInTrace(parsedTrace);
+    const threads = Trace.Handlers.Threads.threadsInTrace(parsedTraceFile.parsedTrace);
     assert.lengthOf(threads, 1);
 
     assert.strictEqual(threads.at(0)?.type, Trace.Handlers.Threads.ThreadType.CPU_PROFILE);

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

@@ -41,11 +41,11 @@ describe('SyntheticEvents', function() {
        async function() {
          const contents = await TraceLoader.fixtureContents(this, 'web-dev.json.gz');
          const rawEvents = 'traceEvents' in contents ? contents.traceEvents as Trace.Types.Events.Event[] : contents;
-         const {parsedTrace} = await TraceLoader.executeTraceEngineOnFileContents(rawEvents);
+         const {parsedTraceFile} = await TraceLoader.executeTraceEngineOnFileContents(rawEvents);
          const allSyntheticEvents = [
-           ...parsedTrace.Animations.animations,
-           ...parsedTrace.NetworkRequests.byTime,
-           ...parsedTrace.Screenshots.legacySyntheticScreenshots ?? [],
+           ...parsedTraceFile.parsedTrace.Animations.animations,
+           ...parsedTraceFile.parsedTrace.NetworkRequests.byTime,
+           ...parsedTraceFile.parsedTrace.Screenshots.legacySyntheticScreenshots ?? [],
          ];
          const syntheticEventsManager = Trace.Helpers.SyntheticEvents.SyntheticEventsManager.getActiveManager();
          for (const syntheticEvent of allSyntheticEvents) {

front_end/models/trace/insights/DuplicatedJavaScript.test.ts

@@ -114,8 +114,8 @@ describeWithEnvironment('DuplicatedJavaScript', function() {
       }
     }
 
-    const parsedTraceData = await TraceLoader.executeTraceEngineOnFileContents(fileContents);
-    const {parsedTrace: data, insights} = parsedTraceData;
+    const {parsedTraceFile} = await TraceLoader.executeTraceEngineOnFileContents(fileContents);
+    const {parsedTrace: data, insights} = parsedTraceFile;
     if (!insights) {
       throw new Error('invalid insights');
     }

front_end/panels/timeline/ModificationsManager.ts

@@ -75,25 +75,19 @@ export class ModificationsManager extends EventTarget {
       activeManager = modificationsManagerByTraceIndex[traceIndex];
       ModificationsManager.activeManager()?.applyModificationsIfPresent();
     }
-    const parsedTrace = traceModel.parsedTrace(traceIndex);
-    if (!parsedTrace) {
+
+    const parsedTraceFile = traceModel.parsedTraceFile(traceIndex);
+    if (!parsedTraceFile) {
       throw new Error('ModificationsManager was initialized without a corresponding trace data');
     }
+
+    const {parsedTrace, metadata, traceEvents, syntheticEventsManager} = parsedTraceFile;
     const traceBounds = parsedTrace.Meta.traceBounds;
-    const traceEvents = traceModel.rawTraceEvents(traceIndex);
-    if (!traceEvents) {
-      throw new Error('ModificationsManager was initialized without a corresponding raw trace events array');
-    }
-    const syntheticEventsManager = traceModel.syntheticTraceEventsManager(traceIndex);
-    if (!syntheticEventsManager) {
-      throw new Error('ModificationsManager was initialized without a corresponding SyntheticEventsManager');
-    }
-    const metadata = traceModel.metadata(traceIndex);
     const newModificationsManager = new ModificationsManager({
       parsedTrace,
       traceBounds,
       rawTraceEvents: traceEvents,
-      modifications: metadata?.modifications,
+      modifications: metadata.modifications,
       syntheticEvents: syntheticEventsManager.getSyntheticTraces(),
     });
     modificationsManagerByTraceIndex[traceIndex] = newModificationsManager;

front_end/panels/timeline/TimelinePanel.ts

@@ -1377,12 +1377,9 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
     if (this.#viewMode.mode !== 'VIEWING_TRACE') {
       return;
     }
-    const trace = this.#traceEngineModel.parsedTrace(this.#viewMode.traceIndex);
-    if (!trace) {
-      return;
-    }
-    let traceEvents = this.#traceEngineModel.rawTraceEvents(this.#viewMode.traceIndex);
-    if (!traceEvents) {
+
+    const parsedTraceFile = this.#traceEngineModel.parsedTraceFile(this.#viewMode.traceIndex);
+    if (!parsedTraceFile) {
       return;
     }
 
@@ -1394,9 +1391,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
       scriptByIdMap.set(`${mapScript.isolate}.${mapScript.scriptId}`, mapScript);
     }
 
-    const metadata = this.#traceEngineModel.metadata(this.#viewMode.traceIndex) ?? {};
-
-    traceEvents = traceEvents.map(event => {
+    const traceEvents = parsedTraceFile.traceEvents.map(event => {
       if (Trace.Types.Events.isAnyScriptCatchupEvent(event) && event.name !== 'StubScriptCatchup') {
         const mappedScript = scriptByIdMap.get(`${event.args.data.isolate}.${event.args.data.scriptId}`);
 
@@ -1422,6 +1417,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
       return event;
     });
 
+    const metadata = parsedTraceFile.metadata;
     metadata.modifications = config.addModifications ? ModificationsManager.activeManager()?.toJSON() : undefined;
 
     // NOTE: we used to export the track configuration changes into the trace

front_end/panels/timeline/components/Utils.test.ts

@@ -71,8 +71,8 @@ describeWithEnvironment('Utils', () => {
 
     before(async function() {
       const events = await TraceLoader.fixtureContents(this, 'load-simple.json.gz');
-      const {parsedTrace} = await TraceLoader.executeTraceEngineOnFileContents(events);
-      req = parsedTrace.NetworkRequests.byId.get('2648544.35');
+      const {parsedTraceFile} = await TraceLoader.executeTraceEngineOnFileContents(events);
+      req = parsedTraceFile.parsedTrace.NetworkRequests.byId.get('2648544.35');
     });
 
     function tweakRequest(

front_end/panels/timeline/utils/AICallTree.test.ts

@@ -60,7 +60,8 @@ describeWithEnvironment('AICallTree', () => {
         rawEvents,
         Trace.Types.Events.ThreadID(1),
     );
-    const {parsedTrace} = await TraceLoader.executeTraceEngineOnFileContents(events);
+    const {parsedTraceFile} = await TraceLoader.executeTraceEngineOnFileContents(events);
+    const parsedTrace = parsedTraceFile.parsedTrace;
     // Find a random function call in the trace.
     const funcCall = parsedTrace.Samples.entryToNode.keys().find(event => {
       return Trace.Types.Events.isProfileCall(event) && event.callFrame.functionName === 'callAndPauseOnStart';

front_end/testing/TraceHelpers.ts

@@ -36,7 +36,7 @@ export interface RenderFlameChartOptions {
    * name so that the TraceLoader can take care of loading and caching the
    * trace.
    */
-  traceFile: string|Trace.Handlers.Types.ParsedTrace;
+  fileNameOrParsedTrace: string|Trace.Handlers.Types.ParsedTrace;
   /**
    * Filter the tracks that will be rendered by their name. The name here is
    * the user visible name that is drawn onto the flame chart.
@@ -90,10 +90,10 @@ export async function renderFlameChartIntoDOM(context: Mocha.Context|null, optio
 
   let parsedTrace: Trace.Handlers.Types.ParsedTrace|null = null;
 
-  if (typeof options.traceFile === 'string') {
-    parsedTrace = (await TraceLoader.traceEngine(context, options.traceFile)).parsedTrace;
+  if (typeof options.fileNameOrParsedTrace === 'string') {
+    parsedTrace = (await TraceLoader.traceEngine(context, options.fileNameOrParsedTrace)).parsedTrace;
   } else {
-    parsedTrace = options.traceFile;
+    parsedTrace = options.fileNameOrParsedTrace;
   }
 
   if (options.preloadScreenshots) {