Skip to content

Commit e3d94e9

Browse files
vidortegDevtools-frontend LUCI CQ
authored andcommitted
Excluding extension content and sourcemap from exported trace.
This CL excludes script content and sourcemaps from chrome-extension and extension URLs when exporting a trace. Including this data does not enhance the debugging experience and, due to privacy concerns, its removal helps reduce both the complexity and size of the trace. Note that events from these sources will still be captured and displayed during regular trace session. The exclusion only applies when the trace is exported. For testing I'm adding a new trace that contains scripts for 'chrome-extensions' both in the sourcemap and souceText(per trace) section. Bug: 432046665 Change-Id: I5fd1c3df65dd66921f17ab11bb5e274217d2ea3a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6866821 Reviewed-by: Connor Clark <[email protected]> Commit-Queue: Vidal Diazleal <[email protected]>
1 parent 93d16c5 commit e3d94e9

File tree

7 files changed

+136
-8
lines changed

7 files changed

+136
-8
lines changed

front_end/models/trace/helpers/Trace.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,10 @@ export function isTopLevelEvent(event: Types.Events.Event): boolean {
527527
return event.cat.includes(DevToolsTimelineEventCategory) && event.name === Types.Events.Name.RUN_TASK;
528528
}
529529

530+
export function isExtensionUrl(url: string): boolean {
531+
return url.startsWith('extensions:') || url.startsWith('chrome-extension:');
532+
}
533+
530534
function topLevelEventIndexEndingAfter(events: Types.Events.Event[], time: Types.Timing.Micro): number {
531535
let index = Platform.ArrayUtilities.upperBound(events, time, (time, event) => time - event.ts) - 1;
532536
while (index > 0 && !isTopLevelEvent(events[index])) {

front_end/models/trace/types/File.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ export interface MetaData {
219219
hostDPR?: number;
220220
}
221221

222-
interface MetadataSourceMap {
222+
export interface MetadataSourceMap {
223223
url: string;
224224
/** If not defined, then this was a data url. */
225225
sourceMapUrl?: string;

front_end/panels/timeline/TimelinePanel.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,5 +446,93 @@ describeWithEnvironment('TimelinePanel', function() {
446446
assert.deepEqual(file.traceEvents, traceEvents);
447447
});
448448
});
449+
450+
describe('removes chrome-extensions content', function() {
451+
it('from trace events when saving a trace with "Include script content" on', async function() {
452+
const {traceEvents, metadata} =
453+
await TraceLoader.traceFile(this, 'chrome-ext-sourcemap-script-content.json.gz');
454+
await timeline.loadingComplete(traceEvents as Trace.Types.Events.Event[], null, metadata);
455+
456+
// 7192505913775043000.8 matches a chrome-extension script in the trace
457+
let extensionTracesWithContent = traceEvents.filter(value => {
458+
return value.cat === 'disabled-by-default-devtools.v8-source-rundown-sources' &&
459+
`${(value as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent).args.data.isolate}.${
460+
(value as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent).args.data.scriptId}` ===
461+
'7192505913775043000.8';
462+
});
463+
464+
// loading the trace and verifying the chrome extension script has associated source text
465+
let castedEvent =
466+
(extensionTracesWithContent[0] as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent);
467+
assert.lengthOf(extensionTracesWithContent, 1);
468+
assert.isDefined(castedEvent.args.data.sourceText);
469+
470+
await timeline.saveToFile({
471+
includeScriptContent: true,
472+
includeSourceMaps: false,
473+
addModifications: false,
474+
});
475+
476+
sinon.assert.calledOnce(saveSpy);
477+
sinon.assert.calledOnce(closeSpy);
478+
479+
const [fileName, contentData] = saveSpy.getCall(0).args;
480+
assert.match(fileName, /EnhancedTrace-[\d|T]+\.json$/);
481+
482+
const file = await contentDataToFile(contentData);
483+
assert.isDefined(file.metadata.enhancedTraceVersion);
484+
485+
// getting the same trace as before, but this time after saving has happened.
486+
extensionTracesWithContent = file.traceEvents?.filter(value => {
487+
return value.cat === 'disabled-by-default-devtools.v8-source-rundown-sources' &&
488+
`${(value as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent).args.data.isolate}.${
489+
(value as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent).args.data.scriptId}` ===
490+
'7192505913775043000.8';
491+
});
492+
493+
// the associated source text is now undefined from the chrome-extension script
494+
castedEvent = (extensionTracesWithContent[0] as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent);
495+
assert.lengthOf(extensionTracesWithContent, 1);
496+
assert.isUndefined(castedEvent.args.data.sourceText);
497+
498+
// non-extension script content is still present (7192505913775043000.10)
499+
extensionTracesWithContent = file.traceEvents?.filter(value => {
500+
return value.cat === 'disabled-by-default-devtools.v8-source-rundown-sources' &&
501+
`${(value as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent).args.data.isolate}.${
502+
(value as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent).args.data.scriptId}` ===
503+
'7192505913775043000.10';
504+
});
505+
castedEvent = (extensionTracesWithContent[0] as Trace.Types.Events.V8SourceRundownSourcesScriptCatchupEvent);
506+
assert.lengthOf(extensionTracesWithContent, 1);
507+
assert.isDefined(castedEvent.args.data.sourceText);
508+
});
509+
510+
it('from trace sourcemaps when saving a trace with "Include source map" on', async function() {
511+
const {traceEvents, metadata} =
512+
await TraceLoader.traceFile(this, 'chrome-ext-sourcemap-script-content.json.gz');
513+
514+
await timeline.innerSaveToFile(traceEvents, metadata, {
515+
includeScriptContent: true,
516+
includeSourceMaps: true,
517+
addModifications: false,
518+
});
519+
520+
sinon.assert.calledOnce(saveSpy);
521+
sinon.assert.calledOnce(closeSpy);
522+
523+
const [fileName, contentData] = saveSpy.getCall(0).args;
524+
assert.match(fileName, /EnhancedTrace-[\d|T]+\.json$/);
525+
526+
const file = await contentDataToFile(contentData);
527+
assert.isDefined(file.metadata.enhancedTraceVersion);
528+
529+
const totalSourceMapsWithChromExtensionProtocol = file.metadata.sourceMaps?.filter(value => {
530+
value.url.startsWith('chrome-extension:');
531+
});
532+
assert.isNotNull(totalSourceMapsWithChromExtensionProtocol);
533+
assert.strictEqual(totalSourceMapsWithChromExtensionProtocol?.length, 0);
534+
});
535+
});
536+
449537
});
450538
});

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1407,11 +1407,24 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14071407
return;
14081408
}
14091409

