Skip to content

Commit 8605148

Browse files
vidortegDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Add dialog for export trace options
This change consolidates 2 different menus in a dialog that contains 3 options to customize an exported performance trace: 1. Include annotations 2. Include script content (Behind experiment flag) 3. Include script source maps (Behind experiment flag) Include annotations This option only shows up if annotations exists on the trace. Replaces the previous option to whether or not include existing annotation in the trace. Include script content This option only shows up when the experiment flag (Enable collecting enhanced traces) is on. Controls whether or not script content will be included. Include script source content This option only shows up when the experiment flag (Enable collecting source text from compiled script) is on, also this option will be disabled if the 'Include script content' is not selected. Controls whether or not to include the script source content (sourcemaps) in the exported trace. Before: https://imgur.com/a/rajLmNc After: https://imgur.com/a/2H0t2uG Bug: 432043591 Change-Id: I0acc5778db84025f19a3f8431793dcf3e3ef1cbe Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6803185 Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Vidal Diazleal <[email protected]>
1 parent 017f278 commit 8605148

File tree

13 files changed

+667
-93
lines changed

13 files changed

+667
-93
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1934,6 +1934,7 @@ grd_files_unbundled_sources = [
19341934
"front_end/panels/timeline/components/BreadcrumbsUI.js",
19351935
"front_end/panels/timeline/components/CPUThrottlingSelector.js",
19361936
"front_end/panels/timeline/components/DetailsView.js",
1937+
"front_end/panels/timeline/components/ExportTraceOptions.js",
19371938
"front_end/panels/timeline/components/FieldSettingsDialog.js",
19381939
"front_end/panels/timeline/components/IgnoreListSetting.js",
19391940
"front_end/panels/timeline/components/InteractionBreakdown.js",
@@ -1954,6 +1955,7 @@ grd_files_unbundled_sources = [
19541955
"front_end/panels/timeline/components/Utils.js",
19551956
"front_end/panels/timeline/components/breadcrumbsUI.css.js",
19561957
"front_end/panels/timeline/components/cpuThrottlingSelector.css.js",
1958+
"front_end/panels/timeline/components/exportTraceOptions.css.js",
19571959
"front_end/panels/timeline/components/fieldSettingsDialog.css.js",
19581960
"front_end/panels/timeline/components/ignoreListSetting.css.js",
19591961
"front_end/panels/timeline/components/insights/BaseInsightComponent.js",

front_end/panels/timeline/TimelinePanel.test.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ describeWithEnvironment('TimelinePanel', function() {
215215
const closeSpy = sinon.stub(fileManager, 'close');
216216

217217
await timeline.saveToFile({
218-
savingEnhancedTrace: false,
218+
includeScriptContent: false,
219+
includeSourceMaps: false,
219220
addModifications: true,
220221
});
221222

@@ -252,7 +253,8 @@ describeWithEnvironment('TimelinePanel', function() {
252253
sinon.stub(fileManager, 'close');
253254

254255
await timeline.saveToFile({
255-
savingEnhancedTrace: false,
256+
includeScriptContent: false,
257+
includeSourceMaps: false,
256258
addModifications: false,
257259
});
258260

@@ -279,7 +281,8 @@ describeWithEnvironment('TimelinePanel', function() {
279281
});
280282
sinon.stub(fileManager, 'close');
281283
await timeline.saveToFile({
282-
savingEnhancedTrace: false,
284+
includeScriptContent: false,
285+
includeSourceMaps: false,
283286
addModifications: true,
284287
});
285288
sinon.assert.calledOnce(saveSpy);
@@ -301,7 +304,8 @@ describeWithEnvironment('TimelinePanel', function() {
301304
sinon.stub(fileManager, 'close');
302305

303306
await timeline.saveToFile({
304-
savingEnhancedTrace: false,
307+
includeScriptContent: false,
308+
includeSourceMaps: false,
305309
addModifications: false,
306310
});
307311

@@ -480,7 +484,8 @@ describeWithEnvironment('TimelinePanel', function() {
480484
});
481485

482486
await timeline.saveToFile({
483-
savingEnhancedTrace: false,
487+
includeScriptContent: false,
488+
includeSourceMaps: false,
484489
addModifications: true,
485490
});
486491

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 25 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -123,18 +123,6 @@ const UIStrings = {
123123
* @description Tooltip text that appears when hovering over the largeicon load button
124124
*/
125125
loadTrace: 'Load trace…',
126-
/**
127-
* @description Tooltip text that appears when hovering over the largeicon download button
128-
*/
129-
saveTrace: 'Save trace…',
130-
/**
131-
* @description An option to save trace with annotations that appears in the menu of the toolbar download button. This is the expected default option, therefore it does not mention annotations.
132-
*/
133-
saveTraceWithAnnotationsMenuOption: 'Save trace',
134-
/**
135-
* @description An option to save trace without annotations that appears in the menu of the toolbar download button
136-
*/
137-
saveTraceWithoutAnnotationsMenuOption: 'Save trace without annotations',
138126
/**
139127
* @description Text to take screenshots
140128
*/
@@ -228,16 +216,6 @@ const UIStrings = {
228216
* @description Text in Timeline Panel of the Performance panel
229217
*/
230218
initializingTracing: 'Initializing tracing…',
231-
/**
232-
*
233-
* @description Text for exporting basic traces
234-
*/
235-
exportNormalTraces: 'Basic performance traces',
236-
/**
237-
*
238-
* @description Text for exporting enhanced traces
239-
*/
240-
exportEnhancedTraces: 'Enhanced performance traces',
241219
/**
242220
* @description Tooltip description for a checkbox that toggles the visibility of data added by extensions of this panel (Performance).
243221
*/
@@ -384,7 +362,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
384362
private cpuProfiler!: SDK.CPUProfilerModel.CPUProfilerModel|null;
385363
private clearButton!: UI.Toolbar.ToolbarButton;
386364
private loadButton!: UI.Toolbar.ToolbarButton;
387-
private saveButton!: UI.Toolbar.ToolbarButton|UI.Toolbar.ToolbarMenuButton;
365+
private saveButton!: UI.Toolbar.ToolbarButton|UI.Toolbar.ToolbarMenuButton|UI.Toolbar.ToolbarItem;
388366
private homeButton?: UI.Toolbar.ToolbarButton;
389367
private statusDialog: StatusDialog|null = null;
390368
private landingPage!: UI.Widget.Widget;
@@ -1017,32 +995,6 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
1017995
this.panelToolbar.removeToolbarItem(this.#sidebarToggleButton);
1018996
}
1019997

1020-
#populateDownloadMenu(contextMenu: UI.ContextMenu.ContextMenu): void {
1021-
// If the current trace is annotated, add an option to save it without annotations.
1022-
const currModificationManager = ModificationsManager.activeManager();
1023-
const annotationsExist = currModificationManager && currModificationManager.getAnnotations()?.length > 0;
1024-
1025-
contextMenu.viewSection().appendItem(i18nString(UIStrings.saveTraceWithAnnotationsMenuOption), () => {
1026-
Host.userMetrics.actionTaken(Host.UserMetrics.Action.PerfPanelTraceExported);
1027-
void this.saveToFile({savingEnhancedTrace: false, addModifications: true});
1028-
}, {
1029-
jslogContext: annotationsExist ? 'timeline.save-to-file-with-annotations' :
1030-
'timeline.save-to-file-without-annotations',
1031-
});
1032-
1033-
if (annotationsExist) {
1034-
contextMenu.viewSection().appendItem(i18nString(UIStrings.saveTraceWithoutAnnotationsMenuOption), () => {
1035-
Host.userMetrics.actionTaken(Host.UserMetrics.Action.PerfPanelTraceExported);
1036-
void this.saveToFile({
1037-
savingEnhancedTrace: false,
1038-
addModifications: false,
1039-
});
1040-
}, {
1041-
jslogContext: 'timeline.save-to-file-without-annotations',
1042-
});
1043-
}
1044-
}
1045-
1046998
/**
1047999
* Returns false if this was loaded in a standalone context such that recording is
10481000
* not possible, like an enhanced trace (which opens a new devtools window) or
@@ -1075,30 +1027,12 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
10751027
this.selectFileToLoad();
10761028
});
10771029

1078-
this.saveButton = new UI.Toolbar.ToolbarMenuButton(
1079-
this.#populateDownloadMenu.bind(this), true, false, 'timeline.save-to-file-more-options', 'download');
1080-
this.saveButton.setTitle(i18nString(UIStrings.saveTrace));
1081-
1082-
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_ENHANCED_TRACES)) {
1083-
this.saveButton.element.addEventListener('contextmenu', event => {
1084-
event.preventDefault();
1085-
event.stopPropagation();
1086-
1087-
if (event.ctrlKey || event.button === 2) {
1088-
const contextMenu = new UI.ContextMenu.ContextMenu(event);
1089-
contextMenu.saveSection().appendItem(i18nString(UIStrings.exportNormalTraces), () => {
1090-
void this.saveToFile({savingEnhancedTrace: false, addModifications: false});
1091-
});
1092-
contextMenu.saveSection().appendItem(i18nString(UIStrings.exportEnhancedTraces), () => {
1093-
void this.saveToFile({savingEnhancedTrace: true, addModifications: false});
1094-
});
1095-
1096-
void contextMenu.show();
1097-
} else {
1098-
void this.saveToFile({savingEnhancedTrace: false, addModifications: false});
1099-
}
1100-
});
1101-
}
1030+
const exportTraceOptions = new TimelineComponents.ExportTraceOptions.ExportTraceOptions();
1031+
exportTraceOptions.data = {
1032+
onExport: this.saveToFile.bind(this),
1033+
buttonEnabled: this.state === State.IDLE && this.#hasActiveTrace(),
1034+
};
1035+
this.saveButton = new UI.Toolbar.ToolbarItem(exportTraceOptions);
11021036

11031037
this.panelToolbar.appendSeparator();
11041038
this.panelToolbar.appendToolbarItem(this.loadButton);
@@ -1372,7 +1306,8 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
13721306
* 3. Visual track configuration (re-ordering or hiding tracks).
13731307
*/
13741308
async saveToFile(config: {
1375-
savingEnhancedTrace: boolean,
1309+
includeScriptContent: boolean,
1310+
includeSourceMaps: boolean,
13761311
addModifications: boolean,
13771312
}): Promise<void> {
13781313
if (this.state !== State.IDLE) {
@@ -1392,8 +1327,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
13921327

13931328
const metadata = this.#traceEngineModel.metadata(this.#viewMode.traceIndex) ?? {};
13941329

1395-
const shouldRetainScriptSources = config.savingEnhancedTrace &&
1396-
Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_COMPILED_SOURCES);
1330+
const shouldRetainScriptSources = config.includeScriptContent && config.includeSourceMaps;
13971331
if (!shouldRetainScriptSources) {
13981332
traceEvents = traceEvents.map(event => {
13991333
if (Trace.Types.Events.isAnyScriptCatchupEvent(event) && event.name !== 'StubScriptCatchup') {
@@ -1426,7 +1360,9 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14261360
}
14271361

14281362
try {
1429-
await this.innerSaveToFile(traceEvents, metadata, config);
1363+
await this.innerSaveToFile(
1364+
traceEvents, metadata,
1365+
{savingEnhancedTrace: config.includeScriptContent, addModifications: config.addModifications});
14301366
} catch (e) {
14311367
// We expect the error to be an Error class, but this deals with any weird case where it's not.
14321368
const error = e instanceof Error ? e : new Error(e);
@@ -1943,7 +1879,13 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
19431879
this.#addSidebarIconToToolbar();
19441880
}
19451881

1946-
this.saveButton.setEnabled(this.state === State.IDLE && this.#hasActiveTrace());
1882+
const exportTraceOptionsElement =
1883+
this.saveButton.element as TimelineComponents.ExportTraceOptions.ExportTraceOptions;
1884+
exportTraceOptionsElement.data = {
1885+
onExport: this.saveToFile.bind(this),
1886+
buttonEnabled: this.state === State.IDLE && this.#hasActiveTrace(),
1887+
};
1888+
19471889
this.#historyManager.setEnabled(this.state === State.IDLE);
19481890
this.clearButton.setEnabled(this.state === State.IDLE);
19491891
this.dropTarget.setEnabled(this.state === State.IDLE);
@@ -2099,6 +2041,8 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
20992041

21002042
const exclusiveFilter = this.#exclusiveFilterPerTrace.get(traceIndex) ?? null;
21012043
this.#applyActiveFilters(parsedTrace.Meta.traceIsGeneric, exclusiveFilter);
2044+
(this.saveButton.element as TimelineComponents.ExportTraceOptions.ExportTraceOptions)
2045+
.updateContentVisibility(currentManager ? currentManager.getAnnotations()?.length > 0 : false);
21022046

21032047
// Add ModificationsManager listeners for annotations change to update the Annotation Overlays.
21042048
currentManager?.addEventListener(AnnotationModifiedEvent.eventName, event => {
@@ -2130,6 +2074,8 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
21302074
const annotations = currentManager.getAnnotations();
21312075
const annotationEntryToColorMap = this.buildColorsAnnotationsMap(annotations);
21322076
this.#sideBar.setAnnotations(annotations, annotationEntryToColorMap);
2077+
(this.saveButton.element as TimelineComponents.ExportTraceOptions.ExportTraceOptions)
2078+
.updateContentVisibility(currentManager ? currentManager.getAnnotations()?.length > 0 : false);
21332079
});
21342080

21352081
// To calculate the activity we might want to zoom in, we use the top-most main-thread track
@@ -3109,7 +3055,7 @@ export class ActionDelegate implements UI.ActionRegistration.ActionDelegate {
31093055
panel.recordReload();
31103056
return true;
31113057
case 'timeline.save-to-file':
3112-
void panel.saveToFile({savingEnhancedTrace: false, addModifications: false});
3058+
void panel.saveToFile({includeScriptContent: false, includeSourceMaps: false, addModifications: false});
31133059
return true;
31143060
case 'timeline.load-from-file':
31153061
panel.selectFileToLoad();

front_end/panels/timeline/components/BUILD.gn

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ generate_css("css_files") {
1111
sources = [
1212
"breadcrumbsUI.css",
1313
"cpuThrottlingSelector.css",
14+
"exportTraceOptions.css",
1415
"fieldSettingsDialog.css",
1516
"ignoreListSetting.css",
1617
"interactionBreakdown.css",
@@ -36,6 +37,7 @@ devtools_module("components") {
3637
"BreadcrumbsUI.ts",
3738
"CPUThrottlingSelector.ts",
3839
"DetailsView.ts",
40+
"ExportTraceOptions.ts",
3941
"FieldSettingsDialog.ts",
4042
"IgnoreListSetting.ts",
4143
"InteractionBreakdown.ts",
@@ -112,6 +114,7 @@ ts_library("unittests") {
112114
sources = [
113115
"BreadcrumbsUI.test.ts",
114116
"CPUThrottlingSelector.test.ts",
117+
"ExportTraceOptions.test.ts",
115118
"FieldSettingsDialog.test.ts",
116119
"IgnoreListSetting.test.ts",
117120
"InteractionBreakdown.test.ts",

0 commit comments

Comments
 (0)