core(tsc): tighten traceOfTab timing types (#5887)

GoogleChrome · Aug 22, 2018 · 4a2c085 · 4a2c085
1 parent 673eedf
commit 4a2c085
Show file tree

Hide file tree

Showing 18 changed files with 208 additions and 89 deletions.
diff --git 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=} 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=} 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
@@ -6,7 +6,6 @@
 'use strict';
 
 const MetricArtifact = require('./metric');
-const LHError = require('../../../lib/errors');
 
 class FirstContentfulPaint extends MetricArtifact {
   get name() {
@@ -17,16 +16,13 @@ class FirstContentfulPaint extends MetricArtifact {
    * @param {LH.Artifacts.MetricComputationData} data
    * @return {Promise<LH.Artifacts.Metric>}
    */
-  computeObservedMetric(data) {
+  async computeObservedMetric(data) {
     const {traceOfTab} = data;
-    if (!traceOfTab.timestamps.firstContentfulPaint) {
-      throw new LHError(LHError.errors.NO_FCP);
-    }
 
-    return Promise.resolve({
+    return {
       timing: 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
@@ -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
@@ -17,16 +17,17 @@ class FirstMeaningfulPaint extends MetricArtifact {
    * @param {LH.Artifacts.MetricComputationData} data
    * @return {Promise<LH.Artifacts.Metric>}
    */
-  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
@@ -148,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-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
@@ -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';
@@ -90,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(
@@ -133,34 +129,44 @@ 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: firstContentfulPaint.ts,
+      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 - navigationStart.ts) / 1000;
+    /** @param {number=} ts */
+    const maybeGetTiming = (ts) => ts === undefined ? undefined : getTiming(ts);
+    /** @type {LH.Artifacts.TraceTimes} */
+    const timings = {
+      navigationStart: 0,
+      firstPaint: maybeGetTiming(timestamps.firstPaint),
+      firstContentfulPaint: getTiming(timestamps.firstContentfulPaint),
+      firstMeaningfulPaint: maybeGetTiming(timestamps.firstMeaningfulPaint),
+      traceEnd: getTiming(timestamps.traceEnd),
+      load: maybeGetTiming(timestamps.load),
+      domContentLoaded: maybeGetTiming(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/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
@@ -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
@@ -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
@@ -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/no_navstart_event.json 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
+    }
+  ]
+}
diff --git 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
@@ -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-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: {