From 69b34b68d3664e61572dbd8708c346d163ccd808 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Mon, 24 Oct 2022 15:18:28 -0700 Subject: [PATCH 01/20] WIP: bfcache audit --- core/audits/bf-cache.js | 116 ++++++++++++++++++ core/config/default-config.js | 8 +- core/config/filters.js | 2 +- core/gather/driver.js | 2 +- core/gather/driver/target-manager.js | 17 ++- core/gather/gatherers/bf-cache-errors.js | 57 +++++++++ core/scripts/i18n/collect-strings.js | 2 + .../reports/sample-flow-result.json | 70 ++++++++++- core/test/results/sample_v2.json | 33 +++++ shared/localization/locales/en-US.json | 18 +++ shared/localization/locales/en-XL.json | 18 +++ types/artifacts.d.ts | 5 + 12 files changed, 338 insertions(+), 10 deletions(-) create mode 100644 core/audits/bf-cache.js create mode 100644 core/gather/gatherers/bf-cache-errors.js diff --git a/core/audits/bf-cache.js b/core/audits/bf-cache.js new file mode 100644 index 000000000000..96e4dfcd13b1 --- /dev/null +++ b/core/audits/bf-cache.js @@ -0,0 +1,116 @@ +/** + * @license Copyright 2022 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. + */ + +import {Audit} from './audit.js'; +import * as i18n from '../lib/i18n/i18n.js'; +import {NotRestoredReasonDescription} from '../lib/bfcache-strings.js'; + +/* eslint-disable max-len */ +const UIStrings = { + /** TODO */ + title: 'Back/forward cache is used', + /** TODO */ + failureTitle: 'Back/forward cache is not used', + /** TODO */ + description: 'The back/forward cache can speed up the page load after navigating away.', + /** TODO */ + failureColumn: 'Failure reason', + /** + * @description [ICU Syntax] Label for an audit identifying the number of back/forward cache failure reasons found in the page. + */ + displayValue: `{itemCount, plural, + =1 {1 failure reason} + other {# failure reasons} + }`, + /** + * @description Error message describing a DevTools error id that was found and has not been identified by this audit. + * @example {platform-not-supported-on-android} reason + */ + unknownReason: `Back/forward cache failure reason '{reason}' is not recognized`, +}; +/* eslint-enable max-len */ + +const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings); + +class BFCache extends Audit { + /** + * @return {LH.Audit.Meta} + */ + static get meta() { + return { + id: 'bf-cache', + title: str_(UIStrings.title), + failureTitle: str_(UIStrings.failureTitle), + description: str_(UIStrings.description), + supportedModes: ['navigation'], + requiredArtifacts: ['BFCacheErrors'], + }; + } + + /** + * @param {LH.Artifacts} artifacts + * @return {Array} + */ + static getBfCacheErrors(artifacts) { + const bfCacheErrors = artifacts.BFCacheErrors.errors; + const i18nErrors = []; + + for (const err of bfCacheErrors) { + // Only show errors which can be addressed by the user. + if (err.type !== 'PageSupportNeeded') continue; + + + const matchingString = NotRestoredReasonDescription[err.reason]; + + // Handle an errorId we don't recognize. + if (matchingString === undefined) { + i18nErrors.push(str_(UIStrings.unknownReason, {reason: err.reason})); + continue; + } + + i18nErrors.push(matchingString.name); + } + + return i18nErrors; + } + + /** + * @param {LH.Artifacts} artifacts + * @return {Promise} + * + */ + static async audit(artifacts) { + const i18nErrors = BFCache.getBfCacheErrors(artifacts); + + /** @type {LH.Audit.Details.Table['headings']} */ + const headings = [ + {key: 'reason', valueType: 'text', label: str_(UIStrings.failureColumn)}, + ]; + + /** @type {LH.Audit.Details.Table['items']} */ + const errorReasons = i18nErrors.map(reason => { + return {reason}; + }); + + const details = Audit.makeTableDetails(headings, errorReasons); + + if (errorReasons.length === 0) { + return { + score: 1, + details, + }; + } + + return { + score: 0, + displayValue: str_(UIStrings.displayValue, {itemCount: errorReasons.length}), + details, + }; + } +} + +export default BFCache; +export {UIStrings}; diff --git a/core/config/default-config.js b/core/config/default-config.js index f460e005f2ec..3d18007b4c2f 100644 --- a/core/config/default-config.js +++ b/core/config/default-config.js @@ -128,6 +128,7 @@ const artifacts = { Trace: '', Accessibility: '', AnchorElements: '', + BFCacheErrors: '', CacheContents: '', ConsoleMessages: '', CSSUsage: '', @@ -216,8 +217,11 @@ const defaultConfig = { {id: artifacts.devtoolsLogs, gatherer: 'devtools-log-compat'}, {id: artifacts.traces, gatherer: 'trace-compat'}, - // FullPageScreenshot comes at the very end so all other node analysis is captured. + // FullPageScreenshot comes at the end so all other node analysis is captured. {id: artifacts.FullPageScreenshot, gatherer: 'full-page-screenshot'}, + + // BFCacheErrors comes at the very end because it can perform a page navigation. + {id: artifacts.BFCacheErrors, gatherer: 'bf-cache-errors'}, ], audits: [ 'is-on-https', @@ -373,6 +377,7 @@ const defaultConfig = { 'seo/canonical', 'seo/manual/structured-data', 'work-during-interaction', + 'bf-cache', ], groups: { 'metrics': { @@ -515,6 +520,7 @@ const defaultConfig = { {id: 'no-unload-listeners', weight: 0}, {id: 'uses-responsive-images-snapshot', weight: 0}, {id: 'work-during-interaction', weight: 0}, + {id: 'bf-cache', weight: 0}, // Budget audits. {id: 'performance-budget', weight: 0, group: 'budgets'}, diff --git a/core/config/filters.js b/core/config/filters.js index 3fe40e319241..f7dfdb86b65e 100644 --- a/core/config/filters.js +++ b/core/config/filters.js @@ -32,7 +32,7 @@ const filterResistantAuditIds = ['full-page-screenshot']; // Some artifacts are used by the report for additional information. // Always run these artifacts even if audits do not request them. // These are similar to base artifacts but they cannot be run in all 3 modes. -const filterResistantArtifactIds = ['Stacks', 'NetworkUserAgent']; +const filterResistantArtifactIds = ['Stacks', 'NetworkUserAgent', 'BFCacheErrors']; /** * Returns the set of audit IDs used in the list of categories. diff --git a/core/gather/driver.js b/core/gather/driver.js index 25b8e9d0af21..cb0561aeed77 100644 --- a/core/gather/driver.js +++ b/core/gather/driver.js @@ -84,7 +84,7 @@ class Driver { /** @return {Promise} */ async disconnect() { if (this.defaultSession === throwingSession) return; - this._targetManager?.disable(); + await this._targetManager?.disable(); await this.defaultSession.dispose(); } } diff --git a/core/gather/driver/target-manager.js b/core/gather/driver/target-manager.js index 6ad2b72b8070..28945b8202a4 100644 --- a/core/gather/driver/target-manager.js +++ b/core/gather/driver/target-manager.js @@ -58,15 +58,22 @@ class TargetManager extends ProtocolEventEmitter { async _onFrameNavigated(frameNavigatedEvent) { // Child frames are handled in `_onSessionAttached`. if (frameNavigatedEvent.frame.parentId) return; + if (!this._enabled) return; // It's not entirely clear when this is necessary, but when the page switches processes on // navigating from about:blank to the `requestedUrl`, resetting `setAutoAttach` has been // necessary in the past. - await this._rootCdpSession.send('Target.setAutoAttach', { - autoAttach: true, - flatten: true, - waitForDebuggerOnStart: true, - }); + try { + await this._rootCdpSession.send('Target.setAutoAttach', { + autoAttach: true, + flatten: true, + waitForDebuggerOnStart: true, + }); + } catch (err) { + // The page can be closed at the end of the run before this CDP function returns. + // In these cases, just ignore the error since we won't need the page anyway. + if (this._enabled) throw err; + } } /** diff --git a/core/gather/gatherers/bf-cache-errors.js b/core/gather/gatherers/bf-cache-errors.js new file mode 100644 index 000000000000..76f8f72b7628 --- /dev/null +++ b/core/gather/gatherers/bf-cache-errors.js @@ -0,0 +1,57 @@ +/** + * @license Copyright 2022 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. + */ + +import FRGatherer from '../base-gatherer.js'; +import {waitForFrameNavigated, waitForLoadEvent} from '../driver/wait-for-condition.js'; + +class BFCacheErrors extends FRGatherer { + /** @type {LH.Gatherer.GathererMeta} */ + meta = { + supportedModes: ['navigation'], + }; + + /** + * @param {LH.Gatherer.FRTransitionalContext} context + * @return {Promise} All visible tap targets with their positions and sizes + */ + async getArtifact(context) { + const session = context.driver.defaultSession; + + /** + * @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} + */ + let errors = []; + + /** + * @param {LH.Crdp.Page.BackForwardCacheNotUsedEvent} event + */ + function onBfCacheNotUsed(event) { + errors = event.notRestoredExplanations; + } + + session.on('Page.backForwardCacheNotUsed', onBfCacheNotUsed); + + const history = await session.sendCommand('Page.getNavigationHistory'); + const entry = history.entries[history.currentIndex]; + + await Promise.all([ + session.sendCommand('Page.navigate', {url: 'chrome://terms'}), + waitForLoadEvent(session, 0).promise, + ]); + + await Promise.all([ + session.sendCommand('Page.navigateToHistoryEntry', {entryId: entry.id}), + waitForFrameNavigated(session).promise, + ]); + + session.off('Page.backForwardCacheNotUsed', onBfCacheNotUsed); + + return {errors}; + } +} + +export default BFCacheErrors; + diff --git a/core/scripts/i18n/collect-strings.js b/core/scripts/i18n/collect-strings.js index d5a4e7162b80..20eec9bed592 100644 --- a/core/scripts/i18n/collect-strings.js +++ b/core/scripts/i18n/collect-strings.js @@ -718,6 +718,8 @@ function checkKnownFixedCollisions(strings) { 'Document has a valid $MARKDOWN_SNIPPET_0$', 'Failing Elements', 'Failing Elements', + 'Failure reason', + 'Failure reason', 'Name', 'Name', 'Pages that use portals are not currently eligible for back/forward cache.', diff --git a/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json b/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json index 2b524d8d5d2f..0b47f3b6322c 100644 --- a/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json +++ b/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json @@ -3675,6 +3675,14 @@ "description": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more about Structured Data](https://web.dev/structured-data/).", "score": null, "scoreDisplayMode": "manual" + }, + "bf-cache": { + "id": "bf-cache", + "title": "Back/forward cache is used", + "description": "The back/forward cache can speed up the page load after navigating away.", + "score": null, + "scoreDisplayMode": "error", + "errorMessage": "Required BFCacheErrors gatherer did not run." } }, "configSettings": { @@ -3981,6 +3989,10 @@ "id": "no-unload-listeners", "weight": 0 }, + { + "id": "bf-cache", + "weight": 0 + }, { "id": "performance-budget", "weight": 0, @@ -6024,12 +6036,18 @@ }, { "startTime": 226, + "name": "lh:audit:bf-cache", + "duration": 1, + "entryType": "measure" + }, + { + "startTime": 227, "name": "lh:runner:generate", "duration": 1, "entryType": "measure" } ], - "total": 227 + "total": 228 }, "i18n": { "rendererFormattedStrings": { @@ -7209,6 +7227,21 @@ "core/audits/seo/manual/structured-data.js | description": [ "audits[structured-data].description" ], + "core/audits/bf-cache.js | title": [ + "audits[bf-cache].title" + ], + "core/audits/bf-cache.js | description": [ + "audits[bf-cache].description" + ], + "core/lib/lh-error.js | missingRequiredArtifact": [ + { + "values": { + "errorCode": "MISSING_REQUIRED_ARTIFACT", + "artifactName": "BFCacheErrors" + }, + "path": "audits[bf-cache].errorMessage" + } + ], "core/config/default-config.js | performanceCategoryTitle": [ "categories.performance.title" ], @@ -17807,6 +17840,14 @@ "description": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more about Structured Data](https://web.dev/structured-data/).", "score": null, "scoreDisplayMode": "manual" + }, + "bf-cache": { + "id": "bf-cache", + "title": "Back/forward cache is used", + "description": "The back/forward cache can speed up the page load after navigating away.", + "score": null, + "scoreDisplayMode": "error", + "errorMessage": "Required BFCacheErrors gatherer did not run." } }, "configSettings": { @@ -18113,6 +18154,10 @@ "id": "no-unload-listeners", "weight": 0 }, + { + "id": "bf-cache", + "weight": 0 + }, { "id": "performance-budget", "weight": 0, @@ -20138,12 +20183,18 @@ }, { "startTime": 223, + "name": "lh:audit:bf-cache", + "duration": 1, + "entryType": "measure" + }, + { + "startTime": 224, "name": "lh:runner:generate", "duration": 1, "entryType": "measure" } ], - "total": 224 + "total": 225 }, "i18n": { "rendererFormattedStrings": { @@ -21339,6 +21390,21 @@ "core/audits/seo/manual/structured-data.js | description": [ "audits[structured-data].description" ], + "core/audits/bf-cache.js | title": [ + "audits[bf-cache].title" + ], + "core/audits/bf-cache.js | description": [ + "audits[bf-cache].description" + ], + "core/lib/lh-error.js | missingRequiredArtifact": [ + { + "values": { + "errorCode": "MISSING_REQUIRED_ARTIFACT", + "artifactName": "BFCacheErrors" + }, + "path": "audits[bf-cache].errorMessage" + } + ], "core/config/default-config.js | performanceCategoryTitle": [ "categories.performance.title" ], diff --git a/core/test/results/sample_v2.json b/core/test/results/sample_v2.json index 1b96a25c244e..149983348de8 100644 --- a/core/test/results/sample_v2.json +++ b/core/test/results/sample_v2.json @@ -5819,6 +5819,14 @@ "description": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more about Structured Data](https://web.dev/structured-data/).", "score": null, "scoreDisplayMode": "manual" + }, + "bf-cache": { + "id": "bf-cache", + "title": "Back/forward cache is used", + "description": "The back/forward cache can speed up the page load after navigating away.", + "score": null, + "scoreDisplayMode": "error", + "errorMessage": "Required BFCacheErrors gatherer did not run." } }, "configSettings": { @@ -6227,6 +6235,10 @@ "id": "no-unload-listeners", "weight": 0 }, + { + "id": "bf-cache", + "weight": 0 + }, { "id": "performance-budget", "weight": 0, @@ -8280,6 +8292,12 @@ "duration": 100, "entryType": "measure" }, + { + "startTime": 0, + "name": "lh:audit:bf-cache", + "duration": 100, + "entryType": "measure" + }, { "startTime": 0, "name": "lh:runner:generate", @@ -9694,6 +9712,21 @@ "core/audits/seo/manual/structured-data.js | description": [ "audits[structured-data].description" ], + "core/audits/bf-cache.js | title": [ + "audits[bf-cache].title" + ], + "core/audits/bf-cache.js | description": [ + "audits[bf-cache].description" + ], + "core/lib/lh-error.js | missingRequiredArtifact": [ + { + "values": { + "errorCode": "MISSING_REQUIRED_ARTIFACT", + "artifactName": "BFCacheErrors" + }, + "path": "audits[bf-cache].errorMessage" + } + ], "core/config/default-config.js | performanceCategoryTitle": [ "categories.performance.title" ], diff --git a/shared/localization/locales/en-US.json b/shared/localization/locales/en-US.json index 57be75a76078..61009602bd44 100644 --- a/shared/localization/locales/en-US.json +++ b/shared/localization/locales/en-US.json @@ -425,6 +425,24 @@ "core/audits/autocomplete.js | warningOrder": { "message": "Review order of tokens: \"{tokens}\" in {snippet}" }, + "core/audits/bf-cache.js | description": { + "message": "The back/forward cache can speed up the page load after navigating away." + }, + "core/audits/bf-cache.js | displayValue": { + "message": "{itemCount, plural,\n =1 {1 failure reason}\n other {# failure reasons}\n }" + }, + "core/audits/bf-cache.js | failureColumn": { + "message": "Failure reason" + }, + "core/audits/bf-cache.js | failureTitle": { + "message": "Back/forward cache is not used" + }, + "core/audits/bf-cache.js | title": { + "message": "Back/forward cache is used" + }, + "core/audits/bf-cache.js | unknownReason": { + "message": "Back/forward cache failure reason '{reason}' is not recognized" + }, "core/audits/bootup-time.js | chromeExtensionsWarning": { "message": "Chrome extensions negatively affected this page's load performance. Try auditing the page in incognito mode or from a Chrome profile without extensions." }, diff --git a/shared/localization/locales/en-XL.json b/shared/localization/locales/en-XL.json index 5caffe894699..871c9d483e25 100644 --- a/shared/localization/locales/en-XL.json +++ b/shared/localization/locales/en-XL.json @@ -425,6 +425,24 @@ "core/audits/autocomplete.js | warningOrder": { "message": "R̂év̂íêẃ ôŕd̂ér̂ óf̂ t́ôḱêńŝ: \"{tokens}\" ín̂ {snippet}" }, + "core/audits/bf-cache.js | description": { + "message": "T̂h́ê b́âćk̂/f́ôŕŵár̂d́ ĉáĉh́ê ćâń ŝṕêéd̂ úp̂ t́ĥé p̂áĝé l̂óâd́ âf́t̂ér̂ ńâv́îǵât́îńĝ áŵáŷ." + }, + "core/audits/bf-cache.js | displayValue": { + "message": "{itemCount, plural,\n =1 {1 f̂áîĺûŕê ŕêáŝón̂}\n other {# f́âíl̂úr̂é r̂éâśôńŝ}\n }" + }, + "core/audits/bf-cache.js | failureColumn": { + "message": "F̂áîĺûŕê ŕêáŝón̂" + }, + "core/audits/bf-cache.js | failureTitle": { + "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé îś n̂ót̂ úŝéd̂" + }, + "core/audits/bf-cache.js | title": { + "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé îś ûśêd́" + }, + "core/audits/bf-cache.js | unknownReason": { + "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé f̂áîĺûŕê ŕêáŝón̂ '{reason}' íŝ ńôt́ r̂éĉóĝńîźêd́" + }, "core/audits/bootup-time.js | chromeExtensionsWarning": { "message": "Ĉh́r̂óm̂é êx́t̂én̂śîón̂ś n̂éĝát̂ív̂él̂ý âf́f̂éĉt́êd́ t̂h́îś p̂áĝé'ŝ ĺôád̂ ṕêŕf̂ór̂ḿâńĉé. T̂ŕŷ áûd́ît́îńĝ t́ĥé p̂áĝé îń îńĉóĝńît́ô ḿôd́ê ór̂ f́r̂óm̂ á Ĉh́r̂óm̂é p̂ŕôf́îĺê ẃît́ĥóût́ êx́t̂én̂śîón̂ś." }, diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 8d548444f540..61287c588325 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -122,6 +122,7 @@ export interface GathererArtifacts extends PublicGathererArtifacts,LegacyBaseArt Accessibility: Artifacts.Accessibility; /** Array of all anchors on the page. */ AnchorElements: Artifacts.AnchorElement[]; + BFCacheErrors: Artifacts.BFCacheErrors; /** Array of all URLs cached in CacheStorage. */ CacheContents: string[]; /** CSS coverage information for styles used by page's final state. */ @@ -410,6 +411,10 @@ declare module Artifacts { }> } + interface BFCacheErrors { + errors: LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]; + } + interface Font { display: string; family: string; From 648f72e293fa2a2c6e9cd6b6f16d8f33df42a2dc Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Mon, 24 Oct 2022 15:41:02 -0700 Subject: [PATCH 02/20] ope --- core/legacy/config/legacy-default-config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/core/legacy/config/legacy-default-config.js b/core/legacy/config/legacy-default-config.js index 7cc286f29308..49311c2e1f1a 100644 --- a/core/legacy/config/legacy-default-config.js +++ b/core/legacy/config/legacy-default-config.js @@ -71,6 +71,7 @@ legacyDefaultConfig.passes = [{ 'inspector-issues', 'source-maps', 'full-page-screenshot', + 'bf-cache-errors', ], }, { From c25b19653b15542f86851f4c82cb3bcaeb98d23a Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 25 Oct 2022 12:58:53 -0700 Subject: [PATCH 03/20] smoke fix --- cli/test/smokehouse/test-definitions/dobetterweb.js | 5 +++++ cli/test/smokehouse/test-definitions/perf-budgets.js | 4 ++++ cli/test/smokehouse/test-definitions/perf-fonts.js | 4 ++++ cli/test/smokehouse/test-definitions/perf-frame-metrics.js | 4 ++++ cli/test/smokehouse/test-definitions/perf-preload.js | 4 ++++ cli/test/smokehouse/test-definitions/perf-trace-elements.js | 4 ++++ 6 files changed, 25 insertions(+) diff --git a/cli/test/smokehouse/test-definitions/dobetterweb.js b/cli/test/smokehouse/test-definitions/dobetterweb.js index 98b6b7a7cbf0..6a548cbb11bd 100644 --- a/cli/test/smokehouse/test-definitions/dobetterweb.js +++ b/cli/test/smokehouse/test-definitions/dobetterweb.js @@ -7,6 +7,11 @@ /** @type {LH.Config.Json} */ const config = { extends: 'lighthouse:default', + settings: { + // BF cache will request the page an additional time, which can initiates additional network requests. + // Disable the audit so we only detect requests from the normal page load. + skipAudits: ['bf-cache'], + }, audits: [ // Test the `ignoredPatterns` audit option. {path: 'errors-in-console', options: {ignoredPatterns: ['An ignored error']}}, diff --git a/cli/test/smokehouse/test-definitions/perf-budgets.js b/cli/test/smokehouse/test-definitions/perf-budgets.js index c25b20072281..7e25bd62e616 100644 --- a/cli/test/smokehouse/test-definitions/perf-budgets.js +++ b/cli/test/smokehouse/test-definitions/perf-budgets.js @@ -13,6 +13,10 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], + // BF cache will request the page an additional time, which can initiates additional network requests. + // Disable the audit so we only detect requests from the normal page load. + skipAudits: ['bf-cache'], + // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-fonts.js b/cli/test/smokehouse/test-definitions/perf-fonts.js index 689272a9701f..3d0fcd9f78cf 100644 --- a/cli/test/smokehouse/test-definitions/perf-fonts.js +++ b/cli/test/smokehouse/test-definitions/perf-fonts.js @@ -13,6 +13,10 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], + // BF cache will request the page an additional time, which can initiates additional network requests. + // Disable the audit so we only detect requests from the normal page load. + skipAudits: ['bf-cache'], + // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-frame-metrics.js b/cli/test/smokehouse/test-definitions/perf-frame-metrics.js index 38cd15dbdd8d..d96a01bbc69b 100644 --- a/cli/test/smokehouse/test-definitions/perf-frame-metrics.js +++ b/cli/test/smokehouse/test-definitions/perf-frame-metrics.js @@ -13,6 +13,10 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], + // BF cache will request the page an additional time, which can initiates additional network requests. + // Disable the audit so we only detect requests from the normal page load. + skipAudits: ['bf-cache'], + // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-preload.js b/cli/test/smokehouse/test-definitions/perf-preload.js index 8f77238dd23b..2d5e6d9bbf37 100644 --- a/cli/test/smokehouse/test-definitions/perf-preload.js +++ b/cli/test/smokehouse/test-definitions/perf-preload.js @@ -13,6 +13,10 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], + // BF cache will request the page an additional time, which can initiates additional network requests. + // Disable the audit so we only detect requests from the normal page load. + skipAudits: ['bf-cache'], + // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-trace-elements.js b/cli/test/smokehouse/test-definitions/perf-trace-elements.js index e27849174e4b..4ac97cc27bf9 100644 --- a/cli/test/smokehouse/test-definitions/perf-trace-elements.js +++ b/cli/test/smokehouse/test-definitions/perf-trace-elements.js @@ -13,6 +13,10 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], + // BF cache will request the page an additional time, which can initiates additional network requests. + // Disable the audit so we only detect requests from the normal page load. + skipAudits: ['bf-cache'], + // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', From 29352f2762d22891cf43f431fb466c7beda25753 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 25 Oct 2022 13:08:03 -0700 Subject: [PATCH 04/20] ope --- core/config/filters.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/config/filters.js b/core/config/filters.js index f7dfdb86b65e..3fe40e319241 100644 --- a/core/config/filters.js +++ b/core/config/filters.js @@ -32,7 +32,7 @@ const filterResistantAuditIds = ['full-page-screenshot']; // Some artifacts are used by the report for additional information. // Always run these artifacts even if audits do not request them. // These are similar to base artifacts but they cannot be run in all 3 modes. -const filterResistantArtifactIds = ['Stacks', 'NetworkUserAgent', 'BFCacheErrors']; +const filterResistantArtifactIds = ['Stacks', 'NetworkUserAgent']; /** * Returns the set of audit IDs used in the list of categories. From b89df496ee6395b5303afc868118312b8dd483eb Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 25 Oct 2022 13:09:36 -0700 Subject: [PATCH 05/20] comments --- cli/test/smokehouse/test-definitions/dobetterweb.js | 2 +- cli/test/smokehouse/test-definitions/perf-budgets.js | 2 +- cli/test/smokehouse/test-definitions/perf-fonts.js | 2 +- cli/test/smokehouse/test-definitions/perf-frame-metrics.js | 2 +- cli/test/smokehouse/test-definitions/perf-preload.js | 2 +- cli/test/smokehouse/test-definitions/perf-trace-elements.js | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/test/smokehouse/test-definitions/dobetterweb.js b/cli/test/smokehouse/test-definitions/dobetterweb.js index 6a548cbb11bd..72ea038fd96a 100644 --- a/cli/test/smokehouse/test-definitions/dobetterweb.js +++ b/cli/test/smokehouse/test-definitions/dobetterweb.js @@ -8,7 +8,7 @@ const config = { extends: 'lighthouse:default', settings: { - // BF cache will request the page an additional time, which can initiates additional network requests. + // BF cache will request the page again, initiating additional network requests. // Disable the audit so we only detect requests from the normal page load. skipAudits: ['bf-cache'], }, diff --git a/cli/test/smokehouse/test-definitions/perf-budgets.js b/cli/test/smokehouse/test-definitions/perf-budgets.js index 7e25bd62e616..72cb8dafc66b 100644 --- a/cli/test/smokehouse/test-definitions/perf-budgets.js +++ b/cli/test/smokehouse/test-definitions/perf-budgets.js @@ -13,7 +13,7 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page an additional time, which can initiates additional network requests. + // BF cache will request the page again, initiating additional network requests. // Disable the audit so we only detect requests from the normal page load. skipAudits: ['bf-cache'], diff --git a/cli/test/smokehouse/test-definitions/perf-fonts.js b/cli/test/smokehouse/test-definitions/perf-fonts.js index 3d0fcd9f78cf..1fd094ca2ab6 100644 --- a/cli/test/smokehouse/test-definitions/perf-fonts.js +++ b/cli/test/smokehouse/test-definitions/perf-fonts.js @@ -13,7 +13,7 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page an additional time, which can initiates additional network requests. + // BF cache will request the page again, initiating additional network requests. // Disable the audit so we only detect requests from the normal page load. skipAudits: ['bf-cache'], diff --git a/cli/test/smokehouse/test-definitions/perf-frame-metrics.js b/cli/test/smokehouse/test-definitions/perf-frame-metrics.js index d96a01bbc69b..f2934c0ec644 100644 --- a/cli/test/smokehouse/test-definitions/perf-frame-metrics.js +++ b/cli/test/smokehouse/test-definitions/perf-frame-metrics.js @@ -13,7 +13,7 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page an additional time, which can initiates additional network requests. + // BF cache will request the page again, initiating additional network requests. // Disable the audit so we only detect requests from the normal page load. skipAudits: ['bf-cache'], diff --git a/cli/test/smokehouse/test-definitions/perf-preload.js b/cli/test/smokehouse/test-definitions/perf-preload.js index 2d5e6d9bbf37..f8262945abbd 100644 --- a/cli/test/smokehouse/test-definitions/perf-preload.js +++ b/cli/test/smokehouse/test-definitions/perf-preload.js @@ -13,7 +13,7 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page an additional time, which can initiates additional network requests. + // BF cache will request the page again, initiating additional network requests. // Disable the audit so we only detect requests from the normal page load. skipAudits: ['bf-cache'], diff --git a/cli/test/smokehouse/test-definitions/perf-trace-elements.js b/cli/test/smokehouse/test-definitions/perf-trace-elements.js index 4ac97cc27bf9..d300cc00cb6d 100644 --- a/cli/test/smokehouse/test-definitions/perf-trace-elements.js +++ b/cli/test/smokehouse/test-definitions/perf-trace-elements.js @@ -13,7 +13,7 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page an additional time, which can initiates additional network requests. + // BF cache will request the page again, initiating additional network requests. // Disable the audit so we only detect requests from the normal page load. skipAudits: ['bf-cache'], From 111abd4d6dceb005b779917377cf36a562400868 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 25 Oct 2022 16:42:10 -0700 Subject: [PATCH 06/20] tree --- core/audits/bf-cache.js | 111 ++++++++++++++++++----- core/gather/gatherers/bf-cache-errors.js | 11 ++- types/artifacts.d.ts | 3 +- 3 files changed, 94 insertions(+), 31 deletions(-) diff --git a/core/audits/bf-cache.js b/core/audits/bf-cache.js index 96e4dfcd13b1..fc756c9ace2d 100644 --- a/core/audits/bf-cache.js +++ b/core/audits/bf-cache.js @@ -51,30 +51,83 @@ class BFCache extends Audit { } /** - * @param {LH.Artifacts} artifacts - * @return {Array} + * We only want to surface errors that are actionable (i.e. have type "PageSupportNeeded") + * + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation} err */ - static getBfCacheErrors(artifacts) { - const bfCacheErrors = artifacts.BFCacheErrors.errors; - const i18nErrors = []; + static shouldIgnoreError(err) { + return err.type !== 'PageSupportNeeded'; + } - for (const err of bfCacheErrors) { - // Only show errors which can be addressed by the user. - if (err.type !== 'PageSupportNeeded') continue; + /** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredReason} reason + */ + static getDescriptionForReason(reason) { + const matchingString = NotRestoredReasonDescription[reason]; + if (matchingString === undefined) { + return str_(UIStrings.unknownReason, {reason: reason}); + } - const matchingString = NotRestoredReasonDescription[err.reason]; + return matchingString.name; + } - // Handle an errorId we don't recognize. - if (matchingString === undefined) { - i18nErrors.push(str_(UIStrings.unknownReason, {reason: err.reason})); - continue; + /** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} errorList + * @return {LH.Audit.Details.TableItem[]} + */ + static constructResultsFromList(errorList) { + const results = []; + + for (const err of errorList) { + if (this.shouldIgnoreError(err)) continue; + results.push({ + reason: this.getDescriptionForReason(err.reason), + }); + } + + return results; + } + + /** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} errorTree + * @return {LH.Audit.Details.TableItem[]} + */ + static constructResultsFromTree(errorTree) { + /** @type {Map} */ + const frameUrlsByFailureReason = new Map(); + + /** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} node + */ + function traverse(node) { + for (const error of node.explanations) { + if (BFCache.shouldIgnoreError(error)) continue; + + const frameUrls = frameUrlsByFailureReason.get(error.reason) || []; + frameUrls.push(node.url); + frameUrlsByFailureReason.set(error.reason, frameUrls); } - i18nErrors.push(matchingString.name); + for (const child of node.children) { + traverse(child); + } } - return i18nErrors; + traverse(errorTree); + + /** @type {LH.Audit.Details.TableItem[]} */ + const results = []; + for (const [reason, frameUrls] of frameUrlsByFailureReason.entries()) { + results.push({ + reason: this.getDescriptionForReason(reason), + subItems: { + type: 'subitems', + items: frameUrls.map(frameUrl => ({frameUrl})), + }, + }); + } + return results; } /** @@ -83,21 +136,29 @@ class BFCache extends Audit { * */ static async audit(artifacts) { - const i18nErrors = BFCache.getBfCacheErrors(artifacts); + const {list, tree} = artifacts.BFCacheErrors; + + // The BF cache failure tree cans sometimes be undefined. + // In this case we can still construct the results from the list result. + // https://bugs.chromium.org/p/chromium/issues/detail?id=1281855 + /** @type {LH.Audit.Details.TableItem[]} */ + let results = []; + if (tree) { + results = BFCache.constructResultsFromTree(tree); + } else if (list) { + results = BFCache.constructResultsFromList(list); + } /** @type {LH.Audit.Details.Table['headings']} */ const headings = [ - {key: 'reason', valueType: 'text', label: str_(UIStrings.failureColumn)}, + /* eslint-disable max-len */ + {key: 'reason', valueType: 'text', subItemsHeading: {key: 'frameUrl', valueType: 'url'}, label: str_(UIStrings.failureColumn)}, + /* eslint-enable max-len */ ]; - /** @type {LH.Audit.Details.Table['items']} */ - const errorReasons = i18nErrors.map(reason => { - return {reason}; - }); - - const details = Audit.makeTableDetails(headings, errorReasons); + const details = Audit.makeTableDetails(headings, results); - if (errorReasons.length === 0) { + if (results.length === 0) { return { score: 1, details, @@ -106,7 +167,7 @@ class BFCache extends Audit { return { score: 0, - displayValue: str_(UIStrings.displayValue, {itemCount: errorReasons.length}), + displayValue: str_(UIStrings.displayValue, {itemCount: results.length}), details, }; } diff --git a/core/gather/gatherers/bf-cache-errors.js b/core/gather/gatherers/bf-cache-errors.js index 76f8f72b7628..a30d503ea55c 100644 --- a/core/gather/gatherers/bf-cache-errors.js +++ b/core/gather/gatherers/bf-cache-errors.js @@ -20,16 +20,17 @@ class BFCacheErrors extends FRGatherer { async getArtifact(context) { const session = context.driver.defaultSession; - /** - * @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} - */ - let errors = []; + /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]|undefined} */ + let errors = undefined; + /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree|undefined} */ + let tree = undefined; /** * @param {LH.Crdp.Page.BackForwardCacheNotUsedEvent} event */ function onBfCacheNotUsed(event) { errors = event.notRestoredExplanations; + tree = event.notRestoredExplanationsTree; } session.on('Page.backForwardCacheNotUsed', onBfCacheNotUsed); @@ -49,7 +50,7 @@ class BFCacheErrors extends FRGatherer { session.off('Page.backForwardCacheNotUsed', onBfCacheNotUsed); - return {errors}; + return {list: errors, tree}; } } diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 61287c588325..96b4f142f236 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -412,7 +412,8 @@ declare module Artifacts { } interface BFCacheErrors { - errors: LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]; + list?: LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]; + tree?: LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree; } interface Font { From d2db29171a6e2f2b1cf2a0ca4b5ec18e86dc2ff5 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 25 Oct 2022 17:40:26 -0700 Subject: [PATCH 07/20] organize results in artifact --- core/audits/bf-cache.js | 82 ++++-------------------- core/gather/gatherers/bf-cache-errors.js | 65 +++++++++++++++++-- types/artifacts.d.ts | 9 +-- 3 files changed, 77 insertions(+), 79 deletions(-) diff --git a/core/audits/bf-cache.js b/core/audits/bf-cache.js index fc756c9ace2d..04046d7841fc 100644 --- a/core/audits/bf-cache.js +++ b/core/audits/bf-cache.js @@ -50,15 +50,6 @@ class BFCache extends Audit { }; } - /** - * We only want to surface errors that are actionable (i.e. have type "PageSupportNeeded") - * - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation} err - */ - static shouldIgnoreError(err) { - return err.type !== 'PageSupportNeeded'; - } - /** * @param {LH.Crdp.Page.BackForwardCacheNotRestoredReason} reason */ @@ -73,52 +64,22 @@ class BFCache extends Audit { } /** - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} errorList - * @return {LH.Audit.Details.TableItem[]} + * @param {LH.Artifacts} artifacts + * @return {Promise} + * */ - static constructResultsFromList(errorList) { + static async audit(artifacts) { + /** @type {LH.Audit.Details.TableItem[]} */ const results = []; - for (const err of errorList) { - if (this.shouldIgnoreError(err)) continue; - results.push({ - reason: this.getDescriptionForReason(err.reason), - }); - } - - return results; - } - - /** - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} errorTree - * @return {LH.Audit.Details.TableItem[]} - */ - static constructResultsFromTree(errorTree) { - /** @type {Map} */ - const frameUrlsByFailureReason = new Map(); - - /** - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} node - */ - function traverse(node) { - for (const error of node.explanations) { - if (BFCache.shouldIgnoreError(error)) continue; - - const frameUrls = frameUrlsByFailureReason.get(error.reason) || []; - frameUrls.push(node.url); - frameUrlsByFailureReason.set(error.reason, frameUrls); - } - - for (const child of node.children) { - traverse(child); - } - } + const actionableErrors = artifacts.BFCacheErrors.PageSupportNeeded; - traverse(errorTree); + // https://github.com/Microsoft/TypeScript/issues/12870 + const reasons = /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredReason[]} */ + (Object.keys(actionableErrors)); - /** @type {LH.Audit.Details.TableItem[]} */ - const results = []; - for (const [reason, frameUrls] of frameUrlsByFailureReason.entries()) { + for (const reason of reasons) { + const frameUrls = actionableErrors[reason] || []; results.push({ reason: this.getDescriptionForReason(reason), subItems: { @@ -127,27 +88,6 @@ class BFCache extends Audit { }, }); } - return results; - } - - /** - * @param {LH.Artifacts} artifacts - * @return {Promise} - * - */ - static async audit(artifacts) { - const {list, tree} = artifacts.BFCacheErrors; - - // The BF cache failure tree cans sometimes be undefined. - // In this case we can still construct the results from the list result. - // https://bugs.chromium.org/p/chromium/issues/detail?id=1281855 - /** @type {LH.Audit.Details.TableItem[]} */ - let results = []; - if (tree) { - results = BFCache.constructResultsFromTree(tree); - } else if (list) { - results = BFCache.constructResultsFromList(list); - } /** @type {LH.Audit.Details.Table['headings']} */ const headings = [ diff --git a/core/gather/gatherers/bf-cache-errors.js b/core/gather/gatherers/bf-cache-errors.js index a30d503ea55c..60421e60311a 100644 --- a/core/gather/gatherers/bf-cache-errors.js +++ b/core/gather/gatherers/bf-cache-errors.js @@ -7,6 +7,60 @@ import FRGatherer from '../base-gatherer.js'; import {waitForFrameNavigated, waitForLoadEvent} from '../driver/wait-for-condition.js'; +/** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} errorList + * @return {LH.Artifacts.BFCacheErrors} + */ +function constructArtifactFromList(errorList) { + /** @type {LH.Artifacts.BFCacheErrors} */ + const bfCacheErrors = { + Circumstantial: {}, + PageSupportNeeded: {}, + SupportPending: {}, + }; + + for (const err of errorList) { + const bfCacheErrorsMap = bfCacheErrors[err.type]; + bfCacheErrorsMap[err.reason] = []; + } + + return bfCacheErrors; +} + +/** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} errorTree + * @return {LH.Artifacts.BFCacheErrors} + */ +function constructArtifactFromTree(errorTree) { + /** @type {LH.Artifacts.BFCacheErrors} */ + const bfCacheErrors = { + Circumstantial: {}, + PageSupportNeeded: {}, + SupportPending: {}, + }; + + /** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} node + */ + function traverse(node) { + for (const error of node.explanations) { + const bfCacheErrorsMap = bfCacheErrors[error.type]; + const frameUrls = bfCacheErrorsMap[error.reason] || []; + frameUrls.push(node.url); + bfCacheErrorsMap[error.reason] = frameUrls; + } + + for (const child of node.children) { + traverse(child); + } + } + + traverse(errorTree); + + return bfCacheErrors; +} + + class BFCacheErrors extends FRGatherer { /** @type {LH.Gatherer.GathererMeta} */ meta = { @@ -20,8 +74,8 @@ class BFCacheErrors extends FRGatherer { async getArtifact(context) { const session = context.driver.defaultSession; - /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]|undefined} */ - let errors = undefined; + /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} */ + let list = []; /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree|undefined} */ let tree = undefined; @@ -29,7 +83,7 @@ class BFCacheErrors extends FRGatherer { * @param {LH.Crdp.Page.BackForwardCacheNotUsedEvent} event */ function onBfCacheNotUsed(event) { - errors = event.notRestoredExplanations; + list = event.notRestoredExplanations; tree = event.notRestoredExplanationsTree; } @@ -50,7 +104,10 @@ class BFCacheErrors extends FRGatherer { session.off('Page.backForwardCacheNotUsed', onBfCacheNotUsed); - return {list: errors, tree}; + if (tree) { + return constructArtifactFromTree(tree); + } + return constructArtifactFromList(list); } } diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 96b4f142f236..57c02b573fa3 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -411,10 +411,11 @@ declare module Artifacts { }> } - interface BFCacheErrors { - list?: LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]; - tree?: LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree; - } + type BFCacheErrorMap = { + [key in LH.Crdp.Page.BackForwardCacheNotRestoredReason]?: string[]; + }; + + type BFCacheErrors = Record; interface Font { display: string; From 7a669a047582e6860e1df66c6d78a388f1712e68 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Wed, 26 Oct 2022 12:29:35 -0700 Subject: [PATCH 08/20] all types --- core/audits/bf-cache.js | 70 +++++++++++++++++--------- core/scripts/i18n/collect-strings.js | 2 - shared/localization/locales/en-US.json | 17 ++++--- shared/localization/locales/en-XL.json | 17 ++++--- 4 files changed, 67 insertions(+), 39 deletions(-) diff --git a/core/audits/bf-cache.js b/core/audits/bf-cache.js index 04046d7841fc..faed667fa939 100644 --- a/core/audits/bf-cache.js +++ b/core/audits/bf-cache.js @@ -17,19 +17,18 @@ const UIStrings = { /** TODO */ description: 'The back/forward cache can speed up the page load after navigating away.', /** TODO */ - failureColumn: 'Failure reason', + actionableColumn: 'Actionable failure', + /** TODO */ + notActionableColumn: 'Not actionable failure', + /** TODO */ + supportPendingColumn: 'Pending browser support', /** * @description [ICU Syntax] Label for an audit identifying the number of back/forward cache failure reasons found in the page. */ displayValue: `{itemCount, plural, - =1 {1 failure reason} - other {# failure reasons} + =1 {1 actionable failure reason} + other {# actionable failure reasons} }`, - /** - * @description Error message describing a DevTools error id that was found and has not been identified by this audit. - * @example {platform-not-supported-on-android} reason - */ - unknownReason: `Back/forward cache failure reason '{reason}' is not recognized`, }; /* eslint-enable max-len */ @@ -57,29 +56,27 @@ class BFCache extends Audit { const matchingString = NotRestoredReasonDescription[reason]; if (matchingString === undefined) { - return str_(UIStrings.unknownReason, {reason: reason}); + return reason; } return matchingString.name; } /** - * @param {LH.Artifacts} artifacts - * @return {Promise} - * + * @param {LH.Artifacts.BFCacheErrorMap} errors + * @param {LH.IcuMessage | string} label + * @return {LH.Audit.Details.Table} */ - static async audit(artifacts) { + static makeTableForFailureType(errors, label) { /** @type {LH.Audit.Details.TableItem[]} */ const results = []; - const actionableErrors = artifacts.BFCacheErrors.PageSupportNeeded; - // https://github.com/Microsoft/TypeScript/issues/12870 const reasons = /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredReason[]} */ - (Object.keys(actionableErrors)); + (Object.keys(errors)); for (const reason of reasons) { - const frameUrls = actionableErrors[reason] || []; + const frameUrls = errors[reason] || []; results.push({ reason: this.getDescriptionForReason(reason), subItems: { @@ -92,23 +89,50 @@ class BFCache extends Audit { /** @type {LH.Audit.Details.Table['headings']} */ const headings = [ /* eslint-disable max-len */ - {key: 'reason', valueType: 'text', subItemsHeading: {key: 'frameUrl', valueType: 'url'}, label: str_(UIStrings.failureColumn)}, + {key: 'reason', valueType: 'text', subItemsHeading: {key: 'frameUrl', valueType: 'url'}, label}, /* eslint-enable max-len */ ]; - const details = Audit.makeTableDetails(headings, results); + return Audit.makeTableDetails(headings, results); + } - if (results.length === 0) { + /** + * @param {LH.Artifacts} artifacts + * @return {Promise} + */ + static async audit(artifacts) { + const {PageSupportNeeded, SupportPending, Circumstantial} = artifacts.BFCacheErrors; + + const actionableTable = + this.makeTableForFailureType(PageSupportNeeded, str_(UIStrings.actionableColumn)); + const notActionableTable = + this.makeTableForFailureType(Circumstantial, str_(UIStrings.notActionableColumn)); + const supportPendingTable = + this.makeTableForFailureType(SupportPending, str_(UIStrings.supportPendingColumn)); + + const items = [ + actionableTable, + notActionableTable, + supportPendingTable, + ]; + + if (actionableTable.items.length === 0) { return { score: 1, - details, + details: { + type: 'list', + items, + }, }; } return { score: 0, - displayValue: str_(UIStrings.displayValue, {itemCount: results.length}), - details, + displayValue: str_(UIStrings.displayValue, {itemCount: actionableTable.items.length}), + details: { + type: 'list', + items, + }, }; } } diff --git a/core/scripts/i18n/collect-strings.js b/core/scripts/i18n/collect-strings.js index 20eec9bed592..d5a4e7162b80 100644 --- a/core/scripts/i18n/collect-strings.js +++ b/core/scripts/i18n/collect-strings.js @@ -718,8 +718,6 @@ function checkKnownFixedCollisions(strings) { 'Document has a valid $MARKDOWN_SNIPPET_0$', 'Failing Elements', 'Failing Elements', - 'Failure reason', - 'Failure reason', 'Name', 'Name', 'Pages that use portals are not currently eligible for back/forward cache.', diff --git a/shared/localization/locales/en-US.json b/shared/localization/locales/en-US.json index 61009602bd44..6bc8f22a34bf 100644 --- a/shared/localization/locales/en-US.json +++ b/shared/localization/locales/en-US.json @@ -425,24 +425,27 @@ "core/audits/autocomplete.js | warningOrder": { "message": "Review order of tokens: \"{tokens}\" in {snippet}" }, + "core/audits/bf-cache.js | actionableColumn": { + "message": "Actionable failure" + }, "core/audits/bf-cache.js | description": { "message": "The back/forward cache can speed up the page load after navigating away." }, "core/audits/bf-cache.js | displayValue": { - "message": "{itemCount, plural,\n =1 {1 failure reason}\n other {# failure reasons}\n }" - }, - "core/audits/bf-cache.js | failureColumn": { - "message": "Failure reason" + "message": "{itemCount, plural,\n =1 {1 actionable failure reason}\n other {# actionable failure reasons}\n }" }, "core/audits/bf-cache.js | failureTitle": { "message": "Back/forward cache is not used" }, + "core/audits/bf-cache.js | notActionableColumn": { + "message": "Not actionable failure" + }, + "core/audits/bf-cache.js | supportPendingColumn": { + "message": "Pending browser support" + }, "core/audits/bf-cache.js | title": { "message": "Back/forward cache is used" }, - "core/audits/bf-cache.js | unknownReason": { - "message": "Back/forward cache failure reason '{reason}' is not recognized" - }, "core/audits/bootup-time.js | chromeExtensionsWarning": { "message": "Chrome extensions negatively affected this page's load performance. Try auditing the page in incognito mode or from a Chrome profile without extensions." }, diff --git a/shared/localization/locales/en-XL.json b/shared/localization/locales/en-XL.json index 871c9d483e25..ae7fd83e7270 100644 --- a/shared/localization/locales/en-XL.json +++ b/shared/localization/locales/en-XL.json @@ -425,24 +425,27 @@ "core/audits/autocomplete.js | warningOrder": { "message": "R̂év̂íêẃ ôŕd̂ér̂ óf̂ t́ôḱêńŝ: \"{tokens}\" ín̂ {snippet}" }, + "core/audits/bf-cache.js | actionableColumn": { + "message": "Âćt̂íôńâb́l̂é f̂áîĺûŕê" + }, "core/audits/bf-cache.js | description": { "message": "T̂h́ê b́âćk̂/f́ôŕŵár̂d́ ĉáĉh́ê ćâń ŝṕêéd̂ úp̂ t́ĥé p̂áĝé l̂óâd́ âf́t̂ér̂ ńâv́îǵât́îńĝ áŵáŷ." }, "core/audits/bf-cache.js | displayValue": { - "message": "{itemCount, plural,\n =1 {1 f̂áîĺûŕê ŕêáŝón̂}\n other {# f́âíl̂úr̂é r̂éâśôńŝ}\n }" - }, - "core/audits/bf-cache.js | failureColumn": { - "message": "F̂áîĺûŕê ŕêáŝón̂" + "message": "{itemCount, plural,\n =1 {1 âćt̂íôńâb́l̂é f̂áîĺûŕê ŕêáŝón̂}\n other {# áĉt́îón̂áb̂ĺê f́âíl̂úr̂é r̂éâśôńŝ}\n }" }, "core/audits/bf-cache.js | failureTitle": { "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé îś n̂ót̂ úŝéd̂" }, + "core/audits/bf-cache.js | notActionableColumn": { + "message": "N̂ót̂ áĉt́îón̂áb̂ĺê f́âíl̂úr̂é" + }, + "core/audits/bf-cache.js | supportPendingColumn": { + "message": "P̂én̂d́îńĝ b́r̂óŵśêŕ ŝúp̂ṕôŕt̂" + }, "core/audits/bf-cache.js | title": { "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé îś ûśêd́" }, - "core/audits/bf-cache.js | unknownReason": { - "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé f̂áîĺûŕê ŕêáŝón̂ '{reason}' íŝ ńôt́ r̂éĉóĝńîźêd́" - }, "core/audits/bootup-time.js | chromeExtensionsWarning": { "message": "Ĉh́r̂óm̂é êx́t̂én̂śîón̂ś n̂éĝát̂ív̂él̂ý âf́f̂éĉt́êd́ t̂h́îś p̂áĝé'ŝ ĺôád̂ ṕêŕf̂ór̂ḿâńĉé. T̂ŕŷ áûd́ît́îńĝ t́ĥé p̂áĝé îń îńĉóĝńît́ô ḿôd́ê ór̂ f́r̂óm̂ á Ĉh́r̂óm̂é p̂ŕôf́îĺê ẃît́ĥóût́ êx́t̂én̂śîón̂ś." }, From 795593fdf7cfea41ec77e9dfc3e8c3d625f37b9c Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 27 Oct 2022 11:02:36 -0700 Subject: [PATCH 09/20] passive flow --- core/audits/bf-cache.js | 2 +- core/gather/gatherers/bf-cache-errors.js | 156 ++++++++++++++--------- core/user-flow.js | 8 ++ 3 files changed, 106 insertions(+), 60 deletions(-) diff --git a/core/audits/bf-cache.js b/core/audits/bf-cache.js index faed667fa939..13879a091479 100644 --- a/core/audits/bf-cache.js +++ b/core/audits/bf-cache.js @@ -44,7 +44,7 @@ class BFCache extends Audit { title: str_(UIStrings.title), failureTitle: str_(UIStrings.failureTitle), description: str_(UIStrings.description), - supportedModes: ['navigation'], + supportedModes: ['navigation', 'timespan'], requiredArtifacts: ['BFCacheErrors'], }; } diff --git a/core/gather/gatherers/bf-cache-errors.js b/core/gather/gatherers/bf-cache-errors.js index 60421e60311a..ba670dc68ce1 100644 --- a/core/gather/gatherers/bf-cache-errors.js +++ b/core/gather/gatherers/bf-cache-errors.js @@ -6,85 +6,93 @@ import FRGatherer from '../base-gatherer.js'; import {waitForFrameNavigated, waitForLoadEvent} from '../driver/wait-for-condition.js'; +import DevtoolsLog from './devtools-log.js'; -/** - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} errorList - * @return {LH.Artifacts.BFCacheErrors} - */ -function constructArtifactFromList(errorList) { - /** @type {LH.Artifacts.BFCacheErrors} */ - const bfCacheErrors = { - Circumstantial: {}, - PageSupportNeeded: {}, - SupportPending: {}, - }; - - for (const err of errorList) { - const bfCacheErrorsMap = bfCacheErrors[err.type]; - bfCacheErrorsMap[err.reason] = []; - } - - return bfCacheErrors; -} - -/** - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} errorTree - * @return {LH.Artifacts.BFCacheErrors} - */ -function constructArtifactFromTree(errorTree) { - /** @type {LH.Artifacts.BFCacheErrors} */ - const bfCacheErrors = { - Circumstantial: {}, - PageSupportNeeded: {}, - SupportPending: {}, +class BFCacheErrors extends FRGatherer { + /** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'>} */ + meta = { + supportedModes: ['navigation', 'timespan'], + dependencies: {DevtoolsLog: DevtoolsLog.symbol}, }; /** - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} node + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} errorList + * @return {LH.Artifacts.BFCacheErrors} */ - function traverse(node) { - for (const error of node.explanations) { - const bfCacheErrorsMap = bfCacheErrors[error.type]; - const frameUrls = bfCacheErrorsMap[error.reason] || []; - frameUrls.push(node.url); - bfCacheErrorsMap[error.reason] = frameUrls; + createArtifactFromList(errorList) { + /** @type {LH.Artifacts.BFCacheErrors} */ + const bfCacheErrors = { + Circumstantial: {}, + PageSupportNeeded: {}, + SupportPending: {}, + }; + + for (const err of errorList) { + const bfCacheErrorsMap = bfCacheErrors[err.type]; + bfCacheErrorsMap[err.reason] = []; } - for (const child of node.children) { - traverse(child); - } + return bfCacheErrors; } - traverse(errorTree); + /** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} errorTree + * @return {LH.Artifacts.BFCacheErrors} + */ + createArtifactFromTree(errorTree) { + /** @type {LH.Artifacts.BFCacheErrors} */ + const bfCacheErrors = { + Circumstantial: {}, + PageSupportNeeded: {}, + SupportPending: {}, + }; - return bfCacheErrors; -} + /** + * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} node + */ + function traverse(node) { + for (const error of node.explanations) { + const bfCacheErrorsMap = bfCacheErrors[error.type]; + const frameUrls = bfCacheErrorsMap[error.reason] || []; + frameUrls.push(node.url); + bfCacheErrorsMap[error.reason] = frameUrls; + } + + for (const child of node.children) { + traverse(child); + } + } + traverse(errorTree); -class BFCacheErrors extends FRGatherer { - /** @type {LH.Gatherer.GathererMeta} */ - meta = { - supportedModes: ['navigation'], - }; + return bfCacheErrors; + } + + /** + * @param {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} event + */ + createArtifactFromEvent(event) { + if (event?.notRestoredExplanationsTree) { + return this.createArtifactFromTree(event.notRestoredExplanationsTree); + } + return this.createArtifactFromList(event?.notRestoredExplanations || []); + } /** * @param {LH.Gatherer.FRTransitionalContext} context - * @return {Promise} All visible tap targets with their positions and sizes + * @return {Promise} */ - async getArtifact(context) { + async activelyCollectBFCacheEvent(context) { const session = context.driver.defaultSession; - /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} */ - let list = []; - /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree|undefined} */ - let tree = undefined; + /** @type {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} */ + let bfCacheEvent = undefined; /** * @param {LH.Crdp.Page.BackForwardCacheNotUsedEvent} event */ function onBfCacheNotUsed(event) { - list = event.notRestoredExplanations; - tree = event.notRestoredExplanationsTree; + bfCacheEvent = event; } session.on('Page.backForwardCacheNotUsed', onBfCacheNotUsed); @@ -104,10 +112,40 @@ class BFCacheErrors extends FRGatherer { session.off('Page.backForwardCacheNotUsed', onBfCacheNotUsed); - if (tree) { - return constructArtifactFromTree(tree); + return bfCacheEvent; + } + + /** + * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context + * @return {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} + */ + passivelyCollectBFCacheEvent(context) { + for (const event of context.dependencies.DevtoolsLog) { + if (event.method === 'Page.backForwardCacheNotUsed') { + return event.params; + } } - return constructArtifactFromList(list); + } + + /** + * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context + * @return {Promise} + */ + async getArtifact(context) { + const event = context.gatherMode === 'navigation' ? + await this.activelyCollectBFCacheEvent(context) : + this.passivelyCollectBFCacheEvent(context); + + return this.createArtifactFromEvent(event); + } + + /** + * @param {LH.Gatherer.PassContext} passContext + * @param {LH.Gatherer.LoadData} loadData + * @return {Promise} + */ + async afterPass(passContext, loadData) { + return this.getArtifact({...passContext, dependencies: {DevtoolsLog: loadData.devtoolsLog}}) } } diff --git a/core/user-flow.js b/core/user-flow.js index a53d3de855ff..538020508d92 100644 --- a/core/user-flow.js +++ b/core/user-flow.js @@ -77,6 +77,14 @@ class UserFlow { newStepFlags.skipAboutBlank = true; } + // BFCache will actively load the page in navigation mode. + // To prevent this from impacting future flow steps, we can disable the audit. + if (!newStepFlags.skipAudits) { + newStepFlags.skipAudits = ['bf-cache']; + } else if (!newStepFlags.skipAudits.includes('bf-cache')) { + newStepFlags.skipAudits.push('bf-cache'); + } + // On repeat navigations, we want to disable storage reset by default (i.e. it's not a cold load). const isSubsequentNavigation = this._gatherSteps .some(step => step.artifacts.GatherContext.gatherMode === 'navigation'); From e4aa601b03a42c5794a8307dd4e9dddfb69a38bf Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 1 Nov 2022 11:51:43 -0700 Subject: [PATCH 10/20] semi --- core/gather/gatherers/bf-cache-errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/gather/gatherers/bf-cache-errors.js b/core/gather/gatherers/bf-cache-errors.js index ba670dc68ce1..3b2d01633e31 100644 --- a/core/gather/gatherers/bf-cache-errors.js +++ b/core/gather/gatherers/bf-cache-errors.js @@ -145,7 +145,7 @@ class BFCacheErrors extends FRGatherer { * @return {Promise} */ async afterPass(passContext, loadData) { - return this.getArtifact({...passContext, dependencies: {DevtoolsLog: loadData.devtoolsLog}}) + return this.getArtifact({...passContext, dependencies: {DevtoolsLog: loadData.devtoolsLog}}); } } From 942f9a4a0d063aa558d926d7aafc3e1a7e04dcfc Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 1 Nov 2022 11:55:43 -0700 Subject: [PATCH 11/20] rm audit --- .../test-definitions/dobetterweb.js | 5 - .../test-definitions/perf-budgets.js | 4 - .../smokehouse/test-definitions/perf-fonts.js | 4 - .../test-definitions/perf-frame-metrics.js | 4 - .../test-definitions/perf-preload.js | 4 - .../test-definitions/perf-trace-elements.js | 4 - core/audits/bf-cache.js | 141 ------------------ core/config/default-config.js | 7 +- .../reports/sample-flow-result.json | 70 +-------- core/test/results/sample_v2.json | 33 ---- core/user-flow.js | 8 - shared/localization/locales/en-US.json | 21 --- shared/localization/locales/en-XL.json | 21 --- 13 files changed, 3 insertions(+), 323 deletions(-) delete mode 100644 core/audits/bf-cache.js diff --git a/cli/test/smokehouse/test-definitions/dobetterweb.js b/cli/test/smokehouse/test-definitions/dobetterweb.js index 98eb969e8475..0aa655a5b4a6 100644 --- a/cli/test/smokehouse/test-definitions/dobetterweb.js +++ b/cli/test/smokehouse/test-definitions/dobetterweb.js @@ -7,11 +7,6 @@ /** @type {LH.Config.Json} */ const config = { extends: 'lighthouse:default', - settings: { - // BF cache will request the page again, initiating additional network requests. - // Disable the audit so we only detect requests from the normal page load. - skipAudits: ['bf-cache'], - }, audits: [ // Test the `ignoredPatterns` audit option. {path: 'errors-in-console', options: {ignoredPatterns: ['An ignored error']}}, diff --git a/cli/test/smokehouse/test-definitions/perf-budgets.js b/cli/test/smokehouse/test-definitions/perf-budgets.js index 72cb8dafc66b..c25b20072281 100644 --- a/cli/test/smokehouse/test-definitions/perf-budgets.js +++ b/cli/test/smokehouse/test-definitions/perf-budgets.js @@ -13,10 +13,6 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page again, initiating additional network requests. - // Disable the audit so we only detect requests from the normal page load. - skipAudits: ['bf-cache'], - // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-fonts.js b/cli/test/smokehouse/test-definitions/perf-fonts.js index 1fd094ca2ab6..689272a9701f 100644 --- a/cli/test/smokehouse/test-definitions/perf-fonts.js +++ b/cli/test/smokehouse/test-definitions/perf-fonts.js @@ -13,10 +13,6 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page again, initiating additional network requests. - // Disable the audit so we only detect requests from the normal page load. - skipAudits: ['bf-cache'], - // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-frame-metrics.js b/cli/test/smokehouse/test-definitions/perf-frame-metrics.js index f2934c0ec644..38cd15dbdd8d 100644 --- a/cli/test/smokehouse/test-definitions/perf-frame-metrics.js +++ b/cli/test/smokehouse/test-definitions/perf-frame-metrics.js @@ -13,10 +13,6 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page again, initiating additional network requests. - // Disable the audit so we only detect requests from the normal page load. - skipAudits: ['bf-cache'], - // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-preload.js b/cli/test/smokehouse/test-definitions/perf-preload.js index f8262945abbd..8f77238dd23b 100644 --- a/cli/test/smokehouse/test-definitions/perf-preload.js +++ b/cli/test/smokehouse/test-definitions/perf-preload.js @@ -13,10 +13,6 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page again, initiating additional network requests. - // Disable the audit so we only detect requests from the normal page load. - skipAudits: ['bf-cache'], - // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/cli/test/smokehouse/test-definitions/perf-trace-elements.js b/cli/test/smokehouse/test-definitions/perf-trace-elements.js index d300cc00cb6d..e27849174e4b 100644 --- a/cli/test/smokehouse/test-definitions/perf-trace-elements.js +++ b/cli/test/smokehouse/test-definitions/perf-trace-elements.js @@ -13,10 +13,6 @@ const config = { // webpages present here, hence the inclusion of 'best-practices'. onlyCategories: ['performance', 'best-practices'], - // BF cache will request the page again, initiating additional network requests. - // Disable the audit so we only detect requests from the normal page load. - skipAudits: ['bf-cache'], - // A mixture of under, over, and meeting budget to exercise all paths. budgets: [{ path: '/', diff --git a/core/audits/bf-cache.js b/core/audits/bf-cache.js deleted file mode 100644 index 13879a091479..000000000000 --- a/core/audits/bf-cache.js +++ /dev/null @@ -1,141 +0,0 @@ -/** - * @license Copyright 2022 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. - */ - -import {Audit} from './audit.js'; -import * as i18n from '../lib/i18n/i18n.js'; -import {NotRestoredReasonDescription} from '../lib/bfcache-strings.js'; - -/* eslint-disable max-len */ -const UIStrings = { - /** TODO */ - title: 'Back/forward cache is used', - /** TODO */ - failureTitle: 'Back/forward cache is not used', - /** TODO */ - description: 'The back/forward cache can speed up the page load after navigating away.', - /** TODO */ - actionableColumn: 'Actionable failure', - /** TODO */ - notActionableColumn: 'Not actionable failure', - /** TODO */ - supportPendingColumn: 'Pending browser support', - /** - * @description [ICU Syntax] Label for an audit identifying the number of back/forward cache failure reasons found in the page. - */ - displayValue: `{itemCount, plural, - =1 {1 actionable failure reason} - other {# actionable failure reasons} - }`, -}; -/* eslint-enable max-len */ - -const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings); - -class BFCache extends Audit { - /** - * @return {LH.Audit.Meta} - */ - static get meta() { - return { - id: 'bf-cache', - title: str_(UIStrings.title), - failureTitle: str_(UIStrings.failureTitle), - description: str_(UIStrings.description), - supportedModes: ['navigation', 'timespan'], - requiredArtifacts: ['BFCacheErrors'], - }; - } - - /** - * @param {LH.Crdp.Page.BackForwardCacheNotRestoredReason} reason - */ - static getDescriptionForReason(reason) { - const matchingString = NotRestoredReasonDescription[reason]; - - if (matchingString === undefined) { - return reason; - } - - return matchingString.name; - } - - /** - * @param {LH.Artifacts.BFCacheErrorMap} errors - * @param {LH.IcuMessage | string} label - * @return {LH.Audit.Details.Table} - */ - static makeTableForFailureType(errors, label) { - /** @type {LH.Audit.Details.TableItem[]} */ - const results = []; - - // https://github.com/Microsoft/TypeScript/issues/12870 - const reasons = /** @type {LH.Crdp.Page.BackForwardCacheNotRestoredReason[]} */ - (Object.keys(errors)); - - for (const reason of reasons) { - const frameUrls = errors[reason] || []; - results.push({ - reason: this.getDescriptionForReason(reason), - subItems: { - type: 'subitems', - items: frameUrls.map(frameUrl => ({frameUrl})), - }, - }); - } - - /** @type {LH.Audit.Details.Table['headings']} */ - const headings = [ - /* eslint-disable max-len */ - {key: 'reason', valueType: 'text', subItemsHeading: {key: 'frameUrl', valueType: 'url'}, label}, - /* eslint-enable max-len */ - ]; - - return Audit.makeTableDetails(headings, results); - } - - /** - * @param {LH.Artifacts} artifacts - * @return {Promise} - */ - static async audit(artifacts) { - const {PageSupportNeeded, SupportPending, Circumstantial} = artifacts.BFCacheErrors; - - const actionableTable = - this.makeTableForFailureType(PageSupportNeeded, str_(UIStrings.actionableColumn)); - const notActionableTable = - this.makeTableForFailureType(Circumstantial, str_(UIStrings.notActionableColumn)); - const supportPendingTable = - this.makeTableForFailureType(SupportPending, str_(UIStrings.supportPendingColumn)); - - const items = [ - actionableTable, - notActionableTable, - supportPendingTable, - ]; - - if (actionableTable.items.length === 0) { - return { - score: 1, - details: { - type: 'list', - items, - }, - }; - } - - return { - score: 0, - displayValue: str_(UIStrings.displayValue, {itemCount: actionableTable.items.length}), - details: { - type: 'list', - items, - }, - }; - } -} - -export default BFCache; -export {UIStrings}; diff --git a/core/config/default-config.js b/core/config/default-config.js index 3d18007b4c2f..ddff2f8e4397 100644 --- a/core/config/default-config.js +++ b/core/config/default-config.js @@ -217,11 +217,8 @@ const defaultConfig = { {id: artifacts.devtoolsLogs, gatherer: 'devtools-log-compat'}, {id: artifacts.traces, gatherer: 'trace-compat'}, - // FullPageScreenshot comes at the end so all other node analysis is captured. + // FullPageScreenshot comes at the very end so all other node analysis is captured. {id: artifacts.FullPageScreenshot, gatherer: 'full-page-screenshot'}, - - // BFCacheErrors comes at the very end because it can perform a page navigation. - {id: artifacts.BFCacheErrors, gatherer: 'bf-cache-errors'}, ], audits: [ 'is-on-https', @@ -377,7 +374,6 @@ const defaultConfig = { 'seo/canonical', 'seo/manual/structured-data', 'work-during-interaction', - 'bf-cache', ], groups: { 'metrics': { @@ -520,7 +516,6 @@ const defaultConfig = { {id: 'no-unload-listeners', weight: 0}, {id: 'uses-responsive-images-snapshot', weight: 0}, {id: 'work-during-interaction', weight: 0}, - {id: 'bf-cache', weight: 0}, // Budget audits. {id: 'performance-budget', weight: 0, group: 'budgets'}, diff --git a/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json b/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json index f82a4b889a67..b938ffea439c 100644 --- a/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json +++ b/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json @@ -3687,14 +3687,6 @@ "description": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more about Structured Data](https://web.dev/structured-data/).", "score": null, "scoreDisplayMode": "manual" - }, - "bf-cache": { - "id": "bf-cache", - "title": "Back/forward cache is used", - "description": "The back/forward cache can speed up the page load after navigating away.", - "score": null, - "scoreDisplayMode": "error", - "errorMessage": "Required BFCacheErrors gatherer did not run." } }, "configSettings": { @@ -4001,10 +3993,6 @@ "id": "no-unload-listeners", "weight": 0 }, - { - "id": "bf-cache", - "weight": 0 - }, { "id": "performance-budget", "weight": 0, @@ -6048,18 +6036,12 @@ }, { "startTime": 226, - "name": "lh:audit:bf-cache", - "duration": 1, - "entryType": "measure" - }, - { - "startTime": 227, "name": "lh:runner:generate", "duration": 1, "entryType": "measure" } ], - "total": 228 + "total": 227 }, "i18n": { "rendererFormattedStrings": { @@ -7239,21 +7221,6 @@ "core/audits/seo/manual/structured-data.js | description": [ "audits[structured-data].description" ], - "core/audits/bf-cache.js | title": [ - "audits[bf-cache].title" - ], - "core/audits/bf-cache.js | description": [ - "audits[bf-cache].description" - ], - "core/lib/lh-error.js | missingRequiredArtifact": [ - { - "values": { - "errorCode": "MISSING_REQUIRED_ARTIFACT", - "artifactName": "BFCacheErrors" - }, - "path": "audits[bf-cache].errorMessage" - } - ], "core/config/default-config.js | performanceCategoryTitle": [ "categories.performance.title" ], @@ -17876,14 +17843,6 @@ "description": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more about Structured Data](https://web.dev/structured-data/).", "score": null, "scoreDisplayMode": "manual" - }, - "bf-cache": { - "id": "bf-cache", - "title": "Back/forward cache is used", - "description": "The back/forward cache can speed up the page load after navigating away.", - "score": null, - "scoreDisplayMode": "error", - "errorMessage": "Required BFCacheErrors gatherer did not run." } }, "configSettings": { @@ -18190,10 +18149,6 @@ "id": "no-unload-listeners", "weight": 0 }, - { - "id": "bf-cache", - "weight": 0 - }, { "id": "performance-budget", "weight": 0, @@ -20219,18 +20174,12 @@ }, { "startTime": 223, - "name": "lh:audit:bf-cache", - "duration": 1, - "entryType": "measure" - }, - { - "startTime": 224, "name": "lh:runner:generate", "duration": 1, "entryType": "measure" } ], - "total": 225 + "total": 224 }, "i18n": { "rendererFormattedStrings": { @@ -21426,21 +21375,6 @@ "core/audits/seo/manual/structured-data.js | description": [ "audits[structured-data].description" ], - "core/audits/bf-cache.js | title": [ - "audits[bf-cache].title" - ], - "core/audits/bf-cache.js | description": [ - "audits[bf-cache].description" - ], - "core/lib/lh-error.js | missingRequiredArtifact": [ - { - "values": { - "errorCode": "MISSING_REQUIRED_ARTIFACT", - "artifactName": "BFCacheErrors" - }, - "path": "audits[bf-cache].errorMessage" - } - ], "core/config/default-config.js | performanceCategoryTitle": [ "categories.performance.title" ], diff --git a/core/test/results/sample_v2.json b/core/test/results/sample_v2.json index 0bf9ec79fcc1..3aa394c02143 100644 --- a/core/test/results/sample_v2.json +++ b/core/test/results/sample_v2.json @@ -5831,14 +5831,6 @@ "description": "Run the [Structured Data Testing Tool](https://search.google.com/structured-data/testing-tool/) and the [Structured Data Linter](http://linter.structured-data.org/) to validate structured data. [Learn more about Structured Data](https://web.dev/structured-data/).", "score": null, "scoreDisplayMode": "manual" - }, - "bf-cache": { - "id": "bf-cache", - "title": "Back/forward cache is used", - "description": "The back/forward cache can speed up the page load after navigating away.", - "score": null, - "scoreDisplayMode": "error", - "errorMessage": "Required BFCacheErrors gatherer did not run." } }, "configSettings": { @@ -6247,10 +6239,6 @@ "id": "no-unload-listeners", "weight": 0 }, - { - "id": "bf-cache", - "weight": 0 - }, { "id": "performance-budget", "weight": 0, @@ -8304,12 +8292,6 @@ "duration": 100, "entryType": "measure" }, - { - "startTime": 0, - "name": "lh:audit:bf-cache", - "duration": 100, - "entryType": "measure" - }, { "startTime": 0, "name": "lh:runner:generate", @@ -9724,21 +9706,6 @@ "core/audits/seo/manual/structured-data.js | description": [ "audits[structured-data].description" ], - "core/audits/bf-cache.js | title": [ - "audits[bf-cache].title" - ], - "core/audits/bf-cache.js | description": [ - "audits[bf-cache].description" - ], - "core/lib/lh-error.js | missingRequiredArtifact": [ - { - "values": { - "errorCode": "MISSING_REQUIRED_ARTIFACT", - "artifactName": "BFCacheErrors" - }, - "path": "audits[bf-cache].errorMessage" - } - ], "core/config/default-config.js | performanceCategoryTitle": [ "categories.performance.title" ], diff --git a/core/user-flow.js b/core/user-flow.js index 0e178644b1b5..e58bef8c3123 100644 --- a/core/user-flow.js +++ b/core/user-flow.js @@ -88,14 +88,6 @@ class UserFlow { nextFlags.skipAboutBlank = true; } - // BFCache will actively load the page in navigation mode. - // To prevent this from impacting future flow steps, we can disable the audit. - if (!newStepFlags.skipAudits) { - newStepFlags.skipAudits = ['bf-cache']; - } else if (!newStepFlags.skipAudits.includes('bf-cache')) { - newStepFlags.skipAudits.push('bf-cache'); - } - // On repeat navigations, we want to disable storage reset by default (i.e. it's not a cold load). const isSubsequentNavigation = this._gatherSteps .some(step => step.artifacts.GatherContext.gatherMode === 'navigation'); diff --git a/shared/localization/locales/en-US.json b/shared/localization/locales/en-US.json index 3eea9be362ea..a8caac294acc 100644 --- a/shared/localization/locales/en-US.json +++ b/shared/localization/locales/en-US.json @@ -425,27 +425,6 @@ "core/audits/autocomplete.js | warningOrder": { "message": "Review order of tokens: \"{tokens}\" in {snippet}" }, - "core/audits/bf-cache.js | actionableColumn": { - "message": "Actionable failure" - }, - "core/audits/bf-cache.js | description": { - "message": "The back/forward cache can speed up the page load after navigating away." - }, - "core/audits/bf-cache.js | displayValue": { - "message": "{itemCount, plural,\n =1 {1 actionable failure reason}\n other {# actionable failure reasons}\n }" - }, - "core/audits/bf-cache.js | failureTitle": { - "message": "Back/forward cache is not used" - }, - "core/audits/bf-cache.js | notActionableColumn": { - "message": "Not actionable failure" - }, - "core/audits/bf-cache.js | supportPendingColumn": { - "message": "Pending browser support" - }, - "core/audits/bf-cache.js | title": { - "message": "Back/forward cache is used" - }, "core/audits/bootup-time.js | chromeExtensionsWarning": { "message": "Chrome extensions negatively affected this page's load performance. Try auditing the page in incognito mode or from a Chrome profile without extensions." }, diff --git a/shared/localization/locales/en-XL.json b/shared/localization/locales/en-XL.json index 00d650bfa32b..940c24c9cfae 100644 --- a/shared/localization/locales/en-XL.json +++ b/shared/localization/locales/en-XL.json @@ -425,27 +425,6 @@ "core/audits/autocomplete.js | warningOrder": { "message": "R̂év̂íêẃ ôŕd̂ér̂ óf̂ t́ôḱêńŝ: \"{tokens}\" ín̂ {snippet}" }, - "core/audits/bf-cache.js | actionableColumn": { - "message": "Âćt̂íôńâb́l̂é f̂áîĺûŕê" - }, - "core/audits/bf-cache.js | description": { - "message": "T̂h́ê b́âćk̂/f́ôŕŵár̂d́ ĉáĉh́ê ćâń ŝṕêéd̂ úp̂ t́ĥé p̂áĝé l̂óâd́ âf́t̂ér̂ ńâv́îǵât́îńĝ áŵáŷ." - }, - "core/audits/bf-cache.js | displayValue": { - "message": "{itemCount, plural,\n =1 {1 âćt̂íôńâb́l̂é f̂áîĺûŕê ŕêáŝón̂}\n other {# áĉt́îón̂áb̂ĺê f́âíl̂úr̂é r̂éâśôńŝ}\n }" - }, - "core/audits/bf-cache.js | failureTitle": { - "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé îś n̂ót̂ úŝéd̂" - }, - "core/audits/bf-cache.js | notActionableColumn": { - "message": "N̂ót̂ áĉt́îón̂áb̂ĺê f́âíl̂úr̂é" - }, - "core/audits/bf-cache.js | supportPendingColumn": { - "message": "P̂én̂d́îńĝ b́r̂óŵśêŕ ŝúp̂ṕôŕt̂" - }, - "core/audits/bf-cache.js | title": { - "message": "B̂áĉḱ/f̂ór̂ẃâŕd̂ ćâćĥé îś ûśêd́" - }, "core/audits/bootup-time.js | chromeExtensionsWarning": { "message": "Ĉh́r̂óm̂é êx́t̂én̂śîón̂ś n̂éĝát̂ív̂él̂ý âf́f̂éĉt́êd́ t̂h́îś p̂áĝé'ŝ ĺôád̂ ṕêŕf̂ór̂ḿâńĉé. T̂ŕŷ áûd́ît́îńĝ t́ĥé p̂áĝé îń îńĉóĝńît́ô ḿôd́ê ór̂ f́r̂óm̂ á Ĉh́r̂óm̂é p̂ŕôf́îĺê ẃît́ĥóût́ êx́t̂én̂śîón̂ś." }, From 01ddc2ae14db76a62c8d663848324eab2ac19374 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 1 Nov 2022 11:58:37 -0700 Subject: [PATCH 12/20] comment --- types/artifacts.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 57c02b573fa3..e67a2140f77f 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -122,6 +122,7 @@ export interface GathererArtifacts extends PublicGathererArtifacts,LegacyBaseArt Accessibility: Artifacts.Accessibility; /** Array of all anchors on the page. */ AnchorElements: Artifacts.AnchorElement[]; + /** Errors when attempting to use the back/forward cache. */ BFCacheErrors: Artifacts.BFCacheErrors; /** Array of all URLs cached in CacheStorage. */ CacheContents: string[]; From 395b442027ff76b5681aedbe3ae91aa6098f05d6 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 1 Nov 2022 14:26:18 -0700 Subject: [PATCH 13/20] unit test --- .../gather/gatherers/bf-cache-errors-test.js | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 core/test/gather/gatherers/bf-cache-errors-test.js diff --git a/core/test/gather/gatherers/bf-cache-errors-test.js b/core/test/gather/gatherers/bf-cache-errors-test.js new file mode 100644 index 000000000000..67b976f8f39e --- /dev/null +++ b/core/test/gather/gatherers/bf-cache-errors-test.js @@ -0,0 +1,188 @@ +/** +* @license Copyright 2022 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. + */ + +import BFCacheErrors from '../../../gather/gatherers/bf-cache-errors.js'; +import {createMockContext} from '../mock-driver.js'; + +describe('BFCacheErrors', () => { + /** @type {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} */ + let context; + let mockContext = createMockContext(); + /** @type {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} */ + let mockBfCacheEvent; + + beforeEach(() => { + mockContext = createMockContext(); + // @ts-expect-error contains DT log dependency + context = mockContext.asContext(); + + mockBfCacheEvent = { + loaderId: 'LOADERID', + frameId: 'FRAMEID', + notRestoredExplanations: [ + {type: 'PageSupportNeeded', reason: 'AppBanner'}, + {type: 'Circumstantial', reason: 'BackForwardCacheDisabled'}, + {type: 'SupportPending', reason: 'CacheControlNoStore'}, + ], + notRestoredExplanationsTree: { + url: 'https://example.com', + explanations: [ + {type: 'PageSupportNeeded', reason: 'AppBanner'}, + {type: 'Circumstantial', reason: 'BackForwardCacheDisabled'}, + ], + children: [ + { + url: 'https://frame.com', + explanations: [ + {type: 'PageSupportNeeded', reason: 'AppBanner'}, + {type: 'SupportPending', reason: 'CacheControlNoStore'}, + ], + children: [], + }, + ], + }, + }; + + context.dependencies.DevtoolsLog = [{ + method: 'Page.backForwardCacheNotUsed', + params: mockBfCacheEvent, + }]; + + mockContext.driver.defaultSession.sendCommand + .mockResponse('Page.getNavigationHistory', { + currentIndex: 1, + entries: [ + {id: 0}, + {id: 1}, + ], + }) + .mockResponse('Page.navigate', undefined) + .mockResponse('Page.navigateToHistoryEntry', () => { + if (mockBfCacheEvent) { + const listener = + mockContext.driver.defaultSession.on.findListener('Page.backForwardCacheNotUsed'); + listener(mockBfCacheEvent); + } + }); + + mockContext.driver.defaultSession.once + .mockEvent('Page.loadEventFired', {}) + .mockEvent('Page.frameNavigated', {}); + }); + + it('actively triggers bf cache in navigation mode', async () => { + const gatherer = new BFCacheErrors(); + const artifact = await gatherer.getArtifact(context); + + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); + + expect(artifact).toEqual({ + PageSupportNeeded: { + AppBanner: ['https://example.com', 'https://frame.com'], + }, + Circumstantial: { + BackForwardCacheDisabled: ['https://example.com'], + }, + SupportPending: { + CacheControlNoStore: ['https://frame.com'], + }, + }); + }); + + it('actively triggers bf cache in legacy navigation mode', async () => { + const gatherer = new BFCacheErrors(); + const artifact = await gatherer.afterPass(mockContext.asLegacyContext(), { + devtoolsLog: context.dependencies.DevtoolsLog, + networkRecords: [], + }); + + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); + + expect(artifact).toEqual({ + PageSupportNeeded: { + AppBanner: ['https://example.com', 'https://frame.com'], + }, + Circumstantial: { + BackForwardCacheDisabled: ['https://example.com'], + }, + SupportPending: { + CacheControlNoStore: ['https://frame.com'], + }, + }); + }); + + it('passively collects bf cache event in timespan mode', async () => { + context.gatherMode = 'timespan'; + + const gatherer = new BFCacheErrors(); + const artifact = await gatherer.getArtifact(context); + + expect(mockContext.driver.defaultSession.sendCommand) + .not.toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + expect(mockContext.driver.defaultSession.sendCommand) + .not.toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); + + expect(artifact).toEqual({ + PageSupportNeeded: { + AppBanner: ['https://example.com', 'https://frame.com'], + }, + Circumstantial: { + BackForwardCacheDisabled: ['https://example.com'], + }, + SupportPending: { + CacheControlNoStore: ['https://frame.com'], + }, + }); + }); + + it('constructs a tree with no frame urls if no frame tree is provided', async () => { + delete mockBfCacheEvent?.notRestoredExplanationsTree; + + const gatherer = new BFCacheErrors(); + const artifact = await gatherer.getArtifact(context); + + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); + + expect(artifact).toEqual({ + PageSupportNeeded: { + AppBanner: [], + }, + Circumstantial: { + BackForwardCacheDisabled: [], + }, + SupportPending: { + CacheControlNoStore: [], + }, + }); + }); + + it('constructs an empty structure if no event was found', async () => { + mockBfCacheEvent = undefined; + + const gatherer = new BFCacheErrors(); + const artifact = await gatherer.getArtifact(context); + + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + expect(mockContext.driver.defaultSession.sendCommand) + .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); + + expect(artifact).toEqual({ + PageSupportNeeded: {}, + Circumstantial: {}, + SupportPending: {}, + }); + }); +}); From 36a0c19cbcd4c8955952f8dcdf2dd3f559cc6e95 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Wed, 9 Nov 2022 15:43:15 -0800 Subject: [PATCH 14/20] restructure --- core/gather/gatherers/bf-cache-errors.js | 55 +++++---- .../gather/gatherers/bf-cache-errors-test.js | 110 ++++++++++-------- types/artifacts.d.ts | 10 +- 3 files changed, 96 insertions(+), 79 deletions(-) diff --git a/core/gather/gatherers/bf-cache-errors.js b/core/gather/gatherers/bf-cache-errors.js index 3b2d01633e31..a9fef2bc198e 100644 --- a/core/gather/gatherers/bf-cache-errors.js +++ b/core/gather/gatherers/bf-cache-errors.js @@ -8,7 +8,7 @@ import FRGatherer from '../base-gatherer.js'; import {waitForFrameNavigated, waitForLoadEvent} from '../driver/wait-for-condition.js'; import DevtoolsLog from './devtools-log.js'; -class BFCacheErrors extends FRGatherer { +class BFCache extends FRGatherer { /** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'>} */ meta = { supportedModes: ['navigation', 'timespan'], @@ -17,31 +17,31 @@ class BFCacheErrors extends FRGatherer { /** * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} errorList - * @return {LH.Artifacts.BFCacheErrors} + * @return {LH.Artifacts.ProcessedBFCacheEvent} */ - createArtifactFromList(errorList) { - /** @type {LH.Artifacts.BFCacheErrors} */ - const bfCacheErrors = { + static processBFCacheEventList(errorList) { + /** @type {LH.Artifacts.BFCacheNotRestoredReasonsTree} */ + const notRestoredReasonsTree = { Circumstantial: {}, PageSupportNeeded: {}, SupportPending: {}, }; for (const err of errorList) { - const bfCacheErrorsMap = bfCacheErrors[err.type]; + const bfCacheErrorsMap = notRestoredReasonsTree[err.type]; bfCacheErrorsMap[err.reason] = []; } - return bfCacheErrors; + return {notRestoredReasonsTree}; } /** * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} errorTree - * @return {LH.Artifacts.BFCacheErrors} + * @return {LH.Artifacts.ProcessedBFCacheEvent} */ - createArtifactFromTree(errorTree) { - /** @type {LH.Artifacts.BFCacheErrors} */ - const bfCacheErrors = { + static processBFCacheEventTree(errorTree) { + /** @type {LH.Artifacts.BFCacheNotRestoredReasonsTree} */ + const notRestoredReasonsTree = { Circumstantial: {}, PageSupportNeeded: {}, SupportPending: {}, @@ -52,7 +52,7 @@ class BFCacheErrors extends FRGatherer { */ function traverse(node) { for (const error of node.explanations) { - const bfCacheErrorsMap = bfCacheErrors[error.type]; + const bfCacheErrorsMap = notRestoredReasonsTree[error.type]; const frameUrls = bfCacheErrorsMap[error.reason] || []; frameUrls.push(node.url); bfCacheErrorsMap[error.reason] = frameUrls; @@ -65,17 +65,18 @@ class BFCacheErrors extends FRGatherer { traverse(errorTree); - return bfCacheErrors; + return {notRestoredReasonsTree}; } /** * @param {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} event + * @return {LH.Artifacts.ProcessedBFCacheEvent} */ - createArtifactFromEvent(event) { + static processBFCacheEvent(event) { if (event?.notRestoredExplanationsTree) { - return this.createArtifactFromTree(event.notRestoredExplanationsTree); + return BFCache.processBFCacheEventTree(event.notRestoredExplanationsTree); } - return this.createArtifactFromList(event?.notRestoredExplanations || []); + return BFCache.processBFCacheEventList(event?.notRestoredExplanations || []); } /** @@ -117,37 +118,41 @@ class BFCacheErrors extends FRGatherer { /** * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context - * @return {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} + * @return {LH.Crdp.Page.BackForwardCacheNotUsedEvent[]} */ passivelyCollectBFCacheEvent(context) { + const events = []; for (const event of context.dependencies.DevtoolsLog) { if (event.method === 'Page.backForwardCacheNotUsed') { - return event.params; + events.push(event.params); } } + return events; } /** * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context - * @return {Promise} + * @return {Promise} */ async getArtifact(context) { - const event = context.gatherMode === 'navigation' ? - await this.activelyCollectBFCacheEvent(context) : - this.passivelyCollectBFCacheEvent(context); + const events = this.passivelyCollectBFCacheEvent(context); + if (context.gatherMode === 'navigation') { + const activelyCollectedEvent = await this.activelyCollectBFCacheEvent(context); + if (activelyCollectedEvent) events.push(activelyCollectedEvent); + } - return this.createArtifactFromEvent(event); + return events.map(BFCache.processBFCacheEvent); } /** * @param {LH.Gatherer.PassContext} passContext * @param {LH.Gatherer.LoadData} loadData - * @return {Promise} + * @return {Promise} */ async afterPass(passContext, loadData) { return this.getArtifact({...passContext, dependencies: {DevtoolsLog: loadData.devtoolsLog}}); } } -export default BFCacheErrors; +export default BFCache; diff --git a/core/test/gather/gatherers/bf-cache-errors-test.js b/core/test/gather/gatherers/bf-cache-errors-test.js index 67b976f8f39e..490e6c1ca052 100644 --- a/core/test/gather/gatherers/bf-cache-errors-test.js +++ b/core/test/gather/gatherers/bf-cache-errors-test.js @@ -4,52 +4,56 @@ * 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. */ -import BFCacheErrors from '../../../gather/gatherers/bf-cache-errors.js'; +import BFCache from '../../../gather/gatherers/bf-cache-errors.js'; import {createMockContext} from '../mock-driver.js'; +/** + * @returns {LH.Crdp.Page.BackForwardCacheNotUsedEvent} + */ +function createMockBfCacheEvent() { + return { + loaderId: 'LOADERID', + frameId: 'FRAMEID', + notRestoredExplanations: [ + {type: 'PageSupportNeeded', reason: 'AppBanner'}, + {type: 'Circumstantial', reason: 'BackForwardCacheDisabled'}, + {type: 'SupportPending', reason: 'CacheControlNoStore'}, + ], + notRestoredExplanationsTree: { + url: 'https://example.com', + explanations: [ + {type: 'PageSupportNeeded', reason: 'AppBanner'}, + {type: 'Circumstantial', reason: 'BackForwardCacheDisabled'}, + ], + children: [ + { + url: 'https://frame.com', + explanations: [ + {type: 'PageSupportNeeded', reason: 'AppBanner'}, + {type: 'SupportPending', reason: 'CacheControlNoStore'}, + ], + children: [], + }, + ], + }, + }; +} + describe('BFCacheErrors', () => { /** @type {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} */ let context; let mockContext = createMockContext(); /** @type {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} */ - let mockBfCacheEvent; + let mockActiveBfCacheEvent; beforeEach(() => { mockContext = createMockContext(); // @ts-expect-error contains DT log dependency context = mockContext.asContext(); - mockBfCacheEvent = { - loaderId: 'LOADERID', - frameId: 'FRAMEID', - notRestoredExplanations: [ - {type: 'PageSupportNeeded', reason: 'AppBanner'}, - {type: 'Circumstantial', reason: 'BackForwardCacheDisabled'}, - {type: 'SupportPending', reason: 'CacheControlNoStore'}, - ], - notRestoredExplanationsTree: { - url: 'https://example.com', - explanations: [ - {type: 'PageSupportNeeded', reason: 'AppBanner'}, - {type: 'Circumstantial', reason: 'BackForwardCacheDisabled'}, - ], - children: [ - { - url: 'https://frame.com', - explanations: [ - {type: 'PageSupportNeeded', reason: 'AppBanner'}, - {type: 'SupportPending', reason: 'CacheControlNoStore'}, - ], - children: [], - }, - ], - }, - }; + mockActiveBfCacheEvent = createMockBfCacheEvent(); - context.dependencies.DevtoolsLog = [{ - method: 'Page.backForwardCacheNotUsed', - params: mockBfCacheEvent, - }]; + context.dependencies.DevtoolsLog = []; mockContext.driver.defaultSession.sendCommand .mockResponse('Page.getNavigationHistory', { @@ -61,10 +65,10 @@ describe('BFCacheErrors', () => { }) .mockResponse('Page.navigate', undefined) .mockResponse('Page.navigateToHistoryEntry', () => { - if (mockBfCacheEvent) { + if (mockActiveBfCacheEvent) { const listener = mockContext.driver.defaultSession.on.findListener('Page.backForwardCacheNotUsed'); - listener(mockBfCacheEvent); + listener(mockActiveBfCacheEvent); } }); @@ -74,7 +78,7 @@ describe('BFCacheErrors', () => { }); it('actively triggers bf cache in navigation mode', async () => { - const gatherer = new BFCacheErrors(); + const gatherer = new BFCache(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) @@ -82,7 +86,8 @@ describe('BFCacheErrors', () => { expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); - expect(artifact).toEqual({ + expect(artifact).toHaveLength(1); + expect(artifact[0].notRestoredReasonsTree).toEqual({ PageSupportNeeded: { AppBanner: ['https://example.com', 'https://frame.com'], }, @@ -96,7 +101,7 @@ describe('BFCacheErrors', () => { }); it('actively triggers bf cache in legacy navigation mode', async () => { - const gatherer = new BFCacheErrors(); + const gatherer = new BFCache(); const artifact = await gatherer.afterPass(mockContext.asLegacyContext(), { devtoolsLog: context.dependencies.DevtoolsLog, networkRecords: [], @@ -107,7 +112,8 @@ describe('BFCacheErrors', () => { expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); - expect(artifact).toEqual({ + expect(artifact).toHaveLength(1); + expect(artifact[0].notRestoredReasonsTree).toEqual({ PageSupportNeeded: { AppBanner: ['https://example.com', 'https://frame.com'], }, @@ -122,8 +128,12 @@ describe('BFCacheErrors', () => { it('passively collects bf cache event in timespan mode', async () => { context.gatherMode = 'timespan'; + context.dependencies.DevtoolsLog = [{ + method: 'Page.backForwardCacheNotUsed', + params: createMockBfCacheEvent(), + }]; - const gatherer = new BFCacheErrors(); + const gatherer = new BFCache(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) @@ -131,7 +141,8 @@ describe('BFCacheErrors', () => { expect(mockContext.driver.defaultSession.sendCommand) .not.toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); - expect(artifact).toEqual({ + expect(artifact).toHaveLength(1); + expect(artifact[0].notRestoredReasonsTree).toEqual({ PageSupportNeeded: { AppBanner: ['https://example.com', 'https://frame.com'], }, @@ -145,9 +156,9 @@ describe('BFCacheErrors', () => { }); it('constructs a tree with no frame urls if no frame tree is provided', async () => { - delete mockBfCacheEvent?.notRestoredExplanationsTree; + delete mockActiveBfCacheEvent?.notRestoredExplanationsTree; - const gatherer = new BFCacheErrors(); + const gatherer = new BFCache(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) @@ -155,7 +166,8 @@ describe('BFCacheErrors', () => { expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); - expect(artifact).toEqual({ + expect(artifact).toHaveLength(1); + expect(artifact[0].notRestoredReasonsTree).toEqual({ PageSupportNeeded: { AppBanner: [], }, @@ -168,10 +180,10 @@ describe('BFCacheErrors', () => { }); }); - it('constructs an empty structure if no event was found', async () => { - mockBfCacheEvent = undefined; + it('returns an empty list if no events were found passively or actively', async () => { + mockActiveBfCacheEvent = undefined; - const gatherer = new BFCacheErrors(); + const gatherer = new BFCache(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) @@ -179,10 +191,6 @@ describe('BFCacheErrors', () => { expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); - expect(artifact).toEqual({ - PageSupportNeeded: {}, - Circumstantial: {}, - SupportPending: {}, - }); + expect(artifact).toHaveLength(0); }); }); diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index e67a2140f77f..f34bdce2e457 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -123,7 +123,7 @@ export interface GathererArtifacts extends PublicGathererArtifacts,LegacyBaseArt /** Array of all anchors on the page. */ AnchorElements: Artifacts.AnchorElement[]; /** Errors when attempting to use the back/forward cache. */ - BFCacheErrors: Artifacts.BFCacheErrors; + BFCache: Artifacts.ProcessedBFCacheEvent[]; /** Array of all URLs cached in CacheStorage. */ CacheContents: string[]; /** CSS coverage information for styles used by page's final state. */ @@ -412,11 +412,15 @@ declare module Artifacts { }> } - type BFCacheErrorMap = { + type BFCacheReasonMap = { [key in LH.Crdp.Page.BackForwardCacheNotRestoredReason]?: string[]; }; - type BFCacheErrors = Record; + type BFCacheNotRestoredReasonsTree = Record; + + interface ProcessedBFCacheEvent { + notRestoredReasonsTree: BFCacheNotRestoredReasonsTree; + } interface Font { display: string; From 6de651fb95486af14d4fdd995124ba4b106ef9d8 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Wed, 9 Nov 2022 15:44:56 -0800 Subject: [PATCH 15/20] rn --- core/config/default-config.js | 2 +- core/gather/gatherers/{bf-cache-errors.js => bf-cache.js} | 0 .../gatherers/{bf-cache-errors-test.js => bf-cache-test.js} | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename core/gather/gatherers/{bf-cache-errors.js => bf-cache.js} (100%) rename core/test/gather/gatherers/{bf-cache-errors-test.js => bf-cache-test.js} (99%) diff --git a/core/config/default-config.js b/core/config/default-config.js index ddff2f8e4397..e06feabd78ad 100644 --- a/core/config/default-config.js +++ b/core/config/default-config.js @@ -128,7 +128,7 @@ const artifacts = { Trace: '', Accessibility: '', AnchorElements: '', - BFCacheErrors: '', + BFCache: '', CacheContents: '', ConsoleMessages: '', CSSUsage: '', diff --git a/core/gather/gatherers/bf-cache-errors.js b/core/gather/gatherers/bf-cache.js similarity index 100% rename from core/gather/gatherers/bf-cache-errors.js rename to core/gather/gatherers/bf-cache.js diff --git a/core/test/gather/gatherers/bf-cache-errors-test.js b/core/test/gather/gatherers/bf-cache-test.js similarity index 99% rename from core/test/gather/gatherers/bf-cache-errors-test.js rename to core/test/gather/gatherers/bf-cache-test.js index 490e6c1ca052..fa854cfaaf61 100644 --- a/core/test/gather/gatherers/bf-cache-errors-test.js +++ b/core/test/gather/gatherers/bf-cache-test.js @@ -4,7 +4,7 @@ * 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. */ -import BFCache from '../../../gather/gatherers/bf-cache-errors.js'; +import BFCache from '../../../gather/gatherers/bf-cache.js'; import {createMockContext} from '../mock-driver.js'; /** From e994f28fcca700ffee6b63bf591b594333419b09 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Wed, 9 Nov 2022 15:58:26 -0800 Subject: [PATCH 16/20] ope --- core/legacy/config/legacy-default-config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/legacy/config/legacy-default-config.js b/core/legacy/config/legacy-default-config.js index 49311c2e1f1a..608a7e34b41c 100644 --- a/core/legacy/config/legacy-default-config.js +++ b/core/legacy/config/legacy-default-config.js @@ -71,7 +71,7 @@ legacyDefaultConfig.passes = [{ 'inspector-issues', 'source-maps', 'full-page-screenshot', - 'bf-cache-errors', + 'bf-cache', ], }, { From 57ac86a3fe814a37b6ebf17f4e7e9638db36bcba Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 10 Nov 2022 11:12:32 -0800 Subject: [PATCH 17/20] rn --- core/config/default-config.js | 2 +- .../{bf-cache.js => bf-cache-failures.js} | 20 +++++++++---------- core/legacy/config/legacy-default-config.js | 2 +- ...ache-test.js => bf-cache-failures-test.js} | 14 ++++++------- types/artifacts.d.ts | 4 ++-- 5 files changed, 21 insertions(+), 21 deletions(-) rename core/gather/gatherers/{bf-cache.js => bf-cache-failures.js} (89%) rename core/test/gather/gatherers/{bf-cache-test.js => bf-cache-failures-test.js} (95%) diff --git a/core/config/default-config.js b/core/config/default-config.js index e06feabd78ad..4af31641767f 100644 --- a/core/config/default-config.js +++ b/core/config/default-config.js @@ -128,7 +128,7 @@ const artifacts = { Trace: '', Accessibility: '', AnchorElements: '', - BFCache: '', + BFCacheFailures: '', CacheContents: '', ConsoleMessages: '', CSSUsage: '', diff --git a/core/gather/gatherers/bf-cache.js b/core/gather/gatherers/bf-cache-failures.js similarity index 89% rename from core/gather/gatherers/bf-cache.js rename to core/gather/gatherers/bf-cache-failures.js index a9fef2bc198e..840ab8a8230c 100644 --- a/core/gather/gatherers/bf-cache.js +++ b/core/gather/gatherers/bf-cache-failures.js @@ -8,7 +8,7 @@ import FRGatherer from '../base-gatherer.js'; import {waitForFrameNavigated, waitForLoadEvent} from '../driver/wait-for-condition.js'; import DevtoolsLog from './devtools-log.js'; -class BFCache extends FRGatherer { +class BFCacheFailures extends FRGatherer { /** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'>} */ meta = { supportedModes: ['navigation', 'timespan'], @@ -17,7 +17,7 @@ class BFCache extends FRGatherer { /** * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanation[]} errorList - * @return {LH.Artifacts.ProcessedBFCacheEvent} + * @return {LH.Artifacts.BFCacheFailure} */ static processBFCacheEventList(errorList) { /** @type {LH.Artifacts.BFCacheNotRestoredReasonsTree} */ @@ -37,7 +37,7 @@ class BFCache extends FRGatherer { /** * @param {LH.Crdp.Page.BackForwardCacheNotRestoredExplanationTree} errorTree - * @return {LH.Artifacts.ProcessedBFCacheEvent} + * @return {LH.Artifacts.BFCacheFailure} */ static processBFCacheEventTree(errorTree) { /** @type {LH.Artifacts.BFCacheNotRestoredReasonsTree} */ @@ -70,13 +70,13 @@ class BFCache extends FRGatherer { /** * @param {LH.Crdp.Page.BackForwardCacheNotUsedEvent|undefined} event - * @return {LH.Artifacts.ProcessedBFCacheEvent} + * @return {LH.Artifacts.BFCacheFailure} */ static processBFCacheEvent(event) { if (event?.notRestoredExplanationsTree) { - return BFCache.processBFCacheEventTree(event.notRestoredExplanationsTree); + return BFCacheFailures.processBFCacheEventTree(event.notRestoredExplanationsTree); } - return BFCache.processBFCacheEventList(event?.notRestoredExplanations || []); + return BFCacheFailures.processBFCacheEventList(event?.notRestoredExplanations || []); } /** @@ -132,7 +132,7 @@ class BFCache extends FRGatherer { /** * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context - * @return {Promise} + * @return {Promise} */ async getArtifact(context) { const events = this.passivelyCollectBFCacheEvent(context); @@ -141,18 +141,18 @@ class BFCache extends FRGatherer { if (activelyCollectedEvent) events.push(activelyCollectedEvent); } - return events.map(BFCache.processBFCacheEvent); + return events.map(BFCacheFailures.processBFCacheEvent); } /** * @param {LH.Gatherer.PassContext} passContext * @param {LH.Gatherer.LoadData} loadData - * @return {Promise} + * @return {Promise} */ async afterPass(passContext, loadData) { return this.getArtifact({...passContext, dependencies: {DevtoolsLog: loadData.devtoolsLog}}); } } -export default BFCache; +export default BFCacheFailures; diff --git a/core/legacy/config/legacy-default-config.js b/core/legacy/config/legacy-default-config.js index 608a7e34b41c..d56bd3e9cc2f 100644 --- a/core/legacy/config/legacy-default-config.js +++ b/core/legacy/config/legacy-default-config.js @@ -71,7 +71,7 @@ legacyDefaultConfig.passes = [{ 'inspector-issues', 'source-maps', 'full-page-screenshot', - 'bf-cache', + 'bf-cache-failures', ], }, { diff --git a/core/test/gather/gatherers/bf-cache-test.js b/core/test/gather/gatherers/bf-cache-failures-test.js similarity index 95% rename from core/test/gather/gatherers/bf-cache-test.js rename to core/test/gather/gatherers/bf-cache-failures-test.js index fa854cfaaf61..4d0e7c33299e 100644 --- a/core/test/gather/gatherers/bf-cache-test.js +++ b/core/test/gather/gatherers/bf-cache-failures-test.js @@ -4,7 +4,7 @@ * 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. */ -import BFCache from '../../../gather/gatherers/bf-cache.js'; +import BFCacheFailures from '../../../gather/gatherers/bf-cache-failures.js'; import {createMockContext} from '../mock-driver.js'; /** @@ -39,7 +39,7 @@ function createMockBfCacheEvent() { }; } -describe('BFCacheErrors', () => { +describe('BFCacheFailures', () => { /** @type {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} */ let context; let mockContext = createMockContext(); @@ -78,7 +78,7 @@ describe('BFCacheErrors', () => { }); it('actively triggers bf cache in navigation mode', async () => { - const gatherer = new BFCache(); + const gatherer = new BFCacheFailures(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) @@ -101,7 +101,7 @@ describe('BFCacheErrors', () => { }); it('actively triggers bf cache in legacy navigation mode', async () => { - const gatherer = new BFCache(); + const gatherer = new BFCacheFailures(); const artifact = await gatherer.afterPass(mockContext.asLegacyContext(), { devtoolsLog: context.dependencies.DevtoolsLog, networkRecords: [], @@ -133,7 +133,7 @@ describe('BFCacheErrors', () => { params: createMockBfCacheEvent(), }]; - const gatherer = new BFCache(); + const gatherer = new BFCacheFailures(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) @@ -158,7 +158,7 @@ describe('BFCacheErrors', () => { it('constructs a tree with no frame urls if no frame tree is provided', async () => { delete mockActiveBfCacheEvent?.notRestoredExplanationsTree; - const gatherer = new BFCache(); + const gatherer = new BFCacheFailures(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) @@ -183,7 +183,7 @@ describe('BFCacheErrors', () => { it('returns an empty list if no events were found passively or actively', async () => { mockActiveBfCacheEvent = undefined; - const gatherer = new BFCache(); + const gatherer = new BFCacheFailures(); const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index f34bdce2e457..b75125360094 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -123,7 +123,7 @@ export interface GathererArtifacts extends PublicGathererArtifacts,LegacyBaseArt /** Array of all anchors on the page. */ AnchorElements: Artifacts.AnchorElement[]; /** Errors when attempting to use the back/forward cache. */ - BFCache: Artifacts.ProcessedBFCacheEvent[]; + BFCacheFailures: Artifacts.BFCacheFailure[]; /** Array of all URLs cached in CacheStorage. */ CacheContents: string[]; /** CSS coverage information for styles used by page's final state. */ @@ -418,7 +418,7 @@ declare module Artifacts { type BFCacheNotRestoredReasonsTree = Record; - interface ProcessedBFCacheEvent { + interface BFCacheFailure { notRestoredReasonsTree: BFCacheNotRestoredReasonsTree; } From 4f330e9b9fc0aa590a9cc8997fdd74a76bb75112 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 10 Nov 2022 11:31:50 -0800 Subject: [PATCH 18/20] about:blank --- core/gather/gatherers/bf-cache-failures.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/gather/gatherers/bf-cache-failures.js b/core/gather/gatherers/bf-cache-failures.js index 840ab8a8230c..b0966a406cf9 100644 --- a/core/gather/gatherers/bf-cache-failures.js +++ b/core/gather/gatherers/bf-cache-failures.js @@ -102,7 +102,7 @@ class BFCacheFailures extends FRGatherer { const entry = history.entries[history.currentIndex]; await Promise.all([ - session.sendCommand('Page.navigate', {url: 'chrome://terms'}), + session.sendCommand('Page.navigate', {url: 'about:blank'}), waitForLoadEvent(session, 0).promise, ]); From b0e30d808d39c96bf4364d2228faee72b2ef9d15 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 29 Nov 2022 10:35:21 -0800 Subject: [PATCH 19/20] ope --- core/test/gather/gatherers/bf-cache-failures-test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/test/gather/gatherers/bf-cache-failures-test.js b/core/test/gather/gatherers/bf-cache-failures-test.js index 4d0e7c33299e..e654322d1ceb 100644 --- a/core/test/gather/gatherers/bf-cache-failures-test.js +++ b/core/test/gather/gatherers/bf-cache-failures-test.js @@ -82,7 +82,7 @@ describe('BFCacheFailures', () => { const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) - .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + .toHaveBeenCalledWith('Page.navigate', {url: 'about:blank'}); expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); @@ -108,7 +108,7 @@ describe('BFCacheFailures', () => { }); expect(mockContext.driver.defaultSession.sendCommand) - .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + .toHaveBeenCalledWith('Page.navigate', {url: 'about:blank'}); expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); @@ -137,7 +137,7 @@ describe('BFCacheFailures', () => { const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) - .not.toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + .not.toHaveBeenCalledWith('Page.navigate', {url: 'about:blank'}); expect(mockContext.driver.defaultSession.sendCommand) .not.toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); @@ -162,7 +162,7 @@ describe('BFCacheFailures', () => { const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) - .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + .toHaveBeenCalledWith('Page.navigate', {url: 'about:blank'}); expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); @@ -187,7 +187,7 @@ describe('BFCacheFailures', () => { const artifact = await gatherer.getArtifact(context); expect(mockContext.driver.defaultSession.sendCommand) - .toHaveBeenCalledWith('Page.navigate', {url: 'chrome://terms'}); + .toHaveBeenCalledWith('Page.navigate', {url: 'about:blank'}); expect(mockContext.driver.defaultSession.sendCommand) .toHaveBeenCalledWith('Page.navigateToHistoryEntry', {entryId: 1}); From 1d858ae5027b4dfbebeac530dc66c8ef2bd39b9b Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Tue, 29 Nov 2022 13:10:51 -0800 Subject: [PATCH 20/20] ope --- core/gather/gatherers/bf-cache-failures.js | 4 ++-- core/test/gather/gatherers/bf-cache-failures-test.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/gather/gatherers/bf-cache-failures.js b/core/gather/gatherers/bf-cache-failures.js index b0966a406cf9..f44453429047 100644 --- a/core/gather/gatherers/bf-cache-failures.js +++ b/core/gather/gatherers/bf-cache-failures.js @@ -120,7 +120,7 @@ class BFCacheFailures extends FRGatherer { * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'>} context * @return {LH.Crdp.Page.BackForwardCacheNotUsedEvent[]} */ - passivelyCollectBFCacheEvent(context) { + passivelyCollectBFCacheEvents(context) { const events = []; for (const event of context.dependencies.DevtoolsLog) { if (event.method === 'Page.backForwardCacheNotUsed') { @@ -135,7 +135,7 @@ class BFCacheFailures extends FRGatherer { * @return {Promise} */ async getArtifact(context) { - const events = this.passivelyCollectBFCacheEvent(context); + const events = this.passivelyCollectBFCacheEvents(context); if (context.gatherMode === 'navigation') { const activelyCollectedEvent = await this.activelyCollectBFCacheEvent(context); if (activelyCollectedEvent) events.push(activelyCollectedEvent); diff --git a/core/test/gather/gatherers/bf-cache-failures-test.js b/core/test/gather/gatherers/bf-cache-failures-test.js index e654322d1ceb..8a2bed544d94 100644 --- a/core/test/gather/gatherers/bf-cache-failures-test.js +++ b/core/test/gather/gatherers/bf-cache-failures-test.js @@ -1,6 +1,6 @@ /** -* @license Copyright 2022 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 + * @license Copyright 2022 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. */