diff --git a/lighthouse-core/audits/manifest-short-name-length.js b/lighthouse-core/audits/manifest-short-name-length.js index 1c2761631332..71979f640a90 100644 --- a/lighthouse-core/audits/manifest-short-name-length.js +++ b/lighthouse-core/audits/manifest-short-name-length.js @@ -6,6 +6,7 @@ 'use strict'; const Audit = require('./audit'); +const ManifestValues = require('../gather/computed/manifest-values'); class ManifestShortNameLength extends Audit { /** @@ -25,10 +26,11 @@ class ManifestShortNameLength extends Audit { /** * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context * @return {Promise} */ - static async audit(artifacts) { - const manifestValues = await artifacts.requestManifestValues(artifacts.Manifest); + static async audit(artifacts, context) { + const manifestValues = await ManifestValues.request(context, artifacts.Manifest); // If there's no valid manifest, this audit is not applicable if (manifestValues.isParseFailure) { return { diff --git a/lighthouse-core/audits/multi-check-audit.js b/lighthouse-core/audits/multi-check-audit.js index 70c724fec26d..fb878279948c 100644 --- a/lighthouse-core/audits/multi-check-audit.js +++ b/lighthouse-core/audits/multi-check-audit.js @@ -14,10 +14,12 @@ const Audit = require('./audit'); class MultiCheckAudit extends Audit { /** * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context * @return {Promise} */ - static audit(artifacts) { - return Promise.resolve(this.audit_(artifacts)).then(result => this.createAuditProduct(result)); + static async audit(artifacts, context) { + const multiProduct = await this.audit_(artifacts, context); + return this.createAuditProduct(multiProduct); } /** @@ -63,9 +65,10 @@ class MultiCheckAudit extends Audit { /** * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context * @return {Promise<{failures: Array, warnings?: Array, manifestValues?: LH.Artifacts.ManifestValues}>} */ - static audit_(artifacts) { + static audit_(artifacts, context) { throw new Error('audit_ unimplemented'); } diff --git a/lighthouse-core/audits/splash-screen.js b/lighthouse-core/audits/splash-screen.js index 376665c4702a..568f938b837f 100644 --- a/lighthouse-core/audits/splash-screen.js +++ b/lighthouse-core/audits/splash-screen.js @@ -6,6 +6,7 @@ 'use strict'; const MultiCheckAudit = require('./multi-check-audit'); +const ManifestValues = require('../gather/computed/manifest-values'); /** * @fileoverview @@ -64,20 +65,20 @@ class SplashScreen extends MultiCheckAudit { /** * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context * @return {Promise<{failures: Array, manifestValues: LH.Artifacts.ManifestValues}>} */ - static audit_(artifacts) { + static async audit_(artifacts, context) { /** @type {Array} */ const failures = []; - return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => { - SplashScreen.assessManifest(manifestValues, failures); + const manifestValues = await ManifestValues.request(context, artifacts.Manifest); + SplashScreen.assessManifest(manifestValues, failures); - return { - failures, - manifestValues, - }; - }); + return { + failures, + manifestValues, + }; } } diff --git a/lighthouse-core/audits/themed-omnibox.js b/lighthouse-core/audits/themed-omnibox.js index 2dd1fb5d7984..50534ecac9b1 100644 --- a/lighthouse-core/audits/themed-omnibox.js +++ b/lighthouse-core/audits/themed-omnibox.js @@ -7,6 +7,7 @@ const MultiCheckAudit = require('./multi-check-audit'); const validColor = require('../lib/web-inspector').Color.parse; +const ManifestValues = require('../gather/computed/manifest-values'); /** * @fileoverview @@ -63,22 +64,22 @@ class ThemedOmnibox extends MultiCheckAudit { /** * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context * @return {Promise<{failures: Array, manifestValues: LH.Artifacts.ManifestValues, themeColor: ?string}>} */ - static audit_(artifacts) { + static async audit_(artifacts, context) { /** @type {Array} */ const failures = []; - return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => { - ThemedOmnibox.assessManifest(manifestValues, failures); - ThemedOmnibox.assessMetaThemecolor(artifacts.ThemeColor, failures); + const manifestValues = await ManifestValues.request(context, artifacts.Manifest); + ThemedOmnibox.assessManifest(manifestValues, failures); + ThemedOmnibox.assessMetaThemecolor(artifacts.ThemeColor, failures); - return { - failures, - manifestValues, - themeColor: artifacts.ThemeColor, - }; - }); + return { + failures, + manifestValues, + themeColor: artifacts.ThemeColor, + }; } } diff --git a/lighthouse-core/audits/webapp-install-banner.js b/lighthouse-core/audits/webapp-install-banner.js index b747ed6bdf48..d08fb5bca367 100644 --- a/lighthouse-core/audits/webapp-install-banner.js +++ b/lighthouse-core/audits/webapp-install-banner.js @@ -7,6 +7,7 @@ const MultiCheckAudit = require('./multi-check-audit'); const SWAudit = require('./service-worker'); +const ManifestValues = require('../gather/computed/manifest-values'); /** * @fileoverview @@ -118,33 +119,33 @@ class WebappInstallBanner extends MultiCheckAudit { /** * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context * @return {Promise<{failures: Array, warnings: Array, manifestValues: LH.Artifacts.ManifestValues}>} */ - static audit_(artifacts) { + static async audit_(artifacts, context) { /** @type {Array} */ let offlineFailures = []; /** @type {Array} */ let offlineWarnings = []; - return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => { - const manifestFailures = WebappInstallBanner.assessManifest(manifestValues); - const swFailures = WebappInstallBanner.assessServiceWorker(artifacts); - if (!swFailures.length) { - const {failures, warnings} = WebappInstallBanner.assessOfflineStartUrl(artifacts); - offlineFailures = failures; - offlineWarnings = warnings; - } + const manifestValues = await ManifestValues.request(context, artifacts.Manifest); + const manifestFailures = WebappInstallBanner.assessManifest(manifestValues); + const swFailures = WebappInstallBanner.assessServiceWorker(artifacts); + if (!swFailures.length) { + const {failures, warnings} = WebappInstallBanner.assessOfflineStartUrl(artifacts); + offlineFailures = failures; + offlineWarnings = warnings; + } - return { - warnings: offlineWarnings, - failures: [ - ...manifestFailures, - ...swFailures, - ...offlineFailures, - ], - manifestValues, - }; - }); + return { + warnings: offlineWarnings, + failures: [ + ...manifestFailures, + ...swFailures, + ...offlineFailures, + ], + manifestValues, + }; } } diff --git a/lighthouse-core/gather/computed/manifest-values.js b/lighthouse-core/gather/computed/manifest-values.js index 88f648ea7b6d..a73c28f9f089 100644 --- a/lighthouse-core/gather/computed/manifest-values.js +++ b/lighthouse-core/gather/computed/manifest-values.js @@ -5,7 +5,7 @@ */ 'use strict'; -const ComputedArtifact = require('./computed-artifact'); +const makeComputedArtifact = require('./new-computed-artifact'); const icons = require('../../lib/icons'); const PWA_DISPLAY_VALUES = ['minimal-ui', 'fullscreen', 'standalone']; @@ -14,11 +14,7 @@ const PWA_DISPLAY_VALUES = ['minimal-ui', 'fullscreen', 'standalone']; // For more discussion, see https://github.com/GoogleChrome/lighthouse/issues/69 and https://developer.chrome.com/apps/manifest/name#short_name const SUGGESTED_SHORTNAME_LENGTH = 12; -class ManifestValues extends ComputedArtifact { - get name() { - return 'ManifestValues'; - } - +class ManifestValues { static get validityIds() { return ['hasManifest', 'hasParseableManifest']; } @@ -88,7 +84,7 @@ class ManifestValues extends ComputedArtifact { * @param {LH.Artifacts['Manifest']} manifest * @return {Promise} */ - async compute_(manifest) { + static async compute_(manifest) { // if the manifest isn't there or is invalid json, we report that and bail let parseFailureReason; @@ -125,4 +121,4 @@ class ManifestValues extends ComputedArtifact { } } -module.exports = ManifestValues; +module.exports = makeComputedArtifact(ManifestValues); diff --git a/lighthouse-core/gather/computed/new-computed-artifact.js b/lighthouse-core/gather/computed/new-computed-artifact.js new file mode 100644 index 000000000000..f77136519042 --- /dev/null +++ b/lighthouse-core/gather/computed/new-computed-artifact.js @@ -0,0 +1,40 @@ +/** + * @license Copyright 2018 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +const ArbitraryEqualityMap = require('../../lib/arbitrary-equality-map.js'); + +/** + * @template {string} N + * @template A + * @template R + * @param {{name: N, compute_: (artifact: A) => Promise}} computableArtifact + * @return {{name: N, request: (context: LH.Audit.Context, artifact: A) => Promise}} + */ +function makeComputedArtifact(computableArtifact) { + /** + * @param {LH.Audit.Context} context + * @param {A} artifact + */ + const request = ({computedCache}, artifact) => { + const cache = computedCache.get(computableArtifact.name) || new ArbitraryEqualityMap(); + computedCache.set(computableArtifact.name, cache); + + const computed = /** @type {Promise|undefined} */ (cache.get(artifact)); + if (computed) { + return computed; + } + + const artifactPromise = computableArtifact.compute_(artifact); + cache.set(artifact, artifactPromise); + + return artifactPromise; + }; + + return Object.assign(computableArtifact, {request}); +} + +module.exports = makeComputedArtifact; diff --git a/lighthouse-core/lib/arbitrary-equality-map.js b/lighthouse-core/lib/arbitrary-equality-map.js index 89901f07fc8b..78a3fd5e818b 100644 --- a/lighthouse-core/lib/arbitrary-equality-map.js +++ b/lighthouse-core/lib/arbitrary-equality-map.js @@ -12,10 +12,10 @@ const isEqual = require('lodash.isequal'); * It is not meant to be performant and is well-suited to use cases where the number of entries is * likely to be small (like computed artifacts). */ -module.exports = class ArbitraryEqualityMap { +class ArbitraryEqualityMap { constructor() { - this._equalsFn = /** @type {function(any,any):boolean} */ ((a, b) => a === b); - /** @type {Array<{key: string, value: *}>} */ + this._equalsFn = /** @type {function(*,*):boolean} */ ((a, b) => a === b); + /** @type {Array<{key: *, value: *}>} */ this._entries = []; } @@ -27,7 +27,7 @@ module.exports = class ArbitraryEqualityMap { } /** - * @param {string} key + * @param {*} key * @return {boolean} */ has(key) { @@ -35,7 +35,7 @@ module.exports = class ArbitraryEqualityMap { } /** - * @param {string} key + * @param {*} key * @return {*} */ get(key) { @@ -44,7 +44,7 @@ module.exports = class ArbitraryEqualityMap { } /** - * @param {string} key + * @param {*} key * @param {*} value */ set(key, value) { @@ -54,7 +54,7 @@ module.exports = class ArbitraryEqualityMap { } /** - * @param {string} key + * @param {*} key * @return {number} */ _findIndexOf(key) { @@ -76,4 +76,6 @@ module.exports = class ArbitraryEqualityMap { static deepEquals(objA, objB) { return isEqual(objA, objB); } -}; +} + +module.exports = ArbitraryEqualityMap; diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index f7574bf1400b..ff134fc02d7d 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -205,10 +205,17 @@ class Runner { } } + // Members of LH.Audit.Context that are shared across all audits. + const sharedAuditContext = { + settings, + LighthouseRunWarnings: runWarnings, + computedCache: new Map(), + }; + // Run each audit sequentially const auditResults = []; for (const auditDefn of audits) { - const auditResult = await Runner._runAudit(auditDefn, artifacts, settings, runWarnings); + const auditResult = await Runner._runAudit(auditDefn, artifacts, sharedAuditContext); auditResults.push(auditResult); } @@ -220,12 +227,11 @@ class Runner { * Otherwise returns error audit result. * @param {LH.Config.AuditDefn} auditDefn * @param {LH.Artifacts} artifacts - * @param {LH.Config.Settings} settings - * @param {Array} runWarnings + * @param {Pick} sharedAuditContext * @return {Promise} * @private */ - static async _runAudit(auditDefn, artifacts, settings, runWarnings) { + static async _runAudit(auditDefn, artifacts, sharedAuditContext) { const audit = auditDefn.implementation; const status = `Evaluating: ${i18n.getFormatted(audit.meta.title, 'en-US')}`; @@ -274,8 +280,7 @@ class Runner { const auditOptions = Object.assign({}, audit.defaultOptions, auditDefn.options); const auditContext = { options: auditOptions, - settings, - LighthouseRunWarnings: runWarnings, + ...sharedAuditContext, }; const product = await audit.audit(artifacts, auditContext); @@ -380,6 +385,10 @@ class Runner { 'metrics', // the sub folder that contains metric names 'metrics/lantern-metric.js', // lantern metric base class 'metrics/metric.js', // computed metric base class + + // Computed artifacts switching to the new system. + 'new-computed-artifact.js', + 'manifest-values.js', ]; const fileList = [ diff --git a/lighthouse-core/test/audits/manifest-short-name-length-test.js b/lighthouse-core/test/audits/manifest-short-name-length-test.js index fe8cf6bd55a5..705e5b3349ec 100644 --- a/lighthouse-core/test/audits/manifest-short-name-length-test.js +++ b/lighthouse-core/test/audits/manifest-short-name-length-test.js @@ -12,14 +12,15 @@ const manifestParser = require('../../lib/manifest-parser'); const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json'; const EXAMPLE_DOC_URL = 'https://example.com/index.html'; -const Runner = require('../../runner.js'); - function generateMockArtifacts() { - const computedArtifacts = Runner.instantiateComputedArtifacts(); - const mockArtifacts = Object.assign({}, computedArtifacts, { + return { Manifest: null, - }); - return mockArtifacts; + }; +} +function generateMockAuditContext() { + return { + computedCache: new Map(), + }; } /* eslint-env jest */ @@ -28,8 +29,9 @@ describe('Manifest: short_name_length audit', () => { it('marked as notApplicable if page had no manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = null; + const context = generateMockAuditContext(); - return ManifestShortNameLengthAudit.audit(artifacts).then(result => { + return ManifestShortNameLengthAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, true); assert.strictEqual(result.notApplicable, true); }); @@ -38,7 +40,8 @@ describe('Manifest: short_name_length audit', () => { it('marked as notApplicable if manifest is present but empty', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = manifestParser('{}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return ManifestShortNameLengthAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ManifestShortNameLengthAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, true); assert.strictEqual(result.notApplicable, true); }); @@ -50,7 +53,8 @@ describe('Manifest: short_name_length audit', () => { name: 'i\'m much longer than the recommended size', }); artifacts.Manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return ManifestShortNameLengthAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ManifestShortNameLengthAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, true); assert.strictEqual(result.notApplicable, true); assert.equal(result.explanation, undefined); @@ -65,7 +69,8 @@ describe('Manifest: short_name_length audit', () => { short_name: 'i\'m much longer than the recommended size', }); artifacts.Manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return ManifestShortNameLengthAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ManifestShortNameLengthAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, false); assert.ok(result.explanation.includes('without truncation'), result.explanation); assert.equal(result.notApplicable, undefined); @@ -78,7 +83,8 @@ describe('Manifest: short_name_length audit', () => { short_name: 'Lighthouse', }); artifacts.Manifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return ManifestShortNameLengthAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ManifestShortNameLengthAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, true); assert.equal(result.explanation, undefined); assert.equal(result.notApplicable, undefined); diff --git a/lighthouse-core/test/audits/splash-screen-test.js b/lighthouse-core/test/audits/splash-screen-test.js index 9a38b7dd0618..b08ecce123fc 100644 --- a/lighthouse-core/test/audits/splash-screen-test.js +++ b/lighthouse-core/test/audits/splash-screen-test.js @@ -14,20 +14,20 @@ const manifestDirtyJpgSrc = JSON.stringify(require('../fixtures/manifest-dirty-j const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json'; const EXAMPLE_DOC_URL = 'https://example.com/index.html'; -const Runner = require('../../runner.js'); - /** * @param {string} src - * @return {!ManifestNode<(!Manifest|undefined)>} */ function generateMockArtifacts(src = manifestSrc) { const exampleManifest = manifestParser(src, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - const computedArtifacts = Runner.instantiateComputedArtifacts(); - const mockArtifacts = Object.assign({}, computedArtifacts, { + return { Manifest: exampleManifest, - }); - return mockArtifacts; + }; +} +function generateMockAuditContext() { + return { + computedCache: new Map(), + }; } /* eslint-env jest */ @@ -36,8 +36,9 @@ describe('PWA: splash screen audit', () => { it('fails if page had no manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = null; + const context = generateMockAuditContext(); - return SplashScreenAudit.audit(artifacts).then(result => { + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('No manifest was fetched'), result.explanation); }); @@ -46,7 +47,8 @@ describe('PWA: splash screen audit', () => { it('fails with a non-parsable manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = manifestParser('{,:}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return SplashScreenAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('failed to parse as valid JSON')); }); @@ -55,7 +57,8 @@ describe('PWA: splash screen audit', () => { it('fails when an empty manifest is present', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = manifestParser('{}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return SplashScreenAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation); assert.strictEqual(result.details.items[0].failures.length, 4); @@ -63,7 +66,8 @@ describe('PWA: splash screen audit', () => { }); it('passes with complete manifest and SW', () => { - return SplashScreenAudit.audit(generateMockArtifacts()).then(result => { + const context = generateMockAuditContext(); + return SplashScreenAudit.audit(generateMockArtifacts(), context).then(result => { assert.strictEqual(result.rawValue, true, result.explanation); assert.strictEqual(result.explanation, undefined, result.explanation); }); @@ -74,8 +78,9 @@ describe('PWA: splash screen audit', () => { it('fails when a manifest contains no name', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.name.value = undefined; + const context = generateMockAuditContext(); - return SplashScreenAudit.audit(artifacts).then(result => { + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('name'), result.explanation); }); @@ -84,8 +89,9 @@ describe('PWA: splash screen audit', () => { it('fails when a manifest contains no background color', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.background_color.value = undefined; + const context = generateMockAuditContext(); - return SplashScreenAudit.audit(artifacts).then(result => { + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('background_color'), result.explanation); }); @@ -95,8 +101,9 @@ describe('PWA: splash screen audit', () => { const artifacts = generateMockArtifacts(JSON.stringify({ background_color: 'no', })); + const context = generateMockAuditContext(); - return SplashScreenAudit.audit(artifacts).then(result => { + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('background_color'), result.explanation); }); @@ -105,8 +112,9 @@ describe('PWA: splash screen audit', () => { it('fails when a manifest contains no theme color', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.theme_color.value = undefined; + const context = generateMockAuditContext(); - return SplashScreenAudit.audit(artifacts).then(result => { + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('theme_color'), result.explanation); }); @@ -115,8 +123,9 @@ describe('PWA: splash screen audit', () => { it('fails if page had no icons in the manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.icons.value = []; + const context = generateMockAuditContext(); - return SplashScreenAudit.audit(artifacts).then(result => { + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('PNG icon'), result.explanation); }); @@ -124,8 +133,9 @@ describe('PWA: splash screen audit', () => { it('fails if icons were present, but no valid PNG present', () => { const artifacts = generateMockArtifacts(manifestDirtyJpgSrc); + const context = generateMockAuditContext(); - return SplashScreenAudit.audit(artifacts).then(result => { + return SplashScreenAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('PNG icon'), result.explanation); const failures = result.details.items[0].failures; diff --git a/lighthouse-core/test/audits/themed-omnibox-test.js b/lighthouse-core/test/audits/themed-omnibox-test.js index 9a4da543841d..49878ae7e509 100644 --- a/lighthouse-core/test/audits/themed-omnibox-test.js +++ b/lighthouse-core/test/audits/themed-omnibox-test.js @@ -14,15 +14,16 @@ const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json'; const EXAMPLE_DOC_URL = 'https://example.com/index.html'; const exampleManifest = noUrlManifestParser(manifestSrc); -const Runner = require('../../runner.js'); - function generateMockArtifacts() { - const computedArtifacts = Runner.instantiateComputedArtifacts(); - const mockArtifacts = Object.assign({}, computedArtifacts, { + return { Manifest: exampleManifest, ThemeColor: '#bada55', - }); - return mockArtifacts; + }; +} +function generateMockAuditContext() { + return { + computedCache: new Map(), + }; } /** @@ -40,8 +41,9 @@ describe('PWA: themed omnibox audit', () => { it('fails if page had no manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = null; + const context = generateMockAuditContext(); - return ThemedOmniboxAudit.audit(artifacts).then(result => { + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('No manifest was fetched'), result.explanation); }); @@ -54,8 +56,9 @@ describe('PWA: themed omnibox audit', () => { artifacts.Manifest = noUrlManifestParser(JSON.stringify({ start_url: '/', })); + const context = generateMockAuditContext(); - return ThemedOmniboxAudit.audit(artifacts).then(result => { + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, false); assert.ok(result.explanation); }); @@ -66,7 +69,8 @@ describe('PWA: themed omnibox audit', () => { artifacts.Manifest = noUrlManifestParser(JSON.stringify({ theme_color: '#bada55', })); - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, true); assert.equal(result.explanation, undefined); }); @@ -75,7 +79,8 @@ describe('PWA: themed omnibox audit', () => { /* eslint-enable camelcase */ it('succeeds when a complete manifest contains a theme_color', () => { const artifacts = generateMockArtifacts(); - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, true); assert.equal(result.explanation, undefined); }); @@ -84,7 +89,8 @@ describe('PWA: themed omnibox audit', () => { it('fails and warns when no theme-color meta tag found', () => { const artifacts = generateMockArtifacts(); artifacts.ThemeColor = null; - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, false); assert.ok(result.explanation); }); @@ -93,7 +99,8 @@ describe('PWA: themed omnibox audit', () => { it('fails and warns when theme-color has an invalid CSS color', () => { const artifacts = generateMockArtifacts(); artifacts.ThemeColor = '#1234567'; - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, false); assert.ok(result.explanation.includes('valid CSS color')); }); @@ -102,7 +109,8 @@ describe('PWA: themed omnibox audit', () => { it('succeeds when theme-color present in the html', () => { const artifacts = generateMockArtifacts(); artifacts.ThemeColor = '#fafa33'; - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, true); assert.equal(result.explanation, undefined); }); @@ -111,7 +119,8 @@ describe('PWA: themed omnibox audit', () => { it('succeeds when theme-color has a CSS nickname content value', () => { const artifacts = generateMockArtifacts(); artifacts.ThemeColor = 'red'; - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, true); assert.equal(result.explanation, undefined); }); @@ -123,7 +132,8 @@ describe('PWA: themed omnibox audit', () => { artifacts.Manifest = noUrlManifestParser(JSON.stringify({ start_url: '/', })); - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, false); assert.ok(result.explanation.includes('does not have `theme_color`'), result.explanation); }); @@ -132,7 +142,8 @@ describe('PWA: themed omnibox audit', () => { it('fails if HTML theme color is bad, and manifest themecolor is good', () => { const artifacts = generateMockArtifacts(); artifacts.ThemeColor = 'not a color'; - return ThemedOmniboxAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return ThemedOmniboxAudit.audit(artifacts, context).then(result => { assert.equal(result.rawValue, false); assert.ok(result.explanation.includes('theme-color meta tag'), result.explanation); }); diff --git a/lighthouse-core/test/audits/webapp-install-banner-test.js b/lighthouse-core/test/audits/webapp-install-banner-test.js index 1a33a1d02c0b..1ed204e5ee09 100644 --- a/lighthouse-core/test/audits/webapp-install-banner-test.js +++ b/lighthouse-core/test/audits/webapp-install-banner-test.js @@ -14,12 +14,9 @@ const manifestDirtyJpgSrc = JSON.stringify(require('../fixtures/manifest-dirty-j const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json'; const EXAMPLE_DOC_URL = 'https://example.com/index.html'; -const Runner = require('../../runner.js'); - function generateMockArtifacts(src = manifestSrc) { const exampleManifest = manifestParser(src, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - const computedArtifacts = Runner.instantiateComputedArtifacts(); const clonedArtifacts = JSON.parse(JSON.stringify({ Manifest: exampleManifest, ServiceWorker: { @@ -31,8 +28,12 @@ function generateMockArtifacts(src = manifestSrc) { StartUrl: {statusCode: 200}, URL: {finalUrl: 'https://example.com'}, })); - const mockArtifacts = Object.assign({}, computedArtifacts, clonedArtifacts); - return mockArtifacts; + return clonedArtifacts; +} +function generateMockAuditContext() { + return { + computedCache: new Map(), + }; } /* eslint-env jest */ @@ -41,8 +42,9 @@ describe('PWA: webapp install banner audit', () => { it('fails if page had no manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = null; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('No manifest was fetched'), result.explanation); }); @@ -51,7 +53,8 @@ describe('PWA: webapp install banner audit', () => { it('fails with a non-parsable manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = manifestParser('{,:}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('failed to parse as valid JSON')); }); @@ -60,7 +63,8 @@ describe('PWA: webapp install banner audit', () => { it('fails when an empty manifest is present', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest = manifestParser('{}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + const context = generateMockAuditContext(); + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation); assert.strictEqual(result.details.items[0].failures.length, 4); @@ -68,7 +72,8 @@ describe('PWA: webapp install banner audit', () => { }); it('passes with complete manifest and SW', () => { - return WebappInstallBannerAudit.audit(generateMockArtifacts()).then(result => { + const context = generateMockAuditContext(); + return WebappInstallBannerAudit.audit(generateMockArtifacts(), context).then(result => { assert.strictEqual(result.rawValue, true, result.explanation); assert.strictEqual(result.explanation, undefined, result.explanation); }); @@ -80,8 +85,9 @@ describe('PWA: webapp install banner audit', () => { it('fails when a manifest contains no start_url', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.start_url.value = undefined; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('start_url'), result.explanation); const failures = result.details.items[0].failures; @@ -93,8 +99,9 @@ describe('PWA: webapp install banner audit', () => { it('fails when a manifest contains no short_name', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.short_name.value = undefined; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('short_name'), result.explanation); const failures = result.details.items[0].failures; @@ -105,8 +112,9 @@ describe('PWA: webapp install banner audit', () => { it('fails when a manifest contains no name', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.name.value = undefined; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('name'), result.explanation); const failures = result.details.items[0].failures; @@ -117,8 +125,9 @@ describe('PWA: webapp install banner audit', () => { it('fails if page had no icons in the manifest', () => { const artifacts = generateMockArtifacts(); artifacts.Manifest.value.icons.value = []; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('PNG icon'), result.explanation); const failures = result.details.items[0].failures; @@ -129,8 +138,9 @@ describe('PWA: webapp install banner audit', () => { it('fails if icons were present, but no valid PNG present', () => { const artifacts = generateMockArtifacts(manifestDirtyJpgSrc); + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('PNG icon'), result.explanation); const failures = result.details.items[0].failures; @@ -142,8 +152,9 @@ describe('PWA: webapp install banner audit', () => { const artifacts = generateMockArtifacts(); artifacts.ServiceWorker.versions = []; artifacts.StartUrl = {statusCode: -1}; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('service worker'), result.explanation); const failures = result.details.items[0].failures; @@ -154,8 +165,9 @@ describe('PWA: webapp install banner audit', () => { it('fails if start_url is not cached', () => { const artifacts = generateMockArtifacts(); artifacts.StartUrl = {statusCode: -1}; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, false); assert.ok(result.explanation.includes('start_url'), result.explanation); const failures = result.details.items[0].failures; @@ -166,8 +178,9 @@ describe('PWA: webapp install banner audit', () => { it('includes warning from start_url', () => { const artifacts = generateMockArtifacts(); artifacts.StartUrl = {statusCode: 200, explanation: 'Warning!'}; + const context = generateMockAuditContext(); - return WebappInstallBannerAudit.audit(artifacts).then(result => { + return WebappInstallBannerAudit.audit(artifacts, context).then(result => { assert.strictEqual(result.rawValue, true); assert.equal(result.warnings[0], 'Warning!'); }); diff --git a/lighthouse-core/test/gather/computed/manifest-values-test.js b/lighthouse-core/test/gather/computed/manifest-values-test.js index 18ca8f4cb824..34a32ca4fd59 100644 --- a/lighthouse-core/test/gather/computed/manifest-values-test.js +++ b/lighthouse-core/test/gather/computed/manifest-values-test.js @@ -13,7 +13,11 @@ const assert = require('assert'); const manifestSrc = JSON.stringify(require('../../fixtures/manifest.json')); const manifestParser = require('../../../lib/manifest-parser'); -const manifestValues = new ManifestValues(); +function getMockContext() { + return { + computedCache: new Map(), + }; +} /** * Simple manifest parsing helper when the manifest URLs aren't material to the @@ -31,7 +35,7 @@ function noUrlManifestParser(manifestSrc) { describe('ManifestValues computed artifact', () => { it('reports a parse failure if page had no manifest', async () => { const manifestArtifact = null; - const results = await manifestValues.compute_(manifestArtifact); + const results = await ManifestValues.request(getMockContext(), manifestArtifact); assert.equal(results.isParseFailure, true); assert.ok(results.parseFailureReason, 'No manifest was fetched'); assert.equal(results.allChecks.length, 0); @@ -39,7 +43,7 @@ describe('ManifestValues computed artifact', () => { it('reports a parse failure if page had an unparseable manifest', async () => { const manifestArtifact = noUrlManifestParser('{:,}'); - const results = await manifestValues.compute_(manifestArtifact); + const results = await ManifestValues.request(getMockContext(), manifestArtifact); assert.equal(results.isParseFailure, true); assert.ok(results.parseFailureReason.includes('failed to parse as valid JSON')); assert.equal(results.allChecks.length, 0); @@ -47,14 +51,14 @@ describe('ManifestValues computed artifact', () => { it('passes the parsing checks on an empty manifest', async () => { const manifestArtifact = noUrlManifestParser('{}'); - const results = await manifestValues.compute_(manifestArtifact); + const results = await ManifestValues.request(getMockContext(), manifestArtifact); assert.equal(results.isParseFailure, false); assert.equal(results.parseFailureReason, undefined); }); it('passes the all checks with fixture manifest', async () => { const manifestArtifact = noUrlManifestParser(manifestSrc); - const results = await manifestValues.compute_(manifestArtifact); + const results = await ManifestValues.request(getMockContext(), manifestArtifact); assert.equal(results.isParseFailure, false); assert.equal(results.parseFailureReason, undefined); @@ -67,7 +71,7 @@ describe('ManifestValues computed artifact', () => { const Manifest = noUrlManifestParser(JSON.stringify({ start_url: '/', })); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const colorResults = results.allChecks.filter(i => i.id.includes('Color')); assert.equal(colorResults.every(i => i.passing === false), true); }); @@ -78,7 +82,7 @@ describe('ManifestValues computed artifact', () => { theme_color: 'no', })); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const colorResults = results.allChecks.filter(i => i.id.includes('Color')); assert.equal(colorResults.every(i => i.passing === false), true); }); @@ -89,7 +93,7 @@ describe('ManifestValues computed artifact', () => { theme_color: '#FAFAFA', })); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const colorResults = results.allChecks.filter(i => i.id.includes('Color')); assert.equal(colorResults.every(i => i.passing === true), true); }); @@ -123,7 +127,7 @@ describe('ManifestValues computed artifact', () => { name: 'NoIconsHere', }); const Manifest = noUrlManifestParser(manifestSrc); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const iconResults = results.allChecks.filter(i => i.id.includes('Icons')); assert.equal(iconResults.every(i => i.passing === false), true); }); @@ -133,7 +137,7 @@ describe('ManifestValues computed artifact', () => { icons: [], }); const Manifest = noUrlManifestParser(manifestSrc); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const iconResults = results.allChecks.filter(i => i.id.includes('Icons')); assert.equal(iconResults.every(i => i.passing === false), true); }); @@ -147,7 +151,7 @@ describe('ManifestValues computed artifact', () => { }], }); const Manifest = noUrlManifestParser(manifestSrc); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const iconResults = results.allChecks.filter(i => i.id.includes('Icons')); assert.equal(iconResults.every(i => i.passing === false), true); @@ -161,7 +165,7 @@ describe('ManifestValues computed artifact', () => { }], }); const Manifest = noUrlManifestParser(manifestSrc); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const iconResults = results.allChecks.filter(i => i.id.includes('Icons')); assert.equal(iconResults.every(i => i.passing === true), true); @@ -177,7 +181,7 @@ describe('ManifestValues computed artifact', () => { }], }); const Manifest = noUrlManifestParser(manifestSrc); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const iconResults = results.allChecks.filter(i => i.id.includes('Icons')); assert.equal(iconResults.every(i => i.passing === true), true); @@ -192,7 +196,7 @@ describe('ManifestValues computed artifact', () => { }], }); const Manifest = noUrlManifestParser(manifestSrc); - const results = await manifestValues.compute_(Manifest); + const results = await ManifestValues.request(getMockContext(), Manifest); const iconResults = results.allChecks.filter(i => i.id.includes('Icons')); assert.equal(iconResults.every(i => i.passing === false), true); diff --git a/typings/artifacts.d.ts b/typings/artifacts.d.ts index dbf98b90b046..0f4747b9e086 100644 --- a/typings/artifacts.d.ts +++ b/typings/artifacts.d.ts @@ -125,7 +125,6 @@ declare global { requestCriticalRequestChains(data: {devtoolsLog: DevtoolsLog, URL: Artifacts['URL']}): Promise; requestLoadSimulator(data: {devtoolsLog: DevtoolsLog, settings: Config.Settings}): Promise; requestMainResource(data: {devtoolsLog: DevtoolsLog, URL: Artifacts['URL']}): Promise; - requestManifestValues(manifest: LH.Artifacts['Manifest']): Promise; requestNetworkAnalysis(devtoolsLog: DevtoolsLog): Promise; requestNetworkThroughput(devtoolsLog: DevtoolsLog): Promise; requestNetworkRecords(devtoolsLog: DevtoolsLog): Promise; diff --git a/typings/audit.d.ts b/typings/audit.d.ts index 15527db54b49..cbde9faa785a 100644 --- a/typings/audit.d.ts +++ b/typings/audit.d.ts @@ -4,6 +4,8 @@ * 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 ArbitraryEqualityMap = require('../lighthouse-core/lib/arbitrary-equality-map.js'); + declare global { module LH.Audit { export interface Context { @@ -12,6 +14,12 @@ declare global { settings: Config.Settings; /** Push to this array to add top-level warnings to the LHR. */ LighthouseRunWarnings: Array; + /** + * Nested cache for already-computed computed artifacts. Keyed first on + * the computed artifact's `name` property, then on input artifact(s). + * Values are Promises resolving to the computedArtifact result. + */ + computedCache: Map; } export interface ScoreOptions { diff --git a/typings/externs.d.ts b/typings/externs.d.ts index 381880326e49..09644ac54473 100644 --- a/typings/externs.d.ts +++ b/typings/externs.d.ts @@ -49,7 +49,7 @@ declare global { /** Obtain the type of the first parameter of a function. */ type FirstParamType any> = - T extends (arg1: infer P, ...args: any[]) => any ? P : any; + T extends (arg1: infer P, ...args: any[]) => any ? P : never; module LH { // re-export useful type modules under global LH module.