core(service-worker): check that test page is in SW scope (#6609)

lighthouse-core/audits/service-worker.js

@@ -5,8 +5,8 @@
  */
 'use strict';
 
-const URL = require('../lib/url-shim');
-const Audit = require('./audit');
+const URL = require('../lib/url-shim.js');
+const Audit = require('./audit.js');
 
 class ServiceWorker extends Audit {
   /**
@@ -29,17 +29,30 @@ class ServiceWorker extends Audit {
    * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
-    // Find active service worker for this URL. Match against
+    const {versions, registrations} = artifacts.ServiceWorker;
+    const pageUrl = new URL(artifacts.URL.finalUrl);
+
+    // Find active service workers for this origin. Match against
     // artifacts.URL.finalUrl so audit accounts for any redirects.
-    const versions = artifacts.ServiceWorker.versions;
-    const url = artifacts.URL.finalUrl;
+    const matchingSWVersions = versions.filter(v => v.status === 'activated')
+      .filter(v => new URL(v.scriptURL).origin === pageUrl.origin);
+
+    if (matchingSWVersions.length === 0) {
+      return {rawValue: false};
+    }
+
+    // Find the normalized scope URLs of possibly-controlling SWs.
+    const matchingScopeUrls = matchingSWVersions
+      .map(v => registrations.find(r => r.registrationId === v.registrationId))
+      .filter(/** @return {r is LH.Crdp.ServiceWorker.ServiceWorkerRegistration} */ r => !!r)
+      .map(r => new URL(r.scopeURL).href);
 
-    const origin = new URL(url).origin;
-    const matchingSW = versions.filter(v => v.status === 'activated')
-        .find(v => new URL(v.scriptURL).origin === origin);
+    // Ensure page is included in a SW's scope.
+    // See https://w3c.github.io/ServiceWorker/v1/#scope-match-algorithm
+    const inScope = matchingScopeUrls.some(scopeUrl => pageUrl.href.startsWith(scopeUrl));
 
     return {
-      rawValue: !!matchingSW,
+      rawValue: inScope,
     };
   }
 }

lighthouse-core/audits/webapp-install-banner.js

@@ -6,7 +6,6 @@
 'use strict';
 
 const MultiCheckAudit = require('./multi-check-audit');
-const SWAudit = require('./service-worker');
 const ManifestValues = require('../computed/manifest-values.js');
 
 /**
@@ -80,20 +79,6 @@ class WebappInstallBanner extends MultiCheckAudit {
     return failures;
   }
 
-  /**
-   * @param {LH.Artifacts} artifacts
-   * @return {Array<string>}
-   */
-  static assessServiceWorker(artifacts) {
-    const failures = [];
-    const hasServiceWorker = SWAudit.audit(artifacts).rawValue;
-    if (!hasServiceWorker) {
-      failures.push('Site does not register a service worker');
-    }
-
-    return failures;
-  }
-
   /**
    * @param {LH.Artifacts} artifacts
    * @param {LH.Audit.Context} context
@@ -102,12 +87,10 @@ class WebappInstallBanner extends MultiCheckAudit {
   static async audit_(artifacts, context) {
     const manifestValues = await ManifestValues.request(artifacts.Manifest, context);
     const manifestFailures = WebappInstallBanner.assessManifest(manifestValues);
-    const swFailures = WebappInstallBanner.assessServiceWorker(artifacts);
 
     return {
       failures: [
         ...manifestFailures,
-        ...swFailures,
       ],
       manifestValues,
     };

lighthouse-core/gather/gatherers/service-worker.js

@@ -14,8 +14,11 @@ class ServiceWorker extends Gatherer {
    */
   async beforePass(passContext) {
     const {versions} = await passContext.driver.getServiceWorkerVersions();
+    const {registrations} = await passContext.driver.getServiceWorkerRegistrations();
+
     return {
       versions,
+      registrations,
     };
   }
 }

lighthouse-core/test/audits/service-worker-test.js

@@ -5,39 +5,168 @@
  */
 'use strict';
 
-const Audit = require('../../audits/service-worker.js');
+const ServiceWorker = require('../../audits/service-worker.js');
+const URL = require('../../lib/url-shim.js');
 const assert = require('assert');
 
 /* eslint-env jest */
 
