Skip to content

Commit 499a9ed

Browse files
Adam RaineDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Add related elements to DOM size insight
https://screenshot.googleplex.com/77JSbU8uKGZ9YGC This also removes the DOM size insight from the experimental list since it is now feature-complete. Bug: 372897811 Change-Id: Id66bcd103fcaef4569d111ef787cd6582b873d54 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6182528 Commit-Queue: Adam Raine <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent 63dc808 commit 499a9ed

File tree

9 files changed

+80
-7
lines changed

9 files changed

+80
-7
lines changed

front_end/panels/timeline/components/SidebarInsightsTab.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ export class SidebarInsightsTab extends HTMLElement {
197197
insightSetKey: id,
198198
activeCategory: this.#selectedCategory,
199199
activeInsight: this.#activeInsight,
200+
parsedTrace: this.#parsedTrace,
200201
};
201202
202203
const contents = html`

front_end/panels/timeline/components/SidebarSingleInsightSet.test.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ function getPassedInsights(component: Components.SidebarSingleInsightSet.Sidebar
3030

3131
describeWithEnvironment('SidebarSingleInsightSet', () => {
3232
it('renders a list of insights', async function() {
33-
const {insights} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
33+
const {insights, parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
3434

3535
assert.isOk(insights);
3636
// only one navigation in this trace.
@@ -46,6 +46,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
4646
insightSetKey: navigationId,
4747
activeCategory: Trace.Insights.Types.InsightCategory.ALL,
4848
activeInsight: null,
49+
parsedTrace,
4950
};
5051
await RenderCoordinator.done();
5152

@@ -57,6 +58,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
5758
'LCP request discovery',
5859
'Render blocking requests',
5960
'Document request latency',
61+
'Optimize DOM size',
6062
'Third parties',
6163
'INP by phase',
6264
'Layout shift culprits',
@@ -88,6 +90,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
8890
insightSetKey: firstNavigation,
8991
activeCategory: Trace.Insights.Types.InsightCategory.ALL,
9092
activeInsight: null,
93+
parsedTrace,
9194
};
9295
await RenderCoordinator.done();
9396
const userVisibleTitles = getUserVisibleInsights(component).flatMap(component => {
@@ -104,6 +107,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
104107
'Render blocking requests',
105108
'Document request latency',
106109
'Optimize viewport for mobile',
110+
'Optimize DOM size',
107111
'CSS Selector costs',
108112
]);
109113

@@ -117,6 +121,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
117121
'Render blocking requests',
118122
'Document request latency',
119123
'Optimize viewport for mobile',
124+
'Optimize DOM size',
120125
'CSS Selector costs',
121126
]);
122127
});
@@ -135,6 +140,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
135140
insightSetKey: firstNavigation,
136141
activeCategory: Trace.Insights.Types.InsightCategory.ALL,
137142
activeInsight: null,
143+
parsedTrace,
138144
};
139145
await RenderCoordinator.done();
140146
const userVisibleTitles = getUserVisibleInsights(component).flatMap(component => {
@@ -171,7 +177,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
171177
});
172178

173179
it('will render the active insight fully', async function() {
174-
const {insights} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
180+
const {insights, parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
175181

176182
assert.isOk(insights);
177183
// only one navigation in this trace.
@@ -195,6 +201,7 @@ describeWithEnvironment('SidebarSingleInsightSet', () => {
195201
model,
196202
insightSetKey: navigationId,
197203
},
204+
parsedTrace,
198205
};
199206
await RenderCoordinator.done();
200207

front_end/panels/timeline/components/SidebarSingleInsightSet.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export interface SidebarSingleInsightSetData {
3838
insightSetKey: Trace.Types.Events.NavigationId|null;
3939
activeCategory: Trace.Insights.Types.InsightCategory;
4040
activeInsight: ActiveInsight|null;
41+
parsedTrace: Trace.Handlers.Types.ParsedTrace|null;
4142
}
4243

4344
/**
@@ -47,7 +48,6 @@ export interface SidebarSingleInsightSetData {
4748
* users. */
4849
const EXPERIMENTAL_INSIGHTS: ReadonlySet<string> = new Set([
4950
'FontDisplay',
50-
'DOMSize',
5151
]);
5252

5353
type InsightNameToComponentMapping =
@@ -82,6 +82,7 @@ export class SidebarSingleInsightSet extends HTMLElement {
8282
insightSetKey: null,
8383
activeCategory: Trace.Insights.Types.InsightCategory.ALL,
8484
activeInsight: null,
85+
parsedTrace: null,
8586
};
8687

8788
set data(data: SidebarSingleInsightSetData) {
@@ -197,7 +198,8 @@ export class SidebarSingleInsightSet extends HTMLElement {
197198
.selected=${this.#data.activeInsight?.model === model}
198199
.model=${model}
199200
.bounds=${insightSet.bounds}
200-
.insightSetKey=${insightSetKey}>
201+
.insightSetKey=${insightSetKey}
202+
.parsedTrace=${this.#data.parsedTrace}>
201203
</${componentClass.litTagName}>
202204
</div>`;
203205
// clang-format on

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44

55
import '../../../../ui/components/markdown_view/markdown_view.js';
66

7+
import * as Common from '../../../../core/common/common.js';
78
import * as i18n from '../../../../core/i18n/i18n.js';
9+
import type * as Protocol from '../../../../generated/protocol.js';
810
import type {InsightModel} from '../../../../models/trace/insights/types.js';
9-
import type * as Trace from '../../../../models/trace/trace.js';
11+
import * as Trace from '../../../../models/trace/trace.js';
1012
import * as Buttons from '../../../../ui/components/buttons/buttons.js';
1113
import * as ComponentHelpers from '../../../../ui/components/helpers/helpers.js';
1214
import * as LitHtml from '../../../../ui/lit-html/lit-html.js';
@@ -59,6 +61,7 @@ export abstract class BaseInsightComponent<T extends InsightModel<{}>> extends H
5961

