Repository: GoogleChrome/lighthouse
Pull request #7197 (2019-02-09 22:04:44)

fix(uses-http2): ignore requests from service-worker
<!-- Thank you for submitting a pull request! -->

**Summary**
Requests from service workers are returned with a http/1.1 protocol so if a service worker is loaded on time and calls the claim function we might route some functions through the SW before we stop listening to the page. This can happen with for example `<link rel="fetch" ../>`

An example that fails regularly is https://remarkable-airboat.glitch.me
![image](https://user-images.githubusercontent.com/1120926/52526810-138e9d80-2cbf-11e9-80b5-18353bcc5f7a.png)

<!-- What kind of change does this PR introduce? -->
<!-- Is this a bugfix, feature, refactoring, build related change, etc? -->

<!-- Describe the need for this change -->

<!-- Link any documentation or information that would help understand this change -->

**Related Issues/PRs**
fixes: #7158 

wardpeet: @patrickhulce @exterkamp  I rebooted travis, PR should be good to go :)


Patch:
diff --git a/lighthouse-core/audits/dobetterweb/uses-http2.js b/lighthouse-core/audits/dobetterweb/uses-http2.js
index 998167b6f..6ca132cc7 100644
--- a/lighthouse-core/audits/dobetterweb/uses-http2.js
+++ b/lighthouse-core/audits/dobetterweb/uses-http2.js
@@ -44,6 +44,9 @@ class UsesHTTP2Audit extends Audit {
       const seenURLs = new Set();
       // Filter requests that are on the same host as the page and not over h2.
       const resources = networkRecords.filter(record => {
+        // check if record is not served through the service worker, servicer worker uses http/1.1 as a protocol
+        // these can generate false positives (bug: https://github.com/GoogleChrome/lighthouse/issues/7158)
+        if (record.fetchedViaServiceWorker) return false;
         // test the protocol first to avoid (potentially) expensive URL parsing
         const isOldHttp = /HTTP\/[01][.\d]?/i.test(record.protocol);
         if (!isOldHttp) return false;


Test patch:
diff --git a/lighthouse-core/test/audits/dobetterweb/uses-http2-test.js b/lighthouse-core/test/audits/dobetterweb/uses-http2-test.js
index c43793ceb..5e8f8d20f 100644
--- a/lighthouse-core/test/audits/dobetterweb/uses-http2-test.js
+++ b/lighthouse-core/test/audits/dobetterweb/uses-http2-test.js
@@ -26,26 +26,29 @@ describe('Resources are fetched over http/2', () => {
   }
 
   it('fails when some resources were requested via http/1.x', () => {
-    return UsesHTTP2Audit.audit(
-      getArtifacts(networkRecords, URL), {computedCache: new Map()}
-    ).then(auditResult => {
-      assert.equal(auditResult.rawValue, false);
-      assert.ok(auditResult.displayValue.match('3 requests 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');
-    });
+    return UsesHTTP2Audit.audit(getArtifacts(networkRecords, URL), {computedCache: new Map()}).then(
+      auditResult => {
+        assert.equal(auditResult.rawValue, false);
+        assert.ok(auditResult.displayValue.match('3 requests 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');
+      }
+    );
   });
 
   it('displayValue is correct when only one resource fails', () => {
     const entryWithHTTP1 = networkRecords.slice(1, 2);
-    return UsesHTTP2Audit.audit(
-      getArtifacts(entryWithHTTP1, URL), {computedCache: new Map()}
-    ).then(auditResult => {
-      assert.ok(auditResult.displayValue.match('1 request not'));
-    });
+    return UsesHTTP2Audit.audit(getArtifacts(entryWithHTTP1, URL), {computedCache: new Map()}).then(
+      auditResult => {
+        assert.ok(auditResult.displayValue.match('1 request not'));
+      }
+    );
   });
 
   it('passes when all resources were requested via http/2', () => {
@@ -54,11 +57,34 @@ describe('Resources are fetched over http/2', () => {
       record.protocol = 'h2';
     });
 
-    return UsesHTTP2Audit.audit(
-      getArtifacts(h2Records, URL), {computedCache: new Map()}
-    ).then(auditResult => {
-      assert.equal(auditResult.rawValue, true);
-      assert.ok(auditResult.displayValue === '');
+    return UsesHTTP2Audit.audit(getArtifacts(h2Records, URL), {computedCache: new Map()}).then(
+      auditResult => {
+        assert.equal(auditResult.rawValue, true);
+        assert.ok(auditResult.displayValue === '');
+      }
+    );
+  });
+
+  it('results are correct when some requests are handled by service worker', () => {
+    const clonedNetworkRecords = JSON.parse(JSON.stringify(networkRecords));
+    clonedNetworkRecords.forEach(record => {
+      // convert http 1.1 to service worker requests
+      if (record.protocol === 'http/1.1') {
+        record.fetchedViaServiceWorker = true;
+      }
+    });
+
+    return UsesHTTP2Audit.audit(getArtifacts(clonedNetworkRecords, URL), {
+      computedCache: new Map(),
+    }).then(auditResult => {
+      assert.equal(auditResult.rawValue, false);
+      assert.ok(auditResult.displayValue.match('1 request not'));
+      // Protocol is http/1.0 which we don't mark as fetched fetchedViaServiceWorker on line 73.
+      assert.equal(
+        auditResult.details.items[0].url,
+        'https://webtide.com/wp-content/themes/clean-retina-pro/library/js/tinynav.js?ver=4.5.4'
+      );
+      assert.equal(auditResult.details.items[0].protocol, 'http/1.0');
     });
   });
 });
diff --git a/lighthouse-core/test/network-records-to-devtools-log.js b/lighthouse-core/test/network-records-to-devtools-log.js
index 0eb6e9e1c..50c380345 100644
--- a/lighthouse-core/test/network-records-to-devtools-log.js
+++ b/lighthouse-core/test/network-records-to-devtools-log.js
@@ -100,7 +100,7 @@ function getResponseReceivedEvent(networkRecord, index) {
         connectionReused: networkRecord.connectionReused || false,
         connectionId: networkRecord.connectionId || 140,
         fromDiskCache: networkRecord.fromDiskCache || undefined,
-        fromServiceWorker: networkRecord.fromServiceWorker || undefined,
+        fromServiceWorker: networkRecord.fetchedViaServiceWorker || undefined,
         encodedDataLength: networkRecord.transferSize || 0,
         timing,
         protocol: networkRecord.protocol || 'http/1.1',