From 0d907a16295dd37f9a2dd0e9cb3ba81a2e0f3ded Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Mon, 8 Jul 2024 16:20:23 +0100 Subject: [PATCH] RPP: add navigationId to layout shift clusters 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 Commit-Queue: Jack Franklin --- .../models/trace/handlers/LayoutShiftsHandler.test.ts | 7 +++++++ front_end/models/trace/handlers/LayoutShiftsHandler.ts | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/front_end/models/trace/handlers/LayoutShiftsHandler.test.ts b/front_end/models/trace/handlers/LayoutShiftsHandler.test.ts index 3634e8ab4ac..7bc27b5bb34 100644 --- a/front_end/models/trace/handlers/LayoutShiftsHandler.test.ts +++ b/front_end/models/trace/handlers/LayoutShiftsHandler.test.ts @@ -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. @@ -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() { diff --git a/front_end/models/trace/handlers/LayoutShiftsHandler.ts b/front_end/models/trace/handlers/LayoutShiftsHandler.ts index 4303b77c1ee..da83803b732 100644 --- a/front_end/models/trace/handlers/LayoutShiftsHandler.ts +++ b/front_end/models/trace/handlers/LayoutShiftsHandler.ts @@ -290,6 +290,12 @@ async function buildLayoutShiftsClusters(): Promise { 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), @@ -299,6 +305,7 @@ async function buildLayoutShiftsClusters(): Promise { needsImprovement: null, bad: null, }, + navigationId, }); firstShiftTime = clusterStartTime; @@ -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/