sort trace by timestamp before calculating FMP (#756)

lighthouse-core/audits/first-meaningful-paint.js

@@ -131,10 +131,6 @@ class FirstMeaningfulPaint extends Audit {
     score = Math.min(100, score);
     score = Math.max(0, score);
 
-    if (!data.navStart) {
-      throw new Error(FAILURE_MESSAGE);
-    }
-
     timings.navStart = data.navStart.ts / 1000;
 
     return {
@@ -159,12 +155,14 @@ class FirstMeaningfulPaint extends Audit {
     // const events = model.timelineModel().mainThreadEvents();
     const events = traceData;
 
-    // Parse the trace for our key events
+    // Parse the trace for our key events and sort them by timestamp.
     events.filter(e => {
       return e.cat.includes('blink.user_timing') ||
         e.name === 'FrameView::performLayout' ||
         e.name === 'Paint' ||
         e.name === 'TracingStartedInPage';
+    }).sort((event0, event1) => {
+      return event0.ts - event1.ts;
     }).forEach(event => {
       // Grab the page's ID from the first TracingStartedInPage in the trace
       if (event.name === 'TracingStartedInPage' && !mainFrameID) {
@@ -179,21 +177,30 @@ class FirstMeaningfulPaint extends Audit {
       // firstContentfulPaint == the first time that text or image content was
       // painted. See src/third_party/WebKit/Source/core/paint/PaintTiming.h
       // COMPAT: firstContentfulPaint trace event first introduced in Chrome 49 (r370921)
-      if (event.name === 'firstContentfulPaint' && event.args.frame === mainFrameID) {
+      if (event.name === 'firstContentfulPaint' && event.args.frame === mainFrameID &&
+          !!navigationStart && event.ts >= navigationStart.ts) {
         firstContentfulPaint = event;
       }
       // COMPAT: frame property requires Chrome 52 (r390306)
       // https://codereview.chromium.org/1922823003
-      if (event.name === 'FrameView::performLayout' && event.args.counters &&
-          event.args.counters.frame === mainFrameID) {
+      if (event.name === 'FrameView::performLayout' &&
+          event.args.counters && event.args.counters.frame === mainFrameID &&
+          !!navigationStart && event.ts >= navigationStart.ts) {
         layouts.set(event, event.args.counters);
       }
 
-      if (event.name === 'Paint' && event.args.data.frame === mainFrameID) {
+      if (event.name === 'Paint' && event.args.data.frame === mainFrameID &&
+        !!navigationStart && event.ts >= navigationStart.ts) {
         paints.push(event);
       }
     });
 
+    // navigationStart is currently essential to FMP calculation.
+    // see: https://github.com/GoogleChrome/lighthouse/issues/753
+    if (!navigationStart) {
+      throw new Error('No `navigationStart` event found after `TracingStartedInPage` in trace');
+    }
+
     return {
       navigationStart,
       firstContentfulPaint,

lighthouse-core/test/audits/first-meaningful-paint-test.js

@@ -15,34 +15,59 @@
  */
 'use strict';
 
-const Audit = require('../../audits/first-meaningful-paint.js');
+const FMPAudit = require('../../audits/first-meaningful-paint.js');
+const Audit = require('../../audits/audit.js');
 const assert = require('assert');
 const traceEvents = require('../fixtures/traces/progressive-app.json');
+const badNavStartTrace = require('../fixtures/traces/bad-nav-start-ts.json');
 
 /* eslint-env mocha */
 describe('Performance: first-meaningful-paint audit', () => {
   it('scores a -1 when no trace data is present', () => {
-    return Audit.audit({}).then(response => {
-      return assert.equal(response.score, -1);
+    return FMPAudit.audit({}).then(result => {
+      assert.equal(result.score, -1);
+      assert.ok(result.debugString);
     });
   });
 
   it('scores a -1 when faulty trace data is present', () => {
-    return Audit.audit({boo: 'ya'}).then(response => {
-      return assert.equal(response.score, -1);
+    const artifacts = {
+      traces: {
+        [Audit.DEFAULT_PASS]: {boo: 'ya'}
+      }
+    };
+    return FMPAudit.audit(artifacts).then(result => {
+      assert.equal(result.rawValue, -1);
+      assert.ok(result.debugString);
+    });
+  });
+
+  it('scores a -1 and returns an error when navigation start is before trace start', () => {
+    const artifacts = {
+      traces: {
+        [Audit.DEFAULT_PASS]: badNavStartTrace
+      }
+    };
+    return FMPAudit.audit(artifacts).then(result => {
+      assert.equal(result.rawValue, -1);
+      assert.ok(/navigationStart/.test(result.debugString));
     });
   });
 
   describe('measures the pwa.rocks example correctly', () => {
     let fmpResult;
 
-    it('processes a valid trace file', done => {
-      assert.doesNotThrow(_ => {
-        Audit.audit({traces: {[Audit.DEFAULT_PASS]: {traceEvents}}})
-          .then(response => {
-            fmpResult = response;
-            done();
-          });
+    it('processes a valid trace file', () => {
+      const artifacts = {
+        traces: {
+          [Audit.DEFAULT_PASS]: {traceEvents}
+        }
+      };
+
+      return FMPAudit.audit(artifacts).then(result => {
+        fmpResult = result;
+      }).catch(_ => {
+        assert.ok(false);
       });
     });