Skip to content

Commit 8d89cca

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: make track configuration global and persisted
This CL updates the track configuration in RPP. Previously, it was stored per trace. This CL makes it be stored globally. This has some implications for how we apply any persisted config: 1. We now store the track name in the configuration. 2. When a user imports or records a trace, we do the following: - For each group, we look to see if its name matches a name in the persisted config. If it does, we apply the expanded / hidden state from the persisted config. This logic is fairly sound but non ideal because some tracks (e.g. "Main - google.com") contain trace specific information. We might want to change this in the future to think about using some other identifier. However, I expect most users do not want to hide the "main" thread, so maybe this is OK. - After that, we try to apply the persisted sorting. We only do this if the number of tracks in the persisted config matches the number in the active trace. This is non-ideal because this scenario won't happen much. However, this CL is already big enough; we should consider if we want to apply this differently in a follow-up CL. 3. We now never apply any saved visual config from a trace file that is imported. The reasoning here is that the visual config is a per-person preference (sort of analogous to light or dark mode, if you want to think of it that way) and so we prefer to maintain the user's preferences. When the user imports or records a new trace, and they have persisted track config, we will apply it. As of crrev.com/c/6861972, they will also see a dismissable banner at the bottom of the view telling them that we have hidden some tracks. Bypass-Check-License: Only updates files, no new files are added. Bug: 432663450 Change-Id: If25512e369163fe473c143b6598dee5c22ae98d6 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6862799 Reviewed-by: Alina Varkki <[email protected]> Commit-Queue: Alina Varkki <[email protected]> Auto-Submit: Jack Franklin <[email protected]>
1 parent 558cd42 commit 8d89cca

17 files changed

+275
-341
lines changed

front_end/models/trace/types/File.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,14 @@ export interface TrackVisualConfig {
180180
expanded: boolean;
181181
originalIndex: number;
182182
visualIndex: number;
183+
trackName: string;
183184
}
184185

185186
/**
186187
* Stores the visual config if the user has modified it. Split into "main" and
187188
* "network" so we can pass the relevant config into the right data provider.
189+
* NOTE: as of August 2025 (M141) we currently do not export this in new
190+
* traces, or use it if an existing trace is imported with it.
188191
*/
189192
export interface PersistedTraceVisualConfig {
190193
main: TrackVisualConfig[]|null;

front_end/panels/timeline/README.md

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ Note: because this approach uses `SyntheticProfileCalls`, which are built from C
268268

269269
We allow the user to edit the visual status and order of tracks in the main flame chart. Initially when we shipped this feature it was not persisted, meaning that if you refreshed or imported another trace, your configuration was lost. This was changed in crrev.com/c/6632596 which added persisting of the track configuration into memory, and added these docs too :)
270270

271+
As of crrev.com/c/6862799, this feature was changed to be persisted across all traces, whereas previously configuration was only applied per trace, which made it much less useful.
272+
271273
### How tracks get rendered
272274

273275
Tracks on the timeline are called `Groups` in the code (`PerfUI.FlameChart.Group`). These get constructed in the data providers and pushed onto the `groups` array.
@@ -323,21 +325,11 @@ We persist the order of the groups and for each group its `hidden` and `expanded
323325
originalIndex: 0,
324326
visualIndex: 2,
325327
hidden: false,
326-
expanded: true
328+
expanded: true,
329+
trackName: 'Frames',
327330
}
328331
```
329332

330333
When the user makes any visual change to the UI, which happens in `FlameChart.ts`, we notify the data provider of that change. It can then choose to persist this data anywhere it likes. In our case, we persist into a DevTools setting which is persisted globally (e.g. it survives restarts).
331334

