[RPP] Fix some bugs related to using the wrong time units

- The insights sorting criteria incorrectly used wrong time units for
  field data
- #getFilmStripFrame incorrectly compared times in different units

These bugs were discovered by a custom eslint rule that has not yet
landed[1].

[1] https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6395588

Bug: 406518012
Change-Id: I7a19181bf1362f200bb9038dca6fff06b16555a3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6417905
Auto-Submit: Connor Clark <[email protected]>
Reviewed-by: Paul Irish <[email protected]>
Commit-Queue: Connor Clark <[email protected]>

Author: Connor Clark

front_end/models/trace/Processor.test.ts

@@ -383,20 +383,20 @@ describeWithEnvironment('TraceProcessor', function() {
       assert.deepEqual(orderWithoutMetadata, [
         'CLSCulprits',
         'Viewport',
+        'Cache',
+        'ImageDelivery',
         'InteractionToNextPaint',
         'LCPPhases',
         'LCPDiscovery',
         'RenderBlocking',
         'NetworkDependencyTree',
-        'ImageDelivery',
         'DocumentLatency',
         'FontDisplay',
         'DOMSize',
         'ThirdParties',
         'DuplicatedJavaScript',
         'SlowCSSSelector',
         'ForcedReflow',
-        'Cache',
         'ModernHTTP',
         'LegacyJavaScript',
       ]);
@@ -406,20 +406,20 @@ describeWithEnvironment('TraceProcessor', function() {
       assert.deepEqual(orderWithMetadata, [
         'Viewport',
         'CLSCulprits',
+        'Cache',
+        'ImageDelivery',
         'InteractionToNextPaint',
         'LCPPhases',
         'LCPDiscovery',
         'RenderBlocking',
         'NetworkDependencyTree',
-        'ImageDelivery',
         'DocumentLatency',
         'FontDisplay',
         'DOMSize',
         'ThirdParties',
         'DuplicatedJavaScript',
         'SlowCSSSelector',
         'ForcedReflow',
-        'Cache',
         'ModernHTTP',
         'LegacyJavaScript',
       ]);

front_end/models/trace/Processor.ts

@@ -381,13 +381,15 @@ export class TraceProcessor extends EventTarget {
 
     // Normalize the estimated savings to a single number, weighted by its relative impact
     // to the page experience based on the same scoring curve that Lighthouse uses.
-    const observedLcp = Insights.Common.getLCP(insights, insightSet.id)?.value;
+    const observedLcpMicro = Insights.Common.getLCP(insights, insightSet.id)?.value;
+    const observedLcp = observedLcpMicro ? Helpers.Timing.microToMilli(observedLcpMicro) : Types.Timing.Milli(0);
     const observedCls = Insights.Common.getCLS(insights, insightSet.id).value;
 
     // INP is special - if users did not interact with the page, we'll have no INP, but we should still
     // be able to prioritize insights based on this metric. When we observe no interaction, instead use
     // a default value for the baseline INP.
-    const observedInp = Insights.Common.getINP(insights, insightSet.id)?.value ?? 200;
+    const observedInpMicro = Insights.Common.getINP(insights, insightSet.id)?.value;
+    const observedInp = observedInpMicro ? Helpers.Timing.microToMilli(observedInpMicro) : Types.Timing.Milli(200);
 
     const observedLcpScore =
         observedLcp !== undefined ? Insights.Common.evaluateLCPMetricScore(observedLcp) : undefined;
@@ -400,8 +402,9 @@ export class TraceProcessor extends EventTarget {
       const inp = model.metricSavings?.INP ?? 0;
       const cls = model.metricSavings?.CLS ?? 0;
 
-      const lcpPostSavings = observedLcp !== undefined ? Math.max(0, observedLcp - lcp) : undefined;
-      const inpPostSavings = Math.max(0, observedInp - inp);
+      const lcpPostSavings =
+          observedLcp !== undefined ? Math.max(0, observedLcp - lcp) as Types.Timing.Milli : undefined;
+      const inpPostSavings = Math.max(0, observedInp - inp) as Types.Timing.Milli;
       const clsPostSavings = Math.max(0, observedCls - cls);
 
       let score = 0;

front_end/models/trace/insights/Common.test.ts

@@ -53,7 +53,7 @@ describeWithEnvironment('Common', function() {
       const {insightSet, metadata} = await process(this, 'image-delivery.json.gz');
 
       const weights = calculateMetricWeightsForSorting(insightSet, metadata);
-      assert.deepEqual(weights, {lcp: 0.48649783990559314, inp: 0.48649783990559314, cls: 0.027004320188813675});
+      assert.deepEqual(weights, {lcp: 0.07778127820223579, inp: 0.5504200439526509, cls: 0.37179867784511333});
     });
   });
 

front_end/models/trace/insights/Common.ts

@@ -84,11 +84,11 @@ export function getCLS(
   return {value: maxScore, worstClusterEvent: worstCluster ?? null};
 }
 
-export function evaluateLCPMetricScore(value: number): number {
+export function evaluateLCPMetricScore(value: Types.Timing.Milli): number {
   return getLogNormalScore({p10: 2500, median: 4000}, value);
 }
 
-export function evaluateINPMetricScore(value: number): number {
+export function evaluateINPMetricScore(value: Types.Timing.Milli): number {
   return getLogNormalScore({p10: 200, median: 500}, value);
 }
 
@@ -212,8 +212,8 @@ export function calculateMetricWeightsForSorting(
   const fieldLcp = fieldMetrics.lcp?.value ?? null;
   const fieldInp = fieldMetrics.inp?.value ?? null;
   const fieldCls = fieldMetrics.cls?.value ?? null;
-  const fieldLcpScore = fieldLcp !== null ? evaluateLCPMetricScore(fieldLcp) : 0;
-  const fieldInpScore = fieldInp !== null ? evaluateINPMetricScore(fieldInp) : 0;
+  const fieldLcpScore = fieldLcp !== null ? evaluateLCPMetricScore(Helpers.Timing.microToMilli(fieldLcp)) : 0;
+  const fieldInpScore = fieldInp !== null ? evaluateINPMetricScore(Helpers.Timing.microToMilli(fieldInp)) : 0;
   const fieldClsScore = fieldCls !== null ? evaluateCLSMetricScore(fieldCls) : 0;
   const fieldLcpScoreInverted = 1 - fieldLcpScore;
   const fieldInpScoreInverted = 1 - fieldInpScore;

front_end/panels/timeline/TimelineDetailsView.ts

@@ -405,13 +405,16 @@ export class TimelineDetailsPane extends
     if (!this.#filmStrip) {
       return null;
     }
+
     const screenshotTime = (frame.idle ? frame.startTime : frame.endTime);
     const filmStripFrame = Trace.Extras.FilmStrip.frameClosestToTimestamp(this.#filmStrip, screenshotTime);
     if (!filmStripFrame) {
       return null;
     }
+
     const frameTimeMilliSeconds = Trace.Helpers.Timing.microToMilli(filmStripFrame.screenshotEvent.ts);
-    return frameTimeMilliSeconds - frame.endTime < 10 ? filmStripFrame : null;
+    const frameEndTimeMilliSeconds = Trace.Helpers.Timing.microToMilli(frame.endTime);
+    return frameTimeMilliSeconds - frameEndTimeMilliSeconds < 10 ? filmStripFrame : null;
   }
 
   #setSelectionForTimelineFrame(frame: Trace.Types.Events.LegacyTimelineFrame): void {