Repository: GoogleChrome/lighthouse
Pull request #6630 (2018-11-22 00:11:16)

core: rename webapp-install-banner audit to installable-manifest
Part of #6395 

Updates `webapp-install-banner` to be the new `installable-manifest` audit. SW detection was already deleted from here, so this mostly just updates the `id` and `title` and tightens some tests/removes some useless stuff.


Patch:
diff --git a/lighthouse-core/audits/webapp-install-banner.js b/lighthouse-core/audits/installable-manifest.js
similarity index 78%
rename from lighthouse-core/audits/webapp-install-banner.js
rename to lighthouse-core/audits/installable-manifest.js
index bbe114413..da50e7048 100644
--- a/lighthouse-core/audits/webapp-install-banner.js
+++ b/lighthouse-core/audits/installable-manifest.js
@@ -5,12 +5,12 @@
  */
 'use strict';
 
-const MultiCheckAudit = require('./multi-check-audit');
+const MultiCheckAudit = require('./multi-check-audit.js');
 const ManifestValues = require('../computed/manifest-values.js');
 
 /**
  * @fileoverview
- * Audits if a page is configured to prompt users with the webapp install banner.
+ * Audits if the page's web app manifest qualifies for triggering a beforeinstallprompt event.
  * https://github.com/GoogleChrome/lighthouse/issues/23#issuecomment-270453303
  *
  * Requirements:
@@ -20,27 +20,21 @@ const ManifestValues = require('../computed/manifest-values.js');
  *   * manifest has a valid shortname
  *   * manifest display property is standalone, minimal-ui, or fullscreen
  *   * manifest contains icon that's a png and size >= 192px
- *   * SW is registered, and it owns this page and the manifest's start url
- *   * Site engagement score of 2 or higher
-
- * This audit covers these requirements with the following exceptions:
- *   * it doesn't consider SW controlling the starturl
- *   * it doesn't consider the site engagement score (naturally)
  */
 
