core(render-blocking-resources): reduce metric savings if LCP is an i…

…mage (#15694)

core/audits/byte-efficiency/render-blocking-resources.js

@@ -17,6 +17,8 @@ import {NetworkRequest} from '../../lib/network-request.js';
 import {ProcessedNavigation} from '../../computed/processed-navigation.js';
 import {LoadSimulator} from '../../computed/load-simulator.js';
 import {FirstContentfulPaint} from '../../computed/metrics/first-contentful-paint.js';
+import {LCPImageRecord} from '../../computed/lcp-image-record.js';
+
 
 /** @typedef {import('../../lib/dependency-graph/simulator/simulator').Simulator} Simulator */
 /** @typedef {import('../../lib/dependency-graph/base-node.js').Node} Node */
@@ -125,7 +127,7 @@ class RenderBlockingResources extends Audit {
   /**
    * @param {LH.Artifacts} artifacts
    * @param {LH.Audit.Context} context
-   * @return {Promise<{wastedMs: number, results: Array<{url: string, totalBytes: number, wastedMs: number}>}>}
+   * @return {Promise<{fcpWastedMs: number, lcpWastedMs: number, results: Array<{url: string, totalBytes: number, wastedMs: number}>}>}
    */
   static async computeResults(artifacts, context) {
     const gatherContext = artifacts.GatherContext;
@@ -179,18 +181,21 @@ class RenderBlockingResources extends Audit {
     }
 
     if (!results.length) {
-      return {results, wastedMs: 0};
+      return {results, fcpWastedMs: 0, lcpWastedMs: 0};
     }
 
-    const wastedMs = RenderBlockingResources.estimateSavingsWithGraphs(
+    const fcpWastedMs = RenderBlockingResources.estimateSavingsWithGraphs(
       simulator,
       fcpSimulation.optimisticGraph,
       deferredNodeIds,
       wastedCssBytes,
       artifacts.Stacks
     );
 
-    return {results, wastedMs};
+    const lcpRecord = await LCPImageRecord.request(metricComputationData, context);
+
+    // In most cases if the LCP is an image, render blocking resources don't affect LCP. For these cases we should reduce its impact.
+    return {results, fcpWastedMs, lcpWastedMs: lcpRecord ? 0 : fcpWastedMs};
   }
 
   /**
@@ -276,11 +281,12 @@ class RenderBlockingResources extends Audit {
    * @return {Promise<LH.Audit.Product>}
    */
   static async audit(artifacts, context) {
-    const {results, wastedMs} = await RenderBlockingResources.computeResults(artifacts, context);
+    const {results, fcpWastedMs, lcpWastedMs} =
+      await RenderBlockingResources.computeResults(artifacts, context);
 
     let displayValue;
     if (results.length > 0) {
-      displayValue = str_(i18n.UIStrings.displayValueMsSavings, {wastedMs});
+      displayValue = str_(i18n.UIStrings.displayValueMsSavings, {wastedMs: fcpWastedMs});
     }
 
     /** @type {LH.Audit.Details.Opportunity['headings']} */
@@ -291,15 +297,15 @@ class RenderBlockingResources extends Audit {
     ];
 
     const details = Audit.makeOpportunityDetails(headings, results,
-      {overallSavingsMs: wastedMs});
+      {overallSavingsMs: fcpWastedMs});
 
     return {
       displayValue,
       score: results.length ? 0 : 1,
-      numericValue: wastedMs,
+      numericValue: fcpWastedMs,
       numericUnit: 'millisecond',
       details,
-      metricSavings: {FCP: wastedMs, LCP: wastedMs},
+      metricSavings: {FCP: fcpWastedMs, LCP: lcpWastedMs},
     };
   }
 }

core/test/audits/byte-efficiency/render-blocking-resources-test.js

@@ -18,6 +18,8 @@ const trace = readJson('../../fixtures/traces/progressive-app-m60.json', import.
 const devtoolsLog = readJson('../../fixtures/traces/progressive-app-m60.devtools.log.json', import.meta);
 const ampTrace = readJson('../../fixtures/traces/amp-m86.trace.json', import.meta);
 const ampDevtoolsLog = readJson('../../fixtures/traces/amp-m86.devtoolslog.json', import.meta);
+const textLcpTrace = readJson('../../fixtures/traces/frame-metrics-m90.json', import.meta);
+const textLcpDevtoolsLog = readJson('../../fixtures/traces/frame-metrics-m90.devtools.log.json', import.meta);
 
 const mobileSlow4G = constants.throttling.mobileSlow4G;
 
@@ -44,6 +46,29 @@ describe('Render blocking resources audit', () => {
     assert.deepStrictEqual(result.metricSavings, {FCP: 0, LCP: 0});
   });
 
+  it('evaluates correct wastedMs when LCP is text', async () => {
+    const artifacts = {
+      URL: getURLArtifactFromDevtoolsLog(textLcpDevtoolsLog),
+      GatherContext: {gatherMode: 'navigation'},
+      traces: {defaultPass: textLcpTrace},
+      devtoolsLogs: {defaultPass: textLcpDevtoolsLog},
+      TagsBlockingFirstPaint: [
+        {
+          tag: {url: 'http://localhost:10200/perf/frame-metrics-inner.html'},
+        },
+        {
+          tag: {url: 'http://localhost:10200/favicon.ico'},
+        },
+      ],
+      Stacks: [],
+    };
+
+    const settings = {throttlingMethod: 'simulate', throttling: mobileSlow4G};
+    const computedCache = new Map();
+    const result = await RenderBlockingResourcesAudit.audit(artifacts, {settings, computedCache});
+    assert.deepStrictEqual(result.metricSavings, {FCP: 783, LCP: 783});
+  });
+
   it('evaluates amp page correctly', async () => {
     const artifacts = {
       URL: getURLArtifactFromDevtoolsLog(ampDevtoolsLog),
@@ -88,7 +113,7 @@ describe('Render blocking resources audit', () => {
       // it look like Montserrat starts after Fira Sans finishes. It would be preferred
       // if eventual simulation improvements list Montserrat here as well.
     ]);
-    expect(result.metricSavings).toEqual({FCP: 469, LCP: 469});
+    expect(result.metricSavings).toEqual({FCP: 469, LCP: 0});
   });
 
   describe('#estimateSavingsWithGraphs', () => {