Skip to content

Commit

Permalink
RPP: add navigationId to layout shift clusters
Browse files Browse the repository at this point in the history
This will more easily enable us to group layout shift information in the
UI when a trace has multiple navigations. You could do this via
timestamps and grouping, but we can easily do this in the handler to
avoid anyone else having to do that work.

Bug: 348591291
Change-Id: I2beb0928c37664a84f6d130960fcf187e60c0062
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5683287
Reviewed-by: Andres Olivares <[email protected]>
Commit-Queue: Jack Franklin <[email protected]>
  • Loading branch information
jackfranklin authored and Devtools-frontend LUCI CQ committed Jul 9, 2024
1 parent 80f8fe3 commit 0d907a1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 0 deletions.
7 changes: 7 additions & 0 deletions front_end/models/trace/handlers/LayoutShiftsHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ describe('LayoutShiftsHandler', function() {
}

assert.strictEqual(layoutShifts.clusters[0].clusterWindow.max, navigations[0].ts);
// The first cluster happens before any navigation
assert.isUndefined(layoutShifts.clusters[0].navigationId);

// We should see an initial cluster here from the first layout shifts,
// followed by 1 for each of the navigations themselves.
Expand All @@ -89,6 +91,11 @@ describe('LayoutShiftsHandler', function() {
const secondCluster = layoutShifts.clusters[1];
// The second cluster should be marked to start at the first shift timestamp.
assert.strictEqual(secondCluster.clusterWindow.min, secondCluster.events[0].ts);

// The second cluster happened after the first navigation, so it should
// have navigationId set to the ID of the first navigation
assert.isDefined(secondCluster.navigationId);
assert.strictEqual(secondCluster.navigationId, navigations[0].args.data?.navigationId);
});

it('creates a cluster after exceeding the continuous shift limit', async function() {
Expand Down
9 changes: 9 additions & 0 deletions front_end/models/trace/handlers/LayoutShiftsHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,12 @@ async function buildLayoutShiftsClusters(): Promise<void> {
updateTraceWindowMax(currentCluster.clusterWindow, Types.Timing.MicroSeconds(previousClusterEndTime));
}

// If this cluster happened after a navigation, set the navigationId to
// the current navigation. This lets us easily group clusters by
// navigation.
const navigationId =
currentShiftNavigation === null ? undefined : navigations[currentShiftNavigation].args.data?.navigationId;

clusters.push({
events: [],
clusterWindow: traceWindowFromTime(clusterStartTime),
Expand All @@ -299,6 +305,7 @@ async function buildLayoutShiftsClusters(): Promise<void> {
needsImprovement: null,
bad: null,
},
navigationId,
});

firstShiftTime = clusterStartTime;
Expand Down Expand Up @@ -467,6 +474,8 @@ export interface LayoutShiftCluster {
needsImprovement: Types.Timing.TraceWindowMicroSeconds|null,
bad: Types.Timing.TraceWindowMicroSeconds|null,
};
// The last navigation that happened before this cluster.
navigationId?: string;
}

// Based on https://web.dev/cls/
Expand Down

0 comments on commit 0d907a1

Please sign in to comment.