Make PerformanceModel processing async

In preparation to resolve function names in the perfomance panel using
source maps, this CL implements two functional changes:

1. The loadingComplete function of the PerformanceModel is set to be
   async. This is because the name resolving will require the result
   of async invocations and this function is ideal for adding the
   calls into (since this function commands the trace processing after
   all trace events are available). With this change, other functions in
   the call chain are updated accordingly (this includes calls in the
   legacy TimlineTestRunner)

2. To avoid race conditions, the PerformanceModel instance owned by the
   TimelinePanel is not used until the TimelineController has finished
   the processing pipeline. Before this CL, this waiting didn't happen
   made the code subject to race conditions, which were made evident
   with the implementation of (1).

Tests in Chromium land that make use of the updated helpers in the test
runner need to be updated before this CL can land.

Bug: 1364601
Change-Id: I2b7586337793eafd570e98947dcfbb574e6e43e5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3941705
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Andres Olivares <andoli@chromium.org>

front_end/legacy_test_runner/performance_test_runner/TimelineTestRunner.js

@@ -102,8 +102,7 @@ PerformanceTestRunner.invokeWithTracing = function(functionName, callback, addit
   }
 
   function onPageActionsDone() {
-    PerformanceTestRunner.runWhenTimelineIsReady(callback);
-    timelineController.stopRecording();
+    timelineController.stopRecording().then(callback);
   }
 };
 
@@ -119,12 +118,12 @@ PerformanceTestRunner.timelineFrameModel = function() {
   return PerformanceTestRunner.performanceModel().frameModel();
 };
 
