Skip to content

Commit b1658cf

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Rename insights to match model/component separation
Also renamed a couple insight models to match their component name. Bug: 371615739 Change-Id: I0776c4ed7cc1c37b9bd10695795b19c430d09317 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5991675 Commit-Queue: Connor Clark <[email protected]> Auto-Submit: Connor Clark <[email protected]> Reviewed-by: Adam Raine <[email protected]>
1 parent 0f8508b commit b1658cf

39 files changed

+161
-165
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,16 +1076,16 @@ grd_files_debug_sources = [
10761076
"front_end/models/trace/helpers/Timing.js",
10771077
"front_end/models/trace/helpers/Trace.js",
10781078
"front_end/models/trace/helpers/TreeHelpers.js",
1079+
"front_end/models/trace/insights/CLSCulprits.js",
10791080
"front_end/models/trace/insights/Common.js",
1080-
"front_end/models/trace/insights/CumulativeLayoutShift.js",
10811081
"front_end/models/trace/insights/DocumentLatency.js",
10821082
"front_end/models/trace/insights/FontDisplay.js",
1083-
"front_end/models/trace/insights/InsightRunners.js",
10841083
"front_end/models/trace/insights/InteractionToNextPaint.js",
10851084
"front_end/models/trace/insights/LargestContentfulPaint.js",
1085+
"front_end/models/trace/insights/Models.js",
10861086
"front_end/models/trace/insights/RenderBlocking.js",
10871087
"front_end/models/trace/insights/SlowCSSSelector.js",
1088-
"front_end/models/trace/insights/ThirdPartyWeb.js",
1088+
"front_end/models/trace/insights/ThirdParties.js",
10891089
"front_end/models/trace/insights/Viewport.js",
10901090
"front_end/models/trace/insights/types.js",
10911091
"front_end/models/trace/lantern/core/LanternError.js",

front_end/models/trace/Processor.test.ts

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ describeWithEnvironment('TraceProcessor', function() {
263263
generateInsight: () => {
264264
throw new Error('forced error');
265265
},
266-
deps: Trace.Insights.InsightRunners.RenderBlocking.deps,
266+
deps: Trace.Insights.Models.RenderBlocking.deps,
267267
},
268268
};
269269
});
@@ -278,8 +278,8 @@ describeWithEnvironment('TraceProcessor', function() {
278278

279279
const insights = Array.from(processor.insights.values());
280280
assert.strictEqual(insights.length, 2);
281-
assert(insights[1].data.RenderBlocking instanceof Error, 'RenderBlocking did not throw an error');
282-
assert.strictEqual(insights[1].data.RenderBlocking.message, 'forced error');
281+
assert(insights[1].model.RenderBlocking instanceof Error, 'RenderBlocking did not throw an error');
282+
assert.strictEqual(insights[1].model.RenderBlocking.message, 'forced error');
283283
});
284284

285285
it('skips insights that are missing one or more dependencies', async function() {
@@ -299,8 +299,8 @@ describeWithEnvironment('TraceProcessor', function() {
299299
]);
300300

301301
const insights = Array.from(processor.insights.values());
302-
assert.isUndefined(insights[0].data.RenderBlocking);
303-
assert.isUndefined(insights[1].data.RenderBlocking);
302+
assert.isUndefined(insights[0].model.RenderBlocking);
303+
assert.isUndefined(insights[1].model.RenderBlocking);
304304
});
305305

