[RPP] Deduplicate |appendEventAtLevel| function of track appenders

Move appendEventAtLevel to CompatibilityBuilder to reduce the duplicate codes.

Bug: 1427378
Change-Id: I3fd4c2b513e0022e7122c3420a788a79c62554c5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4512963
Reviewed-by: Andres Olivares <andoli@chromium.org>
Commit-Queue: Nancy Li <nancyly@chromium.org>

front_end/panels/timeline/CompatibilityTracksAppender.ts

@@ -6,7 +6,11 @@ import * as TraceEngine from '../../models/trace/trace.js';
 import type * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
 import * as SDK from '../../core/sdk/sdk.js';
 import type * as TimelineModel from '../../models/timeline_model/timeline_model.js';
-import {type TimelineFlameChartEntry, type EntryType} from './TimelineFlameChartDataProvider.js';
+import {
+  type TimelineFlameChartEntry,
+  EntryType,
+  InstantEventVisibleDurationMs,
+} from './TimelineFlameChartDataProvider.js';
 import {TimingsTrackAppender} from './TimingsTrackAppender.js';
 import {InteractionsTrackAppender} from './InteractionsTrackAppender.js';
 import {GPUTrackAppender} from './GPUTrackAppender.js';
@@ -33,7 +37,7 @@ export type HighlightedEntryInfo = {
  * removed. This processes of doing this for a track is referred to as
  * "migrating the track" to the new system.
  *
- * The migration implementation will result benefitial among other
+ * The migration implementation will result beneficial among other
  * things because the complexity of rendering the details of each track
  * is distributed among multiple standalone modules.
  * Read more at go/rpp-flamechart-arch
@@ -118,22 +122,21 @@ export class CompatibilityTracksAppender {
     this.#legacyEntryTypeByLevel = legacyEntryTypeByLevel;
     this.#legacyTimelineModel = legacyTimelineModel;
 
-    this.#timingsTrackAppender = new TimingsTrackAppender(
-        this, this.#flameChartData, this.#traceParsedData, this.#entryData, this.#legacyEntryTypeByLevel);
+    this.#timingsTrackAppender =
+        new TimingsTrackAppender(this, this.#flameChartData, this.#traceParsedData, this.#legacyEntryTypeByLevel);
     this.#allTrackAppenders.push(this.#timingsTrackAppender);
 
-    this.#interactionsTrackAppender = new InteractionsTrackAppender(
-        this, this.#flameChartData, this.#traceParsedData, this.#entryData, this.#legacyEntryTypeByLevel);
+    this.#interactionsTrackAppender =
+        new InteractionsTrackAppender(this, this.#flameChartData, this.#traceParsedData, this.#legacyEntryTypeByLevel);
     this.#allTrackAppenders.push(this.#interactionsTrackAppender);
 
-    this.#gpuTrackAppender = new GPUTrackAppender(
-        this, this.#flameChartData, this.#traceParsedData, this.#entryData, this.#legacyEntryTypeByLevel);
+    this.#gpuTrackAppender = new GPUTrackAppender(this, this.#traceParsedData, this.#legacyEntryTypeByLevel);
     this.#allTrackAppenders.push(this.#gpuTrackAppender);
 
     // Layout Shifts track in OPP was called the "Experience" track even though
     // all it shows are layout shifts.
-    this.#layoutShiftsTrackAppender = new LayoutShiftsTrackAppender(
-        this, this.#flameChartData, this.#traceParsedData, this.#entryData, this.#legacyEntryTypeByLevel);
+    this.#layoutShiftsTrackAppender =
+        new LayoutShiftsTrackAppender(this, this.#flameChartData, this.#traceParsedData, this.#legacyEntryTypeByLevel);
     this.#allTrackAppenders.push(this.#layoutShiftsTrackAppender);
   }
 
@@ -306,9 +309,34 @@ export class CompatibilityTracksAppender {
    * style, title, etc.) is needed.
    */
   registerTrackForLevel(level: number, appender: TrackAppender): void {
+    // TODO(crbug.com/1442454) Figure out how to avoid the circular calls.
     this.#trackForLevel.set(level, appender);
   }
 
+  /**
+   * Adds an event to the flame chart data at a defined level.
+   * @param event the event to be appended,
+   * @param level the level to append the event,
+   * @param appender the track which the event belongs to.
+   * @returns the index of the event in all events to be rendered in the flamechart.
+   */
+  appendEventAtLevel(event: TraceEngine.Types.TraceEvents.TraceEventData, level: number, appender: TrackAppender):
+      number {
+    // TODO(crbug.com/1442454) Figure out how to avoid the circular calls.
+    this.#trackForLevel.set(level, appender);
+
+    const index = this.#entryData.length;
+    this.#entryData.push(event);
+    this.#legacyEntryTypeByLevel[level] = EntryType.TrackAppender;
+    this.#flameChartData.entryLevels[index] = level;
+    this.#flameChartData.entryStartTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(event.ts);
+    const msDuration = event.dur ||
+        TraceEngine.Helpers.Timing.millisecondsToMicroseconds(
+            InstantEventVisibleDurationMs as TraceEngine.Types.Timing.MilliSeconds);
+    this.#flameChartData.entryTotalTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(msDuration);
+    return index;
+  }
+
   /**
    * Gets the all track appenders that have been set to be visible.
    */

front_end/panels/timeline/GPUTrackAppender.ts

@@ -1,9 +1,8 @@
 // Copyright 2023 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-import * as TraceEngine from '../../models/trace/trace.js';
-import type * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
 import * as i18n from '../../core/i18n/i18n.js';
+import * as TraceEngine from '../../models/trace/trace.js';
 
 import {
   type TrackAppender,
@@ -13,8 +12,6 @@ import {
 } from './CompatibilityTracksAppender.js';
 import {
   EntryType,
-  InstantEventVisibleDurationMs,
-  type TimelineFlameChartEntry,
 } from './TimelineFlameChartDataProvider.js';
 import {buildGroupStyle, buildTrackHeader, getFormattedTime, getSyncEventLevel} from './AppenderUtils.js';
 
@@ -32,9 +29,7 @@ export class GPUTrackAppender implements TrackAppender {
   readonly appenderName: TrackAppenderName = 'GPU';
 
   #compatibilityBuilder: CompatibilityTracksAppender;
-  #flameChartData: PerfUI.FlameChart.FlameChartTimelineData;
   #traceParsedData: Readonly<TraceEngine.TraceModel.PartialTraceParseDataDuringMigration>;
-  #entryData: TimelineFlameChartEntry[];
   // TODO(crbug.com/1416533)
   // This is used only for compatibility with the legacy flame chart
   // architecture of the panel. Once all tracks have been migrated to
@@ -43,13 +38,11 @@ export class GPUTrackAppender implements TrackAppender {
   #legacyEntryTypeByLevel: EntryType[];
 
   constructor(
-      compatibilityBuilder: CompatibilityTracksAppender, flameChartData: PerfUI.FlameChart.FlameChartTimelineData,
+      compatibilityBuilder: CompatibilityTracksAppender,
       traceParsedData: TraceEngine.TraceModel.PartialTraceParseDataDuringMigration,
-      entryData: TimelineFlameChartEntry[], legacyEntryTypeByLevel: EntryType[]) {
+      legacyEntryTypeByLevel: EntryType[]) {
     this.#compatibilityBuilder = compatibilityBuilder;
-    this.#flameChartData = flameChartData;
     this.#traceParsedData = traceParsedData;
-    this.#entryData = entryData;
     this.#legacyEntryTypeByLevel = legacyEntryTypeByLevel;
   }
 
@@ -108,6 +101,7 @@ export class GPUTrackAppender implements TrackAppender {
 
     this.#legacyEntryTypeByLevel.length = trackStartLevel + maxStackDepth;
     this.#legacyEntryTypeByLevel.fill(EntryType.TrackAppender, trackStartLevel);
+
     return trackStartLevel + maxStackDepth;
   }
 
@@ -117,17 +111,7 @@ export class GPUTrackAppender implements TrackAppender {
    * array, which contains all the events in the timeline.
    */
   #appendEventAtLevel(event: TraceEngine.Types.TraceEvents.TraceEventData, level: number): number {
-    this.#compatibilityBuilder.registerTrackForLevel(level, this);
-    const index = this.#entryData.length;
-    this.#entryData.push(event);
-    this.#legacyEntryTypeByLevel[level] = EntryType.TrackAppender;
-    this.#flameChartData.entryLevels[index] = level;
-    this.#flameChartData.entryStartTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(event.ts);
-    const msDuration = event.dur ||
-        TraceEngine.Helpers.Timing.millisecondsToMicroseconds(
-            InstantEventVisibleDurationMs as TraceEngine.Types.Timing.MilliSeconds);
-    this.#flameChartData.entryTotalTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(msDuration);
-    return index;
+    return this.#compatibilityBuilder.appendEventAtLevel(event, level, this);
   }
 
   /*

front_end/panels/timeline/InteractionsTrackAppender.ts

@@ -6,7 +6,6 @@ import type * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
 
 import {
   EntryType,
-  type TimelineFlameChartEntry,
 } from './TimelineFlameChartDataProvider.js';
 import {
   type CompatibilityTracksAppender,
@@ -38,7 +37,6 @@ export class InteractionsTrackAppender implements TrackAppender {
   #compatibilityBuilder: CompatibilityTracksAppender;
   #flameChartData: PerfUI.FlameChart.FlameChartTimelineData;
   #traceParsedData: Readonly<TraceEngine.TraceModel.PartialTraceParseDataDuringMigration>;
-  #entryData: TimelineFlameChartEntry[];
   // TODO(crbug.com/1416533)
   // This is used only for compatibility with the legacy flame chart
   // architecture of the panel. Once all tracks have been migrated to
@@ -49,7 +47,7 @@ export class InteractionsTrackAppender implements TrackAppender {
   constructor(
       compatibilityBuilder: CompatibilityTracksAppender, flameChartData: PerfUI.FlameChart.FlameChartTimelineData,
       traceParsedData: TraceEngine.TraceModel.PartialTraceParseDataDuringMigration,
-      entryData: TimelineFlameChartEntry[], legacyEntryTypeByLevel: EntryType[]) {
+      legacyEntryTypeByLevel: EntryType[]) {
     this.#compatibilityBuilder = compatibilityBuilder;
     this.#colorGenerator = new Common.Color.Generator(
         {
@@ -60,7 +58,6 @@ export class InteractionsTrackAppender implements TrackAppender {
         {min: 70, max: 100, count: 6}, 50, 0.7);
     this.#flameChartData = flameChartData;
     this.#traceParsedData = traceParsedData;
-    this.#entryData = entryData;
     this.#legacyEntryTypeByLevel = legacyEntryTypeByLevel;
   }
 
@@ -127,26 +124,12 @@ export class InteractionsTrackAppender implements TrackAppender {
    * @returns the position occupied by the new event in the entryData
    * array, which contains all the events in the timeline.
    */
-  #appendEventAtLevel(syntheticEvent: TraceEngine.Types.TraceEvents.SyntheticInteractionEvent, level: number): number {
-    this.#compatibilityBuilder.registerTrackForLevel(level, this);
-    const index = this.#entryData.length;
-
-    // Although the event is a SyntheticInteractionEvent, it extends
-    // TraceEventData, so we can safely push it onto entryData.
-    this.#entryData.push(syntheticEvent);
-    this.#legacyEntryTypeByLevel[level] = EntryType.TrackAppender;
-    this.#flameChartData.entryLevels[index] = level;
-    this.#flameChartData.entryStartTimes[index] =
-        TraceEngine.Helpers.Timing.microSecondsToMilliseconds(syntheticEvent.ts);
+  #appendEventAtLevel(syntheticEvent: TraceEngine.Types.TraceEvents.SyntheticInteractionEvent, level: number): void {
+    const index = this.#compatibilityBuilder.appendEventAtLevel(syntheticEvent, level, this);
     const eventDurationMicroSeconds = syntheticEvent.dur || TraceEngine.Types.Timing.MicroSeconds(0);
-
     if (eventDurationMicroSeconds > LONG_INTERACTION_THRESHOLD) {
       this.#addCandyStripingForLongInteraction(index);
     }
-
-    this.#flameChartData.entryTotalTimes[index] =
-        TraceEngine.Helpers.Timing.microSecondsToMilliseconds(eventDurationMicroSeconds);
-    return index;
   }
 
   #addCandyStripingForLongInteraction(eventIndex: number): void {

front_end/panels/timeline/LayoutShiftsTrackAppender.ts

@@ -6,7 +6,6 @@ import type * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
 
 import {
   EntryType,
-  type TimelineFlameChartEntry,
 } from './TimelineFlameChartDataProvider.js';
 import {
   type CompatibilityTracksAppender,
@@ -33,7 +32,6 @@ export class LayoutShiftsTrackAppender implements TrackAppender {
   #compatibilityBuilder: CompatibilityTracksAppender;
   #flameChartData: PerfUI.FlameChart.FlameChartTimelineData;
   #traceParsedData: Readonly<TraceEngine.TraceModel.PartialTraceParseDataDuringMigration>;
-  #entryData: TimelineFlameChartEntry[];
   // TODO(crbug.com/1416533)
   // This is used only for compatibility with the legacy flame chart
   // architecture of the panel. Once all tracks have been migrated to
@@ -44,11 +42,10 @@ export class LayoutShiftsTrackAppender implements TrackAppender {
   constructor(
       compatibilityBuilder: CompatibilityTracksAppender, flameChartData: PerfUI.FlameChart.FlameChartTimelineData,
       traceParsedData: TraceEngine.TraceModel.PartialTraceParseDataDuringMigration,
-      entryData: TimelineFlameChartEntry[], legacyEntryTypeByLevel: EntryType[]) {
+      legacyEntryTypeByLevel: EntryType[]) {
     this.#compatibilityBuilder = compatibilityBuilder;
     this.#flameChartData = flameChartData;
     this.#traceParsedData = traceParsedData;
-    this.#entryData = entryData;
     this.#legacyEntryTypeByLevel = legacyEntryTypeByLevel;
   }
 
@@ -121,13 +118,8 @@ export class LayoutShiftsTrackAppender implements TrackAppender {
    * @returns the position occupied by the new event in the entryData
    * array, which contains all the events in the timeline.
    */
-  #appendEventAtLevel(event: TraceEngine.Types.TraceEvents.TraceEventData, level: number): number {
-    this.#compatibilityBuilder.registerTrackForLevel(level, this);
-    const index = this.#entryData.length;
-    this.#entryData.push(event);
-    this.#legacyEntryTypeByLevel[level] = EntryType.TrackAppender;
-    this.#flameChartData.entryLevels[index] = level;
-    this.#flameChartData.entryStartTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(event.ts);
+  #appendEventAtLevel(event: TraceEngine.Types.TraceEvents.TraceEventData, level: number): void {
+    const index = this.#compatibilityBuilder.appendEventAtLevel(event, level, this);
     // Bit of a hack: LayoutShifts are instant events, so have no duration. But
     // OPP doesn't do well at making tiny events easy to spot and click. So we
     // set it to a small duration so that the user is able to see and click
@@ -135,7 +127,6 @@ export class LayoutShiftsTrackAppender implements TrackAppender {
     // allow us to do this properly and not hack around it.
     const msDuration = TraceEngine.Types.Timing.MicroSeconds(5_000);
     this.#flameChartData.entryTotalTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(msDuration);
-    return index;
   }
 
   /*

front_end/panels/timeline/TimingsTrackAppender.ts

@@ -6,8 +6,6 @@ import type * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
 
 import {
   EntryType,
-  InstantEventVisibleDurationMs,
-  type TimelineFlameChartEntry,
 } from './TimelineFlameChartDataProvider.js';
 import {
   type CompatibilityTracksAppender,
@@ -42,7 +40,6 @@ export class TimingsTrackAppender implements TrackAppender {
   #compatibilityBuilder: CompatibilityTracksAppender;
   #flameChartData: PerfUI.FlameChart.FlameChartTimelineData;
   #traceParsedData: Readonly<TraceEngine.TraceModel.PartialTraceParseDataDuringMigration>;
-  #entryData: TimelineFlameChartEntry[];
   // TODO(crbug.com/1416533)
   // This is used only for compatibility with the legacy flame chart
   // architecture of the panel. Once all tracks have been migrated to
@@ -53,7 +50,7 @@ export class TimingsTrackAppender implements TrackAppender {
   constructor(
       compatibilityBuilder: CompatibilityTracksAppender, flameChartData: PerfUI.FlameChart.FlameChartTimelineData,
       traceParsedData: TraceEngine.TraceModel.PartialTraceParseDataDuringMigration,
-      entryData: TimelineFlameChartEntry[], legacyEntryTypeByLevel: EntryType[]) {
+      legacyEntryTypeByLevel: EntryType[]) {
     this.#compatibilityBuilder = compatibilityBuilder;
     this.#colorGenerator = new Common.Color.Generator(
         {
@@ -64,7 +61,6 @@ export class TimingsTrackAppender implements TrackAppender {
         {min: 70, max: 100, count: 6}, 50, 0.7);
     this.#flameChartData = flameChartData;
     this.#traceParsedData = traceParsedData;
-    this.#entryData = entryData;
     this.#legacyEntryTypeByLevel = legacyEntryTypeByLevel;
   }
 
@@ -163,8 +159,8 @@ export class TimingsTrackAppender implements TrackAppender {
    */
   #appendConsoleTimings(currentLevel: number): number {
     let newLevel = currentLevel;
-    for (const timestamp of this.#traceParsedData.UserTimings.timestampEvents) {
-      this.#appendEventAtLevel(timestamp, newLevel);
+    for (const timestampEvent of this.#traceParsedData.UserTimings.timestampEvents) {
+      this.#appendEventAtLevel(timestampEvent, newLevel);
     }
     if (this.#traceParsedData.UserTimings.timestampEvents.length !== 0) {
       // Add console.time events on the next level, but only if the
@@ -221,17 +217,7 @@ export class TimingsTrackAppender implements TrackAppender {
    * array, which contains all the events in the timeline.
    */
   #appendEventAtLevel(event: TraceEngine.Types.TraceEvents.TraceEventData, level: number): number {
-    this.#compatibilityBuilder.registerTrackForLevel(level, this);
-    const index = this.#entryData.length;
-    this.#entryData.push(event);
-    this.#legacyEntryTypeByLevel[level] = EntryType.TrackAppender;
-    this.#flameChartData.entryLevels[index] = level;
-    this.#flameChartData.entryStartTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(event.ts);
-    const msDuration = event.dur ||
-        TraceEngine.Helpers.Timing.millisecondsToMicroseconds(
-            InstantEventVisibleDurationMs as TraceEngine.Types.Timing.MilliSeconds);
-    this.#flameChartData.entryTotalTimes[index] = TraceEngine.Helpers.Timing.microSecondsToMilliseconds(msDuration);
-    return index;
+    return this.#compatibilityBuilder.appendEventAtLevel(event, level, this);
   }
 
   /*

test/interactions/panels/performance/timeline/layout_shifts_track_test.ts

@@ -13,6 +13,6 @@ describe('Layout shifts track', () => {
   itScreenshot('renders the layout shifts track correctly', async () => {
     await loadComponentDocExample('performance_panel/track_example.html?track=LayoutShifts&fileName=cls-single-frame');
     const flameChart = await waitFor('.flame-chart-main-pane');
-    await assertElementScreenshotUnchanged(flameChart, 'performance/layout_shifts_track.png', 3);
+    await assertElementScreenshotUnchanged(flameChart, 'performance/layout_shifts_track.png', 2);
   });
 });