6062
#selected = false;
6163
#model: T|null = null;
64+
#parsedTrace: Trace.Handlers.Types.ParsedTrace|null = null;
6265

6366
get model(): T|null {
6467
return this.#model;
@@ -120,6 +123,10 @@ export abstract class BaseInsightComponent<T extends InsightModel<{}>> extends H
120123
void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#boundRender);
121124
}
122125

126+
set parsedTrace(parsedTrace: Trace.Handlers.Types.ParsedTrace) {
127+
this.#parsedTrace = parsedTrace;
128+
}
129+
123130
#dispatchInsightToggle(): void {
124131
if (this.#selected) {
125132
this.dispatchEvent(new SidebarInsight.InsightDeactivated());
@@ -250,6 +257,23 @@ export abstract class BaseInsightComponent<T extends InsightModel<{}>> extends H
250257
return null;
251258
}
252259

260+
protected renderNode(backendNodeId: Protocol.DOM.BackendNodeId, fallbackText?: string): LitHtml.LitTemplate {
261+
const fallback = fallbackText ?? LitHtml.nothing;
262+
if (!this.#parsedTrace) {
263+
return html`${fallback}`;
264+
}
265+
266+
const domNodePromise =
267+
Trace.Extras.FetchNodes.domNodeForBackendNodeID(this.#parsedTrace, backendNodeId).then((node): unknown => {
268+
if (!node) {
269+
return fallback;
270+
}
271+
return Common.Linkifier.Linkifier.linkify(node);
272+
});
273+
274+
return html`${LitHtml.Directives.until(domNodePromise, fallback)}`;
275+
}
276+
253277
#renderWithContent(content: LitHtml.LitTemplate): void {
254278
if (!this.#model) {
255279
LitHtml.render(LitHtml.nothing, this.#shadowRoot, {host: this});

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

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import './Table.js';
77

88
import * as i18n from '../../../../core/i18n/i18n.js';
99
import type {DOMSizeInsightModel} from '../../../../models/trace/insights/DOMSize.js';
10+
import type * as Trace from '../../../../models/trace/trace.js';
1011
import * as LitHtml from '../../../../ui/lit-html/lit-html.js';
1112
import type * as Overlays from '../../overlays/overlays.js';
1213

@@ -22,6 +23,10 @@ const UIStrings = {
2223
* @description Header for a column containing the value of a statistic.
2324
*/
2425
value: 'Value',
26+
/**
27+
* @description Header for a column containing the page element related to a statistc.
28+
*/
29+
element: 'Element',
2530
/**
2631
* @description Label for a value representing the total number of elements on the page.
2732
*/
@@ -58,6 +63,36 @@ export class DOMSize extends BaseInsightComponent<DOMSizeInsightModel> {
5863
}));
5964
}
6065

66+
#renderNodeTable(domStatsData: Trace.Types.Events.DOMStats['args']['data']): LitHtml.LitTemplate {
67+
const rows: TableData['rows'] = [];
68+
69+
if (domStatsData.maxDepth) {
70+
const {nodeId, nodeName} = domStatsData.maxDepth;
71+
rows.push({values: [i18nString(UIStrings.maxDOMDepth), this.renderNode(nodeId, nodeName)]});
72+
}
73+
74+
if (domStatsData.maxChildren) {
75+
const {nodeId, nodeName} = domStatsData.maxChildren;
76+
rows.push({values: [i18nString(UIStrings.maxChildren), this.renderNode(nodeId, nodeName)]});
77+
}
78+
79+
if (!rows.length) {
80+
return LitHtml.nothing;
81+
}
82+
83+
// clang-format off
84+
return html`<div class="insight-section">
85+
<devtools-performance-table
86+
.data=${{
87+
insight: this,
88+
headers: [i18nString(UIStrings.statistic), i18nString(UIStrings.element)],
89+
rows,
90+
} as TableData}>
91+
</devtools-performance-table>
92+
</div>`;
93+
// clang-format on
94+
}
95+
6196
override renderContent(): LitHtml.LitTemplate {
6297
if (!this.model) {
6398
return LitHtml.nothing;
@@ -81,7 +116,9 @@ export class DOMSize extends BaseInsightComponent<DOMSizeInsightModel> {
81116
],
82117
} as TableData}>
83118
</devtools-performance-table>
84-
</div>`;
119+
</div>
120+
${this.#renderNodeTable(domStatsData)}
121+
`;
85122
// clang-format on
86123
}
87124
}

front_end/panels/timeline/components/insights/baseInsightComponent.css

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,11 @@
9292

9393
.insight-section {
9494
padding-top: var(--sys-size-5);
95+
margin-top: var(--sys-size-5);
9596
}
9697

9798
.insight-description:not(:empty) {
98-
padding-bottom: var(--sys-size-5);
99+
margin-bottom: var(--sys-size-5);
99100
}
100101

101102
.insight-section:not(:empty) {

front_end/panels/timeline/components/insights/table.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ table {
88
width: 100%;
99
padding: 5px 0;
1010
border-collapse: collapse;
11+
table-layout: fixed;
1112
}
1213

1314
thead {
2.75 KB
Loading
1.53 KB
Loading

0 commit comments

Comments
 (0)