From 3d219f868d69495afbd777b00a718cfe0f64b8f7 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 21 Aug 2018 15:00:09 -0700 Subject: [PATCH 1/4] core(tsc): tighten traceOfTab timing types --- .../render-blocking-resources.js | 4 ++ lighthouse-core/audits/metrics.js | 24 ++++----- .../metrics/first-contentful-paint.js | 9 ++-- .../gather/computed/metrics/first-cpu-idle.js | 8 +-- .../metrics/first-meaningful-paint.js | 9 ++-- .../gather/computed/metrics/interactive.js | 3 ++ .../metrics/lantern-first-contentful-paint.js | 9 ++++ .../metrics/lantern-first-meaningful-paint.js | 9 ++++ .../gather/computed/trace-of-tab.js | 54 ++++++++++--------- .../computed/metrics/first-cpu-idle-test.js | 1 + typings/artifacts.d.ts | 39 +++++++++----- 11 files changed, 106 insertions(+), 63 deletions(-) diff --git a/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js b/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js index f823e655ef3c..07b499c2b537 100644 --- a/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js +++ b/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js @@ -15,6 +15,7 @@ const BaseNode = require('../../lib/dependency-graph/base-node'); const ByteEfficiencyAudit = require('./byte-efficiency-audit'); const UnusedCSS = require('./unused-css-rules'); const NetworkRequest = require('../../lib/network-request'); +const LHError = require('../../lib/errors'); /** @typedef {import('../../lib/dependency-graph/simulator/simulator')} Simulator */ /** @typedef {import('../../lib/dependency-graph/base-node.js').Node} Node */ @@ -91,6 +92,9 @@ class RenderBlockingResources extends Audit { const metricComputationData = {trace, devtoolsLog, simulator, settings: metricSettings}; // @ts-ignore - TODO(bckenny): allow optional `throttling` settings const fcpSimulation = await artifacts.requestFirstContentfulPaint(metricComputationData); + if (!traceOfTab.timestamps.firstContentfulPaint) { + throw new LHError(LHError.errors.NO_FCP); + } const fcpTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000; const nodesByUrl = getNodesAndTimingByUrl(fcpSimulation.optimisticEstimate.nodeTimings); diff --git a/lighthouse-core/audits/metrics.js b/lighthouse-core/audits/metrics.js index 7f7a53107161..6523d7452df8 100644 --- a/lighthouse-core/audits/metrics.js +++ b/lighthouse-core/audits/metrics.js @@ -115,18 +115,18 @@ class Metrics extends Audit { * @property {number=} estimatedInputLatencyTs * @property {number} observedNavigationStart * @property {number} observedNavigationStartTs - * @property {number} observedFirstPaint - * @property {number} observedFirstPaintTs - * @property {number} observedFirstContentfulPaint - * @property {number} observedFirstContentfulPaintTs - * @property {number} observedFirstMeaningfulPaint - * @property {number} observedFirstMeaningfulPaintTs - * @property {number} observedTraceEnd - * @property {number} observedTraceEndTs - * @property {number} observedLoad - * @property {number} observedLoadTs - * @property {number} observedDomContentLoaded - * @property {number} observedDomContentLoadedTs + * @property {number=} observedFirstPaint + * @property {number=} observedFirstPaintTs + * @property {number=} observedFirstContentfulPaint + * @property {number=} observedFirstContentfulPaintTs + * @property {number=} observedFirstMeaningfulPaint + * @property {number=} observedFirstMeaningfulPaintTs + * @property {number=} observedTraceEnd + * @property {number=} observedTraceEndTs + * @property {number=} observedLoad + * @property {number=} observedLoadTs + * @property {number=} observedDomContentLoaded + * @property {number=} observedDomContentLoadedTs * @property {number} observedFirstVisualChange * @property {number} observedFirstVisualChangeTs * @property {number} observedLastVisualChange diff --git a/lighthouse-core/gather/computed/metrics/first-contentful-paint.js b/lighthouse-core/gather/computed/metrics/first-contentful-paint.js index e84c45b008e3..635c15476532 100644 --- a/lighthouse-core/gather/computed/metrics/first-contentful-paint.js +++ b/lighthouse-core/gather/computed/metrics/first-contentful-paint.js @@ -17,16 +17,17 @@ class FirstContentfulPaint extends MetricArtifact { * @param {LH.Artifacts.MetricComputationData} data * @return {Promise} */ - computeObservedMetric(data) { + async computeObservedMetric(data) { const {traceOfTab} = data; if (!traceOfTab.timestamps.firstContentfulPaint) { throw new LHError(LHError.errors.NO_FCP); } - return Promise.resolve({ - timing: traceOfTab.timings.firstContentfulPaint, + return { + // FCP established as existing, so cast + timing: /** @type {number} */ (traceOfTab.timings.firstContentfulPaint), timestamp: traceOfTab.timestamps.firstContentfulPaint, - }); + }; } } diff --git a/lighthouse-core/gather/computed/metrics/first-cpu-idle.js b/lighthouse-core/gather/computed/metrics/first-cpu-idle.js index 95f485b2b025..2ce2beac65d5 100644 --- a/lighthouse-core/gather/computed/metrics/first-cpu-idle.js +++ b/lighthouse-core/gather/computed/metrics/first-cpu-idle.js @@ -177,14 +177,14 @@ class FirstCPUIdle extends MetricArtifact { const DCL = traceOfTab.timings.domContentLoaded; const traceEnd = traceOfTab.timings.traceEnd; - if (traceEnd - FMP < MAX_QUIET_WINDOW_SIZE) { - throw new LHError(LHError.errors.FMP_TOO_LATE_FOR_FCPUI); - } - if (!FMP || !DCL) { throw new LHError(FMP ? LHError.errors.NO_DCL : LHError.errors.NO_FMP); } + if (traceEnd - FMP < MAX_QUIET_WINDOW_SIZE) { + throw new LHError(LHError.errors.FMP_TOO_LATE_FOR_FCPUI); + } + const longTasksAfterFMP = TracingProcessor.getMainThreadTopLevelEvents(traceOfTab, FMP) .filter(evt => evt.duration >= LONG_TASK_THRESHOLD); const firstInteractive = FirstCPUIdle.findQuietWindow(FMP, traceEnd, longTasksAfterFMP); diff --git a/lighthouse-core/gather/computed/metrics/first-meaningful-paint.js b/lighthouse-core/gather/computed/metrics/first-meaningful-paint.js index 20b7da4f8ea1..ca9bdc44d39b 100644 --- a/lighthouse-core/gather/computed/metrics/first-meaningful-paint.js +++ b/lighthouse-core/gather/computed/metrics/first-meaningful-paint.js @@ -17,16 +17,17 @@ class FirstMeaningfulPaint extends MetricArtifact { * @param {LH.Artifacts.MetricComputationData} data * @return {Promise} */ - computeObservedMetric(data) { + async computeObservedMetric(data) { const {traceOfTab} = data; if (!traceOfTab.timestamps.firstMeaningfulPaint) { throw new LHError(LHError.errors.NO_FMP); } - return Promise.resolve({ - timing: traceOfTab.timings.firstMeaningfulPaint, + return { + // FMP established as existing, so cast + timing: /** @type {number} */ (traceOfTab.timings.firstMeaningfulPaint), timestamp: traceOfTab.timestamps.firstMeaningfulPaint, - }); + }; } } diff --git a/lighthouse-core/gather/computed/metrics/interactive.js b/lighthouse-core/gather/computed/metrics/interactive.js index bb4bd7bcc4c8..08ec019c8fe1 100644 --- a/lighthouse-core/gather/computed/metrics/interactive.js +++ b/lighthouse-core/gather/computed/metrics/interactive.js @@ -90,6 +90,9 @@ class Interactive extends MetricArtifact { * @return {{cpuQuietPeriod: TimePeriod, networkQuietPeriod: TimePeriod, cpuQuietPeriods: Array, networkQuietPeriods: Array}} */ static findOverlappingQuietPeriods(longTasks, networkRecords, traceOfTab) { + if (!traceOfTab.timestamps.firstContentfulPaint) { + throw new LHError(LHError.errors.NO_FCP); + } const FcpTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000; /** @type {function(TimePeriod):boolean} */ diff --git a/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js b/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js index 1f6cc7f6edd7..ffbfe775bbfe 100644 --- a/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js +++ b/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js @@ -7,6 +7,7 @@ const MetricArtifact = require('./lantern-metric'); const BaseNode = require('../../../lib/dependency-graph/base-node'); +const LHError = require('../../../lib/errors'); /** @typedef {BaseNode.Node} Node */ @@ -33,6 +34,10 @@ class FirstContentfulPaint extends MetricArtifact { */ getOptimisticGraph(dependencyGraph, traceOfTab) { const fcp = traceOfTab.timestamps.firstContentfulPaint; + if (!fcp) { + throw new LHError(LHError.errors.NO_FCP); + } + const blockingScriptUrls = MetricArtifact.getScriptUrls(dependencyGraph, node => { return ( node.endTime <= fcp && node.hasRenderBlockingPriority() && node.initiatorType !== 'script' @@ -58,6 +63,10 @@ class FirstContentfulPaint extends MetricArtifact { */ getPessimisticGraph(dependencyGraph, traceOfTab) { const fcp = traceOfTab.timestamps.firstContentfulPaint; + if (!fcp) { + throw new LHError(LHError.errors.NO_FCP); + } + const blockingScriptUrls = MetricArtifact.getScriptUrls(dependencyGraph, node => { return node.endTime <= fcp && node.hasRenderBlockingPriority(); }); diff --git a/lighthouse-core/gather/computed/metrics/lantern-first-meaningful-paint.js b/lighthouse-core/gather/computed/metrics/lantern-first-meaningful-paint.js index 9bbcbe262002..726b592fcfdc 100644 --- a/lighthouse-core/gather/computed/metrics/lantern-first-meaningful-paint.js +++ b/lighthouse-core/gather/computed/metrics/lantern-first-meaningful-paint.js @@ -7,6 +7,7 @@ const MetricArtifact = require('./lantern-metric'); const BaseNode = require('../../../lib/dependency-graph/base-node'); +const LHError = require('../../../lib/errors'); /** @typedef {BaseNode.Node} Node */ @@ -33,6 +34,10 @@ class FirstMeaningfulPaint extends MetricArtifact { */ getOptimisticGraph(dependencyGraph, traceOfTab) { const fmp = traceOfTab.timestamps.firstMeaningfulPaint; + if (!fmp) { + throw new LHError(LHError.errors.NO_FMP); + } + const blockingScriptUrls = MetricArtifact.getScriptUrls(dependencyGraph, node => { return ( node.endTime <= fmp && node.hasRenderBlockingPriority() && node.initiatorType !== 'script' @@ -58,6 +63,10 @@ class FirstMeaningfulPaint extends MetricArtifact { */ getPessimisticGraph(dependencyGraph, traceOfTab) { const fmp = traceOfTab.timestamps.firstMeaningfulPaint; + if (!fmp) { + throw new LHError(LHError.errors.NO_FMP); + } + const requiredScriptUrls = MetricArtifact.getScriptUrls(dependencyGraph, node => { return node.endTime <= fmp && node.hasRenderBlockingPriority(); }); diff --git a/lighthouse-core/gather/computed/trace-of-tab.js b/lighthouse-core/gather/computed/trace-of-tab.js index 696c6fbe470a..baaa3586ef54 100644 --- a/lighthouse-core/gather/computed/trace-of-tab.js +++ b/lighthouse-core/gather/computed/trace-of-tab.js @@ -22,11 +22,6 @@ const TracingProcessor = require('../../lib/traces/tracing-processor'); const LHError = require('../../lib/errors'); const Sentry = require('../../lib/sentry'); -// Bring in web-inspector for side effect of adding [].stableSort -// See https://github.com/GoogleChrome/lighthouse/pull/2415 -// eslint-disable-next-line no-unused-vars -const NetworkRequest = require('../../lib/network-request'); - class TraceOfTab extends ComputedArtifact { get name() { return 'TraceOfTab'; @@ -133,34 +128,41 @@ class TraceOfTab extends ComputedArtifact { const mainThreadEvents = processEvents .filter(e => e.tid === startedInPageEvt.tid); + // traceEnd must exist since at least navigationStart event was verified as existing. const traceEnd = trace.traceEvents.reduce((max, evt) => { return max.ts > evt.ts ? max : evt; }); - - const metrics = { - navigationStart, - firstPaint, - firstContentfulPaint, - firstMeaningfulPaint, - traceEnd: {ts: traceEnd.ts + (traceEnd.dur || 0)}, - load, - domContentLoaded, + const fakeEndOfTraceEvt = {ts: traceEnd.ts + (traceEnd.dur || 0)}; + + /** @param {{ts: number}=} event */ + const getTimestamp = (event) => event && event.ts; + /** @type {LH.Artifacts.TraceTimes} */ + const timestamps = { + navigationStart: navigationStart.ts, + firstPaint: getTimestamp(firstPaint), + firstContentfulPaint: getTimestamp(firstContentfulPaint), + firstMeaningfulPaint: getTimestamp(firstMeaningfulPaint), + traceEnd: fakeEndOfTraceEvt.ts, + load: getTimestamp(load), + domContentLoaded: getTimestamp(domContentLoaded), }; - const timings = {}; - const timestamps = {}; - - Object.keys(metrics).forEach(metric => { - timestamps[metric] = metrics[metric] && metrics[metric].ts; - timings[metric] = (timestamps[metric] - navigationStart.ts) / 1000; - }); + /** @param {number=} ts */ + const getTiming = (ts) => ts === undefined ? undefined : (ts - navigationStart.ts) / 1000; + /** @type {LH.Artifacts.TraceTimes} */ + const timings = { + navigationStart: 0, + firstPaint: getTiming(timestamps.firstPaint), + firstContentfulPaint: getTiming(timestamps.firstContentfulPaint), + firstMeaningfulPaint: getTiming(timestamps.firstMeaningfulPaint), + traceEnd: (timestamps.traceEnd - navigationStart.ts) / 1000, + load: getTiming(timestamps.load), + domContentLoaded: getTiming(timestamps.domContentLoaded), + }; - // @ts-ignore - TODO(bckenny): many of these are actually `|undefined`, but - // undefined case needs to be handled throughout codebase. See also note for - // LH.Artifacts.TraceOfTab. return { - timings: /** @type {LH.Artifacts.TraceTimes} */ (timings), - timestamps: /** @type {LH.Artifacts.TraceTimes} */ (timestamps), + timings, + timestamps, processEvents, mainThreadEvents, startedInPageEvt, diff --git a/lighthouse-core/test/gather/computed/metrics/first-cpu-idle-test.js b/lighthouse-core/test/gather/computed/metrics/first-cpu-idle-test.js index 4d0602972b8d..f5ff6b61ceed 100644 --- a/lighthouse-core/test/gather/computed/metrics/first-cpu-idle-test.js +++ b/lighthouse-core/test/gather/computed/metrics/first-cpu-idle-test.js @@ -95,6 +95,7 @@ describe('FirstInteractive computed artifact:', () => { computeObservedMetric({ timings: { firstMeaningfulPaint: 3400, + domContentLoaded: 2000, traceEnd: 4500, }, timestamps: { diff --git a/typings/artifacts.d.ts b/typings/artifacts.d.ts index 9f3103892b7a..8c79fa10a359 100644 --- a/typings/artifacts.d.ts +++ b/typings/artifacts.d.ts @@ -365,30 +365,43 @@ declare global { export type Speedline = speedline.Output<'speedIndex'>; - // TODO(bckenny): all but navigationStart could actually be undefined. export interface TraceTimes { navigationStart: number; - firstPaint: number; - firstContentfulPaint: number; - firstMeaningfulPaint: number; + firstPaint?: number; + firstContentfulPaint?: number; + firstMeaningfulPaint?: number; traceEnd: number; - load: number; - domContentLoaded: number; + load?: number; + domContentLoaded?: number; } - // TODO(bckenny): events other than started and navStart could be undefined. export interface TraceOfTab { - timings: TraceTimes; + /** The raw timestamps of key metric events, in microseconds. */ timestamps: TraceTimes; + /** The relative times from navigationStart to key metric events, in milliseconds. */ + timings: TraceTimes; + /** The subset of trace events from the page's process, sorted by timestamp. */ processEvents: Array; + /** The subset of trace events from the page's main thread, sorted by timestamp. */ mainThreadEvents: Array; + /** The event marking the start of tracing in the target browser. */ startedInPageEvt: TraceEvent; + /** The trace event marking navigationStart. */ navigationStartEvt: TraceEvent; - firstPaintEvt: TraceEvent; - firstContentfulPaintEvt: TraceEvent; - firstMeaningfulPaintEvt: TraceEvent; - loadEvt: TraceEvent; - domContentLoadedEvt: TraceEvent; + /** The trace event marking firstPaint, if it was found. */ + firstPaintEvt?: TraceEvent; + /** The trace event marking firstContentfulPaint, if it was found. */ + firstContentfulPaintEvt?: TraceEvent; + /** The trace event marking firstMeaningfulPaint, if it was found. */ + firstMeaningfulPaintEvt?: TraceEvent; + /** The trace event marking loadEventEnd, if it was found. */ + loadEvt?: TraceEvent; + /** The trace event marking domContentLoadedEventEnd, if it was found. */ + domContentLoadedEvt?: TraceEvent; + /** + * Whether the firstMeaningfulPaintEvt was the definitive event or a fallback to + * firstMeaningfulPaintCandidate events had to be attempted. + */ fmpFellBack: boolean; } } From d7d5b34c3bea068237d9a9d6596669c5ae37e191 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 21 Aug 2018 17:33:40 -0700 Subject: [PATCH 2/4] throw on lack of fcp --- .../render-blocking-resources.js | 4 --- lighthouse-core/audits/metrics.js | 4 +-- .../metrics/first-contentful-paint.js | 7 +--- .../gather/computed/metrics/interactive.js | 6 ---- .../metrics/lantern-first-contentful-paint.js | 9 ----- .../gather/computed/trace-of-tab.js | 20 ++++++----- .../test/audits/bootup-time-test.js | 2 +- .../audits/mainthread-work-breakdown-test.js | 2 +- .../backgrounded-tab-missing-paints.json | 2 +- .../test/fixtures/traces/load.json | 1 + .../threeframes-blank_content_more.json | 12 +++++++ .../gather/computed/main-thread-tasks-test.js | 5 +-- .../metrics/first-meaningful-paint-test.js | 9 ----- .../test/gather/computed/trace-of-tab-test.js | 34 +++++++++++++------ typings/artifacts.d.ts | 2 +- 15 files changed, 59 insertions(+), 60 deletions(-) diff --git a/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js b/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js index 07b499c2b537..f823e655ef3c 100644 --- a/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js +++ b/lighthouse-core/audits/byte-efficiency/render-blocking-resources.js @@ -15,7 +15,6 @@ const BaseNode = require('../../lib/dependency-graph/base-node'); const ByteEfficiencyAudit = require('./byte-efficiency-audit'); const UnusedCSS = require('./unused-css-rules'); const NetworkRequest = require('../../lib/network-request'); -const LHError = require('../../lib/errors'); /** @typedef {import('../../lib/dependency-graph/simulator/simulator')} Simulator */ /** @typedef {import('../../lib/dependency-graph/base-node.js').Node} Node */ @@ -92,9 +91,6 @@ class RenderBlockingResources extends Audit { const metricComputationData = {trace, devtoolsLog, simulator, settings: metricSettings}; // @ts-ignore - TODO(bckenny): allow optional `throttling` settings const fcpSimulation = await artifacts.requestFirstContentfulPaint(metricComputationData); - if (!traceOfTab.timestamps.firstContentfulPaint) { - throw new LHError(LHError.errors.NO_FCP); - } const fcpTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000; const nodesByUrl = getNodesAndTimingByUrl(fcpSimulation.optimisticEstimate.nodeTimings); diff --git a/lighthouse-core/audits/metrics.js b/lighthouse-core/audits/metrics.js index 6523d7452df8..9113cc5be475 100644 --- a/lighthouse-core/audits/metrics.js +++ b/lighthouse-core/audits/metrics.js @@ -117,8 +117,8 @@ class Metrics extends Audit { * @property {number} observedNavigationStartTs * @property {number=} observedFirstPaint * @property {number=} observedFirstPaintTs - * @property {number=} observedFirstContentfulPaint - * @property {number=} observedFirstContentfulPaintTs + * @property {number} observedFirstContentfulPaint + * @property {number} observedFirstContentfulPaintTs * @property {number=} observedFirstMeaningfulPaint * @property {number=} observedFirstMeaningfulPaintTs * @property {number=} observedTraceEnd diff --git a/lighthouse-core/gather/computed/metrics/first-contentful-paint.js b/lighthouse-core/gather/computed/metrics/first-contentful-paint.js index 635c15476532..d82bf4fb60c1 100644 --- a/lighthouse-core/gather/computed/metrics/first-contentful-paint.js +++ b/lighthouse-core/gather/computed/metrics/first-contentful-paint.js @@ -6,7 +6,6 @@ 'use strict'; const MetricArtifact = require('./metric'); -const LHError = require('../../../lib/errors'); class FirstContentfulPaint extends MetricArtifact { get name() { @@ -19,13 +18,9 @@ class FirstContentfulPaint extends MetricArtifact { */ async computeObservedMetric(data) { const {traceOfTab} = data; - if (!traceOfTab.timestamps.firstContentfulPaint) { - throw new LHError(LHError.errors.NO_FCP); - } return { - // FCP established as existing, so cast - timing: /** @type {number} */ (traceOfTab.timings.firstContentfulPaint), + timing: traceOfTab.timings.firstContentfulPaint, timestamp: traceOfTab.timestamps.firstContentfulPaint, }; } diff --git a/lighthouse-core/gather/computed/metrics/interactive.js b/lighthouse-core/gather/computed/metrics/interactive.js index 08ec019c8fe1..53ec137f12c1 100644 --- a/lighthouse-core/gather/computed/metrics/interactive.js +++ b/lighthouse-core/gather/computed/metrics/interactive.js @@ -90,9 +90,6 @@ class Interactive extends MetricArtifact { * @return {{cpuQuietPeriod: TimePeriod, networkQuietPeriod: TimePeriod, cpuQuietPeriods: Array, networkQuietPeriods: Array}} */ static findOverlappingQuietPeriods(longTasks, networkRecords, traceOfTab) { - if (!traceOfTab.timestamps.firstContentfulPaint) { - throw new LHError(LHError.errors.NO_FCP); - } const FcpTsInMs = traceOfTab.timestamps.firstContentfulPaint / 1000; /** @type {function(TimePeriod):boolean} */ @@ -151,9 +148,6 @@ class Interactive extends MetricArtifact { */ computeObservedMetric(data) { const {traceOfTab, networkRecords} = data; - if (!traceOfTab.timestamps.firstContentfulPaint) { - throw new LHError(LHError.errors.NO_FCP); - } if (!traceOfTab.timestamps.domContentLoaded) { throw new LHError(LHError.errors.NO_DCL); diff --git a/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js b/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js index ffbfe775bbfe..1f6cc7f6edd7 100644 --- a/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js +++ b/lighthouse-core/gather/computed/metrics/lantern-first-contentful-paint.js @@ -7,7 +7,6 @@ const MetricArtifact = require('./lantern-metric'); const BaseNode = require('../../../lib/dependency-graph/base-node'); -const LHError = require('../../../lib/errors'); /** @typedef {BaseNode.Node} Node */ @@ -34,10 +33,6 @@ class FirstContentfulPaint extends MetricArtifact { */ getOptimisticGraph(dependencyGraph, traceOfTab) { const fcp = traceOfTab.timestamps.firstContentfulPaint; - if (!fcp) { - throw new LHError(LHError.errors.NO_FCP); - } - const blockingScriptUrls = MetricArtifact.getScriptUrls(dependencyGraph, node => { return ( node.endTime <= fcp && node.hasRenderBlockingPriority() && node.initiatorType !== 'script' @@ -63,10 +58,6 @@ class FirstContentfulPaint extends MetricArtifact { */ getPessimisticGraph(dependencyGraph, traceOfTab) { const fcp = traceOfTab.timestamps.firstContentfulPaint; - if (!fcp) { - throw new LHError(LHError.errors.NO_FCP); - } - const blockingScriptUrls = MetricArtifact.getScriptUrls(dependencyGraph, node => { return node.endTime <= fcp && node.hasRenderBlockingPriority(); }); diff --git a/lighthouse-core/gather/computed/trace-of-tab.js b/lighthouse-core/gather/computed/trace-of-tab.js index baaa3586ef54..a3addf08e535 100644 --- a/lighthouse-core/gather/computed/trace-of-tab.js +++ b/lighthouse-core/gather/computed/trace-of-tab.js @@ -85,10 +85,11 @@ class TraceOfTab extends ComputedArtifact { // Find our first paint of this frame const firstPaint = frameEvents.find(e => e.name === 'firstPaint' && e.ts > navigationStart.ts); - // FCP will follow at/after the FP + // FCP will follow at/after the FP. Used in so many places we require it. const firstContentfulPaint = frameEvents.find( e => e.name === 'firstContentfulPaint' && e.ts > navigationStart.ts ); + if (!firstContentfulPaint) throw new LHError(LHError.errors.NO_FCP); // fMP will follow at/after the FP let firstMeaningfulPaint = frameEvents.find( @@ -140,24 +141,27 @@ class TraceOfTab extends ComputedArtifact { const timestamps = { navigationStart: navigationStart.ts, firstPaint: getTimestamp(firstPaint), - firstContentfulPaint: getTimestamp(firstContentfulPaint), + firstContentfulPaint: firstContentfulPaint.ts, firstMeaningfulPaint: getTimestamp(firstMeaningfulPaint), traceEnd: fakeEndOfTraceEvt.ts, load: getTimestamp(load), domContentLoaded: getTimestamp(domContentLoaded), }; + + /** @param {number} ts */ + const getTiming = (ts) => (ts - navigationStart.ts) / 1000; /** @param {number=} ts */ - const getTiming = (ts) => ts === undefined ? undefined : (ts - navigationStart.ts) / 1000; + const maybeGetTiming = (ts) => ts === undefined ? undefined : getTiming(ts); /** @type {LH.Artifacts.TraceTimes} */ const timings = { navigationStart: 0, - firstPaint: getTiming(timestamps.firstPaint), + firstPaint: maybeGetTiming(timestamps.firstPaint), firstContentfulPaint: getTiming(timestamps.firstContentfulPaint), - firstMeaningfulPaint: getTiming(timestamps.firstMeaningfulPaint), - traceEnd: (timestamps.traceEnd - navigationStart.ts) / 1000, - load: getTiming(timestamps.load), - domContentLoaded: getTiming(timestamps.domContentLoaded), + firstMeaningfulPaint: maybeGetTiming(timestamps.firstMeaningfulPaint), + traceEnd: getTiming(timestamps.traceEnd), + load: maybeGetTiming(timestamps.load), + domContentLoaded: maybeGetTiming(timestamps.domContentLoaded), }; return { diff --git a/lighthouse-core/test/audits/bootup-time-test.js b/lighthouse-core/test/audits/bootup-time-test.js index 3a3117af04f5..420a9afd7f05 100644 --- a/lighthouse-core/test/audits/bootup-time-test.js +++ b/lighthouse-core/test/audits/bootup-time-test.js @@ -12,7 +12,7 @@ const assert = require('assert'); const acceptableTrace = require('../fixtures/traces/progressive-app-m60.json'); const acceptableDevtoolsLogs = require('../fixtures/traces/progressive-app-m60.devtools.log.json'); -const errorTrace = require('../fixtures/traces/airhorner_no_fcp.json'); +const errorTrace = require('../fixtures/traces/no_fmp_event.json'); describe('Performance: bootup-time audit', () => { const auditOptions = Object.assign({}, BootupTime.defaultOptions, {thresholdInMs: 10}); diff --git a/lighthouse-core/test/audits/mainthread-work-breakdown-test.js b/lighthouse-core/test/audits/mainthread-work-breakdown-test.js index 7061f988133b..5890bab7c7be 100644 --- a/lighthouse-core/test/audits/mainthread-work-breakdown-test.js +++ b/lighthouse-core/test/audits/mainthread-work-breakdown-test.js @@ -14,7 +14,7 @@ const options = PageExecutionTimings.defaultOptions; const acceptableTrace = require('../fixtures/traces/progressive-app-m60.json'); const siteWithRedirectTrace = require('../fixtures/traces/site-with-redirect.json'); const loadTrace = require('../fixtures/traces/load.json'); -const errorTrace = require('../fixtures/traces/airhorner_no_fcp.json'); +const errorTrace = require('../fixtures/traces/no_fmp_event.json'); const acceptableTraceExpectations = { parseHTML: 14, diff --git a/lighthouse-core/test/fixtures/traces/backgrounded-tab-missing-paints.json b/lighthouse-core/test/fixtures/traces/backgrounded-tab-missing-paints.json index 49557d5a2842..d903ac00ab78 100644 --- a/lighthouse-core/test/fixtures/traces/backgrounded-tab-missing-paints.json +++ b/lighthouse-core/test/fixtures/traces/backgrounded-tab-missing-paints.json @@ -91,7 +91,7 @@ "cat": "blink.user_timing,rail", "name": "firstContentfulPaint", "args": { - "frame": "0x1aff390e1e30" + "frame": "0x53965941e30" }, "tts": 239094, "s": "p" diff --git a/lighthouse-core/test/fixtures/traces/load.json b/lighthouse-core/test/fixtures/traces/load.json index 9106ffe97687..0ff31c49de5d 100644 --- a/lighthouse-core/test/fixtures/traces/load.json +++ b/lighthouse-core/test/fixtures/traces/load.json @@ -509,6 +509,7 @@ {"pid":442,"tid":461,"ts":73609802087,"ph":"X","cat":"toplevel","name":"TaskQueueManager::ProcessTaskFromWorkQueue","args":{"src_file":"../../ipc/ipc_channel_proxy.cc","src_func":"OnMessageReceivedNoFilter"},"dur":5851,"tdur":666,"tts":20275155}, {"pid":442,"tid":461,"ts":73609804410,"ph":"I","cat":"disabled-by-default-devtools.timeline","name":"TracingStartedInPage","args":{"data":{"sessionId":"442.138","page":"0xffffffffa1a94000"}},"tts":20275707,"s":"t"}, {"pid":442,"tid":461,"ts":73609804410,"ph":"R","cat":"blink.user_timing","name":"navigationStart","args":{"data":{"sessionId":"442.138"},"frame":"0xffffffffa1a94000"},"tts":20275707,"s":"t"}, +{"pid":442,"tid":461,"ts":73609804428,"ph": "R","cat":"loading,rail,devtools.timeline","name": "firstContentfulPaint","args":{"frame":"0xffffffffa1a94000"},"tts":20275720}, {"pid":442,"tid":461,"ts":73609804428,"ph":"I","cat":"disabled-by-default-devtools.timeline","name":"SetLayerTreeId","args":{"data":{"sessionId":"442.138","layerTreeId":1}},"tts":20275720,"s":"t"}, {"pid":442,"tid":461,"ts":73609807946,"ph":"X","cat":"toplevel","name":"TaskQueueManager::ProcessTaskFromWorkQueue","args":{"src_file":"../../media/audio/fake_audio_worker.cc","src_func":"DoRead"},"dur":88,"tdur":85,"tts":20275831}, {"pid":442,"tid":461,"ts":73609808041,"ph":"X","cat":"toplevel","name":"TaskQueueManager::ProcessTaskFromWorkQueue","args":{"src_file":"../../ipc/ipc_channel_proxy.cc","src_func":"OnMessageReceivedNoFilter"},"dur":193,"tdur":116,"tts":20275927}, diff --git a/lighthouse-core/test/fixtures/traces/threeframes-blank_content_more.json b/lighthouse-core/test/fixtures/traces/threeframes-blank_content_more.json index ab528381c16c..a58b3c2db8e2 100644 --- a/lighthouse-core/test/fixtures/traces/threeframes-blank_content_more.json +++ b/lighthouse-core/test/fixtures/traces/threeframes-blank_content_more.json @@ -45,6 +45,18 @@ "tdur": 186, "tts": 4516530 }, + { + "pid": 93449, + "tid": 1295, + "ts": 900001000000, + "ph": "R", + "cat": "loading,rail,devtools.timeline", + "name": "firstContentfulPaint", + "args": { + "frame": "dummy" + }, + "tts": 63259760 + }, { "pid": 93267, "tid": 1295, diff --git a/lighthouse-core/test/gather/computed/main-thread-tasks-test.js b/lighthouse-core/test/gather/computed/main-thread-tasks-test.js index 2f2ac83fd330..79518a7b33da 100644 --- a/lighthouse-core/test/gather/computed/main-thread-tasks-test.js +++ b/lighthouse-core/test/gather/computed/main-thread-tasks-test.js @@ -23,6 +23,7 @@ describe('MainResource computed artifact', () => { boilerplateTrace = [ {ph: 'I', name: 'TracingStartedInPage', ts: baseTs, args}, {ph: 'I', name: 'navigationStart', ts: baseTs, args}, + {ph: 'R', name: 'firstContentfulPaint', ts: baseTs + 1, args}, ]; computedArtifacts = Runner.instantiateComputedArtifacts(); }); @@ -79,7 +80,7 @@ describe('MainResource computed artifact', () => { attributableURLs: [], children: [taskB], - event: traceEvents[2], + event: traceEvents[3], startTime: 0, endTime: 100, duration: 100, @@ -92,7 +93,7 @@ describe('MainResource computed artifact', () => { attributableURLs: [], children: [taskC], - event: traceEvents[3], + event: traceEvents[4], startTime: 5, endTime: 55, duration: 50, diff --git a/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js b/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js index 278a1bf29dd7..dd57e63f3007 100644 --- a/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js +++ b/lighthouse-core/test/gather/computed/metrics/first-meaningful-paint-test.js @@ -16,7 +16,6 @@ const badNavStartTrace = require(`${TRACE_FIXTURES}/bad-nav-start-ts.json`); const lateTracingStartedTrace = require(`${TRACE_FIXTURES}/tracingstarted-after-navstart.json`); const preactTrace = require(`${TRACE_FIXTURES}/preactjs.com_ts_of_undefined.json`); const noFMPtrace = require(`${TRACE_FIXTURES}/no_fmp_event.json`); -const noFCPtrace = require(`${TRACE_FIXTURES}/airhorner_no_fcp.json`); /* eslint-env jest */ @@ -98,12 +97,4 @@ describe('Metrics: FMP', () => { assert.equal(Math.round(result.timing), 4461); assert.equal(result.timestamp, 2146740268666); }); - - it('handles cases when no FCP exists', async () => { - trace = noFCPtrace; - addEmptyTask(); - const result = await artifacts.requestFirstMeaningfulPaint({trace, devtoolsLog, settings}); - assert.equal(Math.round(result.timing), 482); - assert.equal(result.timestamp, 2149509604903); - }); }); diff --git a/lighthouse-core/test/gather/computed/trace-of-tab-test.js b/lighthouse-core/test/gather/computed/trace-of-tab-test.js index f1b2f4687508..ea5e624b54ee 100644 --- a/lighthouse-core/test/gather/computed/trace-of-tab-test.js +++ b/lighthouse-core/test/gather/computed/trace-of-tab-test.js @@ -15,6 +15,7 @@ const lateTracingStartedTrace = require('../../fixtures/traces/tracingstarted-af const preactTrace = require('../../fixtures/traces/preactjs.com_ts_of_undefined.json'); const noFMPtrace = require('../../fixtures/traces/no_fmp_event.json'); const noFCPtrace = require('../../fixtures/traces/airhorner_no_fcp'); +const noNavStartTrace = require('../../fixtures/traces/no_navstart_event'); const backgroundTabTrace = require('../../fixtures/traces/backgrounded-tab-missing-paints'); /* eslint-env jest */ @@ -89,15 +90,6 @@ describe('Trace of Tab computed artifact:', () => { }); }); - it('handles traces missing an FCP', async () => { - const trace = await traceOfTab.compute_(noFCPtrace); - assert.equal(trace.startedInPageEvt.ts, 2149509117532, 'bad tracingstartedInPage'); - assert.equal(trace.navigationStartEvt.ts, 2149509122585, 'bad navStart'); - assert.equal(trace.firstContentfulPaintEvt, undefined, 'bad fcp'); - assert.equal(trace.firstMeaningfulPaintEvt.ts, 2149509604903, 'bad fmp'); - assert.ok(trace.fmpFellBack); - }); - it('handles traces missing a paints (captured in background tab)', async () => { const trace = await traceOfTab.compute_(backgroundTabTrace); assert.equal(trace.startedInPageEvt.ts, 1966813248134); @@ -108,7 +100,6 @@ describe('Trace of Tab computed artifact:', () => { 1966813258737, 'didnt select navStart event with same timestamp as usertiming measure' ); - assert.equal(trace.firstContentfulPaintEvt, undefined, 'bad fcp'); assert.equal(trace.firstMeaningfulPaintEvt, undefined, 'bad fmp'); }); @@ -147,6 +138,17 @@ describe('Trace of Tab computed artifact:', () => { }, }, 'tts': 141371, + }, { + 'pid': 69920, + 'tid': 1, + 'ts': 2193564790060, + 'ph': 'R', + 'cat': 'loading,rail,devtools.timeline', + 'name': 'firstContentfulPaint', + 'args': { + 'frame': 'B192D1F3355A6F961EC8F0B01623C1FB', + }, + 'tts': 141372, }, { 'pid': 69920, 'tid': 1, @@ -190,4 +192,16 @@ describe('Trace of Tab computed artifact:', () => { assert.deepStrictEqual(sortedEvents, tsGroup); } }); + + it('throws on traces missing a navigationStart', () => { + return traceOfTab.compute_(noNavStartTrace) + .then(_ => assert(false, 'NO_NAVSTART error not throw'), + err => assert.equal(err.message, 'NO_NAVSTART')); + }); + + it('throws on traces missing an FCP', () => { + return traceOfTab.compute_(noFCPtrace) + .then(_ => assert(false, 'NO_FCP error not throw'), + err => assert.equal(err.message, 'NO_FCP')); + }); }); diff --git a/typings/artifacts.d.ts b/typings/artifacts.d.ts index 8c79fa10a359..02c85555dc7e 100644 --- a/typings/artifacts.d.ts +++ b/typings/artifacts.d.ts @@ -368,7 +368,7 @@ declare global { export interface TraceTimes { navigationStart: number; firstPaint?: number; - firstContentfulPaint?: number; + firstContentfulPaint: number; firstMeaningfulPaint?: number; traceEnd: number; load?: number; From bbe79b1705282018912ad0a47e06a4ed83082d9a Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 21 Aug 2018 17:47:03 -0700 Subject: [PATCH 3/4] always firstContentfulPaintEvt --- typings/artifacts.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/typings/artifacts.d.ts b/typings/artifacts.d.ts index 02c85555dc7e..7b61a63d27cd 100644 --- a/typings/artifacts.d.ts +++ b/typings/artifacts.d.ts @@ -391,7 +391,7 @@ declare global { /** The trace event marking firstPaint, if it was found. */ firstPaintEvt?: TraceEvent; /** The trace event marking firstContentfulPaint, if it was found. */ - firstContentfulPaintEvt?: TraceEvent; + firstContentfulPaintEvt: TraceEvent; /** The trace event marking firstMeaningfulPaint, if it was found. */ firstMeaningfulPaintEvt?: TraceEvent; /** The trace event marking loadEventEnd, if it was found. */ From 5f4d13cf36c040cce836dbcc88f505818b5c4a9c Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 21 Aug 2018 17:49:28 -0700 Subject: [PATCH 4/4] new no_navstart trace fixture --- .../fixtures/traces/no_navstart_event.json | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 lighthouse-core/test/fixtures/traces/no_navstart_event.json diff --git a/lighthouse-core/test/fixtures/traces/no_navstart_event.json b/lighthouse-core/test/fixtures/traces/no_navstart_event.json new file mode 100644 index 000000000000..b1865c461a79 --- /dev/null +++ b/lighthouse-core/test/fixtures/traces/no_navstart_event.json @@ -0,0 +1,77 @@ +{ + "traceEvents": [ + { + "pid": 77472, + "tid": 775, + "ts": 2146735802456, + "ph": "I", + "cat": "disabled-by-default-devtools.timeline", + "name": "TracingStartedInPage", + "args": { + "data": { + "frames": [ + { + "frame": "0x150343381dd0", + "name": "", + "url": "about:blank" + } + ], + "page": "0x150343381dd0", + "sessionId": "77472.1" + } + }, + "tts": 341067, + "s": "t" + }, + { + "pid": 77472, + "tid": 775, + "ts": 2146737301957, + "ph": "I", + "cat": "blink.user_timing,rail", + "name": "firstPaint", + "args": { + "frame": "0x150343381dd0" + }, + "tts": 549087, + "s": "p" + }, + { + "pid": 77472, + "tid": 775, + "ts": 2146737302468, + "ph": "R", + "cat": "blink.user_timing,rail", + "name": "firstImagePaint", + "args": { + "frame": "0x150343381dd0" + }, + "tts": 549603 + }, + { + "pid": 77472, + "tid": 775, + "ts": 2146737302468, + "ph": "I", + "cat": "blink.user_timing,rail", + "name": "firstContentfulPaint", + "args": { + "frame": "0x150343381dd0" + }, + "tts": 549599, + "s": "p" + }, + { + "pid": 77472, + "tid": 775, + "ts": 2146740268666, + "ph": "R", + "cat": "loading", + "name": "firstMeaningfulPaintCandidate", + "args": { + "frame": "0x150343381dd0" + }, + "tts": 1009771 + } + ] +} \ No newline at end of file