From 95375384a9712051fab447b5c5bace0c5bfc8f59 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Tue, 18 Dec 2018 07:49:51 +0100 Subject: [PATCH] fix critical redirects --- .../computed/critical-request-chains.js | 9 ++++----- .../computed/critical-request-chains-test.js | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/lighthouse-core/computed/critical-request-chains.js b/lighthouse-core/computed/critical-request-chains.js index c4ee45d3459c..b05f1b32c41e 100644 --- a/lighthouse-core/computed/critical-request-chains.js +++ b/lighthouse-core/computed/critical-request-chains.js @@ -28,15 +28,14 @@ class CriticalRequestChains { return false; } - let resourceType = request.resourceType; // if resourcetype is undefined we check if it's not a 200 and look for a redirect // when we find a redirect we use the redirectDestination's resourcetype #6675 - if (!resourceType && request.statusCode !== 200 && request.redirectDestination) { - resourceType = request.redirectDestination.resourceType; + while (request.redirectDestination) { + request = request.redirectDestination; } // Iframes are considered High Priority but they are not render blocking - const isIframe = resourceType === NetworkRequest.TYPES.Document + const isIframe = request.resourceType === NetworkRequest.TYPES.Document && request.frameId !== mainResource.frameId; // XHRs are fetched at High priority, but we exclude them, as they are unlikely to be critical // Images are also non-critical. @@ -48,7 +47,7 @@ class CriticalRequestChains { NetworkRequest.TYPES.Fetch, NetworkRequest.TYPES.EventSource, ]; - if (nonCriticalResourceTypes.includes(resourceType || 'Other') || + if (nonCriticalResourceTypes.includes(request.resourceType || 'Other') || isIframe || request.mimeType && request.mimeType.startsWith('image/')) { return false; diff --git a/lighthouse-core/test/computed/critical-request-chains-test.js b/lighthouse-core/test/computed/critical-request-chains-test.js index d1d195a5bb96..a9035b1ad48d 100644 --- a/lighthouse-core/test/computed/critical-request-chains-test.js +++ b/lighthouse-core/test/computed/critical-request-chains-test.js @@ -251,13 +251,24 @@ describe('CriticalRequestChain gatherer: extractChain function', () => { }); it('handles redirects', () => { - const networkRecords = mockTracingData([HIGH, HIGH, HIGH], [[0, 1], [1, 2]]); + const networkRecords = mockTracingData([HIGH, HIGH, HIGH, HIGH], [[0, 1], [1, 2], [1, 3]]); const mainResource = networkRecords[0]; // Make a fake redirect networkRecords[1].requestId = '1:redirected.0'; networkRecords[2].requestId = '1'; + networkRecords[3].requestId = '2'; + networkRecords[3].url = 'https://example.com/redirect-stylesheet'; + networkRecords[3].resourceType = undefined; + networkRecords[3].statusCode = 302; + networkRecords[3].redirectDestination = { + redirectDestination: { + resourceType: NetworkRequest.TYPES.Stylesheet, + priority: 'High', + }, + }; + const criticalChains = CriticalRequestChains.extractChain(networkRecords, mainResource); assert.deepEqual(criticalChains, { 0: { @@ -270,6 +281,10 @@ describe('CriticalRequestChain gatherer: extractChain function', () => { request: networkRecords[2], children: {}, }, + 2: { + request: networkRecords[3], + children: {}, + }, }, }, }, @@ -333,6 +348,7 @@ describe('CriticalRequestChain gatherer: extractChain function', () => { networkRecords[3].statusCode = 302; networkRecords[3].redirectDestination = { resourceType: NetworkRequest.TYPES.Document, + priority: 'Low', }; networkRecords[3].frameId = '4';