Skip to content

Commit da151ed

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: add tests + refactor TimelineDetailsView
The setSelection method had grown a bit hard to work with and also lacked tests for its various branches. This CL adds some tests and also extracts some methods just to make it a bit easier to read. Bug: 370716587 Change-Id: Ifb73b8fdb3f85a115d93789bde0142aa87ce7919 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5973697 Commit-Queue: Nancy Li <[email protected]> Reviewed-by: Nancy Li <[email protected]> Commit-Queue: Jack Franklin <[email protected]> Auto-Submit: Jack Franklin <[email protected]>
1 parent e51125d commit da151ed

File tree

2 files changed

+107
-26
lines changed

2 files changed

+107
-26
lines changed

front_end/panels/timeline/TimelineDetailsView.test.ts

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import type * as Trace from '../../models/trace/trace.js';
5+
import * as Root from '../../core/root/root.js';
6+
import * as Trace from '../../models/trace/trace.js';
67
import {doubleRaf} from '../../testing/DOMHelpers.js';
78
import {describeWithEnvironment} from '../../testing/EnvironmentHelpers.js';
89
import {TraceLoader} from '../../testing/TraceLoader.js';
@@ -21,6 +22,7 @@ class MockViewDelegate implements Timeline.TimelinePanel.TimelineModeViewDelegat
2122

