Skip to content

Commit 5f4411c

Browse files
and-oliDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Remove ExtensionDataGatherer
It serves as an intermediary layer between the parsedTrace data and the UI, and it adds some overhead to be properly managed because of our lack of proper support for this type of utils in the RPP architecture. The reason it exists at all is in preparation to support extending the RPP with Chrome extensions, as that was the inital plan. But at the moment, we are focusing on extending the panel with web APIs (performance.measure, console.timeStamp, etc.), which makes the data available via trace events, so right now and for the foreseable future, this extra layer doesn't bring any value to outweight its maintenance cost. Thus this CL. Bug: 381861194 Change-Id: I24a1bc3f7405dc5c8155e7f554007fa48c33583c Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6088062 Reviewed-by: Alina Varkki <[email protected]> Commit-Queue: Andres Olivares <[email protected]>
1 parent 4352acc commit 5f4411c

File tree

10 files changed

+13
-89
lines changed

10 files changed

+13
-89
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1784,7 +1784,6 @@ grd_files_debug_sources = [
17841784
"front_end/panels/timeline/EntriesFilter.js",
17851785
"front_end/panels/timeline/EventsSerializer.js",
17861786
"front_end/panels/timeline/EventsTimelineTreeView.js",
1787-
"front_end/panels/timeline/ExtensionDataGatherer.js",
17881787
"front_end/panels/timeline/ExtensionTrackAppender.js",
17891788
"front_end/panels/timeline/FreshRecording.js",
17901789
"front_end/panels/timeline/GPUTrackAppender.js",

front_end/panels/timeline/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ devtools_module("timeline") {
6363
"EntriesFilter.ts",
6464
"EventsSerializer.ts",
6565
"EventsTimelineTreeView.ts",
66-
"ExtensionDataGatherer.ts",
6766
"ExtensionTrackAppender.ts",
6867
"FreshRecording.ts",
6968
"GPUTrackAppender.ts",

front_end/panels/timeline/CompatibilityTracksAppender.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import * as ThemeSupport from '../../ui/legacy/theme_support/theme_support.js';
1212
import {AnimationsTrackAppender} from './AnimationsTrackAppender.js';
1313
import {getEventLevel, getFormattedTime, type LastTimestampByLevel} from './AppenderUtils.js';
1414
import * as TimelineComponents from './components/components.js';
15-
import {ExtensionDataGatherer} from './ExtensionDataGatherer.js';
1615
import {ExtensionTrackAppender} from './ExtensionTrackAppender.js';
1716
import {GPUTrackAppender} from './GPUTrackAppender.js';
1817
import {InteractionsTrackAppender} from './InteractionsTrackAppender.js';
@@ -23,6 +22,7 @@ import {
2322
EntryType,
2423
InstantEventVisibleDurationMs,
2524
} from './TimelineFlameChartDataProvider.js';
25+
import {TimelinePanel} from './TimelinePanel.js';
2626
import {TimingsTrackAppender} from './TimingsTrackAppender.js';
2727
import * as TimelineUtils from './utils/utils.js';
2828

@@ -280,7 +280,10 @@ export class CompatibilityTracksAppender {
280280
}
281281

282282
#addExtensionAppenders(): void {
283-
const tracks = ExtensionDataGatherer.instance().getExtensionData().extensionTrackData;
283+
if (!TimelinePanel.extensionDataVisibilitySetting().get()) {
284+
return;
285+
}
286+
const tracks = this.#parsedTrace.ExtensionTraceData.extensionTrackData;
284287
for (const trackData of tracks) {
285288
this.#allTrackAppenders.push(new ExtensionTrackAppender(this, trackData));
286289
}

front_end/panels/timeline/ExtensionDataGatherer.ts

Lines changed: 0 additions & 65 deletions
This file was deleted.

front_end/panels/timeline/TimelineFlameChartDataProvider.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import * as UI from '../../ui/legacy/legacy.js';
3838
import * as ThemeSupport from '../../ui/legacy/theme_support/theme_support.js';
3939

4040
import {CompatibilityTracksAppender, type DrawOverride, type TrackAppenderName} from './CompatibilityTracksAppender.js';
41-
import {ExtensionDataGatherer} from './ExtensionDataGatherer.js';
4241
import {initiatorsDataToDraw} from './Initiators.js';
4342
import {ModificationsManager} from './ModificationsManager.js';
4443
import {ThreadAppender} from './ThreadAppender.js';
@@ -404,8 +403,6 @@ export class TimelineFlameChartDataProvider extends Common.ObjectWrapper.ObjectW
404403
setModel(parsedTrace: Trace.Handlers.Types.ParsedTrace|null, isCpuProfile = false): void {
405404
this.reset();
406405
this.parsedTrace = parsedTrace;
407-
ExtensionDataGatherer.instance().modelChanged(parsedTrace);
408-
409406
this.isCpuProfile = isCpuProfile;
410407
if (parsedTrace) {
411408
const {traceBounds} = parsedTrace.Meta;
@@ -524,8 +521,6 @@ export class TimelineFlameChartDataProvider extends Common.ObjectWrapper.ObjectW
524521
this.compatibilityTracksAppender?.threadAppenders().forEach(
525522
threadAppender => threadAppender.setHeaderAppended(false));
526523
}
527-
528-
ExtensionDataGatherer.removeInstance();
529524
}
530525

531526
maxStackDepth(): number {

front_end/panels/timeline/TimingsTrackAppender.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ import {
1313
type TrackAppenderName,
1414
VisualLoggingTrackName,
1515
} from './CompatibilityTracksAppender.js';
16-
import {ExtensionDataGatherer} from './ExtensionDataGatherer.js';
1716
import * as Extensions from './extensions/extensions.js';
1817
import {TimelineFlameChartMarker} from './TimelineFlameChartView.js';
18+
import {TimelinePanel} from './TimelinePanel.js';
1919
import type {TimelineMarkerStyle} from './TimelineUIUtils.js';
2020

2121
const UIStrings = {
@@ -49,13 +49,15 @@ export class TimingsTrackAppender implements TrackAppender {
4949
#colorGenerator: Common.Color.Generator;
5050
#compatibilityBuilder: CompatibilityTracksAppender;
5151
#parsedTrace: Readonly<Trace.Handlers.Types.ParsedTrace>;
52-
52+
#extensionMarkers: readonly Trace.Types.Extensions.SyntheticExtensionMarker[];
5353
constructor(
5454
compatibilityBuilder: CompatibilityTracksAppender, parsedTrace: Trace.Handlers.Types.ParsedTrace,
5555
colorGenerator: Common.Color.Generator) {
5656
this.#compatibilityBuilder = compatibilityBuilder;
5757
this.#colorGenerator = colorGenerator;
5858
this.#parsedTrace = parsedTrace;
59+
const extensionDataEnabled = TimelinePanel.extensionDataVisibilitySetting().get();
60+
this.#extensionMarkers = extensionDataEnabled ? this.#parsedTrace.ExtensionTraceData.extensionMarkers : [];
5961
}
6062

6163
/**
@@ -68,8 +70,7 @@ export class TimingsTrackAppender implements TrackAppender {
6870
* appended the track's events.
6971
*/
7072
appendTrackAtLevel(trackStartLevel: number, expanded?: boolean): number {
71-
const extensionMarkers = ExtensionDataGatherer.instance().getExtensionData().extensionMarkers;
72-
const extensionMarkersAreEmpty = extensionMarkers.length === 0;
73+
const extensionMarkersAreEmpty = this.#extensionMarkers.length === 0;
7374
const performanceMarks = this.#parsedTrace.UserTimings.performanceMarks.filter(
7475
m => !Trace.Handlers.ModelHandlers.ExtensionTraceData.extensionDataInTiming(m));
7576
const performanceMeasures = this.#parsedTrace.UserTimings.performanceMeasures.filter(
@@ -115,8 +116,7 @@ export class TimingsTrackAppender implements TrackAppender {
115116
*/
116117
#appendExtensionsAtLevel(currentLevel: number): number {
117118
let markers: Trace.Types.Extensions.SyntheticExtensionMarker[] = [];
118-
markers = markers.concat(ExtensionDataGatherer.instance().getExtensionData().extensionMarkers)
119-
.sort((m1, m2) => m1.ts - m2.ts);
119+
markers = markers.concat(this.#extensionMarkers).sort((m1, m2) => m1.ts - m2.ts);
120120
if (markers.length === 0) {
121121
return currentLevel;
122122
}

front_end/panels/timeline/timeline.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import * as CountersGraph from './CountersGraph.js';
1212
import * as EntriesFilter from './EntriesFilter.js';
1313
import * as EventsSerializer from './EventsSerializer.js';
1414
import * as EventsTimelineTreeView from './EventsTimelineTreeView.js';
15-
import * as ExtensionDataGatherer from './ExtensionDataGatherer.js';
1615
import * as ExtensionTrackAppender from './ExtensionTrackAppender.js';
1716
import * as FreshRecording from './FreshRecording.js';
1817
import * as GPUTrackAppender from './GPUTrackAppender.js';
@@ -57,7 +56,6 @@ export {
5756
EntriesFilter,
5857
EventsSerializer,
5958
EventsTimelineTreeView,
60-
ExtensionDataGatherer,
6159
ExtensionTrackAppender,
6260
FreshRecording,
6361
GPUTrackAppender,

front_end/panels/timeline/track_appenders/ExtensionTrackAppender.test.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ function initTrackAppender(
1818
flameChartData: PerfUI.FlameChart.FlameChartTimelineData, parsedTrace: Trace.Handlers.Types.ParsedTrace,
1919
entryData: Trace.Types.Events.Event[], entryTypeByLevel: Timeline.TimelineFlameChartDataProvider.EntryType[]):
2020
Timeline.ExtensionTrackAppender.ExtensionTrackAppender[] {
21-
Timeline.ExtensionDataGatherer.ExtensionDataGatherer.instance().modelChanged(parsedTrace);
2221
const compatibilityTracksAppender = new Timeline.CompatibilityTracksAppender.CompatibilityTracksAppender(
2322
flameChartData, parsedTrace, entryData, entryTypeByLevel);
2423

@@ -34,7 +33,6 @@ describeWithEnvironment('ExtensionTrackAppender', function() {
3433
let entryTypeByLevel: Timeline.TimelineFlameChartDataProvider.EntryType[] = [];
3534

3635
beforeEach(async function() {
37-
Timeline.ExtensionDataGatherer.ExtensionDataGatherer.removeInstance();
3836
({parsedTrace} = await TraceLoader.traceEngine(this, 'extension-tracks-and-marks.json.gz'));
3937
extensionTrackAppenders = initTrackAppender(flameChartData, parsedTrace, entryData, entryTypeByLevel);
4038
let level = 0;
@@ -117,7 +115,6 @@ describeWithEnvironment('ExtensionTrackAppender', function() {
117115
] as ExtensionTestData[];
118116
const traceExtensionData = await createTraceExtensionDataFromTestInput(extensionData);
119117
const testParsedTrace = getBaseTraceParseModelData({ExtensionTraceData: traceExtensionData});
120-
Timeline.ExtensionDataGatherer.ExtensionDataGatherer.removeInstance();
121118
entryData = [];
122119
flameChartData = PerfUI.FlameChart.FlameChartTimelineData.createEmpty();
123120
entryTypeByLevel = [];
@@ -205,7 +202,6 @@ describeWithEnvironment('ExtensionTrackAppender', function() {
205202

206203
describe('toggling', function() {
207204
it('Does not append extension data when the configuration is set to disabled', async function() {
208-
Timeline.ExtensionDataGatherer.ExtensionDataGatherer.removeInstance();
209205
entryData = [];
210206
flameChartData = PerfUI.FlameChart.FlameChartTimelineData.createEmpty();
211207
entryTypeByLevel = [];

front_end/panels/timeline/track_appenders/ThreadAppender.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,8 @@ describeWithEnvironment('ThreadAppender', function() {
464464
Meta: {
465465
traceIsGeneric: false,
466466
},
467-
} as Trace.Handlers.Types.ParsedTrace;
467+
ExtensionTraceData: {entryToNode: new Map(), extensionMarkers: [], extensionTrackData: []},
468+
} as unknown as Trace.Handlers.Types.ParsedTrace;
468469

469470
// Add the script to ignore list and then append the flamechart data
470471
ignoreListManager.ignoreListURL(SCRIPT_TO_IGNORE);

front_end/panels/timeline/track_appenders/TimingsTrackAppender.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ function initTrackAppender(
1515
entryData: Trace.Types.Events.Event[],
1616
entryTypeByLevel: Timeline.TimelineFlameChartDataProvider.EntryType[],
1717
): Timeline.TimingsTrackAppender.TimingsTrackAppender {
18-
Timeline.ExtensionDataGatherer.ExtensionDataGatherer.instance().modelChanged(parsedTrace);
1918
const compatibilityTracksAppender = new Timeline.CompatibilityTracksAppender.CompatibilityTracksAppender(
2019
flameChartData, parsedTrace, entryData, entryTypeByLevel);
2120
return compatibilityTracksAppender.timingsTrackAppender();
@@ -327,7 +326,6 @@ describeWithEnvironment('TimingTrackAppender', function() {
327326
});
328327
describe('toggling', function() {
329328
it('Does not append extension data when the configuration is set to disabled', async function() {
330-
Timeline.ExtensionDataGatherer.ExtensionDataGatherer.removeInstance();
331329
entryData = [];
332330
flameChartData = PerfUI.FlameChart.FlameChartTimelineData.createEmpty();
333331
entryTypeByLevel = [];

0 commit comments

Comments
 (0)