Skip to content

Commit d3eb3a1

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Add field data to LCP, FCP metric marker tooltips
Also add FCP to the CrUX request. https://i.imgur.com/UpXzazI.png Bug: 368135130 Change-Id: I11376cbdaab35fc6737060f40ab052041c8a0cd0 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6181300 Commit-Queue: Connor Clark <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent ff13517 commit d3eb3a1

File tree

8 files changed

+192
-32
lines changed

8 files changed

+192
-32
lines changed

front_end/models/crux-manager/CrUXManager.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ describeWithMockConnection('CrUXManager', () => {
155155
{
156156
formFactor: 'DESKTOP',
157157
metrics: [
158+
'first_contentful_paint',
158159
'largest_contentful_paint',
159160
'cumulative_layout_shift',
160161
'interaction_to_next_paint',
@@ -166,6 +167,7 @@ describeWithMockConnection('CrUXManager', () => {
166167
{
167168
formFactor: 'PHONE',
168169
metrics: [
170+
'first_contentful_paint',
169171
'largest_contentful_paint',
170172
'cumulative_layout_shift',
171173
'interaction_to_next_paint',
@@ -176,6 +178,7 @@ describeWithMockConnection('CrUXManager', () => {
176178
},
177179
{
178180
metrics: [
181+
'first_contentful_paint',
179182
'largest_contentful_paint',
180183
'cumulative_layout_shift',
181184
'interaction_to_next_paint',
@@ -187,6 +190,7 @@ describeWithMockConnection('CrUXManager', () => {
187190
{
188191
formFactor: 'DESKTOP',
189192
metrics: [
193+
'first_contentful_paint',
190194
'largest_contentful_paint',
191195
'cumulative_layout_shift',
192196
'interaction_to_next_paint',
@@ -198,6 +202,7 @@ describeWithMockConnection('CrUXManager', () => {
198202
{
199203
formFactor: 'PHONE',
200204
metrics: [
205+
'first_contentful_paint',
201206
'largest_contentful_paint',
202207
'cumulative_layout_shift',
203208
'interaction_to_next_paint',
@@ -208,6 +213,7 @@ describeWithMockConnection('CrUXManager', () => {
208213
},
209214
{
210215
metrics: [
216+
'first_contentful_paint',
211217
'largest_contentful_paint',
212218
'cumulative_layout_shift',
213219
'interaction_to_next_paint',

front_end/models/crux-manager/CrUXManager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export const DEVICE_SCOPE_LIST: DeviceScope[] = ['ALL', 'DESKTOP', 'PHONE'];
103103

104104
const pageScopeList: PageScope[] = ['origin', 'url'];
105105
const metrics: MetricNames[] = [
106+
'first_contentful_paint',
106107
'largest_contentful_paint',
107108
'cumulative_layout_shift',
108109
'interaction_to_next_paint',

front_end/panels/timeline/TimelineFlameChartView.test.ts

Lines changed: 46 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ describeWithEnvironment('TimelineFlameChartView', function() {
6464
});
6565

6666
it('Can search for events by name in the timeline', async function() {
67-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'lcp-images.json.gz');
67+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'lcp-images.json.gz');
6868
// The timeline flamechart view will invoke the `select` method
6969
// of this delegate every time an event has matched on a search.
7070
const mockViewDelegate = new MockViewDelegate();
7171

7272
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
7373
const searchableView = new UI.SearchableView.SearchableView(flameChartView, null);
7474
flameChartView.setSearchableView(searchableView);
75-
flameChartView.setModel(parsedTrace);
75+
flameChartView.setModel(parsedTrace, metadata);
7676

7777
const searchQuery = 'Paint';
7878
const searchConfig =
@@ -103,15 +103,15 @@ describeWithEnvironment('TimelineFlameChartView', function() {
103103
});
104104

105105
it('can search across both flame charts for events', async function() {
106-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
106+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
107107
// The timeline flamechart view will invoke the `select` method
108108
// of this delegate every time an event has matched on a search.
109109
const mockViewDelegate = new MockViewDelegate();
110110

111111
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
112112
const searchableView = new UI.SearchableView.SearchableView(flameChartView, null);
113113
flameChartView.setSearchableView(searchableView);
114-
flameChartView.setModel(parsedTrace);
114+
flameChartView.setModel(parsedTrace, metadata);
115115

116116
const searchQuery = 'app.js';
117117
const searchConfig =
@@ -129,35 +129,35 @@ describeWithEnvironment('TimelineFlameChartView', function() {
129129
// This test is still failing after bumping up the timeout to 20 seconds. So
130130
// skip it while we work on a fix for the trace load speed.
131131
it.skip('[crbug.com/1492405] Shows the network track correctly', async function() {
132-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
132+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
133133
// The timeline flamechart view will invoke the `select` method
134134
// of this delegate every time an event has matched on a search.
135135
const mockViewDelegate = new MockViewDelegate();
136136

137137
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
138-
flameChartView.setModel(parsedTrace);
138+
flameChartView.setModel(parsedTrace, metadata);
139139

140140
assert.isTrue(flameChartView.isNetworkTrackShownForTests());
141141
});
142142

143143
it('Does not show the network track when there is no network request', async function() {
144-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'basic.json.gz');
144+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'basic.json.gz');
145145
// The timeline flamechart view will invoke the `select` method
146146
// of this delegate every time an event has matched on a search.
147147
const mockViewDelegate = new MockViewDelegate();
148148

149149
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
150-
flameChartView.setModel(parsedTrace);
150+
flameChartView.setModel(parsedTrace, metadata);
151151

152152
assert.isFalse(flameChartView.isNetworkTrackShownForTests());
153153
});
154154

155155
it('Adds Hidden Descendants Arrow as a decoration when a Context Menu action is applied on a node', async function() {
156-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
156+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
157157
const mockViewDelegate = new MockViewDelegate();
158158

159159
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
160-
flameChartView.setModel(parsedTrace);
160+
flameChartView.setModel(parsedTrace, metadata);
161161

162162
// Find the main track to later collapse entries of
163163
const mainTrack = flameChartView.getMainFlameChart().timelineData()?.groups.find(group => {
@@ -194,11 +194,11 @@ describeWithEnvironment('TimelineFlameChartView', function() {
194194

195195
it('Adds Hidden Descendants Arrow as a decoration when a Context Menu action is applied on a selected node with a key shortcut event',
196196
async function() {
197-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
197+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
198198
const mockViewDelegate = new MockViewDelegate();
199199

200200
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
201-
flameChartView.setModel(parsedTrace);
201+
flameChartView.setModel(parsedTrace, metadata);
202202

203203
// Find the main track to later collapse entries of
204204
const mainTrack = flameChartView.getMainFlameChart().timelineData()?.groups.find(group => {
@@ -238,11 +238,11 @@ describeWithEnvironment('TimelineFlameChartView', function() {
238238

239239
it('Removes Hidden Descendants Arrow as a decoration when Reset Children action is applied on a node',
240240
async function() {
241-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
241+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'load-simple.json.gz');
242242
const mockViewDelegate = new MockViewDelegate();
243243

244244
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
245-
flameChartView.setModel(parsedTrace);
245+
flameChartView.setModel(parsedTrace, metadata);
246246
Timeline.ModificationsManager.ModificationsManager.activeManager();
247247

248248
// Find the main track to later collapse entries of
@@ -292,16 +292,43 @@ describeWithEnvironment('TimelineFlameChartView', function() {
292292
assert.isUndefined(decorationsForEntry);
293293
});
294294

295+
it('renders metrics as marker overlays w/ tooltips', async function() {
296+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'crux.json.gz');
297+
const mockViewDelegate = new MockViewDelegate();
298+
299+
const flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
300+
flameChartView.setModel(parsedTrace, metadata);
301+
302+
const tooltips =
303+
[...flameChartView.element.querySelectorAll('.overlay-type-TIMINGS_MARKER .marker-title')].map(el => {
304+
el?.dispatchEvent(new MouseEvent('mousemove', {
305+
clientX: 0,
306+
clientY: 0,
307+
}));
308+
return flameChartView.element.querySelector('.timeline-entry-tooltip-element')
309+
?.textContent?.replaceAll('\xa0', ' ');
310+
});
311+
312+
assert.deepEqual(tooltips, [
313+
'0 μsNav',
314+
'43.98 msL',
315+
'75.90 msFCP - Local939.00 msFCP - Field (URL)',
316+
'75.90 msLCP - Local936.00 msLCP - Field (URL)',
317+
'43.75 msDCL',
318+
]);
319+
});
320+
295321
describe('Context Menu', function() {
296322
let flameChartView: Timeline.TimelineFlameChartView.TimelineFlameChartView;
297323
let parsedTrace: Trace.Handlers.Types.ParsedTrace;
324+
let metadata: Trace.Types.File.MetaData|null;
298325

299326
this.beforeEach(async () => {
300-
({parsedTrace} = await TraceLoader.traceEngine(this, 'recursive-blocking-js.json.gz'));
327+
({parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'recursive-blocking-js.json.gz'));
301328
const mockViewDelegate = new MockViewDelegate();
302329

303330
flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
304-
flameChartView.setModel(parsedTrace);
331+
flameChartView.setModel(parsedTrace, metadata);
305332
Timeline.ModificationsManager.ModificationsManager.activeManager();
306333
});
307334

@@ -753,13 +780,14 @@ describeWithEnvironment('TimelineFlameChartView', function() {
753780
describe('Link between entries annotation in progress', function() {
754781
let flameChartView: Timeline.TimelineFlameChartView.TimelineFlameChartView;
755782
let parsedTrace: Trace.Handlers.Types.ParsedTrace;
783+
let metadata: Trace.Types.File.MetaData|null;
756784

757785
this.beforeEach(async () => {
758-
({parsedTrace} = await TraceLoader.traceEngine(this, 'recursive-blocking-js.json.gz'));
786+
({parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'recursive-blocking-js.json.gz'));
759787
const mockViewDelegate = new MockViewDelegate();
760788

761789
flameChartView = new Timeline.TimelineFlameChartView.TimelineFlameChartView(mockViewDelegate);
762-
flameChartView.setModel(parsedTrace);
790+
flameChartView.setModel(parsedTrace, metadata);
763791
Timeline.ModificationsManager.ModificationsManager.activeManager();
764792
});
765793

front_end/panels/timeline/TimelineFlameChartView.ts

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as Platform from '../../core/platform/platform.js';
88
import * as Root from '../../core/root/root.js';
99
import * as SDK from '../../core/sdk/sdk.js';
1010
import * as Bindings from '../../models/bindings/bindings.js';
11+
import type * as CrUXManager from '../../models/crux-manager/crux-manager.js';
1112
import * as Trace from '../../models/trace/trace.js';
1213
import * as TraceBounds from '../../services/trace_bounds/trace_bounds.js';
1314
import * as PerfUI from '../../ui/legacy/components/perf_ui/perf_ui.js';
@@ -121,6 +122,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
121122
private selectedSearchResult?: PerfUI.FlameChart.DataProviderSearchResult;
122123
private searchRegex?: RegExp;
123124
#parsedTrace: Trace.Handlers.Types.ParsedTrace|null;
125+
#traceMetadata: Trace.Types.File.MetaData|null;
124126
#traceInsightSets: Trace.Insights.Types.TraceInsightSets|null = null;
125127
#eventToRelatedInsightsMap: TimelineComponents.RelatedInsightChips.EventToRelatedInsightsMap|null = null;
126128
#selectedGroupName: string|null = null;
@@ -141,7 +143,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
141143

142144
#currentInsightOverlays: Array<Overlays.Overlays.TimelineOverlay> = [];
143145
#activeInsight: TimelineComponents.Sidebar.ActiveInsight|null = null;
144-
#markers: Array<Overlays.Overlays.TimelineOverlay> = [];
146+
#markers: Array<Overlays.Overlays.TimingsMarker> = [];
145147

146148
#tooltipElement = document.createElement('div');
147149

@@ -165,6 +167,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
165167
this.delegate = delegate;
166168
this.eventListeners = [];
167169
this.#parsedTrace = null;
170+
this.#traceMetadata = null;
168171

169172
const flameChartsContainer = new UI.Widget.VBox();
170173
flameChartsContainer.element.classList.add('flame-charts-container');
@@ -521,6 +524,73 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
521524
});
522525
}
523526

527+
#amendMarkerWithFieldData(parsedTrace: Trace.Handlers.Types.ParsedTrace): void {
528+
const cruxFieldData = this.#traceMetadata?.cruxFieldData;
529+
if (!cruxFieldData) {
530+
return;
531+
}
532+
533+
const getPageResult = (url: string, origin: string): CrUXManager.PageResult|undefined => {
534+
return cruxFieldData.find(result => {
535+
const key = (result['url-ALL'] || result['origin-ALL'])?.record.key;
536+
return (key?.url && key.url === url) || (key?.origin && key.origin === origin);
537+
});
538+
};
539+
const getMetricScore = (pageResult: CrUXManager.PageResult, name: CrUXManager.StandardMetricNames):
540+
{value: number, pageScope: CrUXManager.PageScope}|null => {
541+
let value = pageResult['url-ALL']?.record.metrics[name]?.percentiles?.p75;
542+
if (typeof value === 'number') {
543+
return {value, pageScope: 'url'};
544+
}
545+
546+
value = pageResult['origin-ALL']?.record.metrics[name]?.percentiles?.p75;
547+
if (typeof value === 'number') {
548+
return {value, pageScope: 'origin'};
549+
}
550+
551+
return null;
552+
};
553+
554+
for (const marker of this.#markers) {
555+
for (const event of marker.entries) {
556+
const navigationId = event.args?.data?.navigationId;
557+
if (!navigationId) {
558+
continue;
559+
}
560+
561+
let cruxMetricName: CrUXManager.StandardMetricNames;
562+
if (event.name === Trace.Types.Events.Name.MARK_FCP) {
563+
cruxMetricName = 'first_contentful_paint';
564+
} else if (event.name === Trace.Types.Events.Name.MARK_LCP_CANDIDATE) {
565+
cruxMetricName = 'largest_contentful_paint';
566+
} else {
567+
continue;
568+
}
569+
570+
const nav = parsedTrace.Meta.navigationsByNavigationId.get(navigationId);
571+
// TODO(paulirish): Trace.Types.Events.NavigationStart type should be fixed, not working as intended.
572+
const url = nav?.args.data.documentLoaderURL as (string | undefined);
573+
if (!nav || !url) {
574+
continue;
575+
}
576+
577+
const pageResult = getPageResult(url, new URL(url).origin);
578+
if (!pageResult) {
579+
continue;
580+
}
581+
582+
const metricScoreResult = getMetricScore(pageResult, cruxMetricName);
583+
if (!metricScoreResult) {
584+
continue;
585+
}
586+
587+
const tsMs = metricScoreResult.value as Trace.Types.Timing.MilliSeconds;
588+
const ts = Trace.Helpers.Timing.millisecondsToMicroseconds(tsMs);
589+
marker.entryToFieldResult.set(event, {ts, pageScope: metricScoreResult.pageScope});
590+
}
591+
}
592+
}
593+
524594
setMarkers(parsedTrace: Trace.Handlers.Types.ParsedTrace|null): void {
525595
if (!parsedTrace) {
526596
return;
@@ -555,11 +625,12 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
555625
}
556626
}
557627
if (!matchingOverlay) {
558-
const overlay = {
628+
const overlay: Overlays.Overlays.TimingsMarker = {
559629
type: 'TIMINGS_MARKER',
560630
entries: [marker],
631+
entryToFieldResult: new Map(),
561632
adjustedTimestamp,
562-
} as Overlays.Overlays.TimingsMarker;
633+
};
563634
overlayByTs.set(marker.ts, overlay);
564635
}
565636
});
@@ -569,6 +640,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
569640
return;
570641
}
571642

643+
this.#amendMarkerWithFieldData(parsedTrace);
572644
this.bulkAddOverlays(this.#markers);
573645
}
574646

@@ -1047,12 +1119,15 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
10471119
this.#updateDetailViews();
10481120
}
10491121

1050-
setModel(newParsedTrace: Trace.Handlers.Types.ParsedTrace, isCpuProfile = false): void {
1122+
setModel(
1123+
newParsedTrace: Trace.Handlers.Types.ParsedTrace, traceMetadata: Trace.Types.File.MetaData|null,
1124+
isCpuProfile = false): void {
10511125
if (newParsedTrace === this.#parsedTrace) {
10521126
return;
10531127
}
10541128
this.#selectedGroupName = null;
10551129
this.#parsedTrace = newParsedTrace;
1130+
this.#traceMetadata = traceMetadata;
10561131
Common.EventTarget.removeEventListeners(this.eventListeners);
10571132
this.#selectedEvents = null;
10581133
this.mainDataProvider.setModel(newParsedTrace, isCpuProfile);

front_end/panels/timeline/TimelinePanel.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1916,6 +1916,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
19161916
}
19171917
const {traceIndex} = this.#viewMode;
19181918
const parsedTrace = this.#traceEngineModel.parsedTrace(traceIndex);
1919+
const metadata = this.#traceEngineModel.metadata(traceIndex);
19191920
const syntheticEventsManager = this.#traceEngineModel.syntheticTraceEventsManager(traceIndex);
19201921

19211922
if (!parsedTrace || !syntheticEventsManager) {
@@ -1951,7 +1952,7 @@ export class TimelinePanel extends UI.Panel.Panel implements Client, TimelineMod
19511952

19521953
const isCpuProfile =
19531954
this.#traceEngineModel.metadata(traceIndex)?.dataOrigin === Trace.Types.File.DataOrigin.CPU_PROFILE;
1954-
this.flameChart.setModel(parsedTrace, isCpuProfile);
1955+
this.flameChart.setModel(parsedTrace, metadata, isCpuProfile);
19551956
this.flameChart.resizeToPreferredHeights();
19561957
// Reset the visual selection as we've just swapped to a new trace.
19571958
this.flameChart.setSelectionAndReveal(null);

front_end/panels/timeline/fixtures/traces/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ copy_to_gen("traces") {
2020
"cls-multiple-frames.json.gz",
2121
"cls-no-nav.json.gz",
2222
"cls-single-frame.json.gz",
23+
"crux.json.gz",
2324
"dom-size-long.json.gz",
2425
"dom-size.json.gz",
2526
"enhanced-traces.json.gz",
119 KB
Binary file not shown.

0 commit comments

Comments
 (0)