306306
it('returns insights for a navigation', async function() {
@@ -318,15 +318,15 @@ describeWithEnvironment('TraceProcessor', function() {
318318
]);
319319

320320
const insights = Array.from(processor.insights.values());
321-
if (insights[0].data.RenderBlocking instanceof Error) {
321+
if (insights[0].model.RenderBlocking instanceof Error) {
322322
throw new Error('RenderBlocking threw an error');
323323
}
324-
if (insights[1].data.RenderBlocking instanceof Error) {
324+
if (insights[1].model.RenderBlocking instanceof Error) {
325325
throw new Error('RenderBlocking threw an error');
326326
}
327327

328-
assert.strictEqual(insights[0].data.RenderBlocking.renderBlockingRequests.length, 0);
329-
assert.strictEqual(insights[1].data.RenderBlocking.renderBlockingRequests.length, 2);
328+
assert.strictEqual(insights[0].model.RenderBlocking.renderBlockingRequests.length, 0);
329+
assert.strictEqual(insights[1].model.RenderBlocking.renderBlockingRequests.length, 2);
330330
});
331331

332332
it('returns insights for multiple navigations', async function() {
@@ -346,23 +346,23 @@ describeWithEnvironment('TraceProcessor', function() {
346346
]);
347347

348348
const insights = Array.from(processor.insights.values());
349-
if (insights[0].data.RenderBlocking instanceof Error) {
349+
if (insights[0].model.RenderBlocking instanceof Error) {
350350
throw new Error('RenderBlocking threw an error');
351351
}
352-
if (insights[1].data.RenderBlocking instanceof Error) {
352+
if (insights[1].model.RenderBlocking instanceof Error) {
353353
throw new Error('RenderBlocking threw an error');
354354
}
355-
if (insights[2].data.RenderBlocking instanceof Error) {
355+
if (insights[2].model.RenderBlocking instanceof Error) {
356356
throw new Error('RenderBlocking threw an error');
357357
}
358-
if (insights[3].data.RenderBlocking instanceof Error) {
358+
if (insights[3].model.RenderBlocking instanceof Error) {
359359
throw new Error('RenderBlocking threw an error');
360360
}
361361

362-
assert.strictEqual(insights[0].data.RenderBlocking.renderBlockingRequests.length, 0);
363-
assert.strictEqual(insights[1].data.RenderBlocking.renderBlockingRequests.length, 0);
364-
assert.strictEqual(insights[2].data.RenderBlocking.renderBlockingRequests.length, 0);
365-
assert.strictEqual(insights[3].data.RenderBlocking.renderBlockingRequests.length, 1);
362+
assert.strictEqual(insights[0].model.RenderBlocking.renderBlockingRequests.length, 0);
363+
assert.strictEqual(insights[1].model.RenderBlocking.renderBlockingRequests.length, 0);
364+
assert.strictEqual(insights[2].model.RenderBlocking.renderBlockingRequests.length, 0);
365+
assert.strictEqual(insights[3].model.RenderBlocking.renderBlockingRequests.length, 1);
366366
});
367367
});
368368
});

front_end/models/trace/Processor.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ export class TraceProcessor extends EventTarget {
7676
return new TraceProcessor(Handlers.ModelHandlers, Types.Configuration.defaults());
7777
}
7878

79-
static getEnabledInsightRunners(parsedTrace: Handlers.Types.ParsedTrace): Partial<Insights.Types.InsightRunnersType> {
80-
const enabledInsights = {} as Insights.Types.InsightRunnersType;
81-
for (const [name, insight] of Object.entries(Insights.InsightRunners)) {
79+
static getEnabledInsightRunners(parsedTrace: Handlers.Types.ParsedTrace): Partial<Insights.Types.InsightModelsType> {
80+
const enabledInsights = {} as Insights.Types.InsightModelsType;
81+
for (const [name, insight] of Object.entries(Insights.Models)) {
8282
const deps = insight.deps();
8383
if (deps.some(dep => !parsedTrace[dep])) {
8484
continue;
@@ -344,8 +344,8 @@ export class TraceProcessor extends EventTarget {
344344

345345
#computeInsightSets(
346346
insights: Insights.Types.TraceInsightSets, parsedTrace: Handlers.Types.ParsedTrace,
347-
insightRunners: Partial<typeof Insights.InsightRunners>, context: Insights.Types.InsightSetContext): void {
348-
const data = {} as Insights.Types.InsightSets['data'];
347+
insightRunners: Partial<typeof Insights.Models>, context: Insights.Types.InsightSetContext): void {
348+
const model = {} as Insights.Types.InsightSet['model'];
349349

350350
for (const [name, insight] of Object.entries(insightRunners)) {
351351
let insightResult;
@@ -354,7 +354,7 @@ export class TraceProcessor extends EventTarget {
354354
} catch (err) {
355355
insightResult = err;
356356
}
357-
Object.assign(data, {[name]: insightResult});
357+
Object.assign(model, {[name]: insightResult});
358358
}
359359

360360
let id, urlString, navigation;
@@ -382,7 +382,7 @@ export class TraceProcessor extends EventTarget {
382382
navigation,
383383
frameId: context.frameId,
384384
bounds: context.bounds,
385-
data,
385+
model,
386386
};
387387
insights.set(insightSets.id, insightSets);
388388
}

front_end/models/trace/insights/BUILD.gn

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,16 @@ import("../../visibility.gni")
88

99
devtools_module("insights") {
1010
sources = [
11+
"CLSCulprits.ts",
1112
"Common.ts",
12-
"CumulativeLayoutShift.ts",
1313
"DocumentLatency.ts",
1414
"FontDisplay.ts",
15-
"InsightRunners.ts",
1615
"InteractionToNextPaint.ts",
1716
"LargestContentfulPaint.ts",
17+
"Models.ts",
1818
"RenderBlocking.ts",
1919
"SlowCSSSelector.ts",
20-
"ThirdPartyWeb.ts",
20+
"ThirdParties.ts",
2121
"Viewport.ts",
2222
"types.ts",
2323
]
@@ -45,14 +45,14 @@ ts_library("unittests") {
4545
testonly = true
4646

4747
sources = [
48-
"CumulativeLayoutShift.test.ts",
48+
"CLSCulprits.test.ts",
4949
"DocumentLatency.test.ts",
5050
"FontDisplay.test.ts",
5151
"InteractionToNextPaint.test.ts",
5252
"LargestContentfulPaint.test.ts",
5353
"RenderBlocking.test.ts",
5454
"SlowCSSSelector.test.ts",
55-
"ThirdPartyWeb.test.ts",
55+
"ThirdParties.test.ts",
5656
"Viewport.test.ts",
5757
]
5858

front_end/models/trace/insights/CumulativeLayoutShift.test.ts renamed to front_end/models/trace/insights/CLSCulprits.test.ts

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {TraceLoader} from '../../../testing/TraceLoader.js';
88
import * as Helpers from '../helpers/helpers.js';
99
import * as Types from '../types/types.js';
1010

11-
import {InsightRunners} from './insights.js';
11+
import {Models} from './insights.js';
1212

1313
export async function processTrace(testContext: Mocha.Suite|Mocha.Context|null, traceFile: string) {
1414
const {parsedTrace, insights} = await TraceLoader.traceEngine(testContext, traceFile);
@@ -22,12 +22,12 @@ export async function processTrace(testContext: Mocha.Suite|Mocha.Context|null,
2222
// Root cause invalidation window.
2323
const INVALIDATION_WINDOW = Helpers.Timing.secondsToMicroseconds(Types.Timing.Seconds(0.5));
2424

25-
describeWithEnvironment('CumulativeLayoutShift', function() {
25+
describeWithEnvironment('CLSCulprits', function() {
2626
describe('non composited animations', function() {
2727
it('gets the correct non composited animations', async function() {
2828
const {data, insights} = await processTrace(this, 'non-composited-animation.json.gz');
2929
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
30-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
30+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
3131
const {animationFailures} = insight;
3232

3333
const simpleAnimation = data.Animations.animations.find(animation => {
@@ -37,18 +37,18 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
3737
return animation.args.data.beginEvent.args.data.displayName === 'top';
3838
});
3939

40-
const expected: InsightRunners.CumulativeLayoutShift.NoncompositedAnimationFailure[] = [
40+
const expected: Models.CLSCulprits.NoncompositedAnimationFailure[] = [
4141
{
4242
name: 'simple-animation',
43-
failureReasons: [InsightRunners.CumulativeLayoutShift.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
43+
failureReasons: [Models.CLSCulprits.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
4444
unsupportedProperties: ['color'],
4545
animation: simpleAnimation,
4646
},
4747
{
4848
name: 'top',
4949
failureReasons: [
50-
InsightRunners.CumulativeLayoutShift.AnimationFailureReasons.TARGET_HAS_INVALID_COMPOSITING_STATE,
51-
InsightRunners.CumulativeLayoutShift.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY,
50+
Models.CLSCulprits.AnimationFailureReasons.TARGET_HAS_INVALID_COMPOSITING_STATE,
51+
Models.CLSCulprits.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY,
5252
],
5353
unsupportedProperties: ['top'],
5454
animation: top,
@@ -60,7 +60,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
6060
it.skip('[crbug.com/370382177]: gets the correct non composited animations for shift', async function() {
6161
const {data, insights} = await processTrace(this, 'non-composited-animation-shift.json.gz');
6262
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
63-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
63+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
6464
const {shifts, animationFailures} = insight;
6565

6666
const simpleAnimation = data.Animations.animations.find(animation => {
@@ -70,30 +70,30 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
7070
return animation.args.data.beginEvent.args.data.displayName === 'top';
7171
});
7272

73-
const shiftAnimations: InsightRunners.CumulativeLayoutShift.NoncompositedAnimationFailure[] = [];
73+
const shiftAnimations: Models.CLSCulprits.NoncompositedAnimationFailure[] = [];
7474
shifts.forEach(entry => {
7575
shiftAnimations.push(...entry.nonCompositedAnimations);
7676
});
77-
const expectedWithShift: InsightRunners.CumulativeLayoutShift.NoncompositedAnimationFailure[] = [
77+
const expectedWithShift: Models.CLSCulprits.NoncompositedAnimationFailure[] = [
7878
{
7979
name: 'simple-animation',
80-
failureReasons: [InsightRunners.CumulativeLayoutShift.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
80+
failureReasons: [Models.CLSCulprits.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
8181
unsupportedProperties: ['height', 'color', 'top'],
8282
animation: simpleAnimation,
8383
},
8484
];
8585
assert.deepStrictEqual(shiftAnimations, expectedWithShift);
8686

87-
const expectedAll: InsightRunners.CumulativeLayoutShift.NoncompositedAnimationFailure[] = [
87+
const expectedAll: Models.CLSCulprits.NoncompositedAnimationFailure[] = [
8888
{
8989
name: 'simple-animation',
90-
failureReasons: [InsightRunners.CumulativeLayoutShift.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
90+
failureReasons: [Models.CLSCulprits.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
9191
unsupportedProperties: ['height', 'color', 'top'],
9292
animation: simpleAnimation,
9393
},
9494
{
9595
name: 'top',
96-
failureReasons: [InsightRunners.CumulativeLayoutShift.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
96+
failureReasons: [Models.CLSCulprits.AnimationFailureReasons.UNSUPPORTED_CSS_PROPERTY],
9797
unsupportedProperties: ['top'],
9898
animation: top,
9999
},
@@ -105,7 +105,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
105105
it('returns no insights when there are no non-composited animations', async function() {
106106
const {data, insights} = await processTrace(this, 'lcp-images.json.gz');
107107
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
108-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
108+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
109109
const {animationFailures} = insight;
110110

111111
assert.isEmpty(animationFailures);
@@ -115,7 +115,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
115115
it('returns correct layout shifts', async function() {
116116
const {data, insights} = await processTrace(this, 'cls-single-frame.json.gz');
117117
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
118-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
118+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
119119
const {shifts} = insight;
120120

121121
assert.exists(shifts);
@@ -127,7 +127,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
127127
// Trace has a single iframe that gets created before the first layout shift and causes a layout shift.
128128
const {data, insights} = await processTrace(this, 'iframe-shift.json.gz');
129129
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
130-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
130+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
131131
const {shifts} = insight;
132132

133133
assert.exists(shifts);
@@ -159,7 +159,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
159159
// Trace has font load before the second layout shift.
160160
const {data, insights} = await processTrace(this, 'iframe-shift.json.gz');
161161
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
162-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
162+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
163163
const {shifts} = insight;
164164

165165
assert.exists(shifts);
@@ -193,7 +193,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
193193
it('handles potential unsized images root cause correctly', async function() {
194194
const {data, insights} = await processTrace(this, 'unsized-images.json.gz');
195195
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
196-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
196+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
197197
const {shifts} = insight;
198198
assert.exists(shifts);
199199
assert.strictEqual(shifts.size, 2);
@@ -217,7 +217,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
217217
it('returns clusters correctly', async function() {
218218
const {data, insights} = await processTrace(this, 'iframe-shift.json.gz');
219219
const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
220-
const insight = getInsightOrError('CumulativeLayoutShift', insights, firstNav);
220+
const insight = getInsightOrError('CLSCulprits', insights, firstNav);
221221
const {shifts, clusters} = insight;
222222

223223
assert.exists(clusters);
@@ -233,7 +233,7 @@ describeWithEnvironment('CumulativeLayoutShift', function() {
233233

234234
it('returns clusters correctly for non-navigations', async function() {
235235
const {insights} = await processTrace(this, 'cls-no-nav.json.gz');
236-
const insight = getInsightOrError('CumulativeLayoutShift', insights);
236+
const insight = getInsightOrError('CLSCulprits', insights);
237237
const {shifts, clusters} = insight;
238238

239239
assert.exists(clusters);

front_end/models/trace/insights/CumulativeLayoutShift.ts renamed to front_end/models/trace/insights/CLSCulprits.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import type * as Protocol from '../../../generated/protocol.js';
77
import * as Helpers from '../helpers/helpers.js';
88
import * as Types from '../types/types.js';
99

10-
import type {InsightResult, InsightSetContext, RequiredData} from './types.js';
10+
import type {InsightModel, InsightSetContext, RequiredData} from './types.js';
1111

12-
export type CLSInsightResult = InsightResult<{
12+
export type CLSCulpritsInsightModel = InsightModel<{
1313
animationFailures: readonly NoncompositedAnimationFailure[],
1414
shifts: Map<Types.Events.SyntheticLayoutShift, LayoutShiftRootCausesData>,
1515
clusters: Types.Events.SyntheticLayoutShiftCluster[],
@@ -406,7 +406,8 @@ function getFontRootCauses(
406406
return rootCausesByShift;
407407
}
408408

409-
export function generateInsight(parsedTrace: RequiredData<typeof deps>, context: InsightSetContext): CLSInsightResult {
409+
export function generateInsight(
410+
parsedTrace: RequiredData<typeof deps>, context: InsightSetContext): CLSCulpritsInsightModel {
410411
const isWithinContext = (event: Types.Events.Event): boolean => Helpers.Timing.eventIsInBounds(event, context.bounds);
411412

412413
const compositeAnimationEvents = parsedTrace.Animations.animations.filter(isWithinContext);

front_end/models/trace/insights/Common.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import type * as Handlers from '../handlers/handlers.js';
66
import * as Helpers from '../helpers/helpers.js';
77
import type * as Types from '../types/types.js';
88

9-
import type {InsightResults, InsightSetContextWithNavigation, TraceInsightSets} from './types.js';
9+
import type {InsightModels, InsightSetContextWithNavigation, TraceInsightSets} from './types.js';
1010

11-
export function getInsight<InsightName extends keyof InsightResults>(
12-
insightName: InsightName, insights: TraceInsightSets|null, key: string|null): InsightResults[InsightName]|null {
11+
export function getInsight<InsightName extends keyof InsightModels>(
12+
insightName: InsightName, insights: TraceInsightSets|null, key: string|null): InsightModels[InsightName]|null {
1313
if (!insights || !key) {
1414
return null;
1515
}
@@ -19,13 +19,13 @@ export function getInsight<InsightName extends keyof InsightResults>(
1919
return null;
2020
}
2121

22-
const insight = insightSets.data[insightName];
22+
const insight = insightSets.model[insightName];
2323
if (insight instanceof Error) {
2424
return null;
2525
}
2626

2727
// For some reason typescript won't narrow the type by removing Error, so do it manually.
28-
return insight as InsightResults[InsightName];
28+
return insight as InsightModels[InsightName];
2929
}
3030

3131
/**

0 commit comments

Comments
 (0)