+function getBaseDirectory(urlStr) {
+  const url = new URL(urlStr);
+  return url.origin + url.pathname.substring(0, url.pathname.lastIndexOf('/') + 1);
+}
+
+/**
+ * Create a ServiceWorker artifact from an array of SW config opts.
+ * @param {Array<{scriptURL: string, status: string, scopeURL?: string}>} swOpts
+ * @param {string} finalUrl
+ */
+function createArtifacts(swOpts, finalUrl) {
+  const artifact = {versions: [], registrations: []};
+
+  swOpts.forEach((sw, index) => {
+    artifact.versions.push({
+      registrationId: `${index}`,
+      status: sw.status,
+      scriptURL: sw.scriptURL,
+    });
+
+    const scopeURL = sw.scopeURL || getBaseDirectory(sw.scriptURL);
+    assert.ok(scopeURL.endsWith('/')); // required by SW spec.
+
+    artifact.registrations.push({
+      registrationId: `${index}`,
+      scopeURL,
+    });
+  });
+
+  return {
+    ServiceWorker: artifact,
+    URL: {finalUrl},
+  };
+}
+
 describe('Offline: service worker audit', () => {
   it('passes when given a matching service worker version', () => {
-    const output = Audit.audit({
-      ServiceWorker: {
-        versions: [{
-          status: 'activated',
-          scriptURL: 'https://example.com/sw.js',
-        }],
-      },
-      URL: {finalUrl: 'https://example.com'},
-    });
+    const finalUrl = 'https://example.com';
+    const swOpts = [{
+      status: 'activated',
+      scriptURL: 'https://example.com/sw.js',
+    }];
 
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
     assert.equal(output.rawValue, true);
   });
 
+  it('fails when matching service worker is not activated', () => {
+    const finalUrl = 'https://example.com';
+    const swOpts = [{
+      status: 'redundant',
+      scriptURL: 'https://example.com/sw.js',
+    }];
+
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
+    assert.equal(output.rawValue, false);
+  });
+
   it('discards service worker registrations for other origins', () => {
-    const versions = [{
+    const finalUrl = 'https://example.com';
+    const swOpts = [{
       status: 'activated',
       scriptURL: 'https://other-example.com',
     }];
 
-    const output = Audit.audit({
-      ServiceWorker: {
-        versions,
-      },
-      URL: {finalUrl: 'https://example.com'},
-    });
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
+    assert.equal(output.rawValue, false);
+  });
+
+  it('fails when URL is out of scope', () => {
+    const finalUrl = 'https://example.com/index.html';
+    const swOpts = [{
+      status: 'activated',
+      scriptURL: 'https://example.com/serviceworker/sw.js',
+    }];
+
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
+    assert.equal(output.rawValue, false);
+  });
+
+  it('fails when explicit scopeURL puts the URL out of scope', () => {
+    const finalUrl = 'https://example.com/index.html';
+    const swOpts = [{
+      status: 'activated',
+      scriptURL: 'https://example.com/sw.js',
+      scopeURL: 'https://example.com/serviceworker/',
+    }];
+
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
+    assert.equal(output.rawValue, false);
+  });
+
+  it('passes when outside default scope but explicit scopeURL puts it back in', () => {
+    const finalUrl = 'https://example.com/index.html';
+    const swOpts = [{
+      status: 'activated',
+      scriptURL: 'https://example.com/serviceworker/sw.js',
+      // can happen when 'Service-Worker-Allowed' header widens max scope.
+      scopeURL: 'https://example.com/',
+    }];
+
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
+    assert.equal(output.rawValue, true);
+  });
+
+  it('passes when multiple SWs control the scope', () => {
+    const finalUrl = 'https://example.com/project/index.html';
+    const swOpts = [{
+      status: 'activated',
+      scriptURL: 'https://example.com/sw.js',
+    }, {
+      status: 'activated',
+      scriptURL: 'https://example.com/project/sw.js',
+    }];
+
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
+    assert.equal(output.rawValue, true);
+  });
+
+  it('passes when multiple SWs control the origin but only one is in scope', () => {
+    const finalUrl = 'https://example.com/index.html';
+    const swOpts = [{
+      status: 'activated',
+      scriptURL: 'https://example.com/sw.js',
+      scopeURL: 'https://example.com/project/',
+    }, {
+      status: 'activated',
+      scriptURL: 'https://example.com/project/sw.js',
+      scopeURL: 'https://example.com/project/',
+    }, {
+      status: 'activated',
+      scriptURL: 'https://example.com/project/subproject/sw.js',
+      scopeURL: 'https://example.com/',
+    }];
+
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
+    assert.equal(output.rawValue, true);
+  });
+
+  it('fails when multiple SWs control the origin but are all out of scope', () => {
+    const finalUrl = 'https://example.com/index.html';
+    const swOpts = [{
+      status: 'activated',
+      scriptURL: 'https://example.com/sw.js',
+      scopeURL: 'https://example.com/project/',
+    }, {
+      status: 'activated',
+      scriptURL: 'https://example.com/project/sw.js',
+      scopeURL: 'https://example.com/project/',
+    }, {
+      status: 'activated',
+      scriptURL: 'https://example.com/project/subproject/sw.js',
+      scopeURL: 'https://example.com/project/',
+    }];
 
+    const output = ServiceWorker.audit(createArtifacts(swOpts, finalUrl));
     assert.equal(output.rawValue, false);
   });
 });

lighthouse-core/test/audits/webapp-install-banner-test.js

@@ -19,12 +19,6 @@ function generateMockArtifacts(src = manifestSrc) {
 
   const clonedArtifacts = JSON.parse(JSON.stringify({
     Manifest: exampleManifest,
-    ServiceWorker: {
-      versions: [{
-        status: 'activated',
-        scriptURL: 'https://example.com/sw.js',
-      }],
-    },
     URL: {finalUrl: 'https://example.com'},
   }));
   return clonedArtifacts;
@@ -146,17 +140,4 @@ describe('PWA: webapp install banner audit', () => {
       assert.strictEqual(failures.length, 1, failures);
     });
   });
-
-  it('fails if page had no SW', () => {
-    const artifacts = generateMockArtifacts();
-    artifacts.ServiceWorker.versions = [];
-    const context = generateMockAuditContext();
-
-    return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
-      assert.strictEqual(result.rawValue, false);
-      assert.ok(result.explanation.includes('service worker'), result.explanation);
-      const failures = result.details.items[0].failures;
-      assert.strictEqual(failures.length, 1, failures);
-    });
-  });
 });

