throw on lack of fcp

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);

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

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,
     };
   }

lighthouse-core/gather/computed/metrics/interactive.js

@@ -90,9 +90,6 @@ class Interactive extends MetricArtifact {
    * @return {{cpuQuietPeriod: TimePeriod, networkQuietPeriod: TimePeriod, cpuQuietPeriods: Array<TimePeriod>, networkQuietPeriods: Array<TimePeriod>}}
    */
   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);

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();
     });

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 {

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});

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,

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"

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},

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,

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,

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);
-  });
 });

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'));
+  });
 });

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;