[RPP] Simplify PageLoadMetricsHandler map

Instead of using a navigation id as the key, use the navigation event
object directly. Benefits:

* The map can more simply contain soft and hard navs
* Usages don't need to know how to lookup the navigation event by an
  id - a useful property since where to look depends on if its a soft
  or a hard navigation
* Also changed InsightModel to hold the navigation event directly,
  rather than a navigation id. Insights still don't handle soft-navs yet

Bug: 450673887
Change-Id: I58c74cc92fae1f3233fb8191310889caa2d141f1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7214930
Reviewed-by: Paul Irish <paulirish@chromium.org>
Auto-Submit: Connor Clark <cjamcl@chromium.org>
Commit-Queue: Connor Clark <cjamcl@chromium.org>

Author: Connor Clark

front_end/models/ai_assistance/data_formatters/PerformanceInsightFormatter.ts

@@ -13,12 +13,13 @@ import {bytes, millis} from './UnitFormatters.js';
 /**
  * For a given frame ID and navigation ID, returns the LCP Event and the LCP Request, if the resource was an image.
  */
-function getLCPData(parsedTrace: Trace.TraceModel.ParsedTrace, frameId: string, navigationId: string): {
+function getLCPData(
+    parsedTrace: Trace.TraceModel.ParsedTrace, frameId: string, navigation: Trace.Types.Events.NavigationStart): {
   lcpEvent: Trace.Types.Events.LargestContentfulPaintCandidate,
   metricScore: Trace.Handlers.ModelHandlers.PageLoadMetrics.LCPMetricScore,
   lcpRequest?: Trace.Types.Events.SyntheticNetworkRequest,
 }|null {
-  const navMetrics = parsedTrace.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigationId);
+  const navMetrics = parsedTrace.data.PageLoadMetrics.metricScoresByFrameId.get(frameId)?.get(navigation);
   if (!navMetrics) {
     return null;
   }
@@ -32,9 +33,12 @@ function getLCPData(parsedTrace: Trace.TraceModel.ParsedTrace, frameId: string,
     return null;
   }
 
+  const navigationId = navigation.args.data?.navigationId;
+
   return {
     lcpEvent,
-    lcpRequest: parsedTrace.data.LargestImagePaint.lcpRequestByNavigationId.get(navigationId),
+    lcpRequest: navigationId ? parsedTrace.data.LargestImagePaint.lcpRequestByNavigationId.get(navigationId) :
+                               undefined,
     metricScore: metric,
   };
 }
