Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(tsc): tighten traceOfTab timing types #5887

Merged
merged 4 commits into from
Aug 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions lighthouse-core/audits/metrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
'use strict';

const MetricArtifact = require('./metric');
const LHError = require('../../../lib/errors');

class FirstContentfulPaint extends MetricArtifact {
get name() {
Expand All @@ -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,
});
};
}
}

Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/gather/computed/metrics/first-cpu-idle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
};
}
}

Expand Down
3 changes: 0 additions & 3 deletions lighthouse-core/gather/computed/metrics/interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand All @@ -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'
Expand All @@ -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();
});
Expand Down
58 changes: 32 additions & 26 deletions lighthouse-core/gather/computed/trace-of-tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/bootup-time-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"cat": "blink.user_timing,rail",
"name": "firstContentfulPaint",
"args": {
"frame": "0x1aff390e1e30"
"frame": "0x53965941e30"
},
"tts": 239094,
"s": "p"
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/test/fixtures/traces/load.json
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
77 changes: 77 additions & 0 deletions lighthouse-core/test/fixtures/traces/no_navstart_event.json
Original file line number Diff line number Diff line change
@@ -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
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down Expand Up @@ -79,7 +80,7 @@ describe('MainResource computed artifact', () => {
attributableURLs: [],

children: [taskB],
event: traceEvents[2],
event: traceEvents[3],
startTime: 0,
endTime: 100,
duration: 100,
Expand All @@ -92,7 +93,7 @@ describe('MainResource computed artifact', () => {
attributableURLs: [],

children: [taskC],
event: traceEvents[3],
event: traceEvents[4],
startTime: 5,
endTime: 55,
duration: 50,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ describe('FirstInteractive computed artifact:', () => {
computeObservedMetric({
timings: {
firstMeaningfulPaint: 3400,
domContentLoaded: 2000,
traceEnd: 4500,
},
timestamps: {
Expand Down
Loading