core(lantern): refactor fcp graph method signatures

core/computed/metrics/lantern-first-contentful-paint.js

@@ -24,26 +24,30 @@ class LanternFirstContentfulPaint extends LanternMetric {
     };
   }
 
+  /**
+   * @typedef FirstPaintBasedGraphOpts
+   * @property {number} cutoffTimestamp The timestamp used to filter out tasks that occured after
+   *    our paint of interest. Typically this is First Contentful Paint or First Meaningful Paint.
+   * @property {function(NetworkNode):boolean} treatScriptAsBlocking The function that determines
+   *    which resources should be considered *possibly* render-blocking.
+   * @property {(function(CPUNode):boolean)=} additionalCpuNodesToTreatAsBlocking The function that
+   *    determines which CPU nodes should also be included in our blocking node IDs set,
+   *    beyond what getRenderBlockingNodeData() already includes.
+   */
+
   /**
    * This function computes the set of URLs that *appeared* to be render-blocking based on our filter,
    * *but definitely were not* render-blocking based on the timing of their EvaluateScript task.
    * It also computes the set of corresponding CPU node ids that were needed for the paint at the
    * given timestamp.
    *
    * @param {Node} graph
-   * @param {number} filterTimestamp The timestamp used to filter out tasks that occured after our
-   *    paint of interest. Typically this is First Contentful Paint or First Meaningful Paint.
-   * @param {function(NetworkNode):boolean} blockingScriptFilter The function that determines which scripts
-   *    should be considered *possibly* render-blocking.
-   * @param {(function(CPUNode):boolean)=} extraBlockingCpuNodesToIncludeFilter The function that determines which CPU nodes
-   *    should also be included in our blocking node IDs set.
+   * @param {FirstPaintBasedGraphOpts} opts
    * @return {{definitelyNotRenderBlockingScriptUrls: Set<string>, blockingCpuNodeIds: Set<string>}}
    */
