Skip to content

Commit acae917

Browse files
Adam RaineDevtools-frontend LUCI CQ
authored andcommitted
[DOMSize] Filter by pid on cross-origin navigations
DOM stats events can leak across cross-origin navigations because the pages before and after the navigation will not share a process. This CL prevents DOM stats from leaking by also checking for the correct PID for insights related to a navigation. Bug: None Change-Id: I1df8e8451bcefbf6807a6e2d32cb22630ddf7505 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6219828 Commit-Queue: Paul Irish <[email protected]> Auto-Submit: Adam Raine <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent 792886a commit acae917

File tree

5 files changed

+32
-0
lines changed

5 files changed

+32
-0
lines changed

front_end/models/trace/insights/DOMSize.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,23 @@ describeWithEnvironment('DOMSize', function() {
4141
assert.strictEqual(domStats.maxChildren!.numChildren, 4);
4242
assert.strictEqual(domStats.maxChildren!.nodeName, 'BODY');
4343
});
44+
45+
it('separates dom stats in a cross-origin navigation', async () => {
46+
const {data, insights} = await processTrace(this, 'dom-size-overlap.json.gz');
47+
48+
const navigations =
49+
[...data.Meta.navigationsByNavigationId.values()].filter(n => n.args.data?.isOutermostMainFrame);
50+
{
51+
const insight = getInsightOrError('DOMSize', insights, navigations[0]);
52+
const domStats = insight.maxDOMStats!.args.data;
53+
assert.strictEqual(domStats.totalElements, 6811);
54+
}
55+
{
56+
const insight = getInsightOrError('DOMSize', insights, navigations[1]);
57+
const domStats = insight.maxDOMStats!.args.data;
58+
// If we don't filter by process id, the DOM stats events from the previous navigation
59+
// would leak into this one and this # would be much higher
60+
assert.strictEqual(domStats.totalElements, 7);
61+
}
62+
});
4463
});

front_end/models/trace/insights/DOMSize.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,14 @@ export function generateInsight(
124124
const domStatsEvents = parsedTrace.DOMStats.domStatsByFrameId.get(context.frameId)?.filter(isWithinContext) ?? [];
125125
let maxDOMStats: Types.Events.DOMStats|undefined;
126126
for (const domStats of domStatsEvents) {
127+
// While recording a cross-origin navigation, there can be overlapping dom stats from before & after
128+
// the navigation which share a frameId. In this case we should also ensure the pid matches up with
129+
// the navigation we care about (i.e. from after the navigation event).
130+
const navigationPid = context.navigation?.pid;
131+
if (navigationPid && domStats.pid !== navigationPid) {
132+
continue;
133+
}
134+
127135
if (!maxDOMStats || domStats.args.data.totalElements > maxDOMStats.args.data.totalElements) {
128136
maxDOMStats = domStats;
129137
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ copy_to_gen("traces") {
2222
"cls-single-frame.json.gz",
2323
"crux.json.gz",
2424
"dom-size-long.json.gz",
25+
"dom-size-overlap.json.gz",
2526
"dom-size.json.gz",
2627
"enhanced-traces.json.gz",
2728
"extension-tracks-and-marks.json.gz",

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,7 @@ This is a trace where a `LargestImagePaint` event occurs after the last `largest
250250
### web-dev-screenshot-source-ids
251251

252252
A trace generated from crrev.com/c/6197645 (January 2025), which changed the format of screenshots from the legacy OBJECT_SNAPSHOT format to be instant events with more information attached to them.
253+
254+
### dom-size-overlap
255+
256+
Trace containing a cross-origin navigation where DOM size events from the pre-navigation page are emitted *after* the navigation event.
Binary file not shown.

0 commit comments

Comments
 (0)