Skip to content

Commit 12960d2

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Add model property to BaseInsightComponent
Stop having each insight component locate its data manually; just give it to them directly. Bug: 371615739 Change-Id: Id0836393280422e684520b9d520f5db8c7b34a0a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6001800 Reviewed-by: Paul Irish <[email protected]> Commit-Queue: Paul Irish <[email protected]> Auto-Submit: Connor Clark <[email protected]>
1 parent f8a58ef commit 12960d2

File tree

15 files changed

+164
-193
lines changed

15 files changed

+164
-193
lines changed

front_end/models/trace/insights/LCPDiscovery.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export function deps(): ['NetworkRequests', 'PageLoadMetrics', 'LargestImagePain
1212
return ['NetworkRequests', 'PageLoadMetrics', 'LargestImagePaint', 'Meta'];
1313
}
1414

15-
export type LCPDiscoveryModel = InsightModel<{
15+
export type LCPDiscoveryInsightModel = InsightModel<{
1616
lcpEvent?: Types.Events.LargestContentfulPaintCandidate,
1717
shouldRemoveLazyLoading?: boolean,
1818
shouldIncreasePriorityHint?: boolean,
@@ -22,7 +22,8 @@ export type LCPDiscoveryModel = InsightModel<{
2222
earliestDiscoveryTimeTs?: Types.Timing.MicroSeconds,
2323
}>;
2424

25-
export function generateInsight(parsedTrace: RequiredData<typeof deps>, context: InsightSetContext): LCPDiscoveryModel {
25+
export function generateInsight(
26+
parsedTrace: RequiredData<typeof deps>, context: InsightSetContext): LCPDiscoveryInsightModel {
2627
if (!context.navigation) {
2728
return {};
2829
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export async function processTrace(testContext: Mocha.Suite|Mocha.Context|null,
1616
return {data: parsedTrace, insights};
1717
}
1818

19-
describeWithEnvironment('RenderBlockingRequests', function() {
19+
describeWithEnvironment('RenderBlocking', function() {
2020
it('finds render blocking requests', async () => {
2121
const {data, insights} = await processTrace(this, 'load-simple.json.gz');
2222
assert.deepStrictEqual(

front_end/panels/timeline/components/SidebarSingleInsightSet.ts

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -40,27 +40,27 @@ export interface SidebarSingleInsightSetData {
4040
* "enable experimental performance insights" experiment. This is used to enable
4141
* us to ship incrementally without turning insights on by default for all
4242
* users. */
43-
const EXPERIMENTAL_INSIGHTS: ReadonlySet<typeof Insights.Helpers.BaseInsightComponent> = new Set([
44-
Insights.FontDisplay.FontDisplay,
43+
const EXPERIMENTAL_INSIGHTS: ReadonlySet<string> = new Set([
44+
'FontDisplay',
4545
]);
4646

4747
/**
4848
* Every insight (INCLUDING experimental ones)
49-
* The order of this array is the order the insights will be shown in the sidebar.
49+
* The order of these properties is the order the insights will be shown in the sidebar.
5050
* TODO(crbug.com/368135130): sort this in a smart way!
5151
*/
52-
const ALL_INSIGHTS: typeof Insights.Helpers.BaseInsightComponent[] = [
53-
Insights.InteractionToNextPaint.InteractionToNextPaint,
54-
Insights.LCPPhases.LCPPhases,
55-
Insights.LCPDiscovery.LCPDiscovery,
56-
Insights.CLSCulprits.CLSCulprits,
57-
Insights.RenderBlocking.RenderBlockingRequests,
58-
Insights.DocumentLatency.DocumentLatency,
59-
Insights.FontDisplay.FontDisplay,
60-
Insights.Viewport.Viewport,
61-
Insights.ThirdParties.ThirdParties,
62-
Insights.SlowCSSSelector.SlowCSSSelector,
63-
] as const;
52+
const INSIGHT_NAME_TO_COMPONENT = {
53+
InteractionToNextPaint: Insights.InteractionToNextPaint.InteractionToNextPaint,
54+
LCPPhases: Insights.LCPPhases.LCPPhases,
55+
LCPDiscovery: Insights.LCPDiscovery.LCPDiscovery,
56+
CLSCulprits: Insights.CLSCulprits.CLSCulprits,
57+
RenderBlocking: Insights.RenderBlocking.RenderBlocking,
58+
DocumentLatency: Insights.DocumentLatency.DocumentLatency,
59+
FontDisplay: Insights.FontDisplay.FontDisplay,
60+
Viewport: Insights.Viewport.Viewport,
61+
ThirdParties: Insights.ThirdParties.ThirdParties,
62+
SlowCSSSelector: Insights.SlowCSSSelector.SlowCSSSelector,
63+
};
6464

6565
export class SidebarSingleInsightSet extends HTMLElement {
6666
readonly #shadow = this.attachShadow({mode: 'open'});
@@ -193,37 +193,42 @@ export class SidebarSingleInsightSet extends HTMLElement {
193193
`;
194194
}
195195

196-
#insightsForRendering(): typeof Insights.Helpers.BaseInsightComponent[] {
196+
#renderInsights(
197+
insightSets: Trace.Insights.Types.TraceInsightSets|null,
198+
parsedTrace: Trace.Handlers.Types.ParsedTrace|null,
199+
insightSetKey: string,
200+
): LitHtml.TemplateResult {
197201
const includeExperimental = Root.Runtime.experiments.isEnabled(
198202
Root.Runtime.ExperimentName.TIMELINE_EXPERIMENTAL_INSIGHTS,
199203
);
200204

201-
if (includeExperimental) {
202-
return ALL_INSIGHTS;
205+
const models = insightSets?.get(insightSetKey)?.model;
206+
if (!models) {
207+
return html``;
203208
}
204209

205-
return ALL_INSIGHTS.filter(insight => !EXPERIMENTAL_INSIGHTS.has(insight));
206-
}
210+
const components: LitHtml.TemplateResult[] = [];
211+
for (const [name, componentClass] of Object.entries(INSIGHT_NAME_TO_COMPONENT)) {
212+
if (!includeExperimental && EXPERIMENTAL_INSIGHTS.has(name)) {
213+
continue;
214+
}
207215

208-
#renderInsights(
209-
insights: Trace.Insights.Types.TraceInsightSets|null,
210-
parsedTrace: Trace.Handlers.Types.ParsedTrace|null,
211-
insightSetKey: string,
212-
): LitHtml.TemplateResult {
213-
const insightComponents = this.#insightsForRendering();
214-
// clang-format off
215-
return html`${insightComponents.map(component => {
216-
return html`<div data-single-insight-wrapper>
217-
<${component.litTagName}
218-
.insights=${insights}
216+
// clang-format off
217+
const component = html`<div data-single-insight-wrapper>
218+
<${componentClass.litTagName}
219+
.model=${models[name as keyof typeof models]}
219220
.parsedTrace=${parsedTrace}
220221
.insightSetKey=${insightSetKey}
221222
.activeInsight=${this.#data.activeInsight}
222223
.activeCategory=${this.#data.activeCategory}>
223-
</${component.litTagName}>
224+
</${componentClass.litTagName}>
224225
</div>`;
225-
})}`;
226-
// clang-format on
226+
// clang-format on
227+
228+
components.push(component);
229+
}
230+
231+
return html`${components}`;
227232
}
228233

229234
#render(): void {

front_end/panels/timeline/components/insights/CLSCulprits.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
import * as i18n from '../../../../core/i18n/i18n.js';
6+
import type {CLSCulpritsInsightModel} from '../../../../models/trace/insights/CLSCulprits.js';
67
import * as Trace from '../../../../models/trace/trace.js';
78
import * as LitHtml from '../../../../ui/lit-html/lit-html.js';
89
import type * as Overlays from '../../overlays/overlays.js';
@@ -59,18 +60,16 @@ const UIStrings = {
5960
const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/insights/CLSCulprits.ts', UIStrings);
6061
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
6162

62-
export class CLSCulprits extends BaseInsightComponent {
63+
export class CLSCulprits extends BaseInsightComponent<CLSCulpritsInsightModel> {
6364
static override readonly litTagName = LitHtml.literal`devtools-performance-cls-culprits`;
6465
override insightCategory: Category = Category.CLS;
6566
override internalName: string = 'cls-culprits';
6667
override userVisibleTitle: string = i18nString(UIStrings.title);
6768
override description: string = i18nString(UIStrings.description);
6869

6970
override createOverlays(): Overlays.Overlays.TimelineOverlay[] {
70-
const insight = Trace.Insights.Common.getInsight('CLSCulprits', this.data.insights, this.data.insightSetKey);
71-
7271
const clustersByScore =
73-
insight?.clusters.toSorted((a, b) => b.clusterCumulativeScore - a.clusterCumulativeScore) ?? [];
72+
this.model?.clusters.toSorted((a, b) => b.clusterCumulativeScore - a.clusterCumulativeScore) ?? [];
7473
const worstCluster = clustersByScore[0];
7574
if (!worstCluster) {
7675
return [];
@@ -169,31 +168,29 @@ export class CLSCulprits extends BaseInsightComponent {
169168
}
170169

171170
override getRelatedEvents(): Trace.Types.Events.Event[] {
172-
const insight = Trace.Insights.Common.getInsight('CLSCulprits', this.data.insights, this.data.insightSetKey);
173-
return insight?.relatedEvents ?? [];
171+
return this.model?.relatedEvents ?? [];
174172
}
175173

176174
override render(): void {
177-
const insight = Trace.Insights.Common.getInsight('CLSCulprits', this.data.insights, this.data.insightSetKey);
178-
if (!insight) {
175+
if (!this.model) {
179176
return;
180177
}
181178

182-
const culpritsByShift = insight.shifts;
183-
const clusters = insight.clusters ?? [];
184-
if (!clusters.length || !insight.worstCluster) {
179+
const culpritsByShift = this.model.shifts;
180+
const clusters = this.model.clusters ?? [];
181+
if (!clusters.length || !this.model.worstCluster) {
185182
return;
186183
}
187184

188-
const causes = this.getTopCulprits(insight.worstCluster, culpritsByShift);
185+
const causes = this.getTopCulprits(this.model.worstCluster, culpritsByShift);
189186

190187
const hasCulprits = causes.length > 0;
191188

192189
const matchesCategory = shouldRenderForCategory({
193190
activeCategory: this.data.activeCategory,
194191
insightCategory: this.insightCategory,
195192
});
196-
const output = hasCulprits && matchesCategory ? this.#render(causes, insight.worstCluster) : LitHtml.nothing;
193+
const output = hasCulprits && matchesCategory ? this.#render(causes, this.model.worstCluster) : LitHtml.nothing;
197194
LitHtml.render(output, this.shadow, {host: this});
198195
}
199196
}

front_end/panels/timeline/components/insights/DocumentLatency.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import '../../../../ui/components/icon_button/icon_button.js';
66

77
import * as i18n from '../../../../core/i18n/i18n.js';
8+
import type {DocumentLatencyInsightModel} from '../../../../models/trace/insights/DocumentLatency.js';
89
import * as Trace from '../../../../models/trace/trace.js';
910
import * as LitHtml from '../../../../ui/lit-html/lit-html.js';
1011
import type * as Overlays from '../../overlays/overlays.js';
@@ -75,7 +76,7 @@ const UIStrings = {
7576
const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/insights/DocumentLatency.ts', UIStrings);
7677
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
7778

78-
export class DocumentLatency extends BaseInsightComponent {
79+
export class DocumentLatency extends BaseInsightComponent<DocumentLatencyInsightModel> {
7980
static override readonly litTagName = LitHtml.literal`devtools-performance-document-latency`;
8081
override insightCategory: Category = Category.ALL;
8182
override internalName: string = 'document-latency';
@@ -98,26 +99,26 @@ export class DocumentLatency extends BaseInsightComponent {
9899
}
99100

100101
override createOverlays(): Overlays.Overlays.TimelineOverlay[] {
101-
const insight = Trace.Insights.Common.getInsight('DocumentLatency', this.data.insights, this.data.insightSetKey);
102-
if (!insight?.data?.documentRequest) {
102+
if (!this.model?.data?.documentRequest) {
103103
return [];
104104
}
105105

106106
const overlays: Overlays.Overlays.TimelineOverlay[] = [];
107-
const event = insight.data.documentRequest;
108-
const redirectDurationMicro = Trace.Helpers.Timing.millisecondsToMicroseconds(insight.data.redirectDuration);
107+
const event = this.model.data.documentRequest;
108+
const redirectDurationMicro = Trace.Helpers.Timing.millisecondsToMicroseconds(this.model.data.redirectDuration);
109109

110110
const sections = [];
111-
if (insight.data.redirectDuration) {
111+
if (this.model.data.redirectDuration) {
112112
const bounds = Trace.Helpers.Timing.traceWindowFromMicroSeconds(
113113
event.ts,
114114
(event.ts + redirectDurationMicro) as Trace.Types.Timing.MicroSeconds,
115115
);
116116
sections.push({bounds, label: i18nString(UIStrings.redirectsLabel), showDuration: true});
117117
overlays.push({type: 'CANDY_STRIPED_TIME_RANGE', bounds, entry: event});
118118
}
119-
if (insight.data.serverResponseTooSlow) {
120-
const serverResponseTimeMicro = Trace.Helpers.Timing.millisecondsToMicroseconds(insight.data.serverResponseTime);
119+
if (this.model.data.serverResponseTooSlow) {
120+
const serverResponseTimeMicro =
121+
Trace.Helpers.Timing.millisecondsToMicroseconds(this.model.data.serverResponseTime);
121122
// NOTE: NetworkRequestHandlers never makes a synthetic network request event if `timing` is missing.
122123
const sendEnd = event.args.data.timing?.sendEnd ?? Trace.Types.Timing.MilliSeconds(0);
123124
const sendEndMicro = Trace.Helpers.Timing.millisecondsToMicroseconds(sendEnd);
@@ -127,7 +128,7 @@ export class DocumentLatency extends BaseInsightComponent {
127128
);
128129
sections.push({bounds, label: i18nString(UIStrings.serverResponseTimeLabel), showDuration: true});
129130
}
130-
if (insight.data.uncompressedResponseBytes) {
131+
if (this.model.data.uncompressedResponseBytes) {
131132
const bounds = Trace.Helpers.Timing.traceWindowFromMicroSeconds(
132133
event.args.data.syntheticData.downloadStart,
133134
(event.args.data.syntheticData.downloadStart + event.args.data.syntheticData.download) as
@@ -141,15 +142,15 @@ export class DocumentLatency extends BaseInsightComponent {
141142
overlays.push({
142143
type: 'TIMESPAN_BREAKDOWN',
143144
sections,
144-
entry: insight.data.documentRequest,
145+
entry: this.model.data.documentRequest,
145146
// Always render below because the document request is guaranteed to be
146147
// the first request in the network track.
147148
renderLocation: 'BELOW_EVENT',
148149
});
149150
}
150151
overlays.push({
151152
type: 'ENTRY_SELECTED',
152-
entry: insight.data.documentRequest,
153+
entry: this.model.data.documentRequest,
153154
});
154155

155156
return overlays;
@@ -195,22 +196,21 @@ export class DocumentLatency extends BaseInsightComponent {
195196
}
196197

197198
override getRelatedEvents(): Trace.Types.Events.Event[] {
198-
const insight = Trace.Insights.Common.getInsight('DocumentLatency', this.data.insights, this.data.insightSetKey);
199-
return insight?.relatedEvents ?? [];
199+
return this.model?.relatedEvents ?? [];
200200
}
201201

202202
override render(): void {
203-
const insight = Trace.Insights.Common.getInsight('DocumentLatency', this.data.insights, this.data.insightSetKey);
204-
if (insight?.data === undefined) {
203+
if (this.model?.data === undefined) {
205204
return;
206205
}
206+
207207
const matchesCategory = shouldRenderForCategory({
208208
activeCategory: this.data.activeCategory,
209209
insightCategory: this.insightCategory,
210210
});
211-
const hasFailure = insight?.data?.redirectDuration > 0 || insight?.data?.serverResponseTooSlow ||
212-
insight.data.uncompressedResponseBytes > 0;
213-
const output = (matchesCategory && hasFailure) ? this.#renderInsight(insight) : LitHtml.nothing;
211+
const hasFailure = this.model?.data?.redirectDuration > 0 || this.model?.data?.serverResponseTooSlow ||
212+
this.model.data.uncompressedResponseBytes > 0;
213+
const output = (matchesCategory && hasFailure) ? this.#renderInsight(this.model) : LitHtml.nothing;
214214
LitHtml.render(output, this.shadow, {host: this});
215215
}
216216
}

front_end/panels/timeline/components/insights/FontDisplay.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
import './Table.js';
66

77
import * as i18n from '../../../../core/i18n/i18n.js';
8-
import * as Trace from '../../../../models/trace/trace.js';
8+
import type {FontDisplayInsightModel} from '../../../../models/trace/insights/FontDisplay.js';
9+
import type * as Trace from '../../../../models/trace/trace.js';
910
import * as LitHtml from '../../../../ui/lit-html/lit-html.js';
1011
import type * as Overlays from '../../overlays/overlays.js';
1112

@@ -34,7 +35,7 @@ const UIStrings = {
3435
const str_ = i18n.i18n.registerUIStrings('panels/timeline/components/insights/FontDisplay.ts', UIStrings);
3536
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
3637

37-
export class FontDisplay extends BaseInsightComponent {
38+
export class FontDisplay extends BaseInsightComponent<FontDisplayInsightModel> {
3839
static override readonly litTagName = LitHtml.literal`devtools-performance-font-display`;
3940
override insightCategory = Category.INP;
4041
override internalName: string = 'font-display';
@@ -46,12 +47,11 @@ export class FontDisplay extends BaseInsightComponent {
4647
override createOverlays(): Overlays.Overlays.TimelineOverlay[] {
4748
this.#overlayForRequest.clear();
4849

49-
const insight = Trace.Insights.Common.getInsight('FontDisplay', this.data.insights, this.data.insightSetKey);
50-
if (!insight) {
50+
if (!this.model) {
5151
return [];
5252
}
5353

54-
for (const font of insight.fonts) {
54+
for (const font of this.model.fonts) {
5555
this.#overlayForRequest.set(font.request, {
5656
type: 'ENTRY_OUTLINE',
5757
entry: font.request,
@@ -97,19 +97,18 @@ export class FontDisplay extends BaseInsightComponent {
9797
}
9898

9999
override getRelatedEvents(): Trace.Types.Events.Event[] {
100-
const insight = Trace.Insights.Common.getInsight('FontDisplay', this.data.insights, this.data.insightSetKey);
101-
return insight?.relatedEvents ?? [];
100+
return this.model?.relatedEvents ?? [];
102101
}
103102

104103
override render(): void {
105-
const insight = Trace.Insights.Common.getInsight('FontDisplay', this.data.insights, this.data.insightSetKey);
106-
const shouldShow = insight && insight.fonts.find(font => font.wastedTime);
104+
const model = this.model;
105+
const shouldShow = model && model.fonts.find(font => font.wastedTime);
107106

108107
const matchesCategory = shouldRenderForCategory({
109108
activeCategory: this.data.activeCategory,
110109
insightCategory: this.insightCategory,
111110
});
112-
const output = shouldShow && matchesCategory ? this.#render(insight) : LitHtml.nothing;
111+
const output = shouldShow && matchesCategory ? this.#render(model) : LitHtml.nothing;
113112
LitHtml.render(output, this.shadow, {host: this});
114113
}
115114
}

0 commit comments

Comments
 (0)