Skip to content

Commit 3067d40

Browse files
Adriana IxbaDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Use main thread and treeView bottomUpRootNode for 3P insight
This also simplifies the insight shape closer to how we use it in RPP, and how we will use it in LH. This also simplifies some tests. This modifies how we generate eventIDs when grouping by 3P. This fixes a bug where when grouping by 3P, generateEventID can incorrectly group events of different entities. This fixes: - misalignment in main thread times insight<->3P table (by using same data source, including instant events, filters) - incorrect event grouping: breaking bottomUp button and causing further incorrect selftime/transfersize (bottomUp tree node ID generation) Bug: 394651390 Change-Id: I1660a5b5b9a90be89815aad858caea94f153eabb Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6341914 Reviewed-by: Paul Irish <[email protected]> Commit-Queue: Paul Irish <[email protected]>
1 parent a01747c commit 3067d40

File tree

17 files changed

+457
-458
lines changed

17 files changed

+457
-458
lines changed

front_end/models/trace/extras/ThirdParties.test.ts

Lines changed: 24 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -6,44 +6,39 @@ import {describeWithEnvironment} from '../../../testing/EnvironmentHelpers.js';
66
import {TraceLoader} from '../../../testing/TraceLoader.js';
77
import * as Trace from '../trace.js';
88

