Skip to content

Commit

Permalink
core(fcp): handle negative request endTime (#13452)
Browse files Browse the repository at this point in the history
Co-authored-by: Lukas Auer <lukas.auer@mstage.at>
  • Loading branch information
adamraine and LukasAuerMstage committed Dec 10, 2021
1 parent 89a6137 commit d9e84e1
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ class LanternFirstContentfulPaint extends LanternMetric {
return dependencyGraph.cloneWithRelationships(node => {
if (node.type === BaseNode.TYPES.NETWORK) {
// Exclude all nodes that ended after paintTs (except for the main document which we always consider necessary)
if (node.endTime > paintTs && !node.isMainDocument()) return false;
// endTime is negative if request does not finish, make sure startTime isn't after paintTs in this case.
const endedAfterPaint = node.endTime > paintTs || node.startTime > paintTs;
if (endedAfterPaint && !node.isMainDocument()) return false;

const url = node.record.url;
// If the URL definitely wasn't render-blocking then we filter it out.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@

const LanternFirstContentfulPaint = require('../../../computed/metrics/lantern-first-contentful-paint.js'); // eslint-disable-line max-len
const assert = require('assert').strict;
const networkRecordsToDevtoolsLog = require('../../network-records-to-devtools-log.js');
const createTestTrace = require('../../create-test-trace.js');

const trace = require('../../fixtures/traces/progressive-app-m60.json');
const devtoolsLog = require('../../fixtures/traces/progressive-app-m60.devtools.log.json');

/* eslint-env jest */
describe('Metrics: Lantern FCP', () => {
const gatherContext = {gatherMode: 'navigation'};

it('should compute predicted value', async () => {
const settings = {};
const context = {settings, computedCache: new Map()};
Expand All @@ -30,4 +33,45 @@ describe('Metrics: Lantern FCP', () => {
assert.ok(result.optimisticGraph, 'should have created optimistic graph');
assert.ok(result.pessimisticGraph, 'should have created pessimistic graph');
});

it('should handle negative request endTime', async () => {
const settings = {};
const context = {settings, computedCache: new Map()};
const devtoolsLog = networkRecordsToDevtoolsLog([
{
transferSize: 2000,
url: 'https://example.com/',
resourceType: 'Document',
priority: 'High',
startTime: 0,
endTime: 0.0000001, // Before FCP
timing: {sslStart: 50, sslEnd: 100, connectStart: 50, connectEnd: 100},
},
{
transferSize: 2000,
url: 'https://example.com/script.js',
resourceType: 'Script',
priority: 'High',
startTime: 0.000015, // After FCP
endTime: -1,
timing: {sslStart: 50, sslEnd: 100, connectStart: 50, connectEnd: 100},
},
]);
const trace = createTestTrace({timeOrigin: 0, traceEnd: 2000});
const artifacts = {
trace,
devtoolsLog,
gatherContext,
settings,
};
const result = await LanternFirstContentfulPaint.request(artifacts, context);

const optimisticNodes = [];
result.optimisticGraph.traverse(node => optimisticNodes.push(node));
expect(optimisticNodes.map(node => node._record.url)).toEqual(['https://example.com/']);

const pessimisticNodes = [];
result.pessimisticGraph.traverse(node => pessimisticNodes.push(node));
expect(pessimisticNodes.map(node => node._record.url)).toEqual(['https://example.com/']);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"p95": 0.743897672766033
},
"roughEstimateOfLCP": {
"p50": 0.203757225433526,
"p50": 0.2028301886792453,
"p90": 0.6962974535228871,
"p95": 0.9262573279851898
}
Expand Down
Loading

0 comments on commit d9e84e1

Please sign in to comment.