Skip to content

Commit 5e726c5

Browse files
Nancy LiDevtools-frontend LUCI CQ
authored andcommitted
[RPP Insight] Show the insight chip for the network dependency tree related requests
- Show a chip for all requests in the tree - and only show the warning for requests whose depth is at least 2 screenshot: with warning: https://screenshot.googleplex.com/8XVNEcPUTCKBwAZ without warning: https://screenshot.googleplex.com/PEAATy4JcaKB5kA Bug: 372897712 Change-Id: Ie169430089131accd4f60c0b43a91691083b92cf Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6308819 Reviewed-by: Andres Olivares <[email protected]> Commit-Queue: Nancy Li <[email protected]>
1 parent 6ca4ad9 commit 5e726c5

File tree

2 files changed

+49
-10
lines changed

2 files changed

+49
-10
lines changed

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

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {describeWithEnvironment} from '../../../testing/EnvironmentHelpers.js';
66
import {getFirstOrError, getInsightOrError, processTrace} from '../../../testing/InsightHelpers.js';
77
import * as Trace from '../trace.js';
88

9+
import type {RelatedEventsMap} from './types.js';
10+
911
describeWithEnvironment('NetworkDependencyTree', function() {
1012
let insight: Trace.Insights.Types.InsightModels['NetworkDependencyTree'];
1113

@@ -15,7 +17,7 @@ describeWithEnvironment('NetworkDependencyTree', function() {
1517
insight = getInsightOrError('NetworkDependencyTree', insights, firstNav);
1618
});
1719

18-
it('calculates network dependency tree', async () => {
20+
it('calculates network dependency tree', () => {
1921
// The network dependency tree in this trace is
2022
// | .../index.html (ts:566777570990, dur:5005590)
2123
// |
@@ -41,15 +43,15 @@ describeWithEnvironment('NetworkDependencyTree', function() {
4143
assert.lengthOf(child1.children, 0);
4244
});
4345

44-
it('Calculate the max critical path latency', async () => {
46+
it('Calculate the max critical path latency', () => {
4547
// The chain |index.html(root) -> app.js(child1)| is the longest
4648
const root = insight.rootNodes[0];
4749
const child1 = root.children[1];
4850
assert.strictEqual(
4951
insight.maxTime, Trace.Types.Timing.Micro(child1.request.ts + child1.request.dur - root.request.ts));
5052
});
5153

52-
it('Marks the longest network dependency chain', async () => {
54+
it('Marks the longest network dependency chain', () => {
5355
const root = insight.rootNodes[0];
5456
const [child0, child1] = root.children;
5557

@@ -60,7 +62,7 @@ describeWithEnvironment('NetworkDependencyTree', function() {
6062
assert.isNotTrue(child0.isLongest);
6163
});
6264

63-
it('Store the chain in the last request of the chain', async () => {
65+
it('Store the chain in the last request of the chain', () => {
6466
const root = insight.rootNodes[0];
6567
const [child0, child1] = root.children;
6668

@@ -72,4 +74,30 @@ describeWithEnvironment('NetworkDependencyTree', function() {
7274
assert.deepEqual(child0.chain, [root.request, child0.request]);
7375
assert.deepEqual(child1.chain, [root.request, child1.request]);
7476
});
77+
78+
it('Calculates the related events', async () => {
79+
// Need to load a file with longer dependency chain for this test.
80+
// Only those requests whose depth >= 2 will be added to the related events.
81+
const {data, insights} = await processTrace(this, 'web-dev-screenshot-source-ids.json.gz');
82+
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
83+
insight = getInsightOrError('NetworkDependencyTree', insights, firstNav);
84+
85+
// For NetworkDependencyTree, the relatedEvents is a map format.
86+
assert.isFalse(Array.isArray(insight.relatedEvents));
87+
const relatedEvents = insight.relatedEvents as RelatedEventsMap;
88+
89+
// There are a few chains, let test the first chain
90+
// |web.dev -> /css -> 4UasrENHsx...UvQ.woff2|
91+
const root = insight.rootNodes[0];
92+
const child0 = root.children[0];
93+
const child00 = child0.children[0];
94+
95+
// Root's depth is 0, so there isn't any warning message
96+
assert.deepEqual(relatedEvents.get(root.request), []);
97+
// child0's depth is 1, so there isn't any warning message
98+
assert.deepEqual(relatedEvents.get(child0.request), []);
99+
// child00's depth is 2, so there is one warning message
100+
assert.deepEqual(
101+
relatedEvents.get(child00.request), [Trace.Insights.Models.NetworkDependencyTree.UIStrings.warningDescription]);
102+
});
75103
});

front_end/models/trace/insights/NetworkDependencyTree.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
type InsightSetContext,
1616
type InsightSetContextWithNavigation,
1717
type PartialInsightModel,
18+
type RelatedEventsMap,
1819
type RequiredData
1920
} from './types.js';
2021

@@ -28,6 +29,11 @@ export const UIStrings = {
2829
*/
2930
description:
3031
'[Avoid chaining critical requests](https://developer.chrome.com/docs/lighthouse/performance/critical-request-chains) by reducing the length of chains, reducing the download size of resources, or deferring the download of unnecessary resources to improve page load.',
32+
/**
33+
* @description Description of the warning that recommends avoiding chaining critical requests.
34+
*/
35+
warningDescription:
36+
'Avoid chaining critical requests by reducing the length of chains, reducing the download size of resources, or deferring the download of unnecessary resources to improve page load.',
3137
/**
3238
* @description Text status indicating that there isn't long chaining critical network requests.
3339
*/
@@ -125,6 +131,7 @@ export function generateInsight(
125131
}
126132

127133
const rootNodes: CriticalRequestNode[] = [];
134+
const relatedEvents: RelatedEventsMap = new Map();
128135
let maxTime = Types.Timing.Micro(0);
129136

130137
let longestChain: Types.Events.SyntheticNetworkRequest[] = [];
@@ -143,23 +150,26 @@ export function generateInsight(
143150

144151
let currentNodes = rootNodes;
145152

146-
for (const networkRequest of path) {
153+
for (let depth = 0; depth < path.length; ++depth) {
154+
const request = path[depth];
147155
// find the request
148-
let found = currentNodes.find(node => node.request === networkRequest);
156+
let found = currentNodes.find(node => node.request === request);
149157

150158
if (!found) {
151-
const timeFromInitialRequest = Types.Timing.Micro(networkRequest.ts + networkRequest.dur - initialRequest.ts);
159+
const timeFromInitialRequest = Types.Timing.Micro(request.ts + request.dur - initialRequest.ts);
152160
found = {
153-
request: networkRequest,
161+
request,
154162
timeFromInitialRequest,
155163
children: [],
156164
};
157165
currentNodes.push(found);
158166
}
159167

160-
if (networkRequest === lastRequest) {
168+
if (request === lastRequest) {
161169
found.chain = path;
162170
}
171+
// TODO(b/372897712) Switch the UIString to markdown.
172+
relatedEvents.set(request, depth < 2 ? [] : [i18nString(UIStrings.warningDescription)]);
163173

164174
currentNodes = found.children;
165175
}
@@ -183,7 +193,7 @@ export function generateInsight(
183193
return;
184194
}
185195

186-
const networkPath = traversalPath.filter(node => node.type === 'network').reverse().map(node => (node).rawRequest);
196+
const networkPath = traversalPath.filter(node => node.type === 'network').reverse().map(node => node.rawRequest);
187197

188198
// Ignore if some ancestor is not a critical request.
189199
if (networkPath.some(request => (!isCritical(request, context)))) {
@@ -215,5 +225,6 @@ export function generateInsight(
215225
return finalize({
216226
rootNodes,
217227
maxTime,
228+
relatedEvents,
218229
});
219230
}

0 commit comments

Comments
 (0)