9+
function extractUrlsFromSummaries(summaries: Trace.Extras.ThirdParties.Summary[]): Array<[string, string[]]> {
10+
return summaries.map(s => {
11+
const uniqueUrls = new Set<string>();
12+
s.relatedEvents?.forEach(e => {
13+
const url = e.args?.data?.url;
14+
if (url) {
15+
uniqueUrls.add(url);
16+
}
17+
});
18+
return [s.entity.name, Array.from(uniqueUrls)];
19+
});
20+
}
21+
922
describeWithEnvironment('ThirdParties', function() {
1023
describe('byTraceBounds', function() {
1124
it('full trace bounds', async function() {
1225
const {parsedTrace} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
26+
const summaries = Trace.Extras.ThirdParties.summarizeThirdParties(parsedTrace, parsedTrace.Meta.traceBounds);
1327

14-
const requests = parsedTrace.NetworkRequests.byTime.filter(
15-
request => Trace.Helpers.Timing.eventIsInBounds(request, parsedTrace.Meta.traceBounds));
16-
const thirdPartySummary =
17-
Trace.Extras.ThirdParties.summarizeThirdParties(parsedTrace, parsedTrace.Meta.traceBounds, requests);
18-
19-
const results = [...thirdPartySummary.byEntity.entries()].map(([entity, summary]) => [entity.name, summary]);
28+
const results = summaries.map(s => [s.entity.name, s.mainThreadTime, s.transferSize]);
2029
assert.deepEqual(results, [
21-
['localhost', {mainThreadTime: 26381, transferSize: 751}],
22-
['Google Fonts', {mainThreadTime: 0, transferSize: 0}],
23-
]);
24-
25-
assert.deepEqual([...thirdPartySummary.byUrl.entries()], [
26-
['extensions::SafeBuiltins', {mainThreadTime: 1448, transferSize: 0}],
27-
['http://localhost:8080/', {mainThreadTime: 21674, transferSize: 751}],
28-
['http://localhost:8080/blocking.js', {mainThreadTime: 2451, transferSize: 0}],
29-
['http://localhost:8080/module.js', {mainThreadTime: 2256, transferSize: 0}],
30-
['https://fonts.googleapis.com/css2?family=Orelega+One&display=swap', {mainThreadTime: 0, transferSize: 0}],
31-
['http://localhost:8080/styles.css', {mainThreadTime: 0, transferSize: 0}],
32-
[
33-
'https://fonts.gstatic.com/s/orelegaone/v1/3qTpojOggD2XtAdFb-QXZFt93kY.woff2',
34-
{mainThreadTime: 0, transferSize: 0}
35-
],
30+
['localhost', 24.947999954223633, 2254],
31+
['Google Fonts', 0, 25325],
3632
]);
37-
38-
const urls = [...thirdPartySummary.urlsByEntity.entries()].map(([entity, urls]) => [entity.name, [...urls]]);
33+
const urls = extractUrlsFromSummaries(summaries);
3934
assert.deepEqual(urls, [
4035
[
4136
'localhost',
4237
[
4338
'http://localhost:8080/',
39+
'http://localhost:8080/styles.css',
4440
'http://localhost:8080/blocking.js',
4541
'http://localhost:8080/module.js',
46-
'http://localhost:8080/styles.css',
4742
]
4843
],
4944
[
@@ -56,38 +51,6 @@ describeWithEnvironment('ThirdParties', function() {
5651
]);
5752
});
5853

59-
it('works even without network requests', async function() {
60-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
61-
62-
const thirdPartySummary =
63-
Trace.Extras.ThirdParties.summarizeThirdParties(parsedTrace, parsedTrace.Meta.traceBounds, []);
64-
65-
const results = [...thirdPartySummary.byEntity.entries()].map(([entity, summary]) => [entity.name, summary]);
66-
assert.deepEqual(results, [
67-
// Since network requests were not given, there is no transfer size.
68-
['localhost', {mainThreadTime: 26381, transferSize: 0}],
69-
]);
70-
71-
assert.deepEqual([...thirdPartySummary.byUrl.entries()], [
72-
['extensions::SafeBuiltins', {mainThreadTime: 1448, transferSize: 0}],
73-
['http://localhost:8080/', {mainThreadTime: 21674, transferSize: 0}],
74-
['http://localhost:8080/blocking.js', {mainThreadTime: 2451, transferSize: 0}],
75-
['http://localhost:8080/module.js', {mainThreadTime: 2256, transferSize: 0}],
76-
]);
77-
78-
const urls = [...thirdPartySummary.urlsByEntity.entries()].map(([entity, urls]) => [entity.name, [...urls]]);
79-
assert.deepEqual(urls, [
80-
[
81-
'localhost',
82-
[
83-
'http://localhost:8080/',
84-
'http://localhost:8080/blocking.js',
85-
'http://localhost:8080/module.js',
86-
]
87-
],
88-
]);
89-
});
90-
9154
it('partial trace bounds', async function() {
9255
const {parsedTrace} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
9356

@@ -96,14 +59,12 @@ describeWithEnvironment('ThirdParties', function() {
9659
const max = Trace.Types.Timing.Micro(1634222320000);
9760
const bounds = {min, max, range: Trace.Types.Timing.Micro(max - min)};
9861

99-
const requests =
100-
parsedTrace.NetworkRequests.byTime.filter(request => Trace.Helpers.Timing.eventIsInBounds(request, bounds));
101-
const thirdPartySummary = Trace.Extras.ThirdParties.summarizeThirdParties(parsedTrace, bounds, requests);
62+
const summaries = Trace.Extras.ThirdParties.summarizeThirdParties(parsedTrace, bounds);
10263

103-
const results = [...thirdPartySummary.byEntity.entries()].map(([entity, summary]) => [entity.name, summary]);
64+
const results = summaries.map(s => [s.entity.name, s.mainThreadTime, s.transferSize]);
10465
assert.deepEqual(results, [
10566
// No main thread during given bounds. Some network.
106-
['localhost', {mainThreadTime: 0, transferSize: 751}],
67+
['localhost', 0, 419],
10768
]);
10869
});
10970

@@ -113,12 +74,9 @@ describeWithEnvironment('ThirdParties', function() {
11374
const min = Trace.Types.Timing.Micro(1634230000000);
11475
const max = Trace.Types.Timing.Micro(1634231000000);
11576
const bounds = {min, max, range: Trace.Types.Timing.Micro(max - min)};
77+
const summaries = Trace.Extras.ThirdParties.summarizeThirdParties(parsedTrace, bounds);
11678

117-
const requests =
118-
parsedTrace.NetworkRequests.byTime.filter(request => Trace.Helpers.Timing.eventIsInBounds(request, bounds));
119-
const thirdPartySummary = Trace.Extras.ThirdParties.summarizeThirdParties(parsedTrace, bounds, requests);
120-
121-
const results = [...thirdPartySummary.byEntity.entries()].map(([entity, summary]) => [entity.name, summary]);
79+
const results = summaries.map(s => [s.entity.name, s.mainThreadTime, s.transferSize]);
12280
assert.deepEqual(results, []);
12381
});
12482
});

0 commit comments

Comments
 (0)