diff --git a/core/audits/byte-efficiency/render-blocking-resources.js b/core/audits/byte-efficiency/render-blocking-resources.js index 2b350165944c..a4f14f674c3a 100644 --- a/core/audits/byte-efficiency/render-blocking-resources.js +++ b/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,10 +181,10 @@ 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, @@ -190,7 +192,10 @@ class RenderBlockingResources extends Audit { 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} */ 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}, }; } } diff --git a/core/test/audits/byte-efficiency/render-blocking-resources-test.js b/core/test/audits/byte-efficiency/render-blocking-resources-test.js index 54fac15377f6..cd63fa5d9f83 100644 --- a/core/test/audits/byte-efficiency/render-blocking-resources-test.js +++ b/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', () => {