Skip to content

Commit 802453b

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Add marker for LCP after a soft navigation
Bug: 450673887 Change-Id: Id3f470f94c89a9e9784058f2105fc27403fa015f Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7214929 Reviewed-by: Paul Irish <paulirish@chromium.org> Commit-Queue: Connor Clark <cjamcl@chromium.org> Auto-Submit: Connor Clark <cjamcl@chromium.org>
1 parent 073fe02 commit 802453b

18 files changed

Lines changed: 122 additions & 46 deletions

front_end/models/ai_assistance/data_formatters/PerformanceInsightFormatter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ function getLCPData(parsedTrace: Trace.TraceModel.ParsedTrace, frameId: string,
2828
return null;
2929
}
3030
const lcpEvent = metric?.event;
31-
if (!lcpEvent || !Trace.Types.Events.isLargestContentfulPaintCandidate(lcpEvent)) {
31+
if (!lcpEvent || !Trace.Types.Events.isAnyLargestContentfulPaintCandidate(lcpEvent)) {
3232
return null;
3333
}
3434

front_end/models/trace/Styles.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,11 @@ export function maybeInitSylesMap(): EventStylesMap {
879879
defaultCategoryStyles.rendering,
880880
true,
881881
),
882+
[Types.Events.Name.MARK_LCP_CANDIDATE_FOR_SOFT_NAVIGATION]: new TimelineRecordStyle(
883+
i18nString(UIStrings.largestContentfulPaint),
884+
defaultCategoryStyles.rendering,
885+
true,
886+
),
882887

883888
[Types.Events.Name.TIME_STAMP]:
884889
new TimelineRecordStyle(i18nString(UIStrings.timestamp), defaultCategoryStyles.scripting),
@@ -1118,7 +1123,7 @@ export function markerDetailsForEvent(event: Types.Events.Event): {
11181123
color = 'var(--sys-color-green-bright)';
11191124
title = Handlers.ModelHandlers.PageLoadMetrics.MetricName.FCP;
11201125
}
1121-
if (Types.Events.isLargestContentfulPaintCandidate(event)) {
1126+
if (Types.Events.isAnyLargestContentfulPaintCandidate(event)) {
11221127
color = 'var(--sys-color-green)';
11231128
title = Handlers.ModelHandlers.PageLoadMetrics.MetricName.LCP;
11241129
}

front_end/models/trace/handlers/LargestImagePaintHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export async function finalize(): Promise<void> {
5757
for (const [navigationId, navigation] of navigationsByNavigationId) {
5858
const lcpMetric = metricScoresByFrameId.get(navigation.args.frame)?.get(navigationId)?.get(MetricName.LCP);
5959
const lcpEvent = lcpMetric?.event;
60-
if (!lcpEvent || !Types.Events.isLargestContentfulPaintCandidate(lcpEvent)) {
60+
if (!lcpEvent || !Types.Events.isAnyLargestContentfulPaintCandidate(lcpEvent)) {
6161
continue;
6262
}
6363

front_end/models/trace/handlers/PageLoadMetricsHandler.test.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,18 @@ describeWithEnvironment('PageLoadMetricsHandler', function() {
251251

252252
it('extracts all marker events from a trace correctly', () => {
253253
for (const metricName of Trace.Types.Events.MarkerName) {
254+
if (metricName === 'largestContentfulPaint::CandidateForSoftNavigation') {
255+
continue;
256+
}
257+
254258
const markerEventsOfThisType = allMarkerEvents.filter(event => event.name === metricName);
255259
// There should be 2 events for each marker and all of them should correspond to the main frame
256-
assert.lengthOf(markerEventsOfThisType, 2);
257-
assert.isTrue(markerEventsOfThisType.every(
258-
marker => Trace.Handlers.ModelHandlers.PageLoadMetrics.getFrameIdForPageLoadEvent(marker) === mainFrameId));
260+
assert.lengthOf(markerEventsOfThisType, 2, `failed for ${metricName}`);
261+
assert.isTrue(
262+
markerEventsOfThisType.every(
263+
marker =>
264+
Trace.Handlers.ModelHandlers.PageLoadMetrics.getFrameIdForPageLoadEvent(marker) === mainFrameId),
265+
`failed for ${metricName}`);
259266
}
260267
});
261268

@@ -269,20 +276,20 @@ describeWithEnvironment('PageLoadMetricsHandler', function() {
269276
const {data} = await TraceLoader.traceEngine(this, 'multiple-lcp-main-frame.json.gz');
270277
const {PageLoadMetrics} = data;
271278
const pageLoadMarkers = PageLoadMetrics.allMarkerEvents;
272-
const largestContentfulPaints = pageLoadMarkers.filter(Trace.Types.Events.isLargestContentfulPaintCandidate);
279+
const largestContentfulPaints = pageLoadMarkers.filter(Trace.Types.Events.isAnyLargestContentfulPaintCandidate);
273280
assert.lengthOf(largestContentfulPaints, 1);
274281
assert.strictEqual(largestContentfulPaints[0].args.data?.candidateIndex, 2);
275282
});
276283
});
277284

278285
describe('soft navs', () => {
279-
it('detects SoftNavigationStart', async function() {
286+
it('detects SoftNavigationStart and LCP for soft-nav', async function() {
280287
const {data} = await TraceLoader.traceEngine(this, 'soft-navs.json.gz');
281288
const {PageLoadMetrics} = data;
282289
assert.deepEqual(PageLoadMetrics.allMarkerEvents.map(e => e.name), [
283-
'SoftNavigationStart',
284-
'SoftNavigationStart',
285-
'SoftNavigationStart',
290+
'SoftNavigationStart', 'largestContentfulPaint::CandidateForSoftNavigation', 'SoftNavigationStart',
291+
'largestContentfulPaint::CandidateForSoftNavigation', 'SoftNavigationStart',
292+
'largestContentfulPaint::CandidateForSoftNavigation'
286293
]);
287294
});
288295
});

front_end/models/trace/handlers/PageLoadMetricsHandler.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
* and exports them in the shape of a map with the following shape:
88
* Map(FrameId -> Map(navigationID -> metrics) )
99
*
10+
* It also includes soft navigations as separate "navigations", though since soft
11+
* navs use different events, the "navigationID" for them is created in this handler.
12+
*
1013
* It also exports all markers in a trace in an array.
1114
*
1215
* Some metrics are taken directly from a page load events (AKA markers) like DCL.
@@ -22,8 +25,9 @@ import type {HandlerName} from './types.js';
2225

2326
// Small helpers to make the below type easier to read.
2427
type FrameId = string;
25-
type NavigationId = string;
28+
type NavigationId = string|`softnav-${string}`;
2629
type AnyNavigationStart = Types.Events.NavigationStart|Types.Events.SoftNavigationStart;
30+
2731
/**
2832
* This represents the metric scores for all navigations, for all frames in a trace.
2933
* Given a frame id, the map points to another map from navigation id to metric scores.
@@ -55,7 +59,7 @@ let pageLoadEventsArray: Types.Events.PageLoadEvent[] = [];
5559
// trace, we store that and delete the prior event. When we've parsed the
5660
// entire trace this set will contain all the LCP events that were used - e.g.
5761
// the candidates that were the actual LCP events.
58-
let selectedLCPCandidateEvents = new Set<Types.Events.LargestContentfulPaintCandidate>();
62+
let selectedLCPCandidateEvents = new Set<Types.Events.AnyLargestContentfulPaintCandidate>();
5963

6064
export function handleEvent(event: Types.Events.Event): void {
6165
if (!Types.Events.eventIsPageLoadEvent(event)) {
@@ -159,7 +163,7 @@ function storePageLoadMetricAgainstNavigationId(
159163
return;
160164
}
161165

162-
if (Types.Events.isLargestContentfulPaintCandidate(event)) {
166+
if (Types.Events.isAnyLargestContentfulPaintCandidate(event)) {
163167
const candidateIndex = event.args.data?.candidateIndex;
164168
if (!candidateIndex) {
165169
throw new Error('Largest Contentful Paint unexpectedly had no candidateIndex.');
@@ -182,7 +186,7 @@ function storePageLoadMetricAgainstNavigationId(
182186
}
183187
const lastLCPCandidateEvent = lastLCPCandidate.event;
184188

185-
if (!Types.Events.isLargestContentfulPaintCandidate(lastLCPCandidateEvent)) {
189+
if (!Types.Events.isAnyLargestContentfulPaintCandidate(lastLCPCandidateEvent)) {
186190
return;
187191
}
188192
const lastCandidateIndex = lastLCPCandidateEvent.args.data?.candidateIndex;
@@ -220,7 +224,7 @@ function storeMetricScore(frameId: string, navigationId: string, metricScore: Me
220224

221225
export function getFrameIdForPageLoadEvent(event: Types.Events.PageLoadEvent): string {
222226
if (Types.Events.isFirstContentfulPaint(event) || Types.Events.isInteractiveTime(event) ||
223-
Types.Events.isLargestContentfulPaintCandidate(event) || Types.Events.isNavigationStart(event) ||
227+
Types.Events.isAnyLargestContentfulPaintCandidate(event) || Types.Events.isNavigationStart(event) ||
224228
Types.Events.isSoftNavigationStart(event) || Types.Events.isLayoutShift(event) ||
225229
Types.Events.isFirstPaint(event)) {
226230
return event.args.frame;
@@ -236,15 +240,22 @@ export function getFrameIdForPageLoadEvent(event: Types.Events.PageLoadEvent): s
236240
}
237241

238242
function getNavigationForPageLoadEvent(event: Types.Events.PageLoadEvent): AnyNavigationStart|null {
239-
if (Types.Events.isFirstContentfulPaint(event) || Types.Events.isLargestContentfulPaintCandidate(event) ||
243+
if (Types.Events.isFirstContentfulPaint(event) || Types.Events.isAnyLargestContentfulPaintCandidate(event) ||
240244
Types.Events.isFirstPaint(event)) {
241-
const navigationId = event.args.data?.navigationId;
242-
if (!navigationId) {
243-
throw new Error('Trace event unexpectedly had no navigation ID.');
245+
const {navigationsByNavigationId, softNavigationsById} = metaHandlerData();
246+
let navigation;
247+
if (event.name === Types.Events.Name.MARK_LCP_CANDIDATE_FOR_SOFT_NAVIGATION &&
248+
event.args.data?.performanceTimelineNavigationId) {
249+
navigation = softNavigationsById.get(event.args.data.performanceTimelineNavigationId);
244250
}
245-
const {navigationsByNavigationId} = metaHandlerData();
246-
const navigation = navigationsByNavigationId.get(navigationId);
251+
if (!navigation) {
252+
const navigationId = event.args.data?.navigationId;
253+
if (!navigationId) {
254+
throw new Error('Trace event unexpectedly had no navigation ID.');
255+
}
247256

257+
navigation = navigationsByNavigationId.get(navigationId);
258+
}
248259
if (!navigation) {
249260
// This event's navigation has been filtered out by the meta handler as a noise event.
250261
return null;
@@ -390,7 +401,8 @@ export async function finalize(): Promise<void> {
390401
const allFinalLCPEvents = gatherFinalLCPEvents();
391402
const mainFrame = metaHandlerData().mainFrameId;
392403
// Filter out LCP candidates to use only definitive LCP values
393-
const allEventsButLCP = pageLoadEventsArray.filter(event => !Types.Events.isLargestContentfulPaintCandidate(event));
404+
const allEventsButLCP =
405+
pageLoadEventsArray.filter(event => !Types.Events.isAnyLargestContentfulPaintCandidate(event));
394406
const markerEvents = [...allFinalLCPEvents, ...allEventsButLCP].filter(Types.Events.isMarkerEvent);
395407
// Filter by main frame and sort.
396408
allMarkerEvents =
@@ -403,8 +415,10 @@ export interface PageLoadMetricsData {
403415
* Given a frame id, the map points to another map from navigation id to metric scores.
404416
* The metric scores include the event related to the metric as well as the data regarding
405417
* the score itself.
418+
*
419+
* Soft navigations have a faked NavigationId that starts with `softnav-`.
406420
*/
407-
metricScoresByFrameId: Map<string, Map<string, Map<MetricName, MetricScore>>>;
421+
metricScoresByFrameId: Map<string, Map<NavigationId, Map<MetricName, MetricScore>>>;
408422
/**
409423
* Page load events with no associated duration that happened in the
410424
* main frame.

front_end/models/trace/helpers/Timing.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ describeWithEnvironment('Timing helpers', () => {
8080
lcpEvent,
8181
data.Meta.traceBounds,
8282
data.Meta.navigationsByNavigationId,
83+
data.Meta.softNavigationsById,
8384
data.Meta.navigationsByFrameId,
8485
);
8586

@@ -112,6 +113,7 @@ describeWithEnvironment('Timing helpers', () => {
112113
dclEvent,
113114
data.Meta.traceBounds,
114115
data.Meta.navigationsByNavigationId,
116+
data.Meta.softNavigationsById,
115117
data.Meta.navigationsByFrameId,
116118
);
117119

front_end/models/trace/helpers/Timing.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,17 @@ export function timeStampForEventAdjustedByClosestNavigation(
2222
event: Types.Events.Event,
2323
traceBounds: Types.Timing.TraceWindowMicro,
2424
navigationsByNavigationId: Map<string, Types.Events.NavigationStart>,
25+
softNavigationsById: Map<number, Types.Events.SoftNavigationStart>,
2526
navigationsByFrameId: Map<string, Types.Events.NavigationStart[]>,
2627
): Types.Timing.Micro {
2728
let eventTimeStamp = event.ts - traceBounds.min;
28-
if (event.args?.data?.navigationId) {
29+
if (event.name === Types.Events.Name.MARK_LCP_CANDIDATE_FOR_SOFT_NAVIGATION &&
30+
event.args?.data?.performanceTimelineNavigationId) {
31+
const navigationForEvent = softNavigationsById.get(event.args.data.performanceTimelineNavigationId);
32+
if (navigationForEvent) {
33+
eventTimeStamp = event.ts - navigationForEvent.ts;
34+
}
35+
} else if (event.args?.data?.navigationId) {
2936
const navigationForEvent = navigationsByNavigationId.get(event.args.data.navigationId);
3037
if (navigationForEvent) {
3138
eventTimeStamp = event.ts - navigationForEvent.ts;

front_end/models/trace/insights/Common.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export function getInsight<InsightName extends keyof InsightModels>(
2727
}
2828

2929
export function getLCP(insightSet: InsightSet):
30-
{value: Types.Timing.Micro, event: Types.Events.LargestContentfulPaintCandidate}|null {
30+
{value: Types.Timing.Micro, event: Types.Events.AnyLargestContentfulPaintCandidate}|null {
3131
const insight = getInsight(InsightKeys.LCP_BREAKDOWN, insightSet);
3232
if (!insight || !insight.lcpMs || !insight.lcpEvent) {
3333
return null;

front_end/models/trace/insights/LCPBreakdown.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export function isLCPBreakdownInsight(model: InsightModel): model is LCPBreakdow
9595
export type LCPBreakdownInsightModel = InsightModel<typeof UIStrings, {
9696
lcpMs?: Types.Timing.Milli,
9797
lcpTs?: Types.Timing.Milli,
98-
lcpEvent?: Types.Events.LargestContentfulPaintCandidate,
98+
lcpEvent?: Types.Events.AnyLargestContentfulPaintCandidate,
9999
/** The network request for the LCP image, if there was one. */
100100
lcpRequest?: Types.Events.SyntheticNetworkRequest,
101101
subparts?: LCPSubparts,
@@ -112,7 +112,7 @@ function anyValuesNaN(...values: number[]): boolean {
112112
*/
113113
function determineSubparts(
114114
nav: Types.Events.NavigationStart, docRequest: Types.Events.SyntheticNetworkRequest,
115-
lcpEvent: Types.Events.LargestContentfulPaintCandidate,
115+
lcpEvent: Types.Events.AnyLargestContentfulPaintCandidate,
116116
lcpRequest: Types.Events.SyntheticNetworkRequest|undefined): LCPSubparts|null {
117117
const firstDocByteTs = calculateDocFirstByteTs(docRequest);
118118
if (firstDocByteTs === null) {
@@ -224,7 +224,7 @@ export function generateInsight(
224224
}
225225
const metricScore = navMetrics.get(Handlers.ModelHandlers.PageLoadMetrics.MetricName.LCP);
226226
const lcpEvent = metricScore?.event;
227-
if (!lcpEvent || !Types.Events.isLargestContentfulPaintCandidate(lcpEvent)) {
227+
if (!lcpEvent || !Types.Events.isAnyLargestContentfulPaintCandidate(lcpEvent)) {
228228
return finalize({warnings: [InsightWarning.NO_LCP]});
229229
}
230230

front_end/models/trace/insights/LCPDiscovery.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export function isLCPDiscoveryInsight(model: InsightModel): model is LCPDiscover
6666
return model.insightKey === 'LCPDiscovery';
6767
}
6868
export type LCPDiscoveryInsightModel = InsightModel<typeof UIStrings, {
69-
lcpEvent?: Types.Events.LargestContentfulPaintCandidate,
69+
lcpEvent?: Types.Events.AnyLargestContentfulPaintCandidate,
7070
/** The network request for the LCP image, if there was one. */
7171
lcpRequest?: Types.Events.SyntheticNetworkRequest,
7272
earliestDiscoveryTimeTs?: Types.Timing.Micro,
@@ -114,7 +114,7 @@ export function generateInsight(
114114
}
115115
const metricScore = navMetrics.get(Handlers.ModelHandlers.PageLoadMetrics.MetricName.LCP);
116116
const lcpEvent = metricScore?.event;
117-
if (!lcpEvent || !Types.Events.isLargestContentfulPaintCandidate(lcpEvent)) {
117+
if (!lcpEvent || !Types.Events.isAnyLargestContentfulPaintCandidate(lcpEvent)) {
118118
return finalize({warnings: [InsightWarning.NO_LCP]});
119119
}
120120

0 commit comments

Comments
 (0)