lighthouse-core/test/gather/gatherers/service-worker-test.js

@@ -7,32 +7,37 @@
 
 /* eslint-env jest */
 
-const ServiceWorkerGather = require('../../../gather/gatherers/service-worker');
+const ServiceWorkerGather = require('../../../gather/gatherers/service-worker.js');
 const assert = require('assert');
-let serviceWorkerGatherer;
 
 describe('service worker gatherer', () => {
-  // Reset the Gatherer before each test.
-  beforeEach(() => {
-    serviceWorkerGatherer = new ServiceWorkerGather();
-  });
-
-  it('obtains the active service worker registration', () => {
+  it('obtains the active service worker registration', async () => {
     const url = 'https://example.com/';
     const versions = [{
+      registrationId: '123',
       status: 'activated',
       scriptURL: url,
     }];
+    const registrations = [{
+      registrationId: '123',
+      scopeUrl: url,
+      isDeleted: false,
+    }];
 
-    return serviceWorkerGatherer.beforePass({
+    const serviceWorkerGatherer = new ServiceWorkerGather();
+    const artifact = await serviceWorkerGatherer.beforePass({
       driver: {
-        getServiceWorkerVersions() {
-          return Promise.resolve({versions});
+        async getServiceWorkerVersions() {
+          return {versions};
+        },
+        async getServiceWorkerRegistrations() {
+          return {registrations};
         },
       },
       url,
-    }).then(artifact => {
-      assert.deepEqual(artifact.versions, versions);
     });
+
+    assert.deepEqual(artifact.versions, versions);
+    assert.deepEqual(artifact.registrations, registrations);
   });
 });

lighthouse-core/test/results/sample_v2.json

@@ -467,13 +467,12 @@
       "score": 0,
       "scoreDisplayMode": "binary",
       "rawValue": false,
-      "explanation": "Failures: No manifest was fetched,\nSite does not register a service worker.",
+      "explanation": "Failures: No manifest was fetched.",
       "details": {
         "items": [
           {
             "failures": [
-              "No manifest was fetched",
-              "Site does not register a service worker"
+              "No manifest was fetched"
             ],
             "isParseFailure": true,
             "parseFailureReason": "No manifest was fetched"