-PerformanceTestRunner.createPerformanceModelWithEvents = function(events) {
+PerformanceTestRunner.createPerformanceModelWithEvents = async function(events) {
   const tracingModel = new SDK.TracingModel(new Bindings.TempFileBackingStorage('tracing'));
   tracingModel.addEvents(events);
   tracingModel.tracingComplete();
   const performanceModel = new Timeline.PerformanceModel();
-  performanceModel.setTracingModel(tracingModel);
+  await performanceModel.setTracingModel(tracingModel);
   UI.panels.timeline.performanceModel = performanceModel;
   UI.panels.timeline.applyFilters(performanceModel);
   return performanceModel;
@@ -137,7 +136,7 @@ PerformanceTestRunner.createTimelineController = function() {
 };
 
 PerformanceTestRunner.runWhenTimelineIsReady = function(callback) {
-  TestRunner.addSniffer(UI.panels.timeline, 'loadingComplete', () => callback());
+  TestRunner.addSniffer(UI.panels.timeline, 'loadingCompleteForTest', () => callback());
 };
 
 PerformanceTestRunner.startTimeline = function() {
@@ -146,11 +145,8 @@ PerformanceTestRunner.startTimeline = function() {
   return TestRunner.addSnifferPromise(panel, 'recordingStarted');
 };
 
-PerformanceTestRunner.stopTimeline = function() {
-  return new Promise(resolve => {
-    PerformanceTestRunner.runWhenTimelineIsReady(resolve);
-    UI.panels.timeline.toggleRecording();
-  });
+PerformanceTestRunner.stopTimeline = async function() {
+  await UI.panels.timeline.toggleRecording();
 };
 
 PerformanceTestRunner.runPerfTraceWithReload = async function() {
@@ -405,12 +401,8 @@ PerformanceTestRunner.dumpTimelineFlameChart = function(includeGroups) {
   PerformanceTestRunner.dumpFlameChartProvider(provider, includeGroups);
 };
 
-PerformanceTestRunner.loadTimeline = function(timelineData) {
-  const promise = new Promise(fulfill => PerformanceTestRunner.runWhenTimelineIsReady(fulfill));
-
-  UI.panels.timeline.loadFromFile(new Blob([timelineData], {type: 'text/plain'}));
-
-  return promise;
+PerformanceTestRunner.loadTimeline = async function(timelineData) {
+  await UI.panels.timeline.loadFromFile(new Blob([timelineData], {type: 'text/plain'}));
 };
 
 TestRunner.deprecatedInitAsync(`

front_end/models/bindings/FileUtils.ts

@@ -163,19 +163,19 @@ export class ChunkedFileReader implements ChunkedReader {
     }
 
     if (endOfFile) {
-      this.finishRead();
+      void this.finishRead();
       return;
     }
     void this.loadChunk();
   }
 
-  private finishRead(): void {
+  private async finishRead(): Promise<void> {
     if (!this.#output) {
       return;
     }
     this.#file = null;
     this.#reader = null;
-    void this.#output.close();
+    await this.#output.close();
     this.#transferFinished(!this.#errorInternal);
   }
 

front_end/panels/timeline/PerformanceModel.ts

@@ -71,7 +71,7 @@ export class PerformanceModel extends Common.ObjectWrapper.ObjectWrapper<EventTy
     return this.filtersInternal.every(f => f.accept(event));
   }
 
-  setTracingModel(model: SDK.TracingModel.TracingModel): void {
+  async setTracingModel(model: SDK.TracingModel.TracingModel): Promise<void> {
     this.tracingModelInternal = model;
     this.timelineModelInternal.setEvents(model);
 

front_end/panels/timeline/TimelineController.ts

@@ -142,7 +142,11 @@ export class TimelineController implements SDK.TargetManager.SDKModelObserver<SD
 
     this.client.loadingStarted();
     await this.waitForTracingToStop(true);
-    this.allSourcesFinished();
+    await this.allSourcesFinished();
+    return this.performanceModel;
+  }
+
+  getPerformanceModel(): PerformanceModel {
     return this.performanceModel;
   }
 
@@ -232,16 +236,17 @@ export class TimelineController implements SDK.TargetManager.SDKModelObserver<SD
     this.tracingCompleteCallback = null;
   }
 
-  private allSourcesFinished(): void {
+  private async allSourcesFinished(): Promise<void> {
     this.client.processingStarted();
-    window.setTimeout(() => this.finalizeTrace(), 0);
+    await this.finalizeTrace();
   }
 
   private async finalizeTrace(): Promise<void> {
     this.injectCpuProfileEvents();
     await SDK.TargetManager.TargetManager.instance().resumeAllTargets();
     this.tracingModel.tracingComplete();
-    this.client.loadingComplete(this.tracingModel);
+    await this.client.loadingComplete(this.tracingModel);
+    this.client.loadingCompleteForTest();
   }
 
   private injectCpuProfileEvent(pid: number, tid: number, cpuProfile: Protocol.Profiler.Profile|null): void {
@@ -381,6 +386,7 @@ export interface Client {
   processingStarted(): void;
   loadingProgress(progress?: number): void;
   loadingComplete(tracingModel: SDK.TracingModel.TracingModel|null): void;
+  loadingCompleteForTest(): void;
 }
 export interface RecordingOptions {
   enableJSSampling?: boolean;

front_end/panels/timeline/TimelineLoader.ts

@@ -63,18 +63,17 @@ export class TimelineLoader implements Common.StringOutputStream.OutputStream {
     this.jsonTokenizer = new TextUtils.TextUtils.BalancedJSONTokenizer(this.writeBalancedJSON.bind(this), true);
   }
 
-  static loadFromFile(file: File, client: Client): TimelineLoader {
+  static async loadFromFile(file: File, client: Client): Promise<TimelineLoader> {
     const loader = new TimelineLoader(client);
     const fileReader = new Bindings.FileUtils.ChunkedFileReader(file, TransferChunkLengthBytes);
     loader.canceledCallback = fileReader.cancel.bind(fileReader);
     loader.totalSize = file.size;
-    void fileReader.read(loader).then(success => {
-      if (!success && fileReader.error()) {
-        // TODO(crbug.com/1172300) Ignored during the jsdoc to ts migration
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        loader.reportErrorAndCancelLoading((fileReader.error() as any).message);
-      }
-    });
+    const success = await fileReader.read(loader);
+    if (!success && fileReader.error()) {
+      // TODO(crbug.com/1172300) Ignored during the jsdoc to ts migration
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      loader.reportErrorAndCancelLoading((fileReader.error() as any).message);
+    }
     return loader;
   }
 
@@ -247,16 +246,16 @@ export class TimelineLoader implements Common.StringOutputStream.OutputStream {
       return;
     }
     this.client.processingStarted();
-    window.setTimeout(() => this.finalizeTrace(), 0);
+    await this.finalizeTrace();
   }
 
-  private finalizeTrace(): void {
+  private async finalizeTrace(): Promise<void> {
     if (this.state === State.LoadingCPUProfileFormat) {
       this.parseCPUProfileFormat(this.buffer);
       this.buffer = '';
     }
     (this.tracingModel as SDK.TracingModel.TracingModel).tracingComplete();
-    (this.client as Client).loadingComplete(this.tracingModel);
+    await (this.client as Client).loadingComplete(this.tracingModel);
   }
 
   private parseCPUProfileFormat(text: string): void {

front_end/panels/timeline/TimelinePanel.ts

@@ -719,12 +719,12 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     }
   }
 
-  private loadFromFile(file: File): void {
+  private async loadFromFile(file: File): Promise<void> {
     if (this.state !== State.Idle) {
       return;
     }
     this.prepareToLoadTimeline();
-    this.loader = TimelineLoader.loadFromFile(file, this);
+    this.loader = await TimelineLoader.loadFromFile(file, this);
     this.createFileSelector();
   }
 
@@ -889,8 +889,8 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
           .then(widget => widget.stopRecording());
     }
     if (this.controller) {
-      const model = await this.controller.stopRecording();
-      this.performanceModel = model;
+      this.performanceModel = this.controller.getPerformanceModel();
+      await this.controller.stopRecording();
       this.setUIControlsEnabled(true);
       this.controller.dispose();
       this.controller = null;
@@ -963,13 +963,13 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     this.saveButton.setEnabled(this.state === state.Idle && Boolean(this.performanceModel));
   }
 
-  toggleRecording(): void {
+  async toggleRecording(): Promise<void> {
     if (this.state === State.Idle) {
       this.recordingPageReload = false;
-      void this.startRecording();
+      await this.startRecording();
       Host.userMetrics.actionTaken(Host.UserMetrics.Action.TimelineStarted);
     } else if (this.state === State.Recording) {
-      void this.stopRecording();
+      await this.stopRecording();
     }
   }
 
@@ -1148,7 +1148,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     }
   }
 
-  loadingComplete(tracingModel: SDK.TracingModel.TracingModel|null): void {
+  async loadingComplete(tracingModel: SDK.TracingModel.TracingModel|null): Promise<void> {
     delete this.loader;
     this.setState(State.Idle);
 
@@ -1165,7 +1165,8 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     if (!this.performanceModel) {
       this.performanceModel = new PerformanceModel();
     }
-    this.performanceModel.setTracingModel(tracingModel);
+
+    await this.performanceModel.setTracingModel(tracingModel);
     this.setModel(this.performanceModel);
     this.historyManager.addRecording(this.performanceModel);
 
@@ -1178,6 +1179,10 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
     }
   }
 
+  loadingCompleteForTest(): void {
+    // Not implemented, added only for allowing the TimelineTestRunner
+    // to be in sync when a trace load is finished.
+  }
   private showRecordingStarted(): void {
     if (this.statusPane) {
       return;
@@ -1330,7 +1335,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
       if (!file) {
         return;
       }
-      this.loadFromFile(file);
+      void this.loadFromFile(file);
     }
   }
 }
@@ -1588,7 +1593,7 @@ export class ActionDelegate implements UI.ActionRegistration.ActionDelegate {
     console.assert(panel && panel instanceof TimelinePanel);
     switch (actionId) {
       case 'timeline.toggle-recording':
-        panel.toggleRecording();
+        void panel.toggleRecording();
         return true;
       case 'timeline.record-reload':
         panel.recordReload();

test/webtests/http/tests/devtools/a11y-axe-core/performance/performance-pane-a11y-test.js

@@ -37,8 +37,7 @@
   ];
 
   // create dummy data for test
-  const model =
-      PerformanceTestRunner.createPerformanceModelWithEvents(testData);
+  const model = await PerformanceTestRunner.createPerformanceModelWithEvents(testData);
 
   const detailsView = UI.panels.timeline.flameChart.detailsView;
 

test/webtests/http/tests/devtools/a11y-axe-core/performance/performance_event_log_a11y_test.js

@@ -14,7 +14,7 @@
 
   TestRunner.addResult('Loading a performance model.');
   const view = tabbedPane.visibleView;
-  const model = PerformanceTestRunner.createPerformanceModelWithEvents([{}]);
+  const model = await PerformanceTestRunner.createPerformanceModelWithEvents([{}]);
   view.setModel(model, PerformanceTestRunner.mainTrack());
   view.updateContents(Timeline.TimelineSelection.fromRange(
     model.timelineModel().minimumRecordTime(), model.timelineModel().maximumRecordTime()));