332-
Because the user can import/record multiple traces over time, we cannot just persist the array of group states, because then we would mistakenly apply it to other traces. To avoid this we persist the list of groups under a key. The key is the `min` time for each trace (e.g. the time the trace started). This is not guaranteed to be unique, but it is pretty likely to be, given that it is monotonic and microsecond precision.
333-
334-
So, what actually gets stored into the setting is:
335-
336-
```
337-
{
338-
12345: [{...group state as above...}],
339-
56789: [{...group state as above...}],
340-
}
341-
```
342-
343-
When a trace is recorded / imported, we look to read any persisted configuration from disk before applying it.
335+
We store this as two global settings (one for the main flame chart, one for the network flame chart). This means that the user can make changes to one trace and it is applied to all future traces. This is by design - we have lots of feedback that there is a lot of noise that people want to hide.

front_end/panels/timeline/TimelineFlameChartDataProvider.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ describeWithEnvironment('TimelineFlameChartDataProvider', function() {
332332
it('persists track configurations to the setting if it is provided with one', async function() {
333333
const {Settings} = Common.Settings;
334334
const setting =
335-
Settings.instance().createSetting<PerfUi.FlameChart.PersistedConfigPerTrace>('persist-flame-config', {});
335+
Settings.instance().createSetting<PerfUi.FlameChart.PersistedGroupConfig[]|null>('persist-flame-config', null);
336336

337337
const dataProvider = new Timeline.TimelineFlameChartDataProvider.TimelineFlameChartDataProvider();
338338
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
@@ -350,25 +350,27 @@ describeWithEnvironment('TimelineFlameChartDataProvider', function() {
350350
dataProvider.handleTrackConfigurationChange(groups, newVisualOrder);
351351

352352
const newSetting = setting.get();
353-
const traceKey = Timeline.TrackConfiguration.keyForTraceConfig(parsedTrace);
354-
assert.deepEqual(newSetting[traceKey], [
353+
assert.deepEqual(newSetting, [
355354
{
356355
expanded: false,
357356
hidden: false,
358357
originalIndex: 0,
359358
visualIndex: 2,
359+
trackName: 'Frames',
360360
},
361361
{
362362
expanded: false,
363363
hidden: false,
364364
originalIndex: 1,
365365
visualIndex: 0,
366+
trackName: '', // This is screenshots.
366367
},
367368
{
368369
expanded: false,
369370
hidden: false,
370371
originalIndex: 2,
371372
visualIndex: 1,
373+
trackName: 'Animations',
372374
}
373375
]);
374376
});

front_end/panels/timeline/TimelineFlameChartDataProvider.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ import {
5050
selectionsEqual,
5151
type TimelineSelection,
5252
} from './TimelineSelection.js';
53-
import {buildPersistedConfig, keyForTraceConfig} from './TrackConfiguration.js';
53+
import {buildPersistedConfig} from './TrackConfiguration.js';
5454
import * as Utils from './utils/utils.js';
5555

5656
const UIStrings = {
@@ -167,7 +167,7 @@ export class TimelineFlameChartDataProvider extends Common.ObjectWrapper.ObjectW
167167
* have to recalculate. This is reset when the trace changes.
168168
*/
169169
#initiatorsCache = new Map<number, PerfUI.FlameChart.FlameChartInitiatorData[]>();
170-
#persistedGroupConfigSetting: Common.Settings.Setting<PerfUI.FlameChart.PersistedConfigPerTrace>|null = null;
170+
#persistedGroupConfigSetting: Common.Settings.Setting<PerfUI.FlameChart.PersistedGroupConfig[]|null>|null = null;
171171

172172
constructor() {
173173
super();
@@ -218,14 +218,11 @@ export class TimelineFlameChartDataProvider extends Common.ObjectWrapper.ObjectW
218218
return;
219219
}
220220
const persistedDataForTrace = buildPersistedConfig(groups, indexesInVisualOrder);
221-
const traceKey = keyForTraceConfig(this.parsedTrace);
222-
223-
const setting = this.#persistedGroupConfigSetting.get();
224-
setting[traceKey] = persistedDataForTrace;
225-
this.#persistedGroupConfigSetting.set(setting);
221+
this.#persistedGroupConfigSetting.set(persistedDataForTrace);
226222
}
227223

228-
setPersistedGroupConfigSetting(setting: Common.Settings.Setting<PerfUI.FlameChart.PersistedConfigPerTrace>): void {
224+
setPersistedGroupConfigSetting(setting: Common.Settings.Setting<PerfUI.FlameChart.PersistedGroupConfig[]|null>):
225+
void {
229226
this.#persistedGroupConfigSetting = setting;
230227
}
231228

front_end/panels/timeline/TimelineFlameChartNetworkDataProvider.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ describeWithEnvironment('TimelineFlameChartNetworkDataProvider', function() {
224224
it('persists track configurations to the setting if it is provided with one', async function() {
225225
const {Settings} = Common.Settings;
226226
const setting =
227-
Settings.instance().createSetting<PerfUi.FlameChart.PersistedConfigPerTrace>('persist-flame-config', {});
227+
Settings.instance().createSetting<PerfUi.FlameChart.PersistedGroupConfig[]|null>('persist-flame-config', null);
228228

229229
const dataProvider = new Timeline.TimelineFlameChartNetworkDataProvider.TimelineFlameChartNetworkDataProvider();
230230
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
@@ -241,13 +241,13 @@ describeWithEnvironment('TimelineFlameChartNetworkDataProvider', function() {
241241
dataProvider.handleTrackConfigurationChange(groups, [0]);
242242

243243
const newSetting = setting.get();
244-
const traceKey = Timeline.TrackConfiguration.keyForTraceConfig(parsedTrace);
245-
assert.deepEqual(newSetting[traceKey], [
244+
assert.deepEqual(newSetting, [
246245
{
247246
expanded: true,
248247
hidden: false,
249248
originalIndex: 0,
250249
visualIndex: 0,
250+
trackName: 'Network',
251251
},
252252
]);
253253
});

front_end/panels/timeline/TimelineFlameChartNetworkDataProvider.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
selectionsEqual,
2323
type TimelineSelection,
2424
} from './TimelineSelection.js';
25-
import {buildPersistedConfig, keyForTraceConfig} from './TrackConfiguration.js';
25+
import {buildPersistedConfig} from './TrackConfiguration.js';
2626
import * as TimelineUtils from './utils/utils.js';
2727

2828
export class TimelineFlameChartNetworkDataProvider implements PerfUI.FlameChart.FlameChartDataProvider {
@@ -40,7 +40,7 @@ export class TimelineFlameChartNetworkDataProvider implements PerfUI.FlameChart.
4040
#lastInitiatorEntry = -1;
4141
#lastInitiatorsData: PerfUI.FlameChart.FlameChartInitiatorData[] = [];
4242
#entityMapper: TimelineUtils.EntityMapper.EntityMapper|null = null;
43-
#persistedGroupConfigSetting: Common.Settings.Setting<PerfUI.FlameChart.PersistedConfigPerTrace>|null = null;
43+
#persistedGroupConfigSetting: Common.Settings.Setting<PerfUI.FlameChart.PersistedGroupConfig[]|null>|null = null;
4444

4545
constructor() {
4646
this.reset();
@@ -461,13 +461,11 @@ export class TimelineFlameChartNetworkDataProvider implements PerfUI.FlameChart.
461461
return;
462462
}
463463
const persistedDataForTrace = buildPersistedConfig(groups, indexesInVisualOrder);
464-
const traceKey = keyForTraceConfig(this.#parsedTrace);
465-
const setting = this.#persistedGroupConfigSetting.get();
466-
setting[traceKey] = persistedDataForTrace;
467-
this.#persistedGroupConfigSetting.set(setting);
464+
this.#persistedGroupConfigSetting.set(persistedDataForTrace);
468465
}
469466

470-
setPersistedGroupConfigSetting(setting: Common.Settings.Setting<PerfUI.FlameChart.PersistedConfigPerTrace>): void {
467+
setPersistedGroupConfigSetting(setting: Common.Settings.Setting<PerfUI.FlameChart.PersistedGroupConfig[]|null>):
468+
void {
471469
this.#persistedGroupConfigSetting = setting;
472470
}
473471

front_end/panels/timeline/TimelineFlameChartView.test.ts

Lines changed: 64 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,16 @@ class MockViewDelegate implements Timeline.TimelinePanel.TimelineModeViewDelegat
3939
}
4040

4141
function clearPersistTrackConfigSettings() {
42-
const mainGroupSetting = Common.Settings.Settings.instance().createSetting('timeline-main-flame-group-config', {});
42+
const mainGroupSetting =
43+
Common.Settings.Settings.instance().createSetting<PerfUI.FlameChart.PersistedGroupConfig[]|null>(
44+
'timeline-persisted-main-flamechart-track-config', null);
4345
const networkGroupSetting =
44-
Common.Settings.Settings.instance().createSetting('timeline-network-flame-group-config', {});
46+
Common.Settings.Settings.instance().createSetting<PerfUI.FlameChart.PersistedGroupConfig[]|null>(
47+
'timeline-persisted-network-flamechart-track-config', null);
4548

4649
// In case they already existed and need clearing out.
47-
mainGroupSetting.set({});
48-
networkGroupSetting.set({});
50+
mainGroupSetting.set(null);
51+
networkGroupSetting.set(null);
4952
}
5053

5154
describeWithEnvironment('TimelineFlameChartView', function() {
@@ -183,6 +186,26 @@ describeWithEnvironment('TimelineFlameChartView', function() {
183186
});
184187
});
185188

189+
it('knows if the current trace has got hidden tracks', async function() {
190+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
191+
const mockViewDelegate = new MockViewDelegate();
192+
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
193+
renderElementIntoDOM(flameChartView);
194+
flameChartView.setModel(parsedTrace, metadata);
195+
196+
assert.isFalse(flameChartView.hasHiddenTracks());
197+
// Ensure it is true when something in the main flame chart is hidden
198+
flameChartView.getMainFlameChart().hideGroup(0);
199+
assert.isTrue(flameChartView.hasHiddenTracks());
200+
flameChartView.getMainFlameChart().showGroup(0);
201+
assert.isFalse(flameChartView.hasHiddenTracks());
202+
// Ensure it is true when something in the network chart is hidden
203+
// (users cannot technically achieve this via the UI, but in case they can
204+
// in the future let's explicitly check!)
205+
flameChartView.getNetworkFlameChart().hideGroup(0);
206+
assert.isTrue(flameChartView.hasHiddenTracks());
207+
});
208+
186209
it('can gather the visual track config to store as metadata', async function() {
187210
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
188211
const mockViewDelegate = new MockViewDelegate();
@@ -199,82 +222,57 @@ describeWithEnvironment('TimelineFlameChartView', function() {
199222
const networkChart = flameChartView.getNetworkFlameChart();
200223
networkChart.toggleGroupExpand(0);
201224

202-
const visualMetadata = flameChartView.getPersistedConfigMetadata(parsedTrace);
225+
const visualMetadata = flameChartView.getPersistedConfigMetadata();
203226

204-
assert.deepEqual(visualMetadata.network, [{expanded: true, hidden: false, originalIndex: 0, visualIndex: 0}]);
227+
assert.deepEqual(
228+
visualMetadata.network,
229+
[{expanded: true, hidden: false, originalIndex: 0, visualIndex: 0, trackName: 'Network'}]);
205230

206231
assert.deepEqual(visualMetadata.main, [
207-
{expanded: false, hidden: true, originalIndex: 0, visualIndex: 0},
208-
{expanded: false, hidden: false, originalIndex: 1, visualIndex: 1},
209-
{expanded: false, hidden: false, originalIndex: 2, visualIndex: 3},
210-
{expanded: true, hidden: false, originalIndex: 3, visualIndex: 2},
211-
{expanded: false, hidden: false, originalIndex: 4, visualIndex: 4},
212-
{expanded: false, hidden: false, originalIndex: 5, visualIndex: 5},
213-
{expanded: false, hidden: false, originalIndex: 6, visualIndex: 6},
214-
{expanded: false, hidden: false, originalIndex: 7, visualIndex: 7},
215-
{expanded: false, hidden: false, originalIndex: 8, visualIndex: 8},
216-
{expanded: false, hidden: false, originalIndex: 9, visualIndex: 9},
217-
{expanded: false, hidden: false, originalIndex: 10, visualIndex: 10},
218-
{expanded: false, hidden: false, originalIndex: 11, visualIndex: 11},
219-
{expanded: false, hidden: false, originalIndex: 12, visualIndex: 12}
232+
{expanded: false, hidden: true, originalIndex: 0, visualIndex: 0, trackName: 'Frames'}, {
233+
expanded: false,
234+
hidden: false,
235+
originalIndex: 1,
236+
visualIndex: 1,
237+
// screenshots but it has no visible title
238+
trackName: ''
239+
},
240+
{expanded: false, hidden: false, originalIndex: 2, visualIndex: 3, trackName: 'Animations'},
241+
{expanded: true, hidden: false, originalIndex: 3, visualIndex: 2, trackName: 'Main — https://web.dev/'}, {
242+
expanded: false,
243+
hidden: false,
244+
originalIndex: 4,
245+
visualIndex: 4,
246+
trackName:
247+
'Frame — https://shared-storage-demo-content-producer.web.app/paa/scripts/private-aggregation-test.html'
248+
},
249+
{expanded: false, hidden: false, originalIndex: 5, visualIndex: 5, trackName: 'Thread pool'},
250+
{expanded: false, hidden: false, originalIndex: 6, visualIndex: 6, trackName: 'Thread pool worker 1'},
251+
{expanded: false, hidden: false, originalIndex: 7, visualIndex: 7, trackName: 'Thread pool worker 2'},
252+
{expanded: false, hidden: false, originalIndex: 8, visualIndex: 8, trackName: 'Thread pool worker 3'},
253+
{expanded: false, hidden: false, originalIndex: 9, visualIndex: 9, trackName: 'Thread pool worker 4'},
254+
{expanded: false, hidden: false, originalIndex: 10, visualIndex: 10, trackName: 'Thread pool worker 5'},
255+
{expanded: false, hidden: false, originalIndex: 11, visualIndex: 11, trackName: 'StackSamplingProfiler'},
256+
{expanded: false, hidden: false, originalIndex: 12, visualIndex: 12, trackName: 'GPU'}
220257
]);
221258
});
222259

223-
it('will apply metadata on disk to the setting when a trace is imported', async function() {
224-
const {parsedTrace, metadata: originalMetdata} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
225-
226-
const FAKE_VISUAL_CONFIG_MAIN: Trace.Types.File.TrackVisualConfig[] = [
227-
// Move the order of Group 1 and Group 2 around
228-
{expanded: false, hidden: true, originalIndex: 0, visualIndex: 0},
229-
{expanded: false, hidden: false, originalIndex: 1, visualIndex: 1},
230-
{expanded: false, hidden: false, originalIndex: 2, visualIndex: 3},
231-
{expanded: true, hidden: false, originalIndex: 3, visualIndex: 2},
232-
{expanded: false, hidden: false, originalIndex: 4, visualIndex: 4},
233-
{expanded: false, hidden: false, originalIndex: 5, visualIndex: 5},
234-
{expanded: false, hidden: false, originalIndex: 6, visualIndex: 6},
235-
{expanded: false, hidden: false, originalIndex: 7, visualIndex: 7},
236-
{expanded: false, hidden: false, originalIndex: 8, visualIndex: 8},
237-
{expanded: false, hidden: false, originalIndex: 9, visualIndex: 9},
238-
{expanded: false, hidden: false, originalIndex: 10, visualIndex: 10},
239-
{expanded: false, hidden: false, originalIndex: 11, visualIndex: 11},
240-
{expanded: false, hidden: false, originalIndex: 12, visualIndex: 12}
241-
];
242-
243-
const FAKE_VISUAL_CONFIG_NETWORK: Trace.Types.File.TrackVisualConfig[] =
244-
[{expanded: true, hidden: false, originalIndex: 0, visualIndex: 0}];
245-
246-
const metadata: Trace.Types.File.MetaData = {
247-
...originalMetdata,
248-
visualTrackConfig: {
249-
main: FAKE_VISUAL_CONFIG_MAIN,
250-
network: FAKE_VISUAL_CONFIG_NETWORK,
251-
}
252-
};
253-
const mockViewDelegate = new MockViewDelegate();
254-
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
255-
flameChartView.setModel(parsedTrace, metadata);
256-
257-
const metadataInSetting = flameChartView.getPersistedConfigMetadata(parsedTrace);
258-
assert.deepEqual(metadataInSetting, {main: FAKE_VISUAL_CONFIG_MAIN, network: FAKE_VISUAL_CONFIG_NETWORK});
259-
});
260-
261-
it('does not use visual config from file if the user has locally made config changes', async function() {
260+
it('does not apply visual config from a file', async function() {
262261
const {parsedTrace, metadata: originalMetadata} =
263262
await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
264263

265264
const FROM_FILE_VISUAL_CONFIG_NETWORK: Trace.Types.File.TrackVisualConfig[] =
266-
[{expanded: true, hidden: false, originalIndex: 0, visualIndex: 0}];
265+
[{expanded: true, hidden: false, originalIndex: 0, visualIndex: 0, trackName: 'Network'}];
267266

268-
const traceKey = Timeline.TrackConfiguration.keyForTraceConfig(parsedTrace);
269267
// Populate the in-memory setting to pretend the user has already modified
270268
// this trace's visual config.
271269
// Importantly for this test, this is a different setting to FAKE_VISUAL_CONFIG_NETWORK above.
272270
const networkGroupSetting =
273-
Common.Settings.Settings.instance()
274-
.createSetting<PerfUI.FlameChart.PersistedConfigPerTrace>('timeline-network-flame-group-config', {})
275-
.get();
276-
const USER_VISUAL_CONFIG_NETWORK = {hidden: true, expanded: true, originalIndex: 0, visualIndex: 0};
277-
networkGroupSetting[traceKey] = [USER_VISUAL_CONFIG_NETWORK];
271+
Common.Settings.Settings.instance().createSetting<PerfUI.FlameChart.PersistedGroupConfig[]|null>(
272+
'timeline-persisted-network-flamechart-track-config', null);
273+
const USER_VISUAL_CONFIG_NETWORK =
274+
{hidden: true, expanded: true, originalIndex: 0, visualIndex: 0, trackName: 'Network'};
275+
networkGroupSetting.set([USER_VISUAL_CONFIG_NETWORK]);
278276

279277
// Now add network configuration to the metadata that we get from the trace file itself.
280278
const metadata: Trace.Types.File.MetaData = {
@@ -288,7 +286,7 @@ describeWithEnvironment('TimelineFlameChartView', function() {
288286
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
289287
flameChartView.setModel(parsedTrace, metadata);
290288

291-
const metadataInSetting = flameChartView.getPersistedConfigMetadata(parsedTrace);
289+
const metadataInSetting = flameChartView.getPersistedConfigMetadata();
292290
assert.deepEqual(metadataInSetting, {main: null, network: [USER_VISUAL_CONFIG_NETWORK]});
293291
});
294292

0 commit comments

Comments
 (0)