fix critical redirects

GoogleChrome · Dec 18, 2018 · 9537538 · 9537538
1 parent ed3d764
commit 9537538
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 6 deletions.
diff --git 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
@@ -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';