1410+
// Grab the script mapping to be able to filter out by url.
1411+
const mappedScriptsWithData = Trace.Handlers.ModelHandlers.Scripts.data().scripts;
1412+
const scriptByIdMap = new Map<string, Trace.Handlers.ModelHandlers.Scripts.Script>();
1413+
1414+
for (const mapScript of mappedScriptsWithData) {
1415+
scriptByIdMap.set(`${mapScript.isolate}.${mapScript.scriptId}`, mapScript);
1416+
}
1417+
14101418
const metadata = this.#traceEngineModel.metadata(this.#viewMode.traceIndex) ?? {};
14111419

1412-
if (!config.includeScriptContent) {
1413-
traceEvents = traceEvents.map(event => {
1414-
if (Trace.Types.Events.isAnyScriptCatchupEvent(event) && event.name !== 'StubScriptCatchup') {
1420+
traceEvents = traceEvents.map(event => {
1421+
if (Trace.Types.Events.isAnyScriptCatchupEvent(event) && event.name !== 'StubScriptCatchup') {
1422+
const mappedScript = scriptByIdMap.get(`${event.args.data.isolate}.${event.args.data.scriptId}`);
1423+
1424+
// If the checkbox to include script content is not checked or if it comes from and
1425+
// extension we dont include the script content.
1426+
if (!config.includeScriptContent ||
1427+
(mappedScript?.url && Trace.Helpers.Trace.isExtensionUrl(mappedScript.url))) {
14151428
return {
14161429
cat: event.cat,
14171430
name: 'StubScriptCatchup',
@@ -1425,10 +1438,10 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14251438
},
14261439
} as Trace.Types.Events.V8SourceRundownSourcesStubScriptCatchupEvent;
14271440
}
1441+
}
14281442

1429-
return event;
1430-
});
1431-
}
1443+
return event;
1444+
});
14321445

