Skip to content

Commit 59e1ee7

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Fix bug that double-counts transfer size in 3p table
Fixed: 408275043 Change-Id: I966155b37e08f32f2640823f46b0c214095775f1 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6431348 Reviewed-by: Jack Franklin <[email protected]> Commit-Queue: Jack Franklin <[email protected]> Auto-Submit: Connor Clark <[email protected]>
1 parent 210808a commit 59e1ee7

File tree

3 files changed

+22
-9
lines changed

3 files changed

+22
-9
lines changed

front_end/models/trace/extras/ThirdParties.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ describeWithEnvironment('ThirdParties', function() {
2727

2828
const results = summaries.map(s => [s.entity.name, s.mainThreadTime, s.transferSize]);
2929
assert.deepEqual(results, [
30-
['localhost', 24.947999954223633, 2254],
30+
['localhost', 24.947999954223633, 1503],
3131
['Google Fonts', 0, 25325],
3232
]);
3333
const urls = extractUrlsFromSummaries(summaries);

front_end/models/trace/extras/TraceTree.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,20 @@ export class BottomUpRootNode extends Node {
420420
} else {
421421
node.events.push(e);
422422
}
423-
node.transferSize += e.args.data.encodedDataLength;
423+
424+
// ResourceReceivedData events tally up the transfer size over time, but the
425+
// ResourceReceiveResponse / ResourceFinish events hold the final result.
426+
if (e.name === 'ResourceReceivedData') {
427+
node.transferSize += e.args.data.encodedDataLength;
428+
} else if (e.args.data.encodedDataLength > 0) {
429+
// For some reason, ResourceFinish can be zero even if data was sent.
430+
// Ignore that case.
431+
// Note: this will count the entire resource size if just the last bit of a
432+
// request is in view. If it isn't in view, the transfer size is counted
433+
// gradually, in proportion with the ResourceReceivedData events in the
434+
// current view.
435+
node.transferSize = e.args.data.encodedDataLength;
436+
}
424437
}
425438
};
426439

front_end/models/trace/insights/ThirdParties.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describeWithEnvironment('ThirdParties', function() {
2020
]);
2121
const summaryResult = insight.summaries.map(s => [s.entity.name, s.transferSize, s.mainThreadTime.toFixed(2)]);
2222
assert.deepEqual(summaryResult, [
23-
['localhost', 2254, '24.95'],
23+
['localhost', 1503, '24.95'],
2424
['Google Fonts', 25325, '0.00'],
2525
]);
2626
});
@@ -43,12 +43,12 @@ describeWithEnvironment('ThirdParties', function() {
4343

4444
const summaryResult = insight.summaries.map(s => [s.entity.name, s.transferSize, s.mainThreadTime.toFixed(2)]);
4545
assert.deepEqual(summaryResult, [
46-
['paulirish.com', 439223, '85.54'],
47-
['Google Fonts', 169258, '0.00'],
48-
['Google Tag Manager', 367917, '19.95'],
49-
['Google Analytics', 75811, '5.86'],
50-
['Disqus', 3748, '0.34'],
51-
['Firebase', 6564, '0.00'],
46+
['paulirish.com', 157130, '85.54'],
47+
['Google Fonts', 80003, '0.00'],
48+
['Google Tag Manager', 95375, '19.95'],
49+
['Google Analytics', 20865, '5.86'],
50+
['Disqus', 1551, '0.34'],
51+
['Firebase', 2847, '0.00'],
5252
]);
5353
});
5454
});

0 commit comments

Comments
 (0)