diff --git a/lighthouse-core/gather/computed/computed-artifact.js b/lighthouse-core/gather/computed/computed-artifact.js index 3c7c75a6efa1..3196ebc242e6 100644 --- a/lighthouse-core/gather/computed/computed-artifact.js +++ b/lighthouse-core/gather/computed/computed-artifact.js @@ -22,8 +22,34 @@ class ComputedArtifact { this.cache = new Map(); } - request() { - throw new Error('request() not implemented for computed Artifact' + this.name); + /* eslint-disable no-unused-vars */ + + /** + * Override to implement a computed artifact. Can return a Promise or the + * computed artifact itself. + * @param {!Object} artifact Input to computation. + * @return {!Promise|!Object|!Array} + */ + compute_(artifact) { + throw new Error('compute_() not implemented for computed artifact ' + this.name); + } + + /* eslint-enable no-unused-vars */ + + /** + * Request a computed artifact, caching the result on the input artifact. + * @param {!OBject} artifact + * @return {!Promise} + */ + request(artifact) { + if (this.cache.has(artifact)) { + return Promise.resolve(this.cache.get(artifact)); + } + + return Promise.resolve(this.compute_(artifact)).then(computedArtifact => { + this.cache.set(artifact, computedArtifact); + return computedArtifact; + }); } } diff --git a/lighthouse-core/gather/computed/critical-request-chains.js b/lighthouse-core/gather/computed/critical-request-chains.js index 1badc758029b..bd71c2e41eeb 100644 --- a/lighthouse-core/gather/computed/critical-request-chains.js +++ b/lighthouse-core/gather/computed/critical-request-chains.js @@ -50,7 +50,7 @@ class CriticalRequestChains extends ComputedArtifact { return includes(['VeryHigh', 'High', 'Medium'], request.priority()); } - getChains(networkRecords) { + compute_(networkRecords) { // Build a map of requestID -> Node. const requestIdToRequests = new Map(); for (let request of networkRecords) { @@ -132,20 +132,8 @@ class CriticalRequestChains extends ComputedArtifact { }; } - return Promise.resolve(criticalRequestChains); + return criticalRequestChains; } - - request(networkRecords) { - if (this.cache.has(networkRecords)) { - return this.cache.get(networkRecords); - } - - return this.getChains(networkRecords).then(chains => { - this.cache.set(chains, networkRecords); - return chains; - }); - } - } module.exports = CriticalRequestChains; diff --git a/lighthouse-core/gather/computed/pushed-requests.js b/lighthouse-core/gather/computed/pushed-requests.js index 827931d7f09f..6fcd9bd92de2 100644 --- a/lighthouse-core/gather/computed/pushed-requests.js +++ b/lighthouse-core/gather/computed/pushed-requests.js @@ -25,22 +25,15 @@ class PushedRequests extends ComputedArtifact { return 'PushedRequests'; } - filterPushed(records) { + /** + * Return list of network requests that were pushed. + * @param {!Array} records + * @return {!Array} + */ + compute_(records) { const pushedRecords = records.filter(r => r._timing && !!r._timing.pushStart); - return Promise.resolve(pushedRecords); + return pushedRecords; } - - request(networkRecords) { - if (this.cache.has(networkRecords)) { - return this.cache.get(networkRecords); - } - - return this.filterPushed(networkRecords).then(records => { - this.cache.set(records, networkRecords); - return records; - }); - } - } module.exports = PushedRequests; diff --git a/lighthouse-core/gather/computed/screenshots.js b/lighthouse-core/gather/computed/screenshots.js index 230f78857f9c..63bc60383b83 100644 --- a/lighthouse-core/gather/computed/screenshots.js +++ b/lighthouse-core/gather/computed/screenshots.js @@ -36,7 +36,7 @@ class ScreenshotFilmstrip extends ComputedArtifact { * @param {{traceEvents: !Array}} trace * @return {!Promise} */ - getScreenshots(trace) { + compute_(trace) { const model = new DevtoolsTimelineModel(trace.traceEvents); const filmStripFrames = model.filmStripModel().frames(); @@ -49,17 +49,6 @@ class ScreenshotFilmstrip extends ComputedArtifact { return result; }); } - - request(trace) { - if (this.cache.has(trace)) { - return this.cache.get(trace); - } - - return this.getScreenshots(trace).then(screenshots => { - this.cache.set(trace, screenshots); - return screenshots; - }); - } } module.exports = ScreenshotFilmstrip; diff --git a/lighthouse-core/gather/computed/speedline.js b/lighthouse-core/gather/computed/speedline.js index 84db67511646..90ae2caa9381 100644 --- a/lighthouse-core/gather/computed/speedline.js +++ b/lighthouse-core/gather/computed/speedline.js @@ -29,11 +29,7 @@ class Speedline extends ComputedArtifact { /** * @return {!Promise} */ - request(trace) { - if (this.cache.has(trace)) { - return this.cache.get(trace); - } - + compute_(trace) { // speedline() may throw without a promise, so we resolve immediately // to get in a promise chain. return Promise.resolve().then(_ => { @@ -41,7 +37,6 @@ class Speedline extends ComputedArtifact { return speedline(trace.traceEvents); }).then(speedlineResults => { ConsoleQuieter.unmuteAndFlush(); - this.cache.set(trace, speedlineResults); return speedlineResults; }); } diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js index da9dcf626ac9..50ad1a70cf4b 100644 --- a/lighthouse-core/gather/gather-runner.js +++ b/lighthouse-core/gather/gather-runner.js @@ -344,9 +344,7 @@ class GatherRunner { const ArtifactClass = require('./computed/' + file); const artifact = new ArtifactClass(); // define the request* function that will be exposed on `artifacts` - computedArtifacts['request' + artifact.name] = function(artifacts) { - return Promise.resolve(artifact.request(artifacts)); - }; + computedArtifacts['request' + artifact.name] = artifact.request.bind(artifact); }); return computedArtifacts; } diff --git a/lighthouse-core/test/gather/computed/computed-artifact-test.js b/lighthouse-core/test/gather/computed/computed-artifact-test.js new file mode 100644 index 000000000000..8936c378001d --- /dev/null +++ b/lighthouse-core/test/gather/computed/computed-artifact-test.js @@ -0,0 +1,57 @@ +/** + * Copyright 2016 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'; + +/* eslint-env mocha */ + +const assert = require('assert'); + +const ComputedArtifact = require('../../../gather/computed/computed-artifact'); + +class TestComputedArtifact extends ComputedArtifact { + constructor() { + super(); + + this.computeCounter = 0; + } + + get name() { + return 'TestComputedArtifact'; + } + + compute_(_) { + return this.computeCounter++; + } +} + +describe('ComputedArtifact base class', () => { + it('caches computed artifacts', () => { + const testComputedArtifact = new TestComputedArtifact(); + + const obj0 = {}; + const obj1 = {}; + + return testComputedArtifact.request(obj0).then(result => { + assert.equal(result, 0); + }).then(_ => testComputedArtifact.request(obj1)).then(result => { + assert.equal(result, 1); + }).then(_ => testComputedArtifact.request(obj0)).then(result => { + assert.equal(result, 0); + }).then(_ => testComputedArtifact.request(obj1)).then(result => { + assert.equal(result, 1); + }); + }); +});