diff --git a/lighthouse-core/audits/load-fast-enough-for-pwa.js b/lighthouse-core/audits/load-fast-enough-for-pwa.js index c8f70204da77..17b1d7943757 100644 --- a/lighthouse-core/audits/load-fast-enough-for-pwa.js +++ b/lighthouse-core/audits/load-fast-enough-for-pwa.js @@ -11,17 +11,14 @@ * Afterwards, we report if the TTFI is less than 10 seconds. */ +const isDeepEqual = require('lodash.isequal'); const Audit = require('./audit'); -const URL = require('../lib/url-shim'); -const targetLatencyMs = require('../config/constants').throttling.mobile3G.rttMs; -const Sentry = require('../lib/sentry'); +const mobile3GThrottling = require('../config/constants').throttling.mobile3G; const Util = require('../report/v2/renderer/util.js'); -// Maximum TTFI to be considered "fast" for PWA baseline checklist +// Maximum TTI to be considered "fast" for PWA baseline checklist // https://developers.google.com/web/progressive-web-apps/checklist -const MAXIMUM_TTFI = 10 * 1000; - -const WHITELISTED_STATUS_CODES = [307]; +const MAXIMUM_TTI = 10 * 1000; class LoadFastEnough4Pwa extends Audit { /** @@ -32,116 +29,47 @@ class LoadFastEnough4Pwa extends Audit { name: 'load-fast-enough-for-pwa', description: 'Page load is fast enough on 3G', failureDescription: 'Page load is not fast enough on 3G', - helpText: 'A fast page load over a 3G network ensures a good mobile user experience. ' + - '[Learn more](https://developers.google.com/web/tools/lighthouse/audits/fast-3g).', + helpText: + 'A fast page load over a 3G network ensures a good mobile user experience. ' + + '[Learn more](https://developers.google.com/web/tools/lighthouse/audits/fast-3g).', requiredArtifacts: ['traces', 'devtoolsLogs'], }; } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context + * @return {LH.AuditResult} */ - static audit(artifacts) { - const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; - return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => { - const firstRequestLatenciesByOrigin = new Map(); - networkRecords.forEach(record => { - // Ignore requests that don't have valid origin, timing data, came from the cache, were - // redirected by Chrome without going to the network, or are not finished. - const fromCache = record._fromDiskCache || record._fromMemoryCache; - const origin = URL.getOrigin(record._url); - if (!origin || !record._timing || fromCache || - WHITELISTED_STATUS_CODES.includes(record.statusCode) || !record.finished) { - return; - } - - // Disregard requests with an invalid start time, (H2 request start times are sometimes less - // than issue time and even negative which throws off timing) - if (record._startTime < record._issueTime) { - return; - } - - // Use DevTools' definition of Waiting latency: https://github.com/ChromeDevTools/devtools-frontend/blob/66595b8a73a9c873ea7714205b828866630e9e82/front_end/network/RequestTimingView.js#L164 - const latency = record._timing.receiveHeadersEnd - record._timing.sendEnd; - const latencyInfo = { - url: record._url, - startTime: record._startTime, - origin, - latency, - }; - - // Only examine the first request per origin to reduce noisiness from cases like H2 push - // where individual request latency may not apply. - const existing = firstRequestLatenciesByOrigin.get(origin); - if (!existing || latencyInfo.startTime < existing.startTime) { - firstRequestLatenciesByOrigin.set(origin, latencyInfo); - } - }); - - let firstRequestLatencies = Array.from(firstRequestLatenciesByOrigin.values()); - const latency3gMin = targetLatencyMs - 10; - const areLatenciesAll3G = firstRequestLatencies.every(val => val.latency > latency3gMin); - firstRequestLatencies = firstRequestLatencies.map(item => ({ - url: item.url, - latency: Util.formatNumber(item.latency, 0.01), - })); - - const trace = artifacts.traces[Audit.DEFAULT_PASS]; - return artifacts.requestFirstInteractive(trace).then(firstInteractive => { - const timeToFirstInteractive = firstInteractive.timeInMs; - const isFast = timeToFirstInteractive < MAXIMUM_TTFI; - - const extendedInfo = { - value: {areLatenciesAll3G, firstRequestLatencies, isFast, timeToFirstInteractive}, - }; + static async audit(artifacts, context) { + const trace = artifacts.traces[Audit.DEFAULT_PASS]; + const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; + + // If throttling was default devtools or lantern 3G throttling, then reuse the given settings + // Otherwise, we'll force the usage of lantern 3G. + const settingOverrides = {throttlingMethod: 'simulate', throttling: mobile3GThrottling}; + const settings = + context.settings.throttlingMethod !== 'provided' && + isDeepEqual(context.settings.throttling, mobile3GThrottling) + ? context.settings + : Object.assign({}, context.settings, settingOverrides); + + const metricComputationData = {trace, devtoolsLog, settings}; + const tti = await artifacts.requestConsistentlyInteractive(metricComputationData); + + const score = Number(tti.timing < MAXIMUM_TTI); + + let debugString; + if (!score) { + // eslint-disable-next-line max-len + debugString = `First Interactive was ${Util.formatMilliseconds(tti.timing)}. More details in the "Performance" section.`; + } - const details = Audit.makeTableDetails([ - {key: 'url', itemType: 'url', text: 'URL'}, - {key: 'latency', itemType: 'text', text: 'Latency (ms)'}, - ], firstRequestLatencies); - - if (!isFast) { - return { - rawValue: false, - // eslint-disable-next-line max-len - debugString: `First Interactive was at ${Util.formatMilliseconds(timeToFirstInteractive)}. More details in the "Performance" section.`, - extendedInfo, - }; - } - - if (!areLatenciesAll3G) { - const sentryContext = Sentry.getContext(); - const hadThrottlingEnabled = sentryContext && sentryContext.extra && - sentryContext.extra.networkThrottling; - - if (hadThrottlingEnabled) { - // Track these instances in Sentry since there should be no requests that are fast when - // throttling is enabled, and it's likely a throttling bug we should look into. - const violatingLatency = firstRequestLatencies - .find(item => Number(item.latency) < latency3gMin); - Sentry.captureMessage('Network request latencies were not realistic', { - tags: {audit: this.meta.name}, - extra: {violatingLatency}, - level: 'warning', - }); - } - - return { - rawValue: true, - // eslint-disable-next-line max-len - debugString: `First Interactive was found at ${Util.formatMilliseconds(timeToFirstInteractive)}; however, the network request latencies were not sufficiently realistic, so the performance measurements cannot be trusted.`, - extendedInfo, - details, - }; - } - - return { - rawValue: true, - extendedInfo, - }; - }); - }); + return { + score, + debugString, + rawValue: tti.timing, + }; } } diff --git a/lighthouse-core/gather/computed/metrics/metric.js b/lighthouse-core/gather/computed/metrics/metric.js index 8838cfc03368..36148217564f 100644 --- a/lighthouse-core/gather/computed/metrics/metric.js +++ b/lighthouse-core/gather/computed/metrics/metric.js @@ -48,6 +48,10 @@ class ComputedMetric extends ComputedArtifact { */ async compute_(data, artifacts) { const {trace, devtoolsLog, settings} = data; + if (!trace || !devtoolsLog || !settings) { + throw new Error('Did not provide necessary metric computation data'); + } + const augmentedData = Object.assign({ networkRecords: await artifacts.requestNetworkRecords(devtoolsLog), traceOfTab: await artifacts.requestTraceOfTab(trace), diff --git a/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js b/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js index 7b1ac6cb9ee8..8e784c0175fb 100644 --- a/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js +++ b/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js @@ -6,22 +6,24 @@ 'use strict'; const FastPWAAudit = require('../../audits/load-fast-enough-for-pwa'); +const Runner = require('../../runner.js'); const Audit = require('../../audits/audit.js'); +const mobile3GThrottling = require('../../config/constants').throttling.mobile3G; const assert = require('assert'); -function generateArtifacts(firstInteractiveValue, networkRecords = []) { +const trace = require('../fixtures/traces/progressive-app-m60.json'); +const devtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json'); + +function generateArtifacts(ttiValue) { return { devtoolsLogs: { [Audit.DEFAULT_PASS]: [], }, - requestNetworkRecords: () => { - return Promise.resolve(networkRecords); - }, traces: { [Audit.DEFAULT_PASS]: {traceEvents: []}, }, - requestFirstInteractive: () => Promise.resolve({ - timeInMs: firstInteractiveValue, + requestConsistentlyInteractive: () => Promise.resolve({ + timing: ttiValue, }), }; } @@ -29,65 +31,41 @@ function generateArtifacts(firstInteractiveValue, networkRecords = []) { /* eslint-env mocha */ describe('PWA: load-fast-enough-for-pwa audit', () => { it('returns boolean based on TTI value', () => { - return FastPWAAudit.audit(generateArtifacts(5000)).then(result => { - assert.equal(result.rawValue, true, 'fixture trace is not passing audit'); + const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling}; + return FastPWAAudit.audit(generateArtifacts(5000), {settings}).then(result => { + assert.equal(result.score, true, 'fixture trace is not passing audit'); + assert.equal(result.rawValue, 5000); }); }); it('fails a bad TTI value', () => { - return FastPWAAudit.audit(generateArtifacts(15000)).then(result => { - assert.equal(result.rawValue, false, 'not failing a long TTI value'); + const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling}; + return FastPWAAudit.audit(generateArtifacts(15000), {settings}).then(result => { + assert.equal(result.score, false, 'not failing a long TTI value'); + assert.equal(result.rawValue, 15000); assert.ok(result.debugString); }); }); - it('warns on a good TTI value with no throttling', () => { - // latencies are very short - const mockNetworkRecords = [ - {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/'}, - {_timing: {sendEnd: 0, receiveHeadersEnd: 75}, finished: true, _url: 'https://google.com/a'}, - { }, - {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/b'}, - ]; - return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => { - assert.equal(result.rawValue, true); - assert.ok(result.debugString.includes('network request latencies')); - assert.ok(result.details, 'contains details when latencies were not realistic'); - }); - }); + it('respects the observed result when throttling is preset', async () => { + const artifacts = Object.assign({ + traces: {defaultPass: trace}, + devtoolsLogs: {defaultPass: devtoolsLog}, + }, Runner.instantiateComputedArtifacts()); - it('ignores resources coming from cache', () => { - const mockNetworkRecords = [ - {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, _fromDiskCache: true}, - ]; - return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => { - assert.equal(result.rawValue, true); - assert.strictEqual(result.debugString, undefined); - }); + const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling}; + const result = await FastPWAAudit.audit(artifacts, {settings}); + assert.equal(Math.round(result.rawValue), 1582); }); - it('passes a good TTI value and WITH throttling', () => { - // latencies are very long - const urlA = 'https://google.com'; - const urlB = 'https://example.com'; - const urlC = 'https://example-c.com'; - const mockNetworkRecords = [ - {_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true, _url: urlA, _startTime: 0}, - {_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true, _url: urlB}, - // ignored for not having timing - { }, - // ignored for not being the first of the origin - {_timing: {sendEnd: 0, receiveHeadersEnd: 100}, finished: true, _url: urlA, _startTime: 100}, - // ignored for being redirected internally - {_timing: {sendEnd: 0, receiveHeadersEnd: 100}, finished: true, _url: urlC, _startTime: 0, - statusCode: 307}, - // ignored for not finishing - {_timing: {sendEnd: 0, receiveHeadersEnd: -1}, finished: false}, - ]; - return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => { - assert.equal(result.rawValue, true); - assert.strictEqual(result.debugString, undefined); - assert.ok(!result.details, 'does not contain details when latencies are realistic'); - }); + it('overrides with simulated result when throttling is modified', async () => { + const artifacts = Object.assign({ + traces: {defaultPass: trace}, + devtoolsLogs: {defaultPass: devtoolsLog}, + }, Runner.instantiateComputedArtifacts()); + + const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}}; + const result = await FastPWAAudit.audit(artifacts, {settings}); + assert.equal(Math.round(result.rawValue), 5308); }); }); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 207d497290a4..acae515527bb 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -105,24 +105,7 @@ "load-fast-enough-for-pwa": { "score": 1, "displayValue": "", - "rawValue": true, - "extendedInfo": { - "value": { - "areLatenciesAll3G": true, - "firstRequestLatencies": [ - { - "url": "http://localhost:10200/dobetterweb/dbw_tester.html", - "latency": "570.56" - }, - { - "url": "http://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js", - "latency": "564.12" - } - ], - "isFast": true, - "timeToFirstInteractive": 4927.278 - } - }, + "rawValue": 4927.278, "scoreDisplayMode": "binary", "name": "load-fast-enough-for-pwa", "description": "Page load is fast enough on 3G", @@ -5403,6 +5386,6 @@ } }, "timing": { - "total": 877 + "total": 788 } } \ No newline at end of file