From 1d266272006490d4623d5145049420924e4f6656 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 28 Sep 2020 13:46:40 -0700 Subject: [PATCH 01/13] Stripped down LCP preload audit --- lighthouse-core/audits/preload-lcp.js | 185 ++++++++++++++++++ lighthouse-core/config/experimental-config.js | 7 + lighthouse-core/lib/page-functions.js | 21 +- types/artifacts.d.ts | 2 + 4 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 lighthouse-core/audits/preload-lcp.js diff --git a/lighthouse-core/audits/preload-lcp.js b/lighthouse-core/audits/preload-lcp.js new file mode 100644 index 000000000000..75d262914e2c --- /dev/null +++ b/lighthouse-core/audits/preload-lcp.js @@ -0,0 +1,185 @@ +/** + * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +const URL = require('../lib/url-shim.js'); +const Audit = require('./audit.js'); +const i18n = require('../lib/i18n/i18n.js'); +const MainResource = require('../computed/main-resource.js'); +const LanternLCP = require('../computed/metrics/lantern-largest-contentful-paint.js'); +const LoadSimulator = require('../computed/load-simulator.js'); +const UnusedBytes = require('./byte-efficiency/byte-efficiency-audit.js'); + +const UIStrings = { + /** Title of a lighthouse audit that tells a user to preload an image in order to improve their LCP time. */ + title: 'Preload Largest Contentful Paint image', + /** Description of a lighthouse audit that tells a user to preload an image in order to improve their LCP time. */ + description: 'Consider preloading the image used by ' + + 'the LCP element in order to improve your LCP time.', +}; + +const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); + +class PreloadLCPAudit extends Audit { + /** + * @return {LH.Audit.Meta} + */ + static get meta() { + return { + id: 'preload-lcp', + title: str_(UIStrings.title), + description: str_(UIStrings.description), + requiredArtifacts: ['traces', 'devtoolsLogs', 'URL', 'TraceElements'], + scoreDisplayMode: Audit.SCORING_MODES.NUMERIC, + }; + } + + /** + * + * @param {LH.Artifacts.NetworkRequest} request + * @param {LH.Artifacts.NetworkRequest} mainResource + * @param {Array} initiatorPath + * @param {string|undefined} lcpImageSource + * @return {boolean} + */ + static shouldPreloadRequest(request, mainResource, initiatorPath, lcpImageSource) { + const mainResourceDepth = mainResource.redirects ? mainResource.redirects.length : 0; + + // If it's not the request for the LCP image, don't recommend it. + if (request.url !== lcpImageSource) return false; + // If it's already preloaded, no need to recommend it. + if (request.isLinkPreload) return false; + // It's not a request loaded over the network, don't recommend it. + if (URL.NON_NETWORK_PROTOCOLS.includes(request.protocol)) return false; + // It's not at the right depth, don't recommend it. + if (initiatorPath.length < mainResourceDepth + 1) return false; + // Finally, return whether or not it belongs to the main frame + return request.frameId === mainResource.frameId; + } + + /** + * @param {LH.Artifacts.NetworkRequest} mainResource + * @param {LH.Gatherer.Simulation.GraphNode} graph + * @param {LH.Artifacts.TraceElement|undefined} lcpElement + * @return {string|null} + */ + static getURLToPreload(mainResource, graph, lcpElement) { + if (!lcpElement) { + return null; + } + + let lcpUrl = null; + graph.traverse((node, traversalPath) => { + if (node.type !== 'network') return; + const record = node.record; + const imageSource = lcpElement.imageSource; + // Don't include the node itself or any CPU nodes in the initiatorPath + const path = traversalPath.slice(1).filter(initiator => initiator.type === 'network'); + if (!PreloadLCPAudit.shouldPreloadRequest(record, mainResource, path, imageSource)) return; + lcpUrl = node.record.url; + }); + + return lcpUrl; + } + + /** + * Computes the estimated effect of preloading the LCP image. + * @param {string} lcpUrl // The image URL of the LCP + * @param {LH.Gatherer.Simulation.GraphNode} graph + * @param {LH.Gatherer.Simulation.Simulator} simulator + * @return {{wastedMs: number, results: Array<{url: string, wastedMs: number}>}} + */ + static computeWasteWithGraph(lcpUrl, graph, simulator) { + const simulation = simulator.simulate(graph, {flexibleOrdering: true}); + + /** @type {LH.Gatherer.Simulation.GraphNode|null} */ + let lcpNode = null; + /** @type {LH.Gatherer.Simulation.GraphNode|null} */ + let mainDocumentNode = null; + graph.traverse(node => { + if (node.type !== 'network') return; + + const networkNode = /** @type {LH.Gatherer.Simulation.GraphNetworkNode} */ (node); + if (node.isMainDocument()) { + mainDocumentNode = networkNode; + } else if (networkNode.record && lcpUrl === networkNode.record.url) { + lcpNode = networkNode; + } + }); + + if (!mainDocumentNode) { + // Should always find the main document node + throw new Error('Could not find main document node'); + } + + if (!lcpNode) { + // Should always find the LCP node if we've gotten to this point + throw new Error('Could not find the LCP node'); + } + + const timing = simulation.nodeTimings.get(lcpNode); + if (!timing) throw new Error('Missing LCP node'); + return { + wastedMs: timing.duration, + results: [ + {wastedMs: timing.duration, url: lcpUrl}, + ], + }; + } + + /** + * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context + * @return {Promise} + */ + static async audit(artifacts, context) { + const trace = artifacts.traces[PreloadLCPAudit.DEFAULT_PASS]; + const devtoolsLog = artifacts.devtoolsLogs[PreloadLCPAudit.DEFAULT_PASS]; + const URL = artifacts.URL; + const simulatorOptions = {trace, devtoolsLog, settings: context.settings}; + const lcpElement = artifacts.TraceElements + .find(element => element.traceEventType === 'largest-contentful-paint'); + + /** @type {LH.Config.Settings} */ + // @ts-expect-error + const settings = {}; + + const [mainResource, lanternLCP, simulator] = await Promise.all([ + MainResource.request({devtoolsLog, URL}, context), + LanternLCP.request({trace, devtoolsLog, settings}, context), + LoadSimulator.request(simulatorOptions, context), + ]); + + const graph = lanternLCP.pessimisticGraph; + const lcpUrl = PreloadLCPAudit.getURLToPreload(mainResource, graph, lcpElement); + if (!lcpUrl) { + return { + score: 1, + notApplicable: true, + }; + } + + const {results, wastedMs} = PreloadLCPAudit.computeWasteWithGraph(lcpUrl, graph, simulator); + + /** @type {LH.Audit.Details.Opportunity['headings']} */ + const headings = [ + {key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)}, + {key: 'wastedMs', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnWastedMs)}, + ]; + const details = Audit.makeOpportunityDetails(headings, results, wastedMs); + + return { + score: UnusedBytes.scoreForWastedMs(wastedMs), + numericValue: wastedMs, + numericUnit: 'millisecond', + displayValue: wastedMs ? str_(i18n.UIStrings.displayValueMsSavings, {wastedMs}) : '', + details, + }; + } +} + +module.exports = PreloadLCPAudit; +module.exports.UIStrings = UIStrings; diff --git a/lighthouse-core/config/experimental-config.js b/lighthouse-core/config/experimental-config.js index 2c7519602186..67a83de7fe50 100644 --- a/lighthouse-core/config/experimental-config.js +++ b/lighthouse-core/config/experimental-config.js @@ -16,6 +16,7 @@ const config = { audits: [ 'autocomplete', 'full-page-screenshot', + 'preload-lcp', ], passes: [{ passName: 'defaultPass', @@ -31,6 +32,12 @@ const config = { {id: 'autocomplete', weight: 0, group: 'best-practices-ux'}, ], }, + // @ts-ignore: same reason as above + 'performance': { + auditRefs: [ + {id: 'preload-lcp', weight: 0, group: 'load-opportunities'}, + ], + }, }, }; diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8a4350b2b355..94142420429b 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -390,7 +390,6 @@ function getNodeLabel(node) { /** * @param {HTMLElement} element - * @param {LH.Artifacts.Rect} */ /* istanbul ignore next */ function getBoundingClientRect(element) { @@ -406,6 +405,23 @@ function getBoundingClientRect(element) { }; } +/** + * @param {HTMLElement} element + */ +/* instanbul ignore next */ +function getImageSource(element) { + const elementType = element.tagName.toLowerCase(); + if (elementType === 'img') { + return element.getAttribute('src'); + } + + // If the element is not an , it may have an image set via css + const style = window.getComputedStyle(element); + if (!style.backgroundImage) return ''; + + return style.backgroundImage.slice(4, -1).replace(/['"]/g, ''); +} + /* * RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse * penalty for tests run with simulated throttling. Reduces the deadline time to (50 - safetyAllowance) / cpuSlowdownMultiplier to @@ -445,12 +461,15 @@ const getNodeDetailsString = `function getNodeDetails(elem) { ${getBoundingClientRect.toString()}; ${getOuterHTMLSnippet.toString()}; ${getNodeLabel.toString()}; + ${getImageSource.toString()}; return { devtoolsNodePath: getNodePath(elem), selector: getNodeSelector(elem), boundingRect: getBoundingClientRect(elem), snippet: getOuterHTMLSnippet(elem), nodeLabel: getNodeLabel(elem), + imageSource: getImageSource(elem), + elementType: elem.tagName.toLowerCase(), }; }`; diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 8cdcd2f9ff81..89a7e4a24d87 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -163,6 +163,8 @@ declare global { boundingRect: Rect | null, snippet: string, nodeLabel: string, + imageSource: string, + elementType: string, } export interface RuleExecutionError { From ea3b50df0f6a7d0324a69146c719b424e5b7dd10 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Tue, 29 Sep 2020 15:24:33 -0700 Subject: [PATCH 02/13] Apply suggestions from code review Co-authored-by: Patrick Hulce --- lighthouse-core/audits/preload-lcp.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/audits/preload-lcp.js b/lighthouse-core/audits/preload-lcp.js index 75d262914e2c..2a951746f79e 100644 --- a/lighthouse-core/audits/preload-lcp.js +++ b/lighthouse-core/audits/preload-lcp.js @@ -17,7 +17,7 @@ const UIStrings = { /** Title of a lighthouse audit that tells a user to preload an image in order to improve their LCP time. */ title: 'Preload Largest Contentful Paint image', /** Description of a lighthouse audit that tells a user to preload an image in order to improve their LCP time. */ - description: 'Consider preloading the image used by ' + + description: 'Preload the image used by ' + 'the LCP element in order to improve your LCP time.', }; @@ -29,7 +29,7 @@ class PreloadLCPAudit extends Audit { */ static get meta() { return { - id: 'preload-lcp', + id: 'preload-lcp-image', title: str_(UIStrings.title), description: str_(UIStrings.description), requiredArtifacts: ['traces', 'devtoolsLogs', 'URL', 'TraceElements'], @@ -54,8 +54,8 @@ class PreloadLCPAudit extends Audit { if (request.isLinkPreload) return false; // It's not a request loaded over the network, don't recommend it. if (URL.NON_NETWORK_PROTOCOLS.includes(request.protocol)) return false; - // It's not at the right depth, don't recommend it. - if (initiatorPath.length < mainResourceDepth + 1) return false; + // It's already discoverable from the main document, don't recommend it. + if (initiatorPath.length <= mainResourceDepth) return false; // Finally, return whether or not it belongs to the main frame return request.frameId === mainResource.frameId; } @@ -64,7 +64,7 @@ class PreloadLCPAudit extends Audit { * @param {LH.Artifacts.NetworkRequest} mainResource * @param {LH.Gatherer.Simulation.GraphNode} graph * @param {LH.Artifacts.TraceElement|undefined} lcpElement - * @return {string|null} + * @return {string|undefined} */ static getURLToPreload(mainResource, graph, lcpElement) { if (!lcpElement) { From e71ae835a25e2b868b74d5e4a8815fc2b46d292b Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Tue, 29 Sep 2020 15:27:05 -0700 Subject: [PATCH 03/13] Unit test shell --- .../test/audits/preload-lcp-test.js | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 lighthouse-core/test/audits/preload-lcp-test.js diff --git a/lighthouse-core/test/audits/preload-lcp-test.js b/lighthouse-core/test/audits/preload-lcp-test.js new file mode 100644 index 000000000000..0bd8c6da45eb --- /dev/null +++ b/lighthouse-core/test/audits/preload-lcp-test.js @@ -0,0 +1,48 @@ +/** + * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ + +'use strict'; + +/* eslint-env jest */ + +const PreloadLCP = require('../../audits/preload-lcp'); +const assert = require('assert').strict; + +const pwaTrace = require('../fixtures/traces/progressive-app-m60.json'); +const pwaDevtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json'); +const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js'); +const createTestTrace = require('../create-test-trace.js'); + +const defaultMainResourceUrl = 'https://www.example.com/'; +const defaultMainResource = { + requestId: '1', + url: defaultMainResourceUrl, + startTime: 0, + priority: 'VeryHigh', + timing: { + connectStart: 147.848, + connectEnd: 180.71, + sslStart: 151.87, + sslEnd: 180.704, + sendStart: 181.443, + sendEnd: 181.553, + receiveHeadersEnd: 500, + } +}; + +describe('Performance: preload-lcp audit', () => { + const mockArtifacts = (networkRecords, finalUrl) => { + return { + traces: {[PreloadLCP.DEFAULT_PASS]: createTestTrace({traceEnd: 5000})}, + devtoolsLogs: {[PreloadLCP.DEFAULT_PASS]: networkRecordsToDevtoolsLog(networkRecords)}, + URL: {finalUrl}, + }; + }; + + it('should suggest preloading a lcp image', () => { + + }); +}); \ No newline at end of file From 944a0c27d01e1a3d53c14e72bf041dd1fa4d6eea Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Fri, 2 Oct 2020 16:25:15 -0700 Subject: [PATCH 04/13] More unit test progress --- .../{preload-lcp.js => preload-lcp-image.js} | 37 +++--- lighthouse-core/config/experimental-config.js | 4 +- lighthouse-core/lib/page-functions.js | 19 --- .../test/audits/preload-lcp-image-test.js | 110 ++++++++++++++++++ .../test/audits/preload-lcp-test.js | 48 -------- types/artifacts.d.ts | 1 - 6 files changed, 133 insertions(+), 86 deletions(-) rename lighthouse-core/audits/{preload-lcp.js => preload-lcp-image.js} (85%) create mode 100644 lighthouse-core/test/audits/preload-lcp-image-test.js delete mode 100644 lighthouse-core/test/audits/preload-lcp-test.js diff --git a/lighthouse-core/audits/preload-lcp.js b/lighthouse-core/audits/preload-lcp-image.js similarity index 85% rename from lighthouse-core/audits/preload-lcp.js rename to lighthouse-core/audits/preload-lcp-image.js index 2a951746f79e..0cf4a1510cd2 100644 --- a/lighthouse-core/audits/preload-lcp.js +++ b/lighthouse-core/audits/preload-lcp-image.js @@ -23,7 +23,7 @@ const UIStrings = { const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); -class PreloadLCPAudit extends Audit { +class PreloadLCPImageAudit extends Audit { /** * @return {LH.Audit.Meta} */ @@ -32,7 +32,7 @@ class PreloadLCPAudit extends Audit { id: 'preload-lcp-image', title: str_(UIStrings.title), description: str_(UIStrings.description), - requiredArtifacts: ['traces', 'devtoolsLogs', 'URL', 'TraceElements'], + requiredArtifacts: ['traces', 'devtoolsLogs', 'URL', 'TraceElements', 'ImageElements'], scoreDisplayMode: Audit.SCORING_MODES.NUMERIC, }; } @@ -42,7 +42,7 @@ class PreloadLCPAudit extends Audit { * @param {LH.Artifacts.NetworkRequest} request * @param {LH.Artifacts.NetworkRequest} mainResource * @param {Array} initiatorPath - * @param {string|undefined} lcpImageSource + * @param {string} lcpImageSource * @return {boolean} */ static shouldPreloadRequest(request, mainResource, initiatorPath, lcpImageSource) { @@ -64,21 +64,26 @@ class PreloadLCPAudit extends Audit { * @param {LH.Artifacts.NetworkRequest} mainResource * @param {LH.Gatherer.Simulation.GraphNode} graph * @param {LH.Artifacts.TraceElement|undefined} lcpElement + * @param {Array} imageElements * @return {string|undefined} */ - static getURLToPreload(mainResource, graph, lcpElement) { - if (!lcpElement) { - return null; - } + static getURLToPreload(mainResource, graph, lcpElement, imageElements) { + if (!lcpElement) return undefined; + + const lcpImageElement = imageElements.find(elem => { + return elem.devtoolsNodePath === lcpElement.devtoolsNodePath; + }); + + if (!lcpImageElement) return undefined; - let lcpUrl = null; + let lcpUrl; graph.traverse((node, traversalPath) => { if (node.type !== 'network') return; const record = node.record; - const imageSource = lcpElement.imageSource; + const imageSource = lcpImageElement.src; // Don't include the node itself or any CPU nodes in the initiatorPath const path = traversalPath.slice(1).filter(initiator => initiator.type === 'network'); - if (!PreloadLCPAudit.shouldPreloadRequest(record, mainResource, path, imageSource)) return; + if (!PreloadLCPImageAudit.shouldPreloadRequest(record, mainResource, path, imageSource)) return; lcpUrl = node.record.url; }); @@ -136,8 +141,8 @@ class PreloadLCPAudit extends Audit { * @return {Promise} */ static async audit(artifacts, context) { - const trace = artifacts.traces[PreloadLCPAudit.DEFAULT_PASS]; - const devtoolsLog = artifacts.devtoolsLogs[PreloadLCPAudit.DEFAULT_PASS]; + const trace = artifacts.traces[PreloadLCPImageAudit.DEFAULT_PASS]; + const devtoolsLog = artifacts.devtoolsLogs[PreloadLCPImageAudit.DEFAULT_PASS]; const URL = artifacts.URL; const simulatorOptions = {trace, devtoolsLog, settings: context.settings}; const lcpElement = artifacts.TraceElements @@ -149,12 +154,12 @@ class PreloadLCPAudit extends Audit { const [mainResource, lanternLCP, simulator] = await Promise.all([ MainResource.request({devtoolsLog, URL}, context), - LanternLCP.request({trace, devtoolsLog, settings}, context), + LanternLCP.request(simulatorOptions, context), LoadSimulator.request(simulatorOptions, context), ]); const graph = lanternLCP.pessimisticGraph; - const lcpUrl = PreloadLCPAudit.getURLToPreload(mainResource, graph, lcpElement); + const lcpUrl = PreloadLCPImageAudit.getURLToPreload(mainResource, graph, lcpElement, artifacts.ImageElements); if (!lcpUrl) { return { score: 1, @@ -162,7 +167,7 @@ class PreloadLCPAudit extends Audit { }; } - const {results, wastedMs} = PreloadLCPAudit.computeWasteWithGraph(lcpUrl, graph, simulator); + const {results, wastedMs} = PreloadLCPImageAudit.computeWasteWithGraph(lcpUrl, graph, simulator); /** @type {LH.Audit.Details.Opportunity['headings']} */ const headings = [ @@ -181,5 +186,5 @@ class PreloadLCPAudit extends Audit { } } -module.exports = PreloadLCPAudit; +module.exports = PreloadLCPImageAudit; module.exports.UIStrings = UIStrings; diff --git a/lighthouse-core/config/experimental-config.js b/lighthouse-core/config/experimental-config.js index 840d79880ada..fe58fc81d323 100644 --- a/lighthouse-core/config/experimental-config.js +++ b/lighthouse-core/config/experimental-config.js @@ -17,7 +17,7 @@ const config = { 'autocomplete', 'full-page-screenshot', 'large-javascript-libraries', - 'preload-lcp', + 'preload-lcp-image', ], passes: [{ passName: 'defaultPass', @@ -31,7 +31,7 @@ const config = { 'performance': { auditRefs: [ {id: 'large-javascript-libraries', weight: 0, group: 'diagnostics'}, - {id: 'preload-lcp', weight: 0, group: 'load-opportunities'}, + {id: 'preload-lcp-image', weight: 0, group: 'load-opportunities'}, ], }, // @ts-ignore: `title` is required in CategoryJson. setting to the same value as the default diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 94142420429b..cb28cb95eeea 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -405,23 +405,6 @@ function getBoundingClientRect(element) { }; } -/** - * @param {HTMLElement} element - */ -/* instanbul ignore next */ -function getImageSource(element) { - const elementType = element.tagName.toLowerCase(); - if (elementType === 'img') { - return element.getAttribute('src'); - } - - // If the element is not an , it may have an image set via css - const style = window.getComputedStyle(element); - if (!style.backgroundImage) return ''; - - return style.backgroundImage.slice(4, -1).replace(/['"]/g, ''); -} - /* * RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse * penalty for tests run with simulated throttling. Reduces the deadline time to (50 - safetyAllowance) / cpuSlowdownMultiplier to @@ -461,14 +444,12 @@ const getNodeDetailsString = `function getNodeDetails(elem) { ${getBoundingClientRect.toString()}; ${getOuterHTMLSnippet.toString()}; ${getNodeLabel.toString()}; - ${getImageSource.toString()}; return { devtoolsNodePath: getNodePath(elem), selector: getNodeSelector(elem), boundingRect: getBoundingClientRect(elem), snippet: getOuterHTMLSnippet(elem), nodeLabel: getNodeLabel(elem), - imageSource: getImageSource(elem), elementType: elem.tagName.toLowerCase(), }; }`; diff --git a/lighthouse-core/test/audits/preload-lcp-image-test.js b/lighthouse-core/test/audits/preload-lcp-image-test.js new file mode 100644 index 000000000000..3c59e72a7a92 --- /dev/null +++ b/lighthouse-core/test/audits/preload-lcp-image-test.js @@ -0,0 +1,110 @@ +/** + * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ + +'use strict'; + +/* eslint-env jest */ + +const PreloadLCPImage = require('../../audits/preload-lcp-image'); + +const pwaTrace = require('../fixtures/traces/progressive-app-m60.json'); +const pwaDevtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json'); +const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js'); +const createTestTrace = require('../create-test-trace.js'); + +const defaultMainResourceUrl = 'https://www.example.com/'; +const defaultMainResource = { + requestId: '1', + url: defaultMainResourceUrl, + startTime: 0, + priority: 'VeryHigh', + timing: { + connectStart: 147.848, + connectEnd: 180.71, + sslStart: 151.87, + sslEnd: 180.704, + sendStart: 181.443, + sendEnd: 181.553, + receiveHeadersEnd: 500, + } +}; + +describe('Performance: preload-lcp audit', () => { + const mockArtifacts = (networkRecords, finalUrl, imageUrl) => { + return { + traces: {[PreloadLCPImage.DEFAULT_PASS]: createTestTrace({traceEnd: 5000})}, + devtoolsLogs: {[PreloadLCPImage.DEFAULT_PASS]: networkRecordsToDevtoolsLog(networkRecords)}, + URL: {finalUrl}, + TraceElements: [ + { + traceEventType: 'largest-contentful-paint', + devtoolsNodePath: '1,HTML,1,BODY,3,DIV,2,IMG', + }, + ], + ImageElements: [ + { + src: imageUrl, + devtoolsNodePath: '1,HTML,1,BODY,3,DIV,2,IMG', + }, + ], + }; + }; + + it('should suggest preloading a lcp image', () => { + const rootNodeUrl = 'http://example.com:3000'; + const mainDocumentNodeUrl = 'http://www.example.com:3000'; + const scriptNodeUrl = 'http://www.example.com/script.js'; + const imageUrl = 'http://www.example.com/image.png'; + const networkRecords = [ + { + requestId: '2', + priority: 'High', + isLinkPreload: false, + startTime: 0, + endTime: 0.5, + timing: {receiveHeadersEnd: 500}, + url: rootNodeUrl, + }, + { + requestId: '2:redirect', + resourceType: 'Document', + priority: 'High', + isLinkPreload: false, + startTime: 0.5, + endTime: 1, + timing: {receiveHeadersEnd: 500}, + url: mainDocumentNodeUrl, + }, + { + requestId: '3', + resourceType: 'Script', + priority: 'High', + isLinkPreload: false, + startTime: 1, + endTime: 3, + timing: {receiveHeadersEnd: 2000}, + url: scriptNodeUrl, + initiator: {type: 'parser', url: mainDocumentNodeUrl}, + }, + { + requestId: '4', + resourceType: 'Image', + priority: 'High', + isLinkPreload: false, + startTime: 2, + endTime: 3, + timing: {receiveHeadersEnd: 1000}, + url: imageUrl, + initiator: {type: 'script', url: scriptNodeUrl}, + }, + ]; + + const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl); + const context = {settings: {}, computedCache: new Map()}; + const results = PreloadLCPImage.audit(artifacts, context); + expect(results.details).toBeDefined(); + }); +}); \ No newline at end of file diff --git a/lighthouse-core/test/audits/preload-lcp-test.js b/lighthouse-core/test/audits/preload-lcp-test.js deleted file mode 100644 index 0bd8c6da45eb..000000000000 --- a/lighthouse-core/test/audits/preload-lcp-test.js +++ /dev/null @@ -1,48 +0,0 @@ -/** - * @license Copyright 2020 The Lighthouse Authors. All Rights Reserved. - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - */ - -'use strict'; - -/* eslint-env jest */ - -const PreloadLCP = require('../../audits/preload-lcp'); -const assert = require('assert').strict; - -const pwaTrace = require('../fixtures/traces/progressive-app-m60.json'); -const pwaDevtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json'); -const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js'); -const createTestTrace = require('../create-test-trace.js'); - -const defaultMainResourceUrl = 'https://www.example.com/'; -const defaultMainResource = { - requestId: '1', - url: defaultMainResourceUrl, - startTime: 0, - priority: 'VeryHigh', - timing: { - connectStart: 147.848, - connectEnd: 180.71, - sslStart: 151.87, - sslEnd: 180.704, - sendStart: 181.443, - sendEnd: 181.553, - receiveHeadersEnd: 500, - } -}; - -describe('Performance: preload-lcp audit', () => { - const mockArtifacts = (networkRecords, finalUrl) => { - return { - traces: {[PreloadLCP.DEFAULT_PASS]: createTestTrace({traceEnd: 5000})}, - devtoolsLogs: {[PreloadLCP.DEFAULT_PASS]: networkRecordsToDevtoolsLog(networkRecords)}, - URL: {finalUrl}, - }; - }; - - it('should suggest preloading a lcp image', () => { - - }); -}); \ No newline at end of file diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index b44ed4856d67..2a104dd3565b 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -163,7 +163,6 @@ declare global { boundingRect: Rect | null, snippet: string, nodeLabel: string, - imageSource: string, elementType: string, } From 5991c6064a1f7cc6997e643a1cd038ebca9d4d21 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 12 Oct 2020 18:13:09 -0700 Subject: [PATCH 05/13] WIP test --- lighthouse-core/audits/preload-lcp-image.js | 9 +++------ .../test/audits/preload-lcp-image-test.js | 18 ++---------------- 2 files changed, 5 insertions(+), 22 deletions(-) diff --git a/lighthouse-core/audits/preload-lcp-image.js b/lighthouse-core/audits/preload-lcp-image.js index 0cf4a1510cd2..a887d1d3d877 100644 --- a/lighthouse-core/audits/preload-lcp-image.js +++ b/lighthouse-core/audits/preload-lcp-image.js @@ -75,16 +75,17 @@ class PreloadLCPImageAudit extends Audit { }); if (!lcpImageElement) return undefined; - let lcpUrl; graph.traverse((node, traversalPath) => { if (node.type !== 'network') return; const record = node.record; const imageSource = lcpImageElement.src; + console.log(imageSource); + console.log(record.url); // Don't include the node itself or any CPU nodes in the initiatorPath const path = traversalPath.slice(1).filter(initiator => initiator.type === 'network'); if (!PreloadLCPImageAudit.shouldPreloadRequest(record, mainResource, path, imageSource)) return; - lcpUrl = node.record.url; + lcpUrl = record.url; }); return lcpUrl; @@ -148,10 +149,6 @@ class PreloadLCPImageAudit extends Audit { const lcpElement = artifacts.TraceElements .find(element => element.traceEventType === 'largest-contentful-paint'); - /** @type {LH.Config.Settings} */ - // @ts-expect-error - const settings = {}; - const [mainResource, lanternLCP, simulator] = await Promise.all([ MainResource.request({devtoolsLog, URL}, context), LanternLCP.request(simulatorOptions, context), diff --git a/lighthouse-core/test/audits/preload-lcp-image-test.js b/lighthouse-core/test/audits/preload-lcp-image-test.js index 3c59e72a7a92..72332276fdc9 100644 --- a/lighthouse-core/test/audits/preload-lcp-image-test.js +++ b/lighthouse-core/test/audits/preload-lcp-image-test.js @@ -16,26 +16,11 @@ const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log. const createTestTrace = require('../create-test-trace.js'); const defaultMainResourceUrl = 'https://www.example.com/'; -const defaultMainResource = { - requestId: '1', - url: defaultMainResourceUrl, - startTime: 0, - priority: 'VeryHigh', - timing: { - connectStart: 147.848, - connectEnd: 180.71, - sslStart: 151.87, - sslEnd: 180.704, - sendStart: 181.443, - sendEnd: 181.553, - receiveHeadersEnd: 500, - } -}; describe('Performance: preload-lcp audit', () => { const mockArtifacts = (networkRecords, finalUrl, imageUrl) => { return { - traces: {[PreloadLCPImage.DEFAULT_PASS]: createTestTrace({traceEnd: 5000})}, + traces: {[PreloadLCPImage.DEFAULT_PASS]: createTestTrace({traceEnd: 5000, largestContentfulPaint: 2000})}, devtoolsLogs: {[PreloadLCPImage.DEFAULT_PASS]: networkRecordsToDevtoolsLog(networkRecords)}, URL: {finalUrl}, TraceElements: [ @@ -103,6 +88,7 @@ describe('Performance: preload-lcp audit', () => { ]; const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl); + console.log(JSON.stringify(artifacts.traces[PreloadLCPImage.DEFAULT_PASS], null, 4)); const context = {settings: {}, computedCache: new Map()}; const results = PreloadLCPImage.audit(artifacts, context); expect(results.details).toBeDefined(); From 2bdc94d36e7d73a4527ee3e26560ac840be40dee Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Thu, 15 Oct 2020 09:30:47 -0700 Subject: [PATCH 06/13] Unit tests --- lighthouse-core/audits/preload-lcp-image.js | 50 ++++-------- .../test/audits/preload-lcp-image-test.js | 78 ++++++++++++++----- lighthouse-core/test/create-test-trace.js | 2 +- 3 files changed, 75 insertions(+), 55 deletions(-) diff --git a/lighthouse-core/audits/preload-lcp-image.js b/lighthouse-core/audits/preload-lcp-image.js index a887d1d3d877..59c1b44f3e26 100644 --- a/lighthouse-core/audits/preload-lcp-image.js +++ b/lighthouse-core/audits/preload-lcp-image.js @@ -80,10 +80,10 @@ class PreloadLCPImageAudit extends Audit { if (node.type !== 'network') return; const record = node.record; const imageSource = lcpImageElement.src; - console.log(imageSource); - console.log(record.url); // Don't include the node itself or any CPU nodes in the initiatorPath const path = traversalPath.slice(1).filter(initiator => initiator.type === 'network'); + + // eslint-disable-next-line max-len if (!PreloadLCPImageAudit.shouldPreloadRequest(record, mainResource, path, imageSource)) return; lcpUrl = record.url; }); @@ -100,39 +100,17 @@ class PreloadLCPImageAudit extends Audit { */ static computeWasteWithGraph(lcpUrl, graph, simulator) { const simulation = simulator.simulate(graph, {flexibleOrdering: true}); - - /** @type {LH.Gatherer.Simulation.GraphNode|null} */ - let lcpNode = null; - /** @type {LH.Gatherer.Simulation.GraphNode|null} */ - let mainDocumentNode = null; - graph.traverse(node => { - if (node.type !== 'network') return; - - const networkNode = /** @type {LH.Gatherer.Simulation.GraphNetworkNode} */ (node); - if (node.isMainDocument()) { - mainDocumentNode = networkNode; - } else if (networkNode.record && lcpUrl === networkNode.record.url) { - lcpNode = networkNode; - } - }); - - if (!mainDocumentNode) { - // Should always find the main document node - throw new Error('Could not find main document node'); - } - - if (!lcpNode) { - // Should always find the LCP node if we've gotten to this point - throw new Error('Could not find the LCP node'); - } - - const timing = simulation.nodeTimings.get(lcpNode); - if (!timing) throw new Error('Missing LCP node'); + // For now, we are simply using the duration of the LCP image request as the wastedMS value + const timings = Array.from(simulation.nodeTimings.entries()); + // @ts-ignore + const lcpTiming = timings.find(([node, _]) => node.record.url === lcpUrl); + const wastedMs = lcpTiming && lcpTiming[1].duration || 0; return { - wastedMs: timing.duration, - results: [ - {wastedMs: timing.duration, url: lcpUrl}, - ], + wastedMs, + results: [{ + url: lcpUrl, + wastedMs, + }], }; } @@ -156,6 +134,7 @@ class PreloadLCPImageAudit extends Audit { ]); const graph = lanternLCP.pessimisticGraph; + // eslint-disable-next-line max-len const lcpUrl = PreloadLCPImageAudit.getURLToPreload(mainResource, graph, lcpElement, artifacts.ImageElements); if (!lcpUrl) { return { @@ -164,7 +143,8 @@ class PreloadLCPImageAudit extends Audit { }; } - const {results, wastedMs} = PreloadLCPImageAudit.computeWasteWithGraph(lcpUrl, graph, simulator); + const {results, wastedMs} = + PreloadLCPImageAudit.computeWasteWithGraph(lcpUrl, graph, simulator); /** @type {LH.Audit.Details.Opportunity['headings']} */ const headings = [ diff --git a/lighthouse-core/test/audits/preload-lcp-image-test.js b/lighthouse-core/test/audits/preload-lcp-image-test.js index 72332276fdc9..24aad11a6f2d 100644 --- a/lighthouse-core/test/audits/preload-lcp-image-test.js +++ b/lighthouse-core/test/audits/preload-lcp-image-test.js @@ -8,19 +8,27 @@ /* eslint-env jest */ -const PreloadLCPImage = require('../../audits/preload-lcp-image'); +const PreloadLCPImage = require('../../audits/preload-lcp-image.js'); -const pwaTrace = require('../fixtures/traces/progressive-app-m60.json'); -const pwaDevtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json'); +const lcpTrace = require('../fixtures/traces/lcp-m78.json'); +const lcpDevtoolsLog = require('../fixtures/traces/lcp-m78.devtools.log.json'); const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js'); const createTestTrace = require('../create-test-trace.js'); -const defaultMainResourceUrl = 'https://www.example.com/'; +const rootNodeUrl = 'http://example.com:3000'; +const mainDocumentNodeUrl = 'http://www.example.com:3000'; +const scriptNodeUrl = 'http://www.example.com/script.js'; +const imageUrl = 'http://www.example.com/image.png'; describe('Performance: preload-lcp audit', () => { const mockArtifacts = (networkRecords, finalUrl, imageUrl) => { return { - traces: {[PreloadLCPImage.DEFAULT_PASS]: createTestTrace({traceEnd: 5000, largestContentfulPaint: 2000})}, + traces: { + [PreloadLCPImage.DEFAULT_PASS]: createTestTrace({ + traceEnd: 6e3, + largestContentfulPaint: 45e2, + }), + }, devtoolsLogs: {[PreloadLCPImage.DEFAULT_PASS]: networkRecordsToDevtoolsLog(networkRecords)}, URL: {finalUrl}, TraceElements: [ @@ -38,12 +46,8 @@ describe('Performance: preload-lcp audit', () => { }; }; - it('should suggest preloading a lcp image', () => { - const rootNodeUrl = 'http://example.com:3000'; - const mainDocumentNodeUrl = 'http://www.example.com:3000'; - const scriptNodeUrl = 'http://www.example.com/script.js'; - const imageUrl = 'http://www.example.com/image.png'; - const networkRecords = [ + const mockNetworkRecords = () => { + return [ { requestId: '2', priority: 'High', @@ -69,8 +73,8 @@ describe('Performance: preload-lcp audit', () => { priority: 'High', isLinkPreload: false, startTime: 1, - endTime: 3, - timing: {receiveHeadersEnd: 2000}, + endTime: 5, + timing: {receiveHeadersEnd: 4000}, url: scriptNodeUrl, initiator: {type: 'parser', url: mainDocumentNodeUrl}, }, @@ -80,17 +84,53 @@ describe('Performance: preload-lcp audit', () => { priority: 'High', isLinkPreload: false, startTime: 2, - endTime: 3, - timing: {receiveHeadersEnd: 1000}, + endTime: 4.5, + timing: {receiveHeadersEnd: 2500}, url: imageUrl, initiator: {type: 'script', url: scriptNodeUrl}, }, ]; + }; + + it('should suggest preloading a lcp image', async () => { + const networkRecords = mockNetworkRecords(); + const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl); + const context = {settings: {}, computedCache: new Map()}; + const results = await PreloadLCPImage.audit(artifacts, context); + expect(results.numericValue).toEqual(180); + expect(results.details.overallSavingsMs).toEqual(180); + expect(results.details.items[0].url).toEqual(imageUrl); + expect(results.details.items[0].wastedMs).toEqual(180); + }); + it('shouldn\'t be applicable if lcp image is not found', async () => { + const networkRecords = mockNetworkRecords(); const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl); - console.log(JSON.stringify(artifacts.traces[PreloadLCPImage.DEFAULT_PASS], null, 4)); + artifacts.ImageElements = []; + const context = {settings: {}, computedCache: new Map()}; + const results = await PreloadLCPImage.audit(artifacts, context); + expect(results.score).toEqual(1); + expect(results.notApplicable).toBeTruthy(); + expect(results.details).toBeUndefined(); + }); + + it('shouldn\'t be applicable if the lcp is not an image', async () => { + const artifacts = { + traces: {[PreloadLCPImage.DEFAULT_PASS]: lcpTrace}, + devtoolsLogs: {[PreloadLCPImage.DEFAULT_PASS]: lcpDevtoolsLog}, + URL: {finalUrl: 'https://www.paulirish.com/'}, + TraceElements: [ + { + traceEventType: 'largest-contentful-paint', + devtoolsNodePath: '1,HTML,1,BODY,3,DIV,2,P', + }, + ], + ImageElements: [], + }; const context = {settings: {}, computedCache: new Map()}; - const results = PreloadLCPImage.audit(artifacts, context); - expect(results.details).toBeDefined(); + const results = await PreloadLCPImage.audit(artifacts, context); + expect(results.score).toEqual(1); + expect(results.notApplicable).toBeTruthy(); + expect(results.details).toBeUndefined(); }); -}); \ No newline at end of file +}); diff --git a/lighthouse-core/test/create-test-trace.js b/lighthouse-core/test/create-test-trace.js index fcca86e803a0..400fe1a29d0c 100644 --- a/lighthouse-core/test/create-test-trace.js +++ b/lighthouse-core/test/create-test-trace.js @@ -126,7 +126,7 @@ function createTestTrace(options) { if (options.largestContentfulPaint) { traceEvents.push({ name: 'largestContentfulPaint::Candidate', - ts: options.largestContentfulPaint, + ts: options.largestContentfulPaint * 1000, pid, tid, ph: 'R', From eb79c5b8185c00e23eb91dc318817a9ac09a1b3a Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Fri, 16 Oct 2020 14:24:27 -0700 Subject: [PATCH 07/13] Apply suggestions from code review Co-authored-by: Patrick Hulce --- lighthouse-core/audits/preload-lcp-image.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/audits/preload-lcp-image.js b/lighthouse-core/audits/preload-lcp-image.js index 59c1b44f3e26..984747a3c59d 100644 --- a/lighthouse-core/audits/preload-lcp-image.js +++ b/lighthouse-core/audits/preload-lcp-image.js @@ -18,7 +18,7 @@ const UIStrings = { title: 'Preload Largest Contentful Paint image', /** Description of a lighthouse audit that tells a user to preload an image in order to improve their LCP time. */ description: 'Preload the image used by ' + - 'the LCP element in order to improve your LCP time.', + 'the LCP element in order to improve your LCP time. [Learn more](https://web.dev/optimize-lcp/#preload-important-resources).', }; const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); From 106047757607d06e408162232f9899d48daba9e2 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 19 Oct 2020 07:30:56 -0700 Subject: [PATCH 08/13] Address PR comments --- lighthouse-core/audits/preload-lcp-image.js | 69 ++++++++++++--------- lighthouse-core/lib/page-functions.js | 1 - types/artifacts.d.ts | 1 - 3 files changed, 39 insertions(+), 32 deletions(-) diff --git a/lighthouse-core/audits/preload-lcp-image.js b/lighthouse-core/audits/preload-lcp-image.js index 984747a3c59d..15e0be8eb5d6 100644 --- a/lighthouse-core/audits/preload-lcp-image.js +++ b/lighthouse-core/audits/preload-lcp-image.js @@ -42,14 +42,11 @@ class PreloadLCPImageAudit extends Audit { * @param {LH.Artifacts.NetworkRequest} request * @param {LH.Artifacts.NetworkRequest} mainResource * @param {Array} initiatorPath - * @param {string} lcpImageSource * @return {boolean} */ - static shouldPreloadRequest(request, mainResource, initiatorPath, lcpImageSource) { + static shouldPreloadRequest(request, mainResource, initiatorPath) { const mainResourceDepth = mainResource.redirects ? mainResource.redirects.length : 0; - // If it's not the request for the LCP image, don't recommend it. - if (request.url !== lcpImageSource) return false; // If it's already preloaded, no need to recommend it. if (request.isLinkPreload) return false; // It's not a request loaded over the network, don't recommend it. @@ -60,14 +57,36 @@ class PreloadLCPImageAudit extends Audit { return request.frameId === mainResource.frameId; } + /** + * @param {LH.Gatherer.Simulation.GraphNode} graph + * @param {string} imageUrl + * @return {{lcpNode: LH.Gatherer.Simulation.GraphNetworkNode|undefined, path: Array|undefined}} + */ + static findLCPNode(graph, imageUrl) { + let lcpNode; + let path; + graph.traverse((node, traversalPath) => { + if (node.type !== 'network') return; + if (node.record.url === imageUrl) { + lcpNode = node; + path = + traversalPath.slice(1).filter(initiator => initiator.type === 'network'); + } + }); + return { + lcpNode, + path, + }; + } + /** * @param {LH.Artifacts.NetworkRequest} mainResource * @param {LH.Gatherer.Simulation.GraphNode} graph * @param {LH.Artifacts.TraceElement|undefined} lcpElement * @param {Array} imageElements - * @return {string|undefined} + * @return {LH.Gatherer.Simulation.GraphNetworkNode|undefined} */ - static getURLToPreload(mainResource, graph, lcpElement, imageElements) { + static getLCPNodeToPreload(mainResource, graph, lcpElement, imageElements) { if (!lcpElement) return undefined; const lcpImageElement = imageElements.find(elem => { @@ -75,40 +94,30 @@ class PreloadLCPImageAudit extends Audit { }); if (!lcpImageElement) return undefined; - let lcpUrl; - graph.traverse((node, traversalPath) => { - if (node.type !== 'network') return; - const record = node.record; - const imageSource = lcpImageElement.src; - // Don't include the node itself or any CPU nodes in the initiatorPath - const path = traversalPath.slice(1).filter(initiator => initiator.type === 'network'); - - // eslint-disable-next-line max-len - if (!PreloadLCPImageAudit.shouldPreloadRequest(record, mainResource, path, imageSource)) return; - lcpUrl = record.url; - }); - - return lcpUrl; + const lcpUrl = lcpImageElement.src; + const {lcpNode, path} = PreloadLCPImageAudit.findLCPNode(graph, lcpUrl); + if (!lcpNode || !path) return undefined; + // eslint-disable-next-line max-len + const shouldPreload = PreloadLCPImageAudit.shouldPreloadRequest(lcpNode.record, mainResource, path); + return shouldPreload ? lcpNode : undefined; } /** * Computes the estimated effect of preloading the LCP image. - * @param {string} lcpUrl // The image URL of the LCP + * @param {LH.Gatherer.Simulation.GraphNetworkNode} lcpNode * @param {LH.Gatherer.Simulation.GraphNode} graph * @param {LH.Gatherer.Simulation.Simulator} simulator * @return {{wastedMs: number, results: Array<{url: string, wastedMs: number}>}} */ - static computeWasteWithGraph(lcpUrl, graph, simulator) { + static computeWasteWithGraph(lcpNode, graph, simulator) { const simulation = simulator.simulate(graph, {flexibleOrdering: true}); // For now, we are simply using the duration of the LCP image request as the wastedMS value - const timings = Array.from(simulation.nodeTimings.entries()); - // @ts-ignore - const lcpTiming = timings.find(([node, _]) => node.record.url === lcpUrl); - const wastedMs = lcpTiming && lcpTiming[1].duration || 0; + const lcpTiming = simulation.nodeTimings.get(lcpNode); + const wastedMs = lcpTiming && lcpTiming.duration || 0; return { wastedMs, results: [{ - url: lcpUrl, + url: lcpNode.record.url, wastedMs, }], }; @@ -135,8 +144,8 @@ class PreloadLCPImageAudit extends Audit { const graph = lanternLCP.pessimisticGraph; // eslint-disable-next-line max-len - const lcpUrl = PreloadLCPImageAudit.getURLToPreload(mainResource, graph, lcpElement, artifacts.ImageElements); - if (!lcpUrl) { + const lcpNode = PreloadLCPImageAudit.getLCPNodeToPreload(mainResource, graph, lcpElement, artifacts.ImageElements); + if (!lcpNode) { return { score: 1, notApplicable: true, @@ -144,7 +153,7 @@ class PreloadLCPImageAudit extends Audit { } const {results, wastedMs} = - PreloadLCPImageAudit.computeWasteWithGraph(lcpUrl, graph, simulator); + PreloadLCPImageAudit.computeWasteWithGraph(lcpNode, graph, simulator); /** @type {LH.Audit.Details.Opportunity['headings']} */ const headings = [ diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 3c0c5fd54458..7225856bed73 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -458,7 +458,6 @@ const getNodeDetailsString = `function getNodeDetails(elem) { boundingRect: getBoundingClientRect(elem), snippet: getOuterHTMLSnippet(elem), nodeLabel: getNodeLabel(elem), - elementType: elem.tagName.toLowerCase(), }; }`; diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 3472c86283f1..ee59a7a299b6 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -163,7 +163,6 @@ declare global { boundingRect: Rect | null, snippet: string, nodeLabel: string, - elementType: string, } export interface RuleExecutionError { From eb0cfc66baf459ea0e5acf129bdaffa948f2aca2 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 19 Oct 2020 13:12:40 -0700 Subject: [PATCH 09/13] Update/Add additional unit tests --- .../test/audits/preload-lcp-image-test.js | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lighthouse-core/test/audits/preload-lcp-image-test.js b/lighthouse-core/test/audits/preload-lcp-image-test.js index 24aad11a6f2d..cc40541482f2 100644 --- a/lighthouse-core/test/audits/preload-lcp-image-test.js +++ b/lighthouse-core/test/audits/preload-lcp-image-test.js @@ -9,9 +9,6 @@ /* eslint-env jest */ const PreloadLCPImage = require('../../audits/preload-lcp-image.js'); - -const lcpTrace = require('../fixtures/traces/lcp-m78.json'); -const lcpDevtoolsLog = require('../fixtures/traces/lcp-m78.devtools.log.json'); const networkRecordsToDevtoolsLog = require('../network-records-to-devtools-log.js'); const createTestTrace = require('../create-test-trace.js'); @@ -114,19 +111,21 @@ describe('Performance: preload-lcp audit', () => { expect(results.details).toBeUndefined(); }); - it('shouldn\'t be applicable if the lcp is not an image', async () => { - const artifacts = { - traces: {[PreloadLCPImage.DEFAULT_PASS]: lcpTrace}, - devtoolsLogs: {[PreloadLCPImage.DEFAULT_PASS]: lcpDevtoolsLog}, - URL: {finalUrl: 'https://www.paulirish.com/'}, - TraceElements: [ - { - traceEventType: 'largest-contentful-paint', - devtoolsNodePath: '1,HTML,1,BODY,3,DIV,2,P', - }, - ], - ImageElements: [], - }; + it('shouldn\'t be applicable if the lcp is already preloaded', async () => { + const networkRecords = mockNetworkRecords(); + networkRecords[3].isLinkPreload = true; + const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl); + const context = {settings: {}, computedCache: new Map()}; + const results = await PreloadLCPImage.audit(artifacts, context); + expect(results.score).toEqual(1); + expect(results.notApplicable).toBeTruthy(); + expect(results.details).toBeUndefined(); + }); + + it('shouldn\'t be applicable if the lcp request is not from over the network', async () => { + const networkRecords = mockNetworkRecords(); + networkRecords[3].protocol = 'data'; + const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl); const context = {settings: {}, computedCache: new Map()}; const results = await PreloadLCPImage.audit(artifacts, context); expect(results.score).toEqual(1); From ed5d5c20837b669ddcfb3195e2ff48f92aacca7e Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 26 Oct 2020 13:30:59 -0700 Subject: [PATCH 10/13] Quick fix --- lighthouse-core/lib/page-functions.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 7225856bed73..3f6285b3d60c 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -398,6 +398,7 @@ function getNodeLabel(node) { /** * @param {HTMLElement} element + * @return {LH.Artifacts.Rect} */ /* istanbul ignore next */ function getBoundingClientRect(element) { From 136dd718d362d5a6ab9df3079e103ced13dac866 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 26 Oct 2020 13:35:19 -0700 Subject: [PATCH 11/13] Update sample json and test-devtools --- lighthouse-core/lib/i18n/locales/en-US.json | 6 ++++++ lighthouse-core/lib/i18n/locales/en-XL.json | 6 ++++++ .../lighthouse/lighthouse-clear-data-warning-expected.txt | 3 ++- .../devtools/lighthouse/lighthouse-export-run-expected.txt | 6 +++--- .../lighthouse/lighthouse-successful-run-expected.txt | 4 +++- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/lib/i18n/locales/en-US.json b/lighthouse-core/lib/i18n/locales/en-US.json index 9a1b313eafc3..ef5aa6818ad0 100644 --- a/lighthouse-core/lib/i18n/locales/en-US.json +++ b/lighthouse-core/lib/i18n/locales/en-US.json @@ -1058,6 +1058,12 @@ "lighthouse-core/audits/preload-fonts.js | title": { "message": "Fonts with `font-display: optional` are preloaded" }, + "lighthouse-core/audits/preload-lcp-image.js | description": { + "message": "Preload the image used by the LCP element in order to improve your LCP time. [Learn more](https://web.dev/optimize-lcp/#preload-important-resources)." + }, + "lighthouse-core/audits/preload-lcp-image.js | title": { + "message": "Preload Largest Contentful Paint image" + }, "lighthouse-core/audits/redirects-http.js | description": { "message": "If you've already set up HTTPS, make sure that you redirect all HTTP traffic to HTTPS in order to enable secure web features for all your users. [Learn more](https://web.dev/redirects-http/)." }, diff --git a/lighthouse-core/lib/i18n/locales/en-XL.json b/lighthouse-core/lib/i18n/locales/en-XL.json index 6465291c837d..5b4ca31d5e78 100644 --- a/lighthouse-core/lib/i18n/locales/en-XL.json +++ b/lighthouse-core/lib/i18n/locales/en-XL.json @@ -1058,6 +1058,12 @@ "lighthouse-core/audits/preload-fonts.js | title": { "message": "F̂ón̂t́ŝ ẃît́ĥ `font-display: optional` ár̂é p̂ŕêĺôád̂éd̂" }, + "lighthouse-core/audits/preload-lcp-image.js | description": { + "message": "P̂ŕêĺôád̂ t́ĥé îḿâǵê úŝéd̂ b́ŷ t́ĥé L̂ĆP̂ él̂ém̂én̂t́ îń ôŕd̂ér̂ t́ô ím̂ṕr̂óv̂é ŷóûŕ L̂ĆP̂ t́îḿê. [Ĺêár̂ń m̂ór̂é](https://web.dev/optimize-lcp/#preload-important-resources)." + }, + "lighthouse-core/audits/preload-lcp-image.js | title": { + "message": "P̂ŕêĺôád̂ Ĺâŕĝéŝt́ Ĉón̂t́êńt̂f́ûĺ P̂áîńt̂ ím̂áĝé" + }, "lighthouse-core/audits/redirects-http.js | description": { "message": "Îf́ ŷóû'v́ê ál̂ŕêád̂ý ŝét̂ úp̂ H́T̂T́P̂Ś, m̂ák̂é ŝúr̂é t̂h́ât́ ŷóû ŕêd́îŕêćt̂ ál̂ĺ ĤT́T̂Ṕ t̂ŕâf́f̂íĉ t́ô H́T̂T́P̂Ś îń ôŕd̂ér̂ t́ô én̂áb̂ĺê śêćûŕê ẃêb́ f̂éât́ûŕêś f̂ór̂ ál̂ĺ ŷóûŕ ûśêŕŝ. [Ĺêár̂ń m̂ór̂é](https://web.dev/redirects-http/)." }, diff --git a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt index 9104b3dbb12c..0a2ff82f5b82 100644 --- a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt +++ b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt @@ -11,5 +11,6 @@ Tests that Lighthouse panel displays a warning when important data may affect pe [x] Clear storage [x] Simulated throttling Generate report: enabled visible -Warning Text: There may be stored data affecting loading performance in these locations: Web SQL, IndexedDB. Audit this page in an incognito window to prevent those resources from affecting your scores. +PROMISE FAILURE: TypeError: Lighthouse.LighthousePanel.getEvents is not a function + at http://127.0.0.1:8000/devtools/lighthouse/lighthouse-clear-data-warning.js:26:45 diff --git a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt index 30b8393bfba4..57304d5d24dc 100644 --- a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt +++ b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt @@ -2,11 +2,11 @@ Tests that exporting works. ++++++++ testExportHtml -# of .lh-audit divs (original): 135 +# of .lh-audit divs (original): 136 -# of .lh-audit divs (exported html): 135 +# of .lh-audit divs (exported html): 136 ++++++++ testExportJson -# of audits (json): 151 +# of audits (json): 152 diff --git a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt index 1206b14b6bd9..ab129df9602b 100644 --- a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt +++ b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt @@ -329,6 +329,7 @@ Auditing: Properly defines charset Auditing: Avoids an excessive DOM size Auditing: Links to cross-origin destinations are safe Auditing: Avoids requesting the geolocation permission on page load +Auditing: No issues in the `Issues` panel in Chrome Devtools Auditing: Avoids `document.write()` Auditing: Avoids front-end JavaScript libraries with known security vulnerabilities Auditing: Detected JavaScript libraries @@ -457,6 +458,7 @@ form-field-multiple-labels: notApplicable frame-title: notApplicable geolocation-on-start: pass gpt-bids-parallel: notApplicable +has-inspector-issues: pass heading-order: notApplicable hreflang: pass html-has-lang: fail @@ -564,5 +566,5 @@ visual-order-follows-dom: manual without-javascript: pass works-offline: fail -# of .lh-audit divs: 155 +# of .lh-audit divs: 156 From 9a6518232f923c57a5707fa5f4b302c8416c4858 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 26 Oct 2020 14:24:06 -0700 Subject: [PATCH 12/13] Fix rel-preconnect unit test --- lighthouse-core/test/audits/uses-rel-preconnect-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/audits/uses-rel-preconnect-test.js b/lighthouse-core/test/audits/uses-rel-preconnect-test.js index f8cf02793c33..c5154a967c26 100644 --- a/lighthouse-core/test/audits/uses-rel-preconnect-test.js +++ b/lighthouse-core/test/audits/uses-rel-preconnect-test.js @@ -22,7 +22,7 @@ const mainResource = { function buildArtifacts(networkRecords) { const trace = createTestTrace({ timeOrigin: 0, - largestContentfulPaint: 5000e3, + largestContentfulPaint: 5000, topLevelTasks: [{ts: 1000, duration: 50}], }); const devtoolsLog = networkRecordsToDevtoolsLog(networkRecords); From 47f237f4ad85b8fa621c08ef4f8a80d988d5d970 Mon Sep 17 00:00:00 2001 From: Michael Blasingame Date: Mon, 26 Oct 2020 17:49:31 -0700 Subject: [PATCH 13/13] Fix dev tools test --- .../lighthouse/lighthouse-clear-data-warning-expected.txt | 3 +-- .../devtools/lighthouse/lighthouse-export-run-expected.txt | 6 +++--- .../lighthouse/lighthouse-successful-run-expected.txt | 4 +--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt index 0a2ff82f5b82..9104b3dbb12c 100644 --- a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt +++ b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-clear-data-warning-expected.txt @@ -11,6 +11,5 @@ Tests that Lighthouse panel displays a warning when important data may affect pe [x] Clear storage [x] Simulated throttling Generate report: enabled visible -PROMISE FAILURE: TypeError: Lighthouse.LighthousePanel.getEvents is not a function - at http://127.0.0.1:8000/devtools/lighthouse/lighthouse-clear-data-warning.js:26:45 +Warning Text: There may be stored data affecting loading performance in these locations: Web SQL, IndexedDB. Audit this page in an incognito window to prevent those resources from affecting your scores. diff --git a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt index 57304d5d24dc..30b8393bfba4 100644 --- a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt +++ b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt @@ -2,11 +2,11 @@ Tests that exporting works. ++++++++ testExportHtml -# of .lh-audit divs (original): 136 +# of .lh-audit divs (original): 135 -# of .lh-audit divs (exported html): 136 +# of .lh-audit divs (exported html): 135 ++++++++ testExportJson -# of audits (json): 152 +# of audits (json): 151 diff --git a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt index ab129df9602b..1206b14b6bd9 100644 --- a/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt +++ b/third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt @@ -329,7 +329,6 @@ Auditing: Properly defines charset Auditing: Avoids an excessive DOM size Auditing: Links to cross-origin destinations are safe Auditing: Avoids requesting the geolocation permission on page load -Auditing: No issues in the `Issues` panel in Chrome Devtools Auditing: Avoids `document.write()` Auditing: Avoids front-end JavaScript libraries with known security vulnerabilities Auditing: Detected JavaScript libraries @@ -458,7 +457,6 @@ form-field-multiple-labels: notApplicable frame-title: notApplicable geolocation-on-start: pass gpt-bids-parallel: notApplicable -has-inspector-issues: pass heading-order: notApplicable hreflang: pass html-has-lang: fail @@ -566,5 +564,5 @@ visual-order-follows-dom: manual without-javascript: pass works-offline: fail -# of .lh-audit divs: 156 +# of .lh-audit divs: 155