-  static getBlockingNodeData(
+  static getRenderBlockingNodeData(
       graph,
-      filterTimestamp,
-      blockingScriptFilter,
-      extraBlockingCpuNodesToIncludeFilter
+      {cutoffTimestamp, treatScriptAsBlocking, additionalCpuNodesToTreatAsBlocking}
   ) {
     /** @type {Map<string, CPUNode>} A map of blocking script URLs to the earliest EvaluateScript task node that executed them. */
     const scriptUrlToNodeMap = new Map();
@@ -52,9 +56,9 @@ class LanternFirstContentfulPaint extends LanternMetric {
     const cpuNodes = [];
     graph.traverse(node => {
       if (node.type === BaseNode.TYPES.CPU) {
-        // A task is *possibly* render blocking if it *started* before filterTimestamp.
+        // A task is *possibly* render blocking if it *started* before cutoffTimestamp.
         // We use startTime here because the paint event can be *inside* the task that was render blocking.
-        if (node.startTime <= filterTimestamp) cpuNodes.push(node);
+        if (node.startTime <= cutoffTimestamp) cpuNodes.push(node);
 
         // Build our script URL map to find the earliest EvaluateScript task node.
         const scriptUrls = node.getEvaluateScriptURLs();
@@ -68,12 +72,12 @@ class LanternFirstContentfulPaint extends LanternMetric {
 
     cpuNodes.sort((a, b) => a.startTime - b.startTime);
 
-    // A script is *possibly* render blocking if it finished loading before filterTimestamp.
+    // A script is *possibly* render blocking if it finished loading before cutoffTimestamp.
     const possiblyRenderBlockingScriptUrls = LanternMetric.getScriptUrls(graph, node => {
-      return node.endTime <= filterTimestamp && blockingScriptFilter(node);
+      return node.endTime <= cutoffTimestamp && treatScriptAsBlocking(node);
     });
 
-    // A script is *definitely not* render blocking if its EvaluateScript task started after filterTimestamp.
+    // A script is *definitely not* render blocking if its EvaluateScript task started after cutoffTimestamp.
     /** @type {Set<string>} */
     const definitelyNotRenderBlockingScriptUrls = new Set();
     /** @type {Set<string>} */
@@ -85,7 +89,7 @@ class LanternFirstContentfulPaint extends LanternMetric {
       // If we can't find it at all, we can't conclude anything, so just skip it.
       if (!cpuNodeForUrl) continue;
 
-      // If we found it and it was in our `cpuNodes` set that means it finished before filterTimestamp, so it really is render-blocking.
+      // If we found it and it was in our `cpuNodes` set that means it finished before cutoffTimestamp, so it really is render-blocking.
       if (cpuNodes.includes(cpuNodeForUrl)) {
         blockingCpuNodeIds.add(cpuNodeForUrl.id);
         continue;
@@ -106,9 +110,9 @@ class LanternFirstContentfulPaint extends LanternMetric {
     if (firstParse) blockingCpuNodeIds.add(firstParse.id);
 
     // If a CPU filter was passed in, we also want to include those extra nodes.
-    if (extraBlockingCpuNodesToIncludeFilter) {
+    if (additionalCpuNodesToTreatAsBlocking) {
       cpuNodes
-        .filter(extraBlockingCpuNodesToIncludeFilter)
+        .filter(additionalCpuNodesToTreatAsBlocking)
         .forEach(node => blockingCpuNodeIds.add(node.id));
     }
 
@@ -122,35 +126,25 @@ class LanternFirstContentfulPaint extends LanternMetric {
    * This function computes the graph required for the first paint of interest.
    *
    * @param {Node} dependencyGraph
-   * @param {number} paintTs The timestamp used to filter out tasks that occured after our
-   *    paint of interest. Typically this is First Contentful Paint or First Meaningful Paint.
-   * @param {function(NetworkNode):boolean} blockingResourcesFilter The function that determines which resources
-   *    should be considered *possibly* render-blocking.
-   * @param {(function(CPUNode):boolean)=} extraBlockingCpuNodesToIncludeFilter The function that determines which CPU nodes
-   *    should also be included in our blocking node IDs set.
+   * @param {FirstPaintBasedGraphOpts} opts
    * @return {Node}
    */
   static getFirstPaintBasedGraph(
       dependencyGraph,
-      paintTs,
-      blockingResourcesFilter,
-      extraBlockingCpuNodesToIncludeFilter
+      {cutoffTimestamp, treatScriptAsBlocking, additionalCpuNodesToTreatAsBlocking}
   ) {
-    const {
-      definitelyNotRenderBlockingScriptUrls,
-      blockingCpuNodeIds,
-    } = this.getBlockingNodeData(
-      dependencyGraph,
-      paintTs,
-      blockingResourcesFilter,
-      extraBlockingCpuNodesToIncludeFilter
-    );
+    const {definitelyNotRenderBlockingScriptUrls, blockingCpuNodeIds} =
+      this.getRenderBlockingNodeData(dependencyGraph, {
+        cutoffTimestamp,
+        treatScriptAsBlocking,
+        additionalCpuNodesToTreatAsBlocking,
+      });
 
     return dependencyGraph.cloneWithRelationships(node => {
       if (node.type === BaseNode.TYPES.NETWORK) {
-        // Exclude all nodes that ended after paintTs (except for the main document which we always consider necessary)
-        // endTime is negative if request does not finish, make sure startTime isn't after paintTs in this case.
-        const endedAfterPaint = node.endTime > paintTs || node.startTime > paintTs;
+        // Exclude all nodes that ended after cutoffTimestamp (except for the main document which we always consider necessary)
+        // endTime is negative if request does not finish, make sure startTime isn't after cutoffTimestamp in this case.
+        const endedAfterPaint = node.endTime > cutoffTimestamp || node.startTime > cutoffTimestamp;
         if (endedAfterPaint && !node.isMainDocument()) return false;
 
         const url = node.record.url;
@@ -159,7 +153,7 @@ class LanternFirstContentfulPaint extends LanternMetric {
           return false;
         }
 
-        return blockingResourcesFilter(node);
+        return treatScriptAsBlocking(node);
       } else {
         // If it's a CPU node, just check if it was blocking.
         return blockingCpuNodeIds.has(node.id);
@@ -173,14 +167,14 @@ class LanternFirstContentfulPaint extends LanternMetric {
    * @return {Node}
    */
   static getOptimisticGraph(dependencyGraph, processedNavigation) {
-    return this.getFirstPaintBasedGraph(
-      dependencyGraph,
-      processedNavigation.timestamps.firstContentfulPaint,
+    return this.getFirstPaintBasedGraph(dependencyGraph, {
+      cutoffTimestamp: processedNavigation.timestamps.firstContentfulPaint,
       // In the optimistic graph we exclude resources that appeared to be render blocking but were
       // initiated by a script. While they typically have a very high importance and tend to have a
       // significant impact on the page's content, these resources don't technically block rendering.
-      node => node.hasRenderBlockingPriority() && node.initiatorType !== 'script'
-    );
+      treatScriptAsBlocking: node =>
+        node.hasRenderBlockingPriority() && node.initiatorType !== 'script',
+    });
   }
 
   /**
@@ -189,11 +183,10 @@ class LanternFirstContentfulPaint extends LanternMetric {
    * @return {Node}
    */
   static getPessimisticGraph(dependencyGraph, processedNavigation) {
-    return this.getFirstPaintBasedGraph(
-      dependencyGraph,
-      processedNavigation.timestamps.firstContentfulPaint,
-      node => node.hasRenderBlockingPriority()
-    );
+    return this.getFirstPaintBasedGraph(dependencyGraph, {
+      cutoffTimestamp: processedNavigation.timestamps.firstContentfulPaint,
+      treatScriptAsBlocking: node => node.hasRenderBlockingPriority(),
+    });
   }
 }
 

core/computed/metrics/lantern-first-meaningful-paint.js

@@ -33,14 +33,13 @@ class LanternFirstMeaningfulPaint extends LanternMetric {
     if (!fmp) {
       throw new LighthouseError(LighthouseError.errors.NO_FMP);
     }
-
-    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(
-      dependencyGraph,
-      fmp,
+    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(dependencyGraph, {
+      cutoffTimestamp: fmp,
       // See LanternFirstContentfulPaint's getOptimisticGraph implementation for a longer description
       // of why we exclude script initiated resources here.
-      node => node.hasRenderBlockingPriority() && node.initiatorType !== 'script'
-    );
+      treatScriptAsBlocking: node =>
+        node.hasRenderBlockingPriority() && node.initiatorType !== 'script',
+    });
   }
 
   /**
@@ -54,13 +53,12 @@ class LanternFirstMeaningfulPaint extends LanternMetric {
       throw new LighthouseError(LighthouseError.errors.NO_FMP);
     }
 
-    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(
-      dependencyGraph,
-      fmp,
-      node => node.hasRenderBlockingPriority(),
+    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(dependencyGraph, {
+      cutoffTimestamp: fmp,
+      treatScriptAsBlocking: node => node.hasRenderBlockingPriority(),
       // For pessimistic FMP we'll include *all* layout nodes
-      node => node.didPerformLayout()
-    );
+      additionalCpuNodesToTreatAsBlocking: node => node.didPerformLayout(),
+    });
   }
 
   /**

core/computed/metrics/lantern-largest-contentful-paint.js

@@ -49,11 +49,10 @@ class LanternLargestContentfulPaint extends LanternMetric {
       throw new LighthouseError(LighthouseError.errors.NO_LCP);
     }
 
-    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(
-      dependencyGraph,
-      lcp,
-      LanternLargestContentfulPaint.isNotLowPriorityImageNode
-    );
+    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(dependencyGraph, {
+      cutoffTimestamp: lcp,
+      treatScriptAsBlocking: LanternLargestContentfulPaint.isNotLowPriorityImageNode,
+    });
   }
 
   /**
@@ -67,13 +66,12 @@ class LanternLargestContentfulPaint extends LanternMetric {
       throw new LighthouseError(LighthouseError.errors.NO_LCP);
     }
 
-    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(
-      dependencyGraph,
-      lcp,
-      _ => true,
+    return LanternFirstContentfulPaint.getFirstPaintBasedGraph(dependencyGraph, {
+      cutoffTimestamp: lcp,
+      treatScriptAsBlocking: _ => true,
       // For pessimistic LCP we'll include *all* layout nodes
-      node => node.didPerformLayout()
-    );
+      additionalCpuNodesToTreatAsBlocking: node => node.didPerformLayout(),
+    });
   }
 
   /**