Skip to content

Commit 948827b

Browse files
vidortegDevtools-frontend LUCI CQ
authored andcommitted
Fixing flipped logic in TimelinePanel export trace.
This CL fixes some logic that is flipped in the recently added export trace dialog. Before the fix: Script content will only be added to the exported trace if "Include Script Content" and "Include Sourcemap" are BOTH selected, something similar happening to "Include Sourcemap" After the fix: Script content will be added correctly if the "Include Script Content" checkbox is selected, same for the "Include Sourcemap" checkbox. I added a test for this behavior. Bug: 438908750 Change-Id: I0d08ca31361788a7fb168498775a42a6fa5c3ec1 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6854692 Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Vidal Diazleal <[email protected]>
1 parent 8d89cca commit 948827b

File tree

2 files changed

+42
-16
lines changed

2 files changed

+42
-16
lines changed

front_end/panels/timeline/TimelinePanel.test.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,8 @@ describeWithEnvironment('TimelinePanel', function() {
311311
it('saves a regular trace file', async function() {
312312
const {traceEvents, metadata} = await TraceLoader.traceFile(this, 'web-dev.json.gz');
313313
await timeline.innerSaveToFile(traceEvents, metadata, {
314-
savingEnhancedTrace: false,
314+
includeScriptContent: false,
315+
includeSourceMaps: false,
315316
addModifications: false,
316317
});
317318

@@ -340,7 +341,8 @@ describeWithEnvironment('TimelinePanel', function() {
340341
const {traceEvents, metadata} = file;
341342

342343
await timeline.innerSaveToFile(traceEvents, metadata, {
343-
savingEnhancedTrace: false,
344+
includeScriptContent: false,
345+
includeSourceMaps: false,
344346
addModifications: false,
345347
});
346348

@@ -356,10 +358,11 @@ describeWithEnvironment('TimelinePanel', function() {
356358
assert.deepEqual(cpuFile, profile2);
357359
});
358360

359-
it('saves an enhanced trace file', async function() {
361+
it('saves an enhanced trace file without sourcemaps', async function() {
360362
const {traceEvents, metadata} = await TraceLoader.traceFile(this, 'enhanced-traces.json.gz');
361363
await timeline.innerSaveToFile(traceEvents, metadata, {
362-
savingEnhancedTrace: true,
364+
includeScriptContent: true,
365+
includeSourceMaps: false,
363366
addModifications: false,
364367
});
365368

@@ -371,6 +374,26 @@ describeWithEnvironment('TimelinePanel', function() {
371374

372375
const file = await contentDataToFile(contentData);
373376
assert.isDefined(file.metadata.enhancedTraceVersion);
377+
assert.isUndefined(file.metadata.sourceMaps);
378+
});
379+
380+
it('saves an enhanced trace file with sourcemaps', async function() {
381+
const {traceEvents, metadata} = await TraceLoader.traceFile(this, 'dupe-js-inline-maps.json.gz');
382+
await timeline.innerSaveToFile(traceEvents, metadata, {
383+
includeScriptContent: true,
384+
includeSourceMaps: true,
385+
addModifications: false,
386+
});
387+
388+
sinon.assert.calledOnce(saveSpy);
389+
sinon.assert.calledOnce(closeSpy);
390+
391+
const [fileName, contentData] = saveSpy.getCall(0).args;
392+
assert.match(fileName, /EnhancedTrace-[\d|T]+\.json\.gz$/);
393+
394+
const file = await contentDataToFile(contentData);
395+
assert.isDefined(file.metadata.enhancedTraceVersion);
396+
assert.isDefined(file.metadata.sourceMaps);
374397
});
375398

376399
it('saves a trace file with modifications', async function() {
@@ -407,7 +430,8 @@ describeWithEnvironment('TimelinePanel', function() {
407430
it('saves a regular trace file', async function() {
408431
const {traceEvents, metadata} = await TraceLoader.traceFile(this, 'web-dev.json.gz');
409432
await timeline.innerSaveToFile(traceEvents, metadata, {
410-
savingEnhancedTrace: false,
433+
includeScriptContent: false,
434+
includeSourceMaps: false,
411435
addModifications: false,
412436
});
413437

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,8 +1409,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14091409

14101410
const metadata = this.#traceEngineModel.metadata(this.#viewMode.traceIndex) ?? {};
14111411

1412-
const shouldRetainScriptSources = config.includeScriptContent && config.includeSourceMaps;
1413-
if (!shouldRetainScriptSources) {
1412+
if (!config.includeScriptContent) {
14141413
traceEvents = traceEvents.map(event => {
14151414
if (Trace.Types.Events.isAnyScriptCatchupEvent(event) && event.name !== 'StubScriptCatchup') {
14161415
return {
@@ -1442,9 +1441,11 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14421441
// export the config.
14431442

14441443
try {
1445-
await this.innerSaveToFile(
1446-
traceEvents, metadata,
1447-
{savingEnhancedTrace: config.includeScriptContent, addModifications: config.addModifications});
1444+
await this.innerSaveToFile(traceEvents, metadata, {
1445+
includeScriptContent: config.includeScriptContent,
1446+
includeSourceMaps: config.includeSourceMaps,
1447+
addModifications: config.addModifications
1448+
});
14481449
} catch (e) {
14491450
// We expect the error to be an Error class, but this deals with any weird case where it's not.
14501451
const error = e instanceof Error ? e : new Error(e);
@@ -1460,21 +1461,22 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14601461
}
14611462

14621463
async innerSaveToFile(traceEvents: readonly Trace.Types.Events.Event[], metadata: Trace.Types.File.MetaData, config: {
1463-
savingEnhancedTrace: boolean,
1464+
includeScriptContent: boolean,
1465+
includeSourceMaps: boolean,
14641466
addModifications: boolean,
14651467
}): Promise<void> {
14661468
// Base the filename on the trace's time of recording
14671469
const isoDate =
14681470
Platform.DateUtilities.toISO8601Compact(metadata.startTime ? new Date(metadata.startTime) : new Date());
14691471

14701472
const isCpuProfile = metadata.dataOrigin === Trace.Types.File.DataOrigin.CPU_PROFILE;
1471-
const {savingEnhancedTrace} = config;
1473+
const {includeScriptContent, includeSourceMaps} = config;
14721474
metadata.enhancedTraceVersion =
1473-
savingEnhancedTrace ? SDK.EnhancedTracesParser.EnhancedTracesParser.enhancedTraceVersion : undefined;
1475+
includeScriptContent ? SDK.EnhancedTracesParser.EnhancedTracesParser.enhancedTraceVersion : undefined;
14741476

14751477
let fileName = (isCpuProfile ? `CPU-${isoDate}.cpuprofile` :
1476-
savingEnhancedTrace ? `EnhancedTrace-${isoDate}.json` :
1477-
`Trace-${isoDate}.json`) as Platform.DevToolsPath.RawPathString;
1478+
includeScriptContent ? `EnhancedTrace-${isoDate}.json` :
1479+
`Trace-${isoDate}.json`) as Platform.DevToolsPath.RawPathString;
14781480

14791481
let blobParts: string[] = [];
14801482
if (isCpuProfile) {
@@ -1483,7 +1485,7 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14831485
} else {
14841486
const formattedTraceIter = traceJsonGenerator(traceEvents, {
14851487
...metadata,
1486-
sourceMaps: savingEnhancedTrace ? metadata.sourceMaps : undefined,
1488+
sourceMaps: includeScriptContent && includeSourceMaps ? metadata.sourceMaps : undefined,
14871489
});
14881490
blobParts = Array.from(formattedTraceIter);
14891491
}

0 commit comments

Comments
 (0)