Fix: Catch all images as non-critical (#2233)

GoogleChrome · May 12, 2017 · 00373ed · 00373ed
1 parent b44b174
commit 00373ed
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 11 deletions.
diff --git a/lighthouse-core/gather/computed/critical-request-chains.js b/lighthouse-core/gather/computed/critical-request-chains.js
@@ -37,17 +37,13 @@ class CriticalRequestChains extends ComputedArtifact {
 
     // XHRs are fetched at High priority, but we exclude them, as they are unlikely to be critical
     // Images are also non-critical.
+    // Treat any images missed by category, primarily favicons, as non-critical resources
     const nonCriticalResourceTypes = [
       WebInspector.resourceTypes.Image._category,
       WebInspector.resourceTypes.XHR._category
     ];
-    if (nonCriticalResourceTypes.includes(resourceTypeCategory)) {
-      return false;
-    }
-
-    // Treat favicons as non-critical resources
-    if (request.mimeType === 'image/x-icon' ||
-        (request.parsedURL && request.parsedURL.lastPathComponent === 'favicon.ico')) {
+    if (nonCriticalResourceTypes.includes(resourceTypeCategory) ||
+        request.mimeType && request.mimeType.startsWith('image/')) {
       return false;
     }
 

diff --git a/lighthouse-core/scripts/run-mocha.sh b/lighthouse-core/scripts/run-mocha.sh
@@ -17,5 +17,5 @@ elif [ "$flag" == '--core' ]; then
 else
   echo "lighthouse-core tests" && _runmocha 'lighthouse-core' && \
   echo "lighthouse-cli tests" && _runmocha 'lighthouse-cli' && \
- echo "lighthouse-viewer tests" && _runmocha 'lighthouse-viewer'
+  echo "lighthouse-viewer tests" && _runmocha 'lighthouse-viewer'
 fi
diff --git a/lighthouse-core/test/gather/computed/critical-request-chains-test.js b/lighthouse-core/test/gather/computed/critical-request-chains-test.js
@@ -288,15 +288,27 @@ describe('CriticalRequestChain gatherer: getCriticalChain function', () => {
   });
 
   it('discards favicons as non-critical', () => {
-    const networkRecords = mockTracingData([HIGH, HIGH, HIGH], [[0, 1], [0, 2]]);
+    const networkRecords = mockTracingData([HIGH, HIGH, HIGH, HIGH], [[0, 1], [0, 2], [0, 3]]);
 
     // 2nd record is a favicon
     networkRecords[1].url = 'https://example.com/favicon.ico';
+    networkRecords[1].mimeType = 'image/x-icon';
     networkRecords[1].parsedURL = {
       lastPathComponent: 'favicon.ico'
     };
-    // 3rd record is also a favicon
-    networkRecords[2].mimeType = 'image/x-icon';
+    // 3rd record is a favicon
+    networkRecords[2].url = 'https://example.com/favicon-32x32.png';
+    networkRecords[2].mimeType = 'image/png';
+    networkRecords[2].parsedURL = {
+      lastPathComponent: 'favicon-32x32.png'
+    };
+    // 4th record is a favicon
+    networkRecords[3].url = 'https://example.com/android-chrome-192x192.png';
+    networkRecords[3].mimeType = 'image/png';
+    networkRecords[3].parsedURL = {
+      lastPathComponent: 'android-chrome-192x192.png'
+    };
+
     return Gatherer.request(networkRecords).then(criticalChains => {
       assert.deepEqual(criticalChains, {
         0: {