14331446
metadata.modifications = config.addModifications ? ModificationsManager.activeManager()?.toJSON() : undefined;
14341447

@@ -1483,9 +1496,11 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
14831496
const profile = Trace.Helpers.SamplesIntegrator.SamplesIntegrator.extractCpuProfileFromFakeTrace(traceEvents);
14841497
blobParts = [JSON.stringify(profile)];
14851498
} else {
1499+
const filteredMetadataSourceMaps =
1500+
includeScriptContent && includeSourceMaps ? this.#filterMetadataSourceMaps(metadata) : undefined;
14861501
const formattedTraceIter = traceJsonGenerator(traceEvents, {
14871502
...metadata,
1488-
sourceMaps: includeScriptContent && includeSourceMaps ? metadata.sourceMaps : undefined,
1503+
sourceMaps: filteredMetadataSourceMaps,
14891504
});
14901505
blobParts = Array.from(formattedTraceIter);
14911506
}
@@ -1532,6 +1547,18 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
15321547
}
15331548
}
15341549

1550+
#filterMetadataSourceMaps(metadata: Trace.Types.File.MetaData): Trace.Types.File.MetadataSourceMap[]|undefined {
1551+
if (!metadata.sourceMaps) {
1552+
return undefined;
1553+
}
1554+
1555+
// extensions sourcemaps provide little to no-value for the exported trace
1556+
// debugging, so they are filtered out.
1557+
return metadata.sourceMaps.filter(value => {
1558+
return value.url && Trace.Helpers.Trace.isExtensionUrl(value.url);
1559+
});
1560+
}
1561+
15351562
#showExportTraceErrorDialog(error: Error): void {
15361563
if (this.statusDialog) {
15371564
this.statusDialog.remove();

front_end/panels/timeline/fixtures/traces/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ copy_to_gen("traces") {
1515
"basic.json.gz",
1616
"byte-efficiency.json.gz",
1717
"changing-priority.json.gz",
18+
"chrome-ext-sourcemap-script-content.json.gz",
1819
"cls-cluster-max-duration.json.gz",
1920
"cls-cluster-max-timeout.json.gz",
2021
"cls-cluster-navigation.json.gz",

front_end/panels/timeline/fixtures/traces/README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,14 @@ A very barebones trace; contains info on browser processes and threads but very
4848

4949
A recording of a page load of the Lighthouse byte-efficiency tester. https://github.com/GoogleChrome/lighthouse/blob/main/cli/test/fixtures/byte-efficiency/tester.html
5050

51+
### chrome-ext-sourcemap-script-content.json.gz
52+
53+
Trace generated after installing https://chromewebstore.google.com/detail/react-developer-tools/. A regular trace that contains two sections
54+
not found in other traces:
55+
56+
1. A sourcemap in the metadata array with a 'chrome-extension' protocol.
57+
2. 'V8SourceRundownSourcesScriptCatchupEvent' traces that have a 'chrome-extension' protocol with associated source text.
58+
5159
### dom-size.json.gz
5260

5361
Generate from a recording of [this HTML file](https://gist.github.com/adamraine/bfdb3cecca2322bf74f1e725d9a4699d) with the following steps:
Binary file not shown.

0 commit comments

Comments
 (0)