diff --git a/core/audits/uses-rel-preconnect.js b/core/audits/uses-rel-preconnect.js index 2301a8d14661..dabd1898b0e3 100644 --- a/core/audits/uses-rel-preconnect.js +++ b/core/audits/uses-rel-preconnect.js @@ -14,6 +14,7 @@ import {LoadSimulator} from '../computed/load-simulator.js'; import {ProcessedNavigation} from '../computed/processed-navigation.js'; import {PageDependencyGraph} from '../computed/page-dependency-graph.js'; import {LanternLargestContentfulPaint} from '../computed/metrics/lantern-largest-contentful-paint.js'; +import {LanternFirstContentfulPaint} from '../computed/metrics/lantern-first-contentful-paint.js'; // Preconnect establishes a "clean" socket. Chrome's socket manager will keep an unused socket // around for 10s. Meaning, the time delta between processing preconnect a request should be <10s, @@ -125,7 +126,8 @@ class UsesRelPreconnectAudit extends Audit { const devtoolsLog = artifacts.devtoolsLogs[UsesRelPreconnectAudit.DEFAULT_PASS]; const settings = context.settings; - let maxWasted = 0; + let maxWastedLcp = 0; + let maxWastedFcp = 0; /** @type {Array} */ const warnings = []; @@ -144,7 +146,14 @@ class UsesRelPreconnectAudit extends Audit { /** @type {Set} */ const lcpGraphURLs = new Set(); lcpGraph.traverse(node => { - if (node.type === 'network' ) lcpGraphURLs.add(node.record.url); + if (node.type === 'network') lcpGraphURLs.add(node.record.url); + }); + + const fcpGraph = + await LanternFirstContentfulPaint.getPessimisticGraph(pageGraph, processedNavigation); + const fcpGraphURLs = new Set(); + fcpGraph.traverse(node => { + if (node.type === 'network') fcpGraphURLs.add(node.record.url); }); /** @type {Map} */ @@ -216,7 +225,11 @@ class UsesRelPreconnectAudit extends Audit { return; } - maxWasted = Math.max(wastedMs, maxWasted); + maxWastedLcp = Math.max(wastedMs, maxWastedLcp); + + if (fcpGraphURLs.has(firstRecordOfOrigin.url)) { + maxWastedFcp = Math.max(wastedMs, maxWastedLcp); + } results.push({ url: securityOrigin, wastedMs: wastedMs, @@ -240,6 +253,7 @@ class UsesRelPreconnectAudit extends Audit { score: 1, warnings: preconnectLinks.length >= 3 ? [...warnings, str_(UIStrings.tooManyPreconnectLinksWarning)] : warnings, + metricSavings: {LCP: maxWastedLcp, FCP: maxWastedFcp}, }; } @@ -250,17 +264,18 @@ class UsesRelPreconnectAudit extends Audit { ]; const details = Audit.makeOpportunityDetails(headings, results, - {overallSavingsMs: maxWasted, sortedBy: ['wastedMs']}); + {overallSavingsMs: maxWastedLcp, sortedBy: ['wastedMs']}); return { - score: ByteEfficiencyAudit.scoreForWastedMs(maxWasted), - numericValue: maxWasted, + score: ByteEfficiencyAudit.scoreForWastedMs(maxWastedLcp), + numericValue: maxWastedLcp, numericUnit: 'millisecond', - displayValue: maxWasted ? - str_(i18n.UIStrings.displayValueMsSavings, {wastedMs: maxWasted}) : + displayValue: maxWastedLcp ? + str_(i18n.UIStrings.displayValueMsSavings, {wastedMs: maxWastedLcp}) : '', warnings, details, + metricSavings: {LCP: maxWastedLcp, FCP: maxWastedFcp}, }; } } diff --git a/core/test/audits/uses-rel-preconnect-test.js b/core/test/audits/uses-rel-preconnect-test.js index 54fd39c8de74..00475a55eba1 100644 --- a/core/test/audits/uses-rel-preconnect-test.js +++ b/core/test/audits/uses-rel-preconnect-test.js @@ -14,12 +14,14 @@ const mainResource = { url: 'https://www.example.com/', timing: {receiveHeadersEnd: 0.5}, networkEndTime: 1000, + priority: 'High', }; -function buildArtifacts(networkRecords) { +function buildArtifacts(networkRecords, fcpTs) { const trace = createTestTrace({ timeOrigin: 0, largestContentfulPaint: 5000, + firstContentfulPaint: fcpTs ? fcpTs : 5000, topLevelTasks: [{ts: 1000, duration: 50}], }); const devtoolsLog = networkRecordsToDevtoolsLog(networkRecords); @@ -177,6 +179,7 @@ describe('Performance: uses-rel-preconnect audit', () => { connectEnd: 300, receiveHeadersEnd: 2.3, }, + priority: 'High', }, { url: 'https://cdn.example.com/second', @@ -188,17 +191,20 @@ describe('Performance: uses-rel-preconnect audit', () => { connectEnd: 400, receiveHeadersEnd: 3.4, }, + priority: 'High', }, ]; const artifacts = buildArtifacts(networkRecords); const context = {settings: {}, computedCache: new Map()}; - const {numericValue, details} = await UsesRelPreconnect.audit(artifacts, context); + const {numericValue, details, metricSavings} = + await UsesRelPreconnect.audit(artifacts, context); assert.equal(numericValue, 300); assert.equal(details.items.length, 1); assert.deepStrictEqual(details.items, [ {url: 'https://cdn.example.com', wastedMs: 300}, ]); + assert.deepStrictEqual(metricSavings, {LCP: 300, FCP: 300}); }); it(`should give a list of important preconnected origins`, async () => { @@ -214,6 +220,7 @@ describe('Performance: uses-rel-preconnect audit', () => { connectEnd: 300, receiveHeadersEnd: 2.3, }, + priority: 'High', }, { url: 'https://othercdn.example.com/second', @@ -225,6 +232,7 @@ describe('Performance: uses-rel-preconnect audit', () => { connectEnd: 600, receiveHeadersEnd: 1.8, }, + priority: 'High', }, { url: 'https://unimportant.example.com/second', @@ -237,6 +245,7 @@ describe('Performance: uses-rel-preconnect audit', () => { connectEnd: 600, receiveHeadersEnd: 1.8, }, + priority: 'High', }, ]; @@ -246,6 +255,7 @@ describe('Performance: uses-rel-preconnect audit', () => { numericValue, details, warnings, + metricSavings, } = await UsesRelPreconnect.audit(artifacts, context); assert.equal(numericValue, 300); assert.equal(details.items.length, 2); @@ -253,6 +263,7 @@ describe('Performance: uses-rel-preconnect audit', () => { {url: 'https://othercdn.example.com', wastedMs: 300}, {url: 'http://cdn.example.com', wastedMs: 150}, ]); + assert.deepStrictEqual(metricSavings, {LCP: 300, FCP: 300}); assert.equal(warnings.length, 0); }); @@ -343,4 +354,33 @@ describe('Performance: uses-rel-preconnect audit', () => { assert.equal(result.score, 1); assert.equal(result.warnings.length, 1); }); + + it('should have LCP savings and not FCP savings', async () => { + const networkRecords = [ + mainResource, + { + url: 'https://cdn.example.com/second', + initiator: {}, + networkRequestTime: 4000, // starts *after* FCP + networkEndTime: 4500, // ends *before* LCP + timing: { + dnsStart: 100, + connectStart: 200, + connectEnd: 300, + }, + priority: 'High', + }, + ]; + + const artifacts = buildArtifacts(networkRecords, 4000); + const context = {settings: {}, computedCache: new Map()}; + const {numericValue, details, metricSavings} = + await UsesRelPreconnect.audit(artifacts, context); + assert.equal(numericValue, 300); + assert.equal(details.items.length, 1); + assert.deepStrictEqual(details.items, [ + {url: 'https://cdn.example.com', wastedMs: 300}, + ]); + assert.deepStrictEqual(metricSavings, {LCP: 300, FCP: 0}); + }); }); diff --git a/core/test/create-test-trace.js b/core/test/create-test-trace.js index 57d06a5a964e..ef90fabe8e06 100644 --- a/core/test/create-test-trace.js +++ b/core/test/create-test-trace.js @@ -20,6 +20,7 @@ const lcpImageUrl = 'http://www.example.com/image.png'; * @property {string} [frameUrl] * @property {number} [timeOrigin] * @property {number} [largestContentfulPaint] + * @property {number} [firstContentfulPaint] * @property {number} [traceEnd] * @property {Array} [topLevelTasks] * @property {Array} [childFrames] Add a child frame with a known `frame` id for easy insertion of child frame events. @@ -140,7 +141,7 @@ function createTestTrace(options) { args: {frame: rootFrame}, }, { name: 'firstContentfulPaint', - ts: timeOrigin + 10, + ts: options.firstContentfulPaint ? options.firstContentfulPaint * 1000 : timeOrigin + 10, pid, tid, ph: 'R',