2223
describeWithEnvironment('TimelineDetailsView', function() {
2324
const mockViewDelegate = new MockViewDelegate();
25+
2426
it('displays the details of a network request event correctly', async function() {
2527
const {parsedTrace, insights} = await TraceLoader.traceEngine(this, 'lcp-web-font.json.gz');
2628
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
@@ -63,4 +65,64 @@ describeWithEnvironment('TimelineDetailsView', function() {
6365
assert.isOk(duration);
6466
assert.strictEqual(duration.innerText, 'Duration37.85 ms (at 109.82 ms)');
6567
});
68+
69+
it('renders the layout shift component for a single layout shift', async function() {
70+
Root.Runtime.experiments.enableForTest(Root.Runtime.ExperimentName.TIMELINE_INSIGHTS);
71+
72+
const {parsedTrace} = await TraceLoader.traceEngine(this, 'shift-attribution.json.gz');
73+
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
74+
await detailsView.setModel(
75+
{parsedTrace, selectedEvents: null, traceInsightsSets: null, eventToRelatedInsightsMap: null});
76+
77+
const layoutShift = parsedTrace.LayoutShifts.clusters.at(0)?.events.at(0);
78+
assert.isOk(layoutShift);
79+
const selection = Timeline.TimelineSelection.TimelineSelection.fromTraceEvent(layoutShift);
80+
await detailsView.setSelection(selection);
81+
const detailsContentElement = detailsView.getDetailsContentElementForTest();
82+
// Assert that the right component is rendered. This component has its own
83+
// tests for its contents so no need to duplicate those here.
84+
const layoutShiftDetails = detailsContentElement.querySelector('devtools-performance-layout-shift-details');
85+
assert.isNotNull(layoutShiftDetails);
86+
});
87+
88+
it('renders the layout shift component for a selected cluster', async function() {
89+
Root.Runtime.experiments.enableForTest(Root.Runtime.ExperimentName.TIMELINE_INSIGHTS);
90+
91+
const {parsedTrace} = await TraceLoader.traceEngine(this, 'shift-attribution.json.gz');
92+
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
93+
await detailsView.setModel(
94+
{parsedTrace, selectedEvents: null, traceInsightsSets: null, eventToRelatedInsightsMap: null});
95+
96+
const layoutShiftCluster = parsedTrace.LayoutShifts.clusters.at(0);
97+
assert.isOk(layoutShiftCluster);
98+
const selection = Timeline.TimelineSelection.TimelineSelection.fromTraceEvent(layoutShiftCluster);
99+
await detailsView.setSelection(selection);
100+
const detailsContentElement = detailsView.getDetailsContentElementForTest();
101+
// Assert that the right component is rendered. This component has its own
102+
// tests for its contents so no need to duplicate those here.
103+
const layoutShiftDetails = detailsContentElement.querySelector('devtools-performance-layout-shift-details');
104+
assert.isNotNull(layoutShiftDetails);
105+
});
106+
107+
it('updates the range details when the user has a range selected', async function() {
108+
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
109+
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsView(mockViewDelegate);
110+
await detailsView.setModel({
111+
parsedTrace,
112+
// We have to set selected events for the range selection UI to be drawn
113+
// (without the set of events we can't generate the range stats)
114+
selectedEvents: parsedTrace.Renderer.allTraceEntries,
115+
traceInsightsSets: null,
116+
eventToRelatedInsightsMap: null,
117+
});
118+
const bounds = Trace.Helpers.Timing.traceWindowMilliSeconds(parsedTrace.Meta.traceBounds);
119+
const selection = Timeline.TimelineSelection.TimelineSelection.fromRange(
120+
bounds.min,
121+
bounds.max,
122+
);
123+
await detailsView.setSelection(selection);
124+
const detailsContentElement = detailsView.getDetailsContentElementForTest();
125+
const title = detailsContentElement.querySelector<HTMLElement>('.timeline-details-chip-title');
126+
assert.strictEqual(title?.innerText, 'Range: 0 ms – 5.39 s');
127+
});
66128
});

front_end/panels/timeline/TimelineDetailsView.ts

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,48 @@ export class TimelineDetailsView extends UI.Widget.VBox {
303303
}
304304
}
305305
}
306+
307+
async #setSelectionForNetworkEvent(networkRequest: Trace.Types.Events.SyntheticNetworkRequest): Promise<void> {
308+
if (!this.#parsedTrace) {
309+
return;
310+
}
311+
const maybeTarget = targetForEvent(this.#parsedTrace, networkRequest);
312+
await this.#networkRequestDetails.setData(this.#parsedTrace, networkRequest, maybeTarget);
313+
this.#relatedInsightChips.activeEvent = networkRequest;
314+
if (this.#eventToRelatedInsightsMap) {
315+
this.#relatedInsightChips.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
316+
}
317+
318+
this.setContent(this.#networkRequestDetails);
319+
}
320+
321+
async #setSelectionForTraceEvent(event: Trace.Types.Events.Event): Promise<void> {
322+
if (!this.#parsedTrace) {
323+
return;
324+
}
325+
326+
this.#relatedInsightChips.activeEvent = event;
327+
if (this.#eventToRelatedInsightsMap) {
328+
this.#relatedInsightChips.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
329+
}
330+
331+
// Special case: if Insights experiment is enabled (on by default in M131)
332+
// and the user selects a layout shift or a layout shift cluster, render
333+
// the new layout shift details component.
334+
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_INSIGHTS) &&
335+
(Trace.Types.Events.isSyntheticLayoutShift(event) || Trace.Types.Events.isSyntheticLayoutShiftCluster(event))) {
336+
const isFreshRecording = Boolean(this.#parsedTrace && Tracker.instance().recordingIsFresh(this.#parsedTrace));
337+
this.#layoutShiftDetails.setData(event, this.#traceInsightsSets, this.#parsedTrace, isFreshRecording);
338+
this.setContent(this.#layoutShiftDetails);
339+
return;
340+
}
341+
342+
// Otherwise, build the generic trace event details UI.
343+
const traceEventDetails =
344+
await TimelineUIUtils.buildTraceEventDetails(this.#parsedTrace, event, this.detailsLinkifier, true);
345+
this.appendDetailsTabsForTraceEventAndShowDetails(event, traceEventDetails);
346+
}
347+
306348
async setSelection(selection: TimelineSelection|null): Promise<void> {
307349
if (!this.#parsedTrace) {
308350
// You can't make a selection if we have no trace data.
@@ -319,32 +361,9 @@ export class TimelineDetailsView extends UI.Widget.VBox {
319361
}
320362
const selectionObject = this.selection.object;
321363
if (TimelineSelection.isSyntheticNetworkRequestDetailsEventSelection(selectionObject)) {
322-
const networkRequest = selectionObject;
323-
const maybeTarget = targetForEvent(this.#parsedTrace, networkRequest);
324-
await this.#networkRequestDetails.setData(this.#parsedTrace, networkRequest, maybeTarget);
325-
this.#relatedInsightChips.activeEvent = networkRequest;
326-
if (this.#eventToRelatedInsightsMap) {
327-
this.#relatedInsightChips.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
328-
}
329-
330-
this.setContent(this.#networkRequestDetails);
364+
await this.#setSelectionForNetworkEvent(selectionObject);
331365
} else if (TimelineSelection.isTraceEventSelection(selectionObject)) {
332-
const event = selectionObject;
333-
this.#relatedInsightChips.activeEvent = event;
334-
if (this.#eventToRelatedInsightsMap) {
335-
this.#relatedInsightChips.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
336-
}
337-
if (Root.Runtime.experiments.isEnabled(Root.Runtime.ExperimentName.TIMELINE_INSIGHTS) &&
338-
(Trace.Types.Events.isSyntheticLayoutShift(event) ||
339-
Trace.Types.Events.isSyntheticLayoutShiftCluster(event))) {
340-
const isFreshRecording = Boolean(this.#parsedTrace && Tracker.instance().recordingIsFresh(this.#parsedTrace));
341-
this.#layoutShiftDetails.setData(event, this.#traceInsightsSets, this.#parsedTrace, isFreshRecording);
342-
this.setContent(this.#layoutShiftDetails);
343-
} else {
344-
const traceEventDetails =
345-
await TimelineUIUtils.buildTraceEventDetails(this.#parsedTrace, event, this.detailsLinkifier, true);
346-
this.appendDetailsTabsForTraceEventAndShowDetails(event, traceEventDetails);
347-
}
366+
await this.#setSelectionForTraceEvent(selectionObject);
348367
} else if (TimelineSelection.isLegacyTimelineFrame(selectionObject)) {
349368
this.#setSelectionForTimelineFrame(selectionObject);
350369
} else if (TimelineSelection.isRangeSelection(selectionObject)) {

0 commit comments

Comments
 (0)