Skip to content

Commit d5ca504

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Ignore cached requests in Lantern network analysis
Also check if the network analysis failed to compute a result, and proactively decide not to make a Lantern context, rather than waiting for it to throw an error. Bug: None Change-Id: Iffdb256114a56effe9c73818d74b8e9490e6d545 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5949390 Commit-Queue: Paul Irish <[email protected]> Commit-Queue: Connor Clark <[email protected]> Auto-Submit: Connor Clark <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent f7d9bba commit d5ca504

File tree

5 files changed

+23
-11
lines changed

5 files changed

+23
-11
lines changed

front_end/models/trace/Processor.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,10 @@ export class TraceProcessor extends EventTarget {
319319
const processedNavigation = LanternComputationData.createProcessedNavigation(parsedTrace, frameId, navigationId);
320320

321321
const networkAnalysis = Lantern.Core.NetworkAnalyzer.analyze(requests);
322+
if (!networkAnalysis) {
323+
return;
324+
}
325+
322326
const simulator: Lantern.Simulation.Simulator<Types.Events.SyntheticNetworkRequest> =
323327
Lantern.Simulation.Simulator.createSimulator({
324328
// TODO(crbug.com/372674229): if devtools throttling was on, does this network analysis capture
@@ -451,7 +455,7 @@ export class TraceProcessor extends EventTarget {
451455
} else if (!expectedErrors.some(err => e.message === err)) {
452456
// To reduce noise from tests, only print errors that are not expected to occur because a trace is
453457
// too old (for which there is no single check).
454-
console.error(e.message);
458+
console.error(e);
455459
}
456460
}
457461

front_end/models/trace/lantern/core/NetworkAnalyzer.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('NetworkAnalyzer', () => {
4242
connectionReused: false,
4343
networkRequestTime: 10,
4444
networkEndTime: 10,
45-
transferSize: 0,
45+
transferSize: 10000,
4646
protocol: opts.protocol || 'http/1.1',
4747
parsedURL: {scheme: url.match(/https?/)[0], securityOrigin: url.match(/.*\.com/)[0]},
4848
timing: opts.timing || null,
@@ -339,9 +339,9 @@ describe('NetworkAnalyzer', () => {
339339
);
340340
}
341341

342-
it('should return Infinity for no/missing records', () => {
343-
assert.strictEqual(estimateThroughput([]), Infinity);
344-
assert.strictEqual(estimateThroughput([createThroughputRecord(0, 0, {finished: false})]), Infinity);
342+
it('should return null for no/missing records', () => {
343+
assert.strictEqual(estimateThroughput([]), null);
344+
assert.strictEqual(estimateThroughput([createThroughputRecord(0, 0, {finished: false})]), null);
345345
});
346346

347347
it('should compute correctly for a basic waterfall', () => {

front_end/models/trace/lantern/core/NetworkAnalyzer.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ class NetworkAnalyzer {
412412
function collectEstimates(estimator: (e: RequestInfo) => number[] | number | undefined, multiplier = 1): void {
413413
for (const request of originRequests) {
414414
const timing = request.timing;
415-
if (!timing) {
415+
if (!timing || !request.transferSize) {
416416
continue;
417417
}
418418

@@ -489,9 +489,9 @@ class NetworkAnalyzer {
489489
/**
490490
* Computes the average throughput for the given requests in bits/second.
491491
* Excludes data URI, failed or otherwise incomplete, and cached requests.
492-
* Returns Infinity if there were no analyzable network requests.
492+
* Returns null if there were no analyzable network requests.
493493
*/
494-
static estimateThroughput(records: Lantern.NetworkRequest[]): number {
494+
static estimateThroughput(records: Lantern.NetworkRequest[]): number|null {
495495
let totalBytes = 0;
496496

497497
// We will measure throughput by summing the total bytes downloaded by the total time spent
@@ -518,7 +518,7 @@ class NetworkAnalyzer {
518518
.sort((a, b) => a.time - b.time);
519519

520520
if (!timeBoundaries.length) {
521-
return Infinity;
521+
return null;
522522
}
523523

524524
let inflight = 0;
@@ -576,8 +576,12 @@ class NetworkAnalyzer {
576576
};
577577
}
578578

579-
static analyze(records: Lantern.NetworkRequest[]): Lantern.Simulation.Settings['networkAnalysis'] {
579+
static analyze(records: Lantern.NetworkRequest[]): Lantern.Simulation.Settings['networkAnalysis']|null {
580580
const throughput = NetworkAnalyzer.estimateThroughput(records);
581+
if (throughput === null) {
582+
return null;
583+
}
584+
581585
return {
582586
throughput,
583587
...NetworkAnalyzer.computeRTTAndServerResponseTime(records),

front_end/models/trace/lantern/simulation/Simulator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ class Simulator<T = Lantern.AnyNetworkObject> {
143143
throw new Core.LanternError(`Invalid rtt ${this._rtt}`);
144144
}
145145
if (!Number.isFinite(this.throughput)) {
146-
throw new Core.LanternError(`Invalid rtt ${this.throughput}`);
146+
throw new Core.LanternError(`Invalid throughput ${this.throughput}`);
147147
}
148148
}
149149

front_end/models/trace/lantern/testing/MetricTestUtils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ async function getComputationDataFromFixture({trace, settings, url}: {
3232
const parsedTrace = await runTrace(trace);
3333
const requests = Trace.LanternComputationData.createNetworkRequests(trace, parsedTrace);
3434
const networkAnalysis = Lantern.Core.NetworkAnalyzer.analyze(requests);
35+
if (!networkAnalysis) {
36+
throw new Error('no networkAnalysis');
37+
}
38+
3539
const frameId = parsedTrace.Meta.mainFrameId;
3640
const navigationId = parsedTrace.Meta.mainFrameNavigations[0].args.data?.navigationId;
3741
if (!navigationId) {

0 commit comments

Comments
 (0)