core: de-dupe URLs in is-on-http, uses-http2 (#4950)

GoogleChrome · Apr 11, 2018 · ff4f111 · ff4f111
1 parent 82c8381
commit ff4f111
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 16 deletions.
diff --git a/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js b/lighthouse-cli/test/smokehouse/dobetterweb/dbw-expectations.js
@@ -35,13 +35,13 @@ module.exports = [
         extendedInfo: {
           value: {
             results: {
-              length: 18,
+              length: 17,
             },
           },
         },
         details: {
           items: {
-            length: 18,
+            length: 17,
           },
         },
       },

diff --git a/lighthouse-core/audits/dobetterweb/uses-http2.js b/lighthouse-core/audits/dobetterweb/uses-http2.js
@@ -39,6 +39,7 @@ class UsesHTTP2Audit extends Audit {
     return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => {
       const finalHost = new URL(artifacts.URL.finalUrl).host;
 
+      const seenURLs = new Set();
       // Filter requests that are on the same host as the page and not over h2.
       const resources = networkRecords.filter(record => {
         // test the protocol first to avoid (potentially) expensive URL parsing
@@ -49,8 +50,12 @@ class UsesHTTP2Audit extends Audit {
       }).map(record => {
         return {
           protocol: record.protocol,
-          url: record.url, // .url is a getter and not copied over for the assign.
+          url: record._url,
         };
+      }).filter(record => {
+        if (seenURLs.has(record.url)) return false;
+        seenURLs.add(record.url);
+        return true;
       });
 
       let displayValue = '';

diff --git a/lighthouse-core/audits/is-on-https.js b/lighthouse-core/audits/is-on-https.js
@@ -47,30 +47,28 @@ class HTTPS extends Audit {
   static audit(artifacts) {
     const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
     return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => {
-      const insecureRecords = networkRecords
+      const insecureURLs = networkRecords
           .filter(record => !HTTPS.isSecureRecord(record))
-          .map(record => ({url: URL.elideDataURI(record.url)}));
+          .map(record => URL.elideDataURI(record.url));
 
       let displayValue = '';
-      if (insecureRecords.length > 1) {
-        displayValue = `${Util.formatNumber(insecureRecords.length)} insecure requests found`;
-      } else if (insecureRecords.length === 1) {
-        displayValue = `${insecureRecords.length} insecure request found`;
+      if (insecureURLs.length > 1) {
+        displayValue = `${Util.formatNumber(insecureURLs.length)} insecure requests found`;
+      } else if (insecureURLs.length === 1) {
+        displayValue = `${insecureURLs.length} insecure request found`;
       }
 
-      const items = insecureRecords.map(record => ({
-        url: record.url,
-      }));
+      const items = Array.from(new Set(insecureURLs)).map(url => ({url}));
 
       const headings = [
         {key: 'url', itemType: 'url', text: 'Insecure URL'},
       ];
 
       return {
-        rawValue: insecureRecords.length === 0,
+        rawValue: items.length === 0,
         displayValue,
         extendedInfo: {
-          value: insecureRecords,
+          value: items,
         },
         details: Audit.makeTableDetails(headings, items),
       };

diff --git a/lighthouse-core/test/audits/dobetterweb/uses-http2-test.js b/lighthouse-core/test/audits/dobetterweb/uses-http2-test.js
@@ -22,8 +22,9 @@ describe('Resources are fetched over http/2', () => {
       requestNetworkRecords: () => Promise.resolve(networkRecords),
     }).then(auditResult => {
       assert.equal(auditResult.rawValue, false);
-      assert.ok(auditResult.displayValue.match('4 requests were not'));
-      assert.equal(auditResult.details.items.length, 4);
+      assert.ok(auditResult.displayValue.match('3 requests were not'));
+      assert.equal(auditResult.details.items.length, 3);
+      assert.equal(auditResult.details.items[0].url, 'https://webtide.com/wp-content/plugins/wp-pagenavi/pagenavi-css.css?ver=2.70');
       const headers = auditResult.details.headings;
       assert.equal(headers[0].text, 'URL', 'table headings are correct and in order');
       assert.equal(headers[1].text, 'Protocol', 'table headings are correct and in order');

diff --git a/lighthouse-core/test/audits/is-on-https-test.js b/lighthouse-core/test/audits/is-on-https-test.js
@@ -22,6 +22,7 @@ describe('Security: HTTPS audit', () => {
     return Audit.audit(getArtifacts([
       {url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
       {url: 'http://insecure.com/image.jpeg', scheme: 'http', domain: 'insecure.com'},
+      {url: 'http://insecure.com/image.jpeg', scheme: 'http', domain: 'insecure.com'}, // should be de-duped
       {url: 'http://insecure.com/image2.jpeg', scheme: 'http', domain: 'insecure.com'},
       {url: 'https://google.com/', scheme: 'https', domain: 'google.com'},
     ])).then(result => {