-class WebappInstallBanner extends MultiCheckAudit {
+class InstallableManifest extends MultiCheckAudit {
   /**
    * @return {LH.Audit.Meta}
    */
   static get meta() {
     return {
-      id: 'webapp-install-banner',
-      title: 'User can be prompted to Install the Web App',
-      failureTitle: 'User will not be prompted to Install the Web App',
+      id: 'installable-manifest',
+      title: 'Web app manifest meets the installability requirements',
+      failureTitle: 'Web app manifest does not meet the installability requirements',
       description: 'Browsers can proactively prompt users to add your app to their homescreen, ' +
           'which can lead to higher engagement. ' +
           '[Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).',
-      requiredArtifacts: ['URL', 'ServiceWorker', 'Manifest'],
+      requiredArtifacts: ['URL', 'Manifest'],
     };
   }
 
@@ -86,7 +80,7 @@ class WebappInstallBanner extends MultiCheckAudit {
    */
   static async audit_(artifacts, context) {
     const manifestValues = await ManifestValues.request(artifacts.Manifest, context);
-    const manifestFailures = WebappInstallBanner.assessManifest(manifestValues);
+    const manifestFailures = InstallableManifest.assessManifest(manifestValues);
 
     return {
       failures: [
@@ -97,4 +91,4 @@ class WebappInstallBanner extends MultiCheckAudit {
   }
 }
 
-module.exports = WebappInstallBanner;
+module.exports = InstallableManifest;
diff --git a/lighthouse-core/audits/multi-check-audit.js b/lighthouse-core/audits/multi-check-audit.js
index 35e62a59f..a41cb33e0 100644
--- a/lighthouse-core/audits/multi-check-audit.js
+++ b/lighthouse-core/audits/multi-check-audit.js
@@ -32,7 +32,6 @@ class MultiCheckAudit extends Audit {
       ...result,
       ...result.manifestValues,
       manifestValues: undefined,
-      warnings: undefined,
       allChecks: undefined,
     };
 
diff --git a/lighthouse-core/computed/manifest-values.js b/lighthouse-core/computed/manifest-values.js
index 113cb46af..722ad5372 100644
--- a/lighthouse-core/computed/manifest-values.js
+++ b/lighthouse-core/computed/manifest-values.js
@@ -15,10 +15,6 @@ const PWA_DISPLAY_VALUES = ['minimal-ui', 'fullscreen', 'standalone'];
 const SUGGESTED_SHORTNAME_LENGTH = 12;
 
 class ManifestValues {
-  static get validityIds() {
-    return ['hasManifest', 'hasParseableManifest'];
-  }
-
   /** @typedef {(val: NonNullable<LH.Artifacts.Manifest['value']>) => boolean} Validator */
 
   /**
@@ -86,8 +82,6 @@ class ManifestValues {
    */
   static async compute_(manifest) {
     // if the manifest isn't there or is invalid json, we report that and bail
-    let parseFailureReason;
-
     if (manifest === null) {
       return {
         isParseFailure: true,
@@ -115,7 +109,6 @@ class ManifestValues {
 
     return {
       isParseFailure: false,
-      parseFailureReason,
       allChecks: remainingChecks,
     };
   }
diff --git a/lighthouse-core/config/default-config.js b/lighthouse-core/config/default-config.js
index 39ad7100f..645dfd2dd 100644
--- a/lighthouse-core/config/default-config.js
+++ b/lighthouse-core/config/default-config.js
@@ -151,7 +151,7 @@ const defaultConfig = {
     'user-timings',
     'critical-request-chains',
     'redirects',
-    'webapp-install-banner',
+    'installable-manifest',
     'splash-screen',
     'themed-omnibox',
     'content-width',
@@ -371,7 +371,7 @@ const defaultConfig = {
         // Installable
         {id: 'is-on-https', weight: 2, group: 'pwa-installable'},
         {id: 'service-worker', weight: 1, group: 'pwa-installable'},
-        {id: 'webapp-install-banner', weight: 2, group: 'pwa-installable'},
+        {id: 'installable-manifest', weight: 2, group: 'pwa-installable'},
         // PWA Optimized
         {id: 'redirects-http', weight: 2, group: 'pwa-optimized'},
         {id: 'splash-screen', weight: 1, group: 'pwa-optimized'},
diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json
index 9e8c3c343..2febe8a80 100644
--- a/proto/sample_v2_round_trip.json
+++ b/proto/sample_v2_round_trip.json
@@ -933,6 +933,25 @@
             "scoreDisplayMode": "not_applicable", 
             "title": "`<input type=\"image\">` elements have `[alt]` text"
         }, 
+        "installable-manifest": {
+            "description": "Browsers can proactively prompt users to add your app to their homescreen, which can lead to higher engagement. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).", 
+            "details": {
+                "items": [
+                    {
+                        "failures": [
+                            "No manifest was fetched"
+                        ], 
+                        "isParseFailure": true, 
+                        "parseFailureReason": "No manifest was fetched"
+                    }
+                ]
+            }, 
+            "explanation": "Failures: No manifest was fetched.", 
+            "id": "installable-manifest", 
+            "score": 0.0, 
+            "scoreDisplayMode": "binary", 
+            "title": "Web app manifest does not meet the installability requirements"
+        }, 
         "interactive": {
             "description": "Interactive marks the time at which the page is fully interactive. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/consistently-interactive).", 
             "displayValue": "4.9\u00a0s", 
@@ -2542,25 +2561,6 @@
             "scoreDisplayMode": "manual", 
             "title": "Visual order on the page follows DOM order"
         }, 
-        "webapp-install-banner": {
-            "description": "Browsers can proactively prompt users to add your app to their homescreen, which can lead to higher engagement. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).", 
-            "details": {
-                "items": [
-                    {
-                        "failures": [
-                            "No manifest was fetched"
-                        ], 
-                        "isParseFailure": true, 
-                        "parseFailureReason": "No manifest was fetched"
-                    }
-                ]
-            }, 
-            "explanation": "Failures: No manifest was fetched.", 
-            "id": "webapp-install-banner", 
-            "score": 0.0, 
-            "scoreDisplayMode": "binary", 
-            "title": "User will not be prompted to Install the Web App"
-        }, 
         "without-javascript": {
             "description": "Your app should display some content when JavaScript is disabled, even if it's just a warning to the user that JavaScript is required to use the app. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/no-js).", 
             "id": "without-javascript", 
@@ -3065,7 +3065,7 @@
                 }, 
                 {
                     "group": "pwa-installable", 
-                    "id": "webapp-install-banner", 
+                    "id": "installable-manifest", 
                     "weight": 2.0
                 }, 
                 {
@@ -3554,7 +3554,7 @@
             {
                 "duration": 100.0, 
                 "entryType": "measure", 
-                "name": "lh:audit:webapp-install-banner", 
+                "name": "lh:audit:installable-manifest", 
                 "startTime": 0.0
             }, 
             {
diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts
index a8dff54d2..aebaa284d 100644
--- a/types/artifacts.d.ts
+++ b/types/artifacts.d.ts
@@ -296,9 +296,16 @@ declare global {
 
       export type ManifestValueCheckID = 'hasStartUrl'|'hasIconsAtLeast192px'|'hasIconsAtLeast512px'|'hasPWADisplayValue'|'hasBackgroundColor'|'hasThemeColor'|'hasShortName'|'hasName'|'shortNameLength';
 
-      export interface ManifestValues {
-        isParseFailure: boolean;
-        parseFailureReason: string | undefined;
+      export type ManifestValues = {
+        isParseFailure: false;
+        allChecks: {
+          id: ManifestValueCheckID;
+          failureText: string;
+          passing: boolean;
+        }[];
+      } | {
+        isParseFailure: true;
+        parseFailureReason: string;
         allChecks: {
           id: ManifestValueCheckID;
           failureText: string;
diff --git a/types/audit.d.ts b/types/audit.d.ts
index 3ec154b77..f2596f444 100644
--- a/types/audit.d.ts
+++ b/types/audit.d.ts
@@ -179,7 +179,6 @@ declare global {
     type MultiCheckAuditP2 = Partial<Artifacts.ManifestValues>;
     interface MultiCheckAuditP3 {
       failures: Array<string>;
-      warnings?: undefined;
       manifestValues?: undefined;
       allChecks?: undefined;
     }


Test patch:
diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
index d2f3e5cca..691971d6a 100644
--- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
+++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
@@ -64,7 +64,7 @@ Object {
       "path": "redirects",
     },
     Object {
-      "path": "webapp-install-banner",
+      "path": "installable-manifest",
     },
     Object {
       "path": "splash-screen",
@@ -840,7 +840,7 @@ Object {
         },
         Object {
           "group": "pwa-installable",
-          "id": "webapp-install-banner",
+          "id": "installable-manifest",
           "weight": 2,
         },
         Object {
diff --git a/lighthouse-cli/test/smokehouse/offline-config.js b/lighthouse-cli/test/smokehouse/offline-config.js
index 01a263ae9..a46f2fcfd 100644
--- a/lighthouse-cli/test/smokehouse/offline-config.js
+++ b/lighthouse-cli/test/smokehouse/offline-config.js
@@ -24,7 +24,7 @@ module.exports = {
       'user-timings',
       'critical-request-chains',
       'render-blocking-resources',
-      'webapp-install-banner',
+      'installable-manifest',
       'splash-screen',
       'themed-omnibox',
       'aria-valid-attr',
diff --git a/lighthouse-cli/test/smokehouse/offline-local/offline-expectations.js b/lighthouse-cli/test/smokehouse/offline-local/offline-expectations.js
index 12f3f23f3..8a04d9260 100644
--- a/lighthouse-cli/test/smokehouse/offline-local/offline-expectations.js
+++ b/lighthouse-cli/test/smokehouse/offline-local/offline-expectations.js
@@ -63,8 +63,10 @@ module.exports = [
       'critical-request-chains': {
         scoreDisplayMode: 'not-applicable',
       },
-      'webapp-install-banner': {
+      'installable-manifest': {
         score: 0,
+        explanation: 'Failures: No manifest was fetched.',
+        details: {items: [{isParseFailure: true}]},
       },
       'splash-screen': {
         score: 0,
@@ -124,7 +126,7 @@ module.exports = [
       'critical-request-chains': {
         scoreDisplayMode: 'not-applicable',
       },
-      'webapp-install-banner': {
+      'installable-manifest': {
         score: 1,
       },
       'splash-screen': {
diff --git a/lighthouse-cli/test/smokehouse/pwa-expectations.js b/lighthouse-cli/test/smokehouse/pwa-expectations.js
index dc61ca6b0..9db56360c 100644
--- a/lighthouse-cli/test/smokehouse/pwa-expectations.js
+++ b/lighthouse-cli/test/smokehouse/pwa-expectations.js
@@ -50,7 +50,7 @@ module.exports = [
       'load-fast-enough-for-pwa': {
         // Ignore speed test; just verify that it ran.
       },
-      'webapp-install-banner': {
+      'installable-manifest': {
         score: 1,
         details: {items: [pwaDetailsExpectations]},
       },
@@ -110,7 +110,7 @@ module.exports = [
       'load-fast-enough-for-pwa': {
         // Ignore speed test; just verify that it ran.
       },
-      'webapp-install-banner': {
+      'installable-manifest': {
         score: 1,
         details: {items: [pwaDetailsExpectations]},
       },
diff --git a/lighthouse-cli/test/smokehouse/pwa2-expectations.js b/lighthouse-cli/test/smokehouse/pwa2-expectations.js
index 28857e8bd..0adc92c0e 100644
--- a/lighthouse-cli/test/smokehouse/pwa2-expectations.js
+++ b/lighthouse-cli/test/smokehouse/pwa2-expectations.js
@@ -44,9 +44,10 @@ module.exports = [
       'load-fast-enough-for-pwa': {
         // Ignore speed test; just verify that it ran.
       },
-      'webapp-install-banner': {
+      'installable-manifest': {
         score: 0,
         details: {items: [jakeExpectations]},
+        explanation: /^Failures: .*short_name/,
       },
       'splash-screen': {
         score: 1,
@@ -104,7 +105,7 @@ module.exports = [
       'load-fast-enough-for-pwa': {
         // Ignore speed test; just verify that it ran.
       },
-      'webapp-install-banner': {
+      'installable-manifest': {
         score: 1,
         details: {items: [pwaDetailsExpectations]},
       },
diff --git a/lighthouse-cli/test/smokehouse/pwa3-expectations.js b/lighthouse-cli/test/smokehouse/pwa3-expectations.js
index 6554e9fa3..205ade18c 100644
--- a/lighthouse-cli/test/smokehouse/pwa3-expectations.js
+++ b/lighthouse-cli/test/smokehouse/pwa3-expectations.js
@@ -42,7 +42,7 @@ module.exports = [
       'load-fast-enough-for-pwa': {
         // Ignore speed test; just verify that it ran .
       },
-      'webapp-install-banner': {
+      'installable-manifest': {
         score: 1,
         details: {items: [pwaRocksExpectations]},
       },
diff --git a/lighthouse-core/test/audits/webapp-install-banner-test.js b/lighthouse-core/test/audits/installable-manifest-test.js
similarity index 72%
rename from lighthouse-core/test/audits/webapp-install-banner-test.js
rename to lighthouse-core/test/audits/installable-manifest-test.js
index 2835a33a1..79b51f60a 100644
--- a/lighthouse-core/test/audits/webapp-install-banner-test.js
+++ b/lighthouse-core/test/audits/installable-manifest-test.js
@@ -5,7 +5,7 @@
  */
 'use strict';
 
-const WebappInstallBannerAudit = require('../../audits/webapp-install-banner');
+const InstallableManifestAudit = require('../../audits/installable-manifest.js');
 const assert = require('assert');
 const manifestParser = require('../../lib/manifest-parser');
 
@@ -37,7 +37,7 @@ describe('PWA: webapp install banner audit', () => {
       artifacts.Manifest = null;
       const context = generateMockAuditContext();
 
-      return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+      return InstallableManifestAudit.audit(artifacts, context).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('No manifest was fetched'), result.explanation);
       });
@@ -47,7 +47,7 @@ describe('PWA: webapp install banner audit', () => {
       const artifacts = generateMockArtifacts();
       artifacts.Manifest = manifestParser('{,:}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
       const context = generateMockAuditContext();
-      return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+      return InstallableManifestAudit.audit(artifacts, context).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('failed to parse as valid JSON'));
       });
@@ -57,7 +57,7 @@ describe('PWA: webapp install banner audit', () => {
       const artifacts = generateMockArtifacts();
       artifacts.Manifest = manifestParser('{}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
       const context = generateMockAuditContext();
-      return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+      return InstallableManifestAudit.audit(artifacts, context).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation);
         assert.strictEqual(result.details.items[0].failures.length, 4);
@@ -66,7 +66,7 @@ describe('PWA: webapp install banner audit', () => {
 
     it('passes with complete manifest and SW', () => {
       const context = generateMockAuditContext();
-      return WebappInstallBannerAudit.audit(generateMockArtifacts(), context).then(result => {
+      return InstallableManifestAudit.audit(generateMockArtifacts(), context).then(result => {
         assert.strictEqual(result.rawValue, true, result.explanation);
         assert.strictEqual(result.explanation, undefined, result.explanation);
       });
@@ -74,31 +74,35 @@ describe('PWA: webapp install banner audit', () => {
   });
 
   describe('one-off-failures', () => {
-    /* eslint-disable camelcase */ // because start_url
     it('fails when a manifest contains no start_url', () => {
       const artifacts = generateMockArtifacts();
       artifacts.Manifest.value.start_url.value = undefined;
       const context = generateMockAuditContext();
 
-      return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+      return InstallableManifestAudit.audit(artifacts, context).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('start_url'), result.explanation);
-        const failures = result.details.items[0].failures;
-        assert.strictEqual(failures.length, 1, failures);
+
+        const details = result.details.items[0];
+        assert.strictEqual(details.failures.length, 1, details.failures);
+        assert.strictEqual(details.hasStartUrl, false);
+        assert.strictEqual(details.hasShortName, true);
       });
     });
 
-    /* eslint-disable camelcase */ // because short_name
     it('fails when a manifest contains no short_name', () => {
       const artifacts = generateMockArtifacts();
       artifacts.Manifest.value.short_name.value = undefined;
       const context = generateMockAuditContext();
 
-      return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+      return InstallableManifestAudit.audit(artifacts, context).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('short_name'), result.explanation);
-        const failures = result.details.items[0].failures;
-        assert.strictEqual(failures.length, 1, failures);
+
+        const details = result.details.items[0];
+        assert.strictEqual(details.failures.length, 1, details.failures);
+        assert.strictEqual(details.hasStartUrl, true);
+        assert.strictEqual(details.hasShortName, false);
       });
     });
 
@@ -107,11 +111,14 @@ describe('PWA: webapp install banner audit', () => {
       artifacts.Manifest.value.name.value = undefined;
       const context = generateMockAuditContext();
 
-      return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+      return InstallableManifestAudit.audit(artifacts, context).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('name'), result.explanation);
-        const failures = result.details.items[0].failures;
-        assert.strictEqual(failures.length, 1, failures);
+
+        const details = result.details.items[0];
+        assert.strictEqual(details.failures.length, 1, details.failures);
+        assert.strictEqual(details.hasStartUrl, true);
+        assert.strictEqual(details.hasName, false);
       });
     });
 
@@ -120,11 +127,14 @@ describe('PWA: webapp install banner audit', () => {
       artifacts.Manifest.value.icons.value = [];
       const context = generateMockAuditContext();
 
-      return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+      return InstallableManifestAudit.audit(artifacts, context).then(result => {
         assert.strictEqual(result.rawValue, false);
         assert.ok(result.explanation.includes('PNG icon'), result.explanation);
-        const failures = result.details.items[0].failures;
-        assert.strictEqual(failures.length, 1, failures);
+
+        const details = result.details.items[0];
+        assert.strictEqual(details.failures.length, 1, details.failures);
+        assert.strictEqual(details.hasStartUrl, true);
+        assert.strictEqual(details.hasIconsAtLeast192px, false);
       });
     });
   });
@@ -133,11 +143,14 @@ describe('PWA: webapp install banner audit', () => {
     const artifacts = generateMockArtifacts(manifestDirtyJpgSrc);
     const context = generateMockAuditContext();
 
-    return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
+    return InstallableManifestAudit.audit(artifacts, context).then(result => {
       assert.strictEqual(result.rawValue, false);
       assert.ok(result.explanation.includes('PNG icon'), result.explanation);
-      const failures = result.details.items[0].failures;
-      assert.strictEqual(failures.length, 1, failures);
+
+      const details = result.details.items[0];
+      assert.strictEqual(details.failures.length, 1, details.failures);
+      assert.strictEqual(details.hasStartUrl, true);
+      assert.strictEqual(details.hasIconsAtLeast192px, false);
     });
   });
 });
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
index c527d7ac5..44cf8eeb0 100644
--- a/lighthouse-core/test/results/sample_v2.json
+++ b/lighthouse-core/test/results/sample_v2.json
@@ -460,9 +460,9 @@
         "overallSavingsMs": 0
       }
     },
-    "webapp-install-banner": {
-      "id": "webapp-install-banner",
-      "title": "User will not be prompted to Install the Web App",
+    "installable-manifest": {
+      "id": "installable-manifest",
+      "title": "Web app manifest does not meet the installability requirements",
       "description": "Browsers can proactively prompt users to add your app to their homescreen, which can lead to higher engagement. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).",
       "score": 0,
       "scoreDisplayMode": "binary",
@@ -2951,7 +2951,7 @@
           "group": "pwa-installable"
         },
         {
-          "id": "webapp-install-banner",
+          "id": "installable-manifest",
           "weight": 2,
           "group": "pwa-installable"
         },
@@ -3697,7 +3697,7 @@
       },
       {
         "startTime": 0,
-        "name": "lh:audit:webapp-install-banner",
+        "name": "lh:audit:installable-manifest",
         "duration": 100,
         "entryType": "measure"
       },