@@ -89,15 +93,15 @@ export class PerformanceInsightFormatter {
    * Information about LCP which we pass to the LLM for all insights that relate to LCP.
    */
   #lcpMetricSharedContext(): string {
-    if (!this.#insight.navigationId) {
+    if (!this.#insight.navigation) {
       // No navigation ID = no LCP.
       return '';
     }
-    if (!this.#insight.frameId || !this.#insight.navigationId) {
+    if (!this.#insight.frameId || !this.#insight.navigation) {
       return '';
     }
 
-    const data = getLCPData(this.#parsedTrace, this.#insight.frameId, this.#insight.navigationId);
+    const data = getLCPData(this.#parsedTrace, this.#insight.frameId, this.#insight.navigation);
     if (!data) {
       return '';
     }

front_end/models/trace/LanternComputationData.ts

@@ -10,14 +10,15 @@ import type * as Types from './types/types.js';
 
 type NetworkRequest = Lantern.Types.NetworkRequest<Types.Events.SyntheticNetworkRequest>;
 
-function createProcessedNavigation(data: Handlers.Types.HandlerData, frameId: string, navigationId: string):
-    Lantern.Types.Simulation.ProcessedNavigation {
+function createProcessedNavigation(
+    data: Handlers.Types.HandlerData, frameId: string,
+    navigation: Types.Events.NavigationStart): Lantern.Types.Simulation.ProcessedNavigation {
   const scoresByNav = data.PageLoadMetrics.metricScoresByFrameId.get(frameId);
   if (!scoresByNav) {
     throw new Lantern.Core.LanternError('missing metric scores for frame');
   }
 
-  const scores = scoresByNav.get(navigationId);
+  const scores = scoresByNav.get(navigation);
   if (!scores) {
     throw new Lantern.Core.LanternError('missing metric scores for specified navigation');
   }

front_end/models/trace/Processor.ts

@@ -268,7 +268,8 @@ export class TraceProcessor extends EventTarget {
 
   #createLanternContext(
       data: Handlers.Types.HandlerData, traceEvents: readonly Types.Events.Event[], frameId: string,
-      navigationId: string, options: Types.Configuration.ParseOptions): Insights.Types.LanternContext|undefined {
+      navigation: Types.Events.NavigationStart,
+      options: Types.Configuration.ParseOptions): Insights.Types.LanternContext|undefined {
     // Check for required handlers.
     if (!data.NetworkRequests || !data.Workers || !data.PageLoadMetrics) {
       return;
@@ -278,7 +279,7 @@ export class TraceProcessor extends EventTarget {
     }
 
     const navStarts = data.Meta.navigationsByFrameId.get(frameId);
-    const navStartIndex = navStarts?.findIndex(n => n.args.data?.navigationId === navigationId);
+    const navStartIndex = navStarts?.findIndex(n => n === navigation);
     if (!navStarts || navStartIndex === undefined || navStartIndex === -1) {
       throw new Lantern.Core.LanternError('Could not find navigation start');
     }
@@ -295,7 +296,7 @@ export class TraceProcessor extends EventTarget {
 
     const requests = LanternComputationData.createNetworkRequests(trace, data, startTime, endTime);
     const graph = LanternComputationData.createGraph(requests, trace, data);
-    const processedNavigation = LanternComputationData.createProcessedNavigation(data, frameId, navigationId);
+    const processedNavigation = LanternComputationData.createProcessedNavigation(data, frameId, navigation);
 
     const networkAnalysis = Lantern.Core.NetworkAnalyzer.analyze(requests);
     if (!networkAnalysis) {
@@ -455,7 +456,7 @@ export class TraceProcessor extends EventTarget {
         model.frameId = context.frameId;
         const navId = context.navigation?.args.data?.navigationId;
         if (navId) {
-          model.navigationId = navId;
+          model.navigation = context.navigation;
         }
         model.createOverlays = () => {
           // @ts-expect-error: model is a union of all possible insight model types.
@@ -571,7 +572,7 @@ export class TraceProcessor extends EventTarget {
     let lantern: Insights.Types.LanternContext|undefined;
     try {
       options.logger?.start('insights:createLanternContext');
-      lantern = this.#createLanternContext(data, traceEvents, frameId, navigationId, options);
+      lantern = this.#createLanternContext(data, traceEvents, frameId, navigation, options);
     } catch (e) {
       // Handle Lantern errors gracefully
       // Don't allow an error in constructing the Lantern graphs to break the rest of the trace processor.

front_end/models/trace/handlers/LargestImagePaintHandler.ts

@@ -55,7 +55,7 @@ export async function finalize(): Promise<void> {
   const metricScoresByFrameId = pageLoadMetricsData().metricScoresByFrameId;
 
   for (const [navigationId, navigation] of navigationsByNavigationId) {
-    const lcpMetric = metricScoresByFrameId.get(navigation.args.frame)?.get(navigationId)?.get(MetricName.LCP);
+    const lcpMetric = metricScoresByFrameId.get(navigation.args.frame)?.get(navigation)?.get(MetricName.LCP);
     const lcpEvent = lcpMetric?.event;
     if (!lcpEvent || !Types.Events.isAnyLargestContentfulPaintCandidate(lcpEvent)) {
       continue;

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

@@ -72,9 +72,8 @@ describeWithEnvironment('PageLoadMetricsHandler', function() {
       const {Meta, PageLoadMetrics} = data;
       const {mainFrameId, navigationsByFrameId} = Meta;
       const navigationBeforeMetrics = navigationsByFrameId.get(mainFrameId)?.[0];
-      const navigationId = navigationBeforeMetrics?.args.data?.navigationId;
-      if (!navigationBeforeMetrics || !navigationId) {
-        assert.fail('Could not find expected navigation event or its navigation ID');
+      if (!navigationBeforeMetrics) {
+        assert.fail('Could not find expected navigation event');
       }
       const pageLoadMetricsData = PageLoadMetrics.metricScoresByFrameId;
       // Only one frame to deal with
@@ -84,7 +83,7 @@ describeWithEnvironment('PageLoadMetricsHandler', function() {
       assert.isOk(pageLoadEventsForMainFrame, 'Page load events for main frame were unexpectedly null.');
       // Single FCP event that occurred after the refresh.
       assert.strictEqual(pageLoadEventsForMainFrame.size, 1);
-      const events = pageLoadEventsForMainFrame.get(navigationId);
+      const events = pageLoadEventsForMainFrame.get(navigationBeforeMetrics);
       const allFoundMetricScoresForMainFrame = events ? Array.from(events.values()) : [];
       for (const score of allFoundMetricScoresForMainFrame) {
         assert.strictEqual(score.navigation, navigationBeforeMetrics);

front_end/models/trace/handlers/PageLoadMetricsHandler.ts

@@ -5,10 +5,9 @@
 /**
  * This handler stores page load metrics, including web vitals,
  * and exports them in the shape of a map with the following shape:
- * Map(FrameId -> Map(navigationID -> metrics) )
+ * Map(FrameId -> Map(navigation -> metrics) )
  *
- * It also includes soft navigations as separate "navigations", though since soft
- * navs use different events, the "navigationID" for them is created in this handler.
+ * Includes soft navigations.
  *
  * It also exports all markers in a trace in an array.
  *
@@ -25,7 +24,6 @@ import type {HandlerName} from './types.js';
 
 // Small helpers to make the below type easier to read.
 type FrameId = string;
-type NavigationId = string|`softnav-${string}`;
 type AnyNavigationStart = Types.Events.NavigationStart|Types.Events.SoftNavigationStart;
 
 /**
@@ -34,7 +32,7 @@ type AnyNavigationStart = Types.Events.NavigationStart|Types.Events.SoftNavigati
  * The metric scores include the event related to the metric as well as the data regarding
  * the score itself.
  */
-let metricScoresByFrameId = new Map<FrameId, Map<NavigationId, Map<MetricName, MetricScore>>>();
+let metricScoresByFrameId = new Map<FrameId, Map<AnyNavigationStart, Map<MetricName, MetricScore>>>();
 
 /**
  * Page load events with no associated duration that happened in the
@@ -70,12 +68,6 @@ export function handleEvent(event: Types.Events.Event): void {
 
 function storePageLoadMetricAgainstNavigationId(
     navigation: AnyNavigationStart, event: Types.Events.PageLoadEvent): void {
-  const navigationId = Types.Events.isSoftNavigationStart(navigation) ?
-      `softnav-${navigation.args.context.performanceTimelineNavigationId}` :
-      navigation.args.data?.navigationId;
-  if (!navigationId) {
-    throw new Error('Navigation event unexpectedly had no navigation ID.');
-  }
   const frameId = getFrameIdForPageLoadEvent(event);
   const {rendererProcessesByFrame} = metaHandlerData();
 
@@ -102,15 +94,15 @@ function storePageLoadMetricAgainstNavigationId(
     const fcpTime = Types.Timing.Micro(event.ts - navigation.ts);
     const classification = scoreClassificationForFirstContentfulPaint(fcpTime);
     const metricScore = {event, metricName: MetricName.FCP, classification, navigation, timing: fcpTime};
-    storeMetricScore(frameId, navigationId, metricScore);
+    storeMetricScore(frameId, navigation, metricScore);
     return;
   }
 
   if (Types.Events.isFirstPaint(event)) {
     const paintTime = Types.Timing.Micro(event.ts - navigation.ts);
     const classification = ScoreClassification.UNCLASSIFIED;
     const metricScore = {event, metricName: MetricName.FP, classification, navigation, timing: paintTime};
-    storeMetricScore(frameId, navigationId, metricScore);
+    storeMetricScore(frameId, navigation, metricScore);
     return;
   }
 
@@ -123,7 +115,7 @@ function storePageLoadMetricAgainstNavigationId(
       navigation,
       timing: dclTime,
     };
-    storeMetricScore(frameId, navigationId, metricScore);
+    storeMetricScore(frameId, navigation, metricScore);
     return;
   }
 
@@ -136,7 +128,7 @@ function storePageLoadMetricAgainstNavigationId(
       navigation,
       timing: ttiValue,
     };
-    storeMetricScore(frameId, navigationId, tti);
+    storeMetricScore(frameId, navigation, tti);
 
     const tbtValue = Helpers.Timing.milliToMicro(Types.Timing.Milli(event.args.args.total_blocking_time_ms));
     const tbt = {
@@ -146,7 +138,7 @@ function storePageLoadMetricAgainstNavigationId(
       navigation,
       timing: tbtValue,
     };
-    storeMetricScore(frameId, navigationId, tbt);
+    storeMetricScore(frameId, navigation, tbt);
     return;
   }
 
@@ -159,7 +151,7 @@ function storePageLoadMetricAgainstNavigationId(
       navigation,
       timing: loadTime,
     };
-    storeMetricScore(frameId, navigationId, metricScore);
+    storeMetricScore(frameId, navigation, metricScore);
     return;
   }
 
@@ -177,11 +169,11 @@ function storePageLoadMetricAgainstNavigationId(
       timing: lcpTime,
     };
     const metricsByNavigation = Platform.MapUtilities.getWithDefault(metricScoresByFrameId, frameId, () => new Map());
-    const metrics = Platform.MapUtilities.getWithDefault(metricsByNavigation, navigationId, () => new Map());
+    const metrics = Platform.MapUtilities.getWithDefault(metricsByNavigation, navigation, () => new Map());
     const lastLCPCandidate = metrics.get(MetricName.LCP);
     if (lastLCPCandidate === undefined) {
       selectedLCPCandidateEvents.add(lcp.event);
-      storeMetricScore(frameId, navigationId, lcp);
+      storeMetricScore(frameId, navigation, lcp);
       return;
     }
     const lastLCPCandidateEvent = lastLCPCandidate.event;
@@ -199,7 +191,7 @@ function storePageLoadMetricAgainstNavigationId(
     if (lastCandidateIndex < candidateIndex) {
       selectedLCPCandidateEvents.delete(lastLCPCandidateEvent);
       selectedLCPCandidateEvents.add(lcp.event);
-      storeMetricScore(frameId, navigationId, lcp);
+      storeMetricScore(frameId, navigation, lcp);
     }
     return;
   }
@@ -212,9 +204,9 @@ function storePageLoadMetricAgainstNavigationId(
   return Platform.assertNever(event, `Unexpected event type: ${event}`);
 }
 
-function storeMetricScore(frameId: string, navigationId: string, metricScore: MetricScore): void {
+function storeMetricScore(frameId: string, navigation: AnyNavigationStart, metricScore: MetricScore): void {
   const metricsByNavigation = Platform.MapUtilities.getWithDefault(metricScoresByFrameId, frameId, () => new Map());
-  const metrics = Platform.MapUtilities.getWithDefault(metricsByNavigation, navigationId, () => new Map());
+  const metrics = Platform.MapUtilities.getWithDefault(metricsByNavigation, navigation, () => new Map());
   // If an entry with that metric name is present, delete it so that the new entry that
   // will replace it is added at the end of the map. This way we guarantee the map entries
   // are ordered in ASC manner by timestamp.
@@ -416,9 +408,9 @@ export interface PageLoadMetricsData {
    * The metric scores include the event related to the metric as well as the data regarding
    * the score itself.
    *
-   * Soft navigations have a faked NavigationId that starts with `softnav-`.
+   * Includes soft navigations.
    */
-  metricScoresByFrameId: Map<string, Map<NavigationId, Map<MetricName, MetricScore>>>;
+  metricScoresByFrameId: Map<string, Map<AnyNavigationStart, Map<MetricName, MetricScore>>>;
   /**
    * Page load events with no associated duration that happened in the
    * main frame.

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

@@ -124,15 +124,15 @@ describeWithEnvironment('Trace helpers', function() {
     it('returns the correct navigation for a page load event', async function() {
       const parsedTrace = await TraceLoader.traceEngine(this, 'multiple-navigations.json.gz');
       const {Meta, PageLoadMetrics} = parsedTrace.data;
-      const firstNavigationId = Meta.navigationsByNavigationId.keys().next().value!;
+      const firstNavigation = Meta.navigationsByNavigationId.values().next().value!;
 
       const fcp = PageLoadMetrics.metricScoresByFrameId.get(Meta.mainFrameId)
-                      ?.get(firstNavigationId)
+                      ?.get(firstNavigation)
                       ?.get(Trace.Handlers.ModelHandlers.PageLoadMetrics.MetricName.FCP);
       assert.isOk(fcp?.event, 'FCP not found');
       const navigationForFirstRequest =
           Trace.Helpers.Trace.getNavigationForTraceEvent(fcp.event, Meta.mainFrameId, Meta.navigationsByFrameId);
-      assert.strictEqual(navigationForFirstRequest?.args.data?.navigationId, firstNavigationId);
+      assert.strictEqual(navigationForFirstRequest, firstNavigation);
     });
   });
 

front_end/models/trace/insights/LCPBreakdown.ts

@@ -218,7 +218,7 @@ export function generateInsight(
     throw new Error('no frame metrics');
   }
 
-  const navMetrics = frameMetrics.get(context.navigationId);
+  const navMetrics = frameMetrics.get(context.navigation);
   if (!navMetrics) {
     throw new Error('no navigation metrics');
   }

front_end/models/trace/insights/LCPDiscovery.ts

@@ -108,7 +108,7 @@ export function generateInsight(
     throw new Error('no frame metrics');
   }
 
-  const navMetrics = frameMetrics.get(context.navigationId);
+  const navMetrics = frameMetrics.get(context.navigation);
   if (!navMetrics) {
     throw new Error('no navigation metrics');
   }

front_end/models/trace/insights/RenderBlocking.ts

@@ -185,7 +185,7 @@ export function generateInsight(
   }
 
   const firstPaintTs = data.PageLoadMetrics.metricScoresByFrameId.get(context.frameId)
-                           ?.get(context.navigationId)
+                           ?.get(context.navigation)
                            ?.get(Handlers.ModelHandlers.PageLoadMetrics.MetricName.FP)
                            ?.event?.ts;
   if (!firstPaintTs) {