Merge c3c0a4b into cc69781

lighthouse-cli/test/cli/__snapshots__/index-test.js.snap

@@ -102,6 +102,9 @@ Object {
     Object {
       "path": "metrics",
     },
+    Object {
+      "path": "offline-start-url",
+    },
     Object {
       "path": "manual/pwa-cross-browser",
     },
@@ -820,6 +823,11 @@ Object {
           "id": "works-offline",
           "weight": 5,
         },
+        Object {
+          "group": "pwa-fast-reliable",
+          "id": "offline-start-url",
+          "weight": 1,
+        },
         Object {
           "group": "pwa-installable",
           "id": "is-on-https",
@@ -833,7 +841,7 @@ Object {
         Object {
           "group": "pwa-installable",
           "id": "webapp-install-banner",
-          "weight": 3,
+          "weight": 2,
         },
         Object {
           "group": "pwa-engaging",

lighthouse-cli/test/smokehouse/pwa-expectations.js

@@ -38,6 +38,9 @@ module.exports = [
       'works-offline': {
         score: 1,
       },
+      'offline-start-url': {
+        score: 1,
+      },
       'viewport': {
         score: 1,
       },
@@ -95,6 +98,9 @@ module.exports = [
       'works-offline': {
         score: 0,
       },
+      'offline-start-url': {
+        score: 1,
+      },
       'viewport': {
         score: 1,
       },

lighthouse-cli/test/smokehouse/pwa2-expectations.js

@@ -32,6 +32,9 @@ module.exports = [
       'works-offline': {
         score: 1,
       },
+      'offline-start-url': {
+        score: 1,
+      },
       'viewport': {
         score: 1,
       },
@@ -89,6 +92,9 @@ module.exports = [
       'works-offline': {
         score: 1,
       },
+      'offline-start-url': {
+        score: 1,
+      },
       'viewport': {
         score: 1,
       },

lighthouse-cli/test/smokehouse/pwa3-expectations.js

@@ -30,6 +30,9 @@ module.exports = [
       'works-offline': {
         score: 1,
       },
+      'offline-start-url': {
+        score: 1,
+      },
       'viewport': {
         score: 1,
       },

lighthouse-core/audits/multi-check-audit.js

@@ -23,7 +23,7 @@ class MultiCheckAudit extends Audit {
   }
 
   /**
-   * @param {{failures: Array<string>, warnings?: Array<string>, manifestValues?: LH.Artifacts.ManifestValues}} result
+   * @param {{failures: Array<string>, manifestValues?: LH.Artifacts.ManifestValues}} result
    * @return {LH.Audit.Product}
    */
   static createAuditProduct(result) {
@@ -57,7 +57,6 @@ class MultiCheckAudit extends Audit {
     return {
       rawValue: true,
       details,
-      warnings: result.warnings,
     };
   }
 
@@ -66,7 +65,7 @@ class MultiCheckAudit extends Audit {
   /**
    * @param {LH.Artifacts} artifacts
    * @param {LH.Audit.Context} context
-   * @return {Promise<{failures: Array<string>, warnings?: Array<string>, manifestValues?: LH.Artifacts.ManifestValues}>}
+   * @return {Promise<{failures: Array<string>, manifestValues?: LH.Artifacts.ManifestValues}>}
    */
   static audit_(artifacts, context) {
     throw new Error('audit_ unimplemented');

lighthouse-core/audits/offline-start-url.js

@@ -0,0 +1,49 @@
+/**
+ * @license Copyright 2018 Google Inc. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+'use strict';
+
+const Audit = require('./audit.js');
+
+class OfflineStartUrl extends Audit {
+  /**
+   * @return {LH.Audit.Meta}
+   */
+  static get meta() {
+    return {
+      id: 'offline-start-url',
+      title: 'start_url responds with a 200 when offline',
+      failureTitle: 'start_url does not respond with a 200 when offline',
+      description: 'A service worker enables your web app to be reliable in unpredictable network conditions. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/http-200-when-offline).',
+      requiredArtifacts: ['Manifest', 'StartUrl'],
+    };
+  }
+
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @return {LH.Audit.Product}
+   */
+  static audit(artifacts) {
+    // StartUrl gatherer will give explanations for failures, but need to take manifest parsing
+    // warnings from the manifest itself (e.g. invalid `start_url`, so fell back to document URL).
+    const warnings = [];
+    const manifest = artifacts.Manifest;
+    if (manifest && manifest.value && manifest.value.start_url.warning) {
+      const manifestWarning = manifest.value.start_url.warning;
+      warnings.push('We couldn\'t read the start_url from the manifest. As a result, the ' +
+          `start_url was assumed to be the document's URL. Error message: '${manifestWarning}'.`);
+    }
+
+    const hasOfflineStartUrl = artifacts.StartUrl.statusCode === 200;
+
+    return {
+      rawValue: hasOfflineStartUrl,
+      explanation: artifacts.StartUrl.explanation,
+      warnings,
+    };
+  }
+}
+
+module.exports = OfflineStartUrl;

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

@@ -41,7 +41,7 @@ class WebappInstallBanner extends MultiCheckAudit {
       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', 'StartUrl'],
+      requiredArtifacts: ['URL', 'ServiceWorker', 'Manifest'],
     };
   }
 
@@ -94,55 +94,20 @@ class WebappInstallBanner extends MultiCheckAudit {
     return failures;
   }
 
-  /**
-   * @param {LH.Artifacts} artifacts
-   * @return {{failures: Array<string>, warnings: Array<string>}}
-   */
-  static assessOfflineStartUrl(artifacts) {
-    const failures = [];
-    const warnings = [];
-    const hasOfflineStartUrl = artifacts.StartUrl.statusCode === 200;
-
-    if (!hasOfflineStartUrl) {
-      failures.push('Service worker does not successfully serve the manifest\'s start_url');
-      if (artifacts.StartUrl.explanation) {
-        failures.push(artifacts.StartUrl.explanation);
-      }
-    }
-
-    if (artifacts.StartUrl.explanation) {
-      warnings.push(artifacts.StartUrl.explanation);
-    }
-
-    return {failures, warnings};
-  }
-
   /**
    * @param {LH.Artifacts} artifacts
    * @param {LH.Audit.Context} context
-   * @return {Promise<{failures: Array<string>, warnings: Array<string>, manifestValues: LH.Artifacts.ManifestValues}>}
+   * @return {Promise<{failures: Array<string>, manifestValues: LH.Artifacts.ManifestValues}>}
    */
   static async audit_(artifacts, context) {
-    /** @type {Array<string>} */
-    let offlineFailures = [];
-    /** @type {Array<string>} */
-    let offlineWarnings = [];
-
     const manifestValues = await ManifestValues.request(artifacts.Manifest, context);
     const manifestFailures = WebappInstallBanner.assessManifest(manifestValues);
     const swFailures = WebappInstallBanner.assessServiceWorker(artifacts);
-    if (!swFailures.length) {
-      const {failures, warnings} = WebappInstallBanner.assessOfflineStartUrl(artifacts);
-      offlineFailures = failures;
-      offlineWarnings = warnings;
-    }
 
     return {
-      warnings: offlineWarnings,
       failures: [
         ...manifestFailures,
         ...swFailures,
-        ...offlineFailures,
       ],
       manifestValues,
     };

lighthouse-core/config/default-config.js

@@ -132,6 +132,7 @@ const defaultConfig = {
     'font-display',
     'network-requests',
     'metrics',
+    'offline-start-url',
     'manual/pwa-cross-browser',
     'manual/pwa-page-transitions',
     'manual/pwa-each-page-has-url',
@@ -334,10 +335,11 @@ const defaultConfig = {
         // Fast and Reliable
         {id: 'load-fast-enough-for-pwa', weight: 7, group: 'pwa-fast-reliable'},
         {id: 'works-offline', weight: 5, group: 'pwa-fast-reliable'},
+        {id: 'offline-start-url', weight: 1, group: 'pwa-fast-reliable'},
         // Installable
         {id: 'is-on-https', weight: 2, group: 'pwa-installable'},
         {id: 'service-worker', weight: 1, group: 'pwa-installable'},
-        {id: 'webapp-install-banner', weight: 3, group: 'pwa-installable'},
+        {id: 'webapp-install-banner', weight: 2, group: 'pwa-installable'},
         // Engaging
         {id: 'redirects-http', weight: 2, group: 'pwa-engaging'},
         {id: 'splash-screen', weight: 1, group: 'pwa-engaging'},

lighthouse-core/gather/gatherers/start-url.js

@@ -7,7 +7,8 @@
 
 const Gatherer = require('./gatherer');
 const manifestParser = require('../../lib/manifest-parser');
-const Driver = require('../driver.js'); // eslint-disable-line no-unused-vars
+
+/** @typedef {import('../driver.js')} Driver */
 
 class StartUrl extends Gatherer {
   /**
@@ -29,33 +30,27 @@ class StartUrl extends Gatherer {
 
         return this._attemptManifestFetch(passContext.driver, startUrlInfo.startUrl);
       }).catch(() => {
-        return {statusCode: -1, explanation: 'Unable to fetch start URL via service worker'};
+        return {statusCode: -1, explanation: 'Unable to fetch start URL via service worker.'};
       });
   }
 
   /**
    * Read the parsed manifest and return failure reasons or the startUrl
-   * @param {?{value?: {start_url: {value?: string, warning?: string}}, warning?: string}} manifest
+   * @param {?{value?: {start_url: {value: string, warning?: string}}, warning?: string}} manifest
    * @return {{isReadFailure: true, reason: string}|{isReadFailure: false, startUrl: string}}
    */
   _readManifestStartUrl(manifest) {
     if (!manifest || !manifest.value) {
       const detailedMsg = manifest && manifest.warning;
 
       if (detailedMsg) {
-        return {isReadFailure: true, reason: `Error fetching web app manifest: ${detailedMsg}`};
+        return {isReadFailure: true, reason: `Error fetching web app manifest: ${detailedMsg}.`};
       } else {
-        return {isReadFailure: true, reason: `No usable web app manifest found on page`};
+        return {isReadFailure: true, reason: `No usable web app manifest found on page.`};
       }
     }
 
-    // Even if the start URL had an error, the browser will still supply a fallback URL.
-    // Therefore, we only set the warning here and continue with the fetch.
-    if (manifest.value.start_url.warning) {
-      return {isReadFailure: true, reason: manifest.value.start_url.warning};
-    }
-
-    // @ts-ignore - TODO(bckenny): should actually be testing value above, not warning
+    // Even if the start URL had a parser warning, the browser will still supply a fallback URL.
     return {isReadFailure: false, startUrl: manifest.value.start_url.value};
   }
 
@@ -70,7 +65,7 @@ class StartUrl extends Gatherer {
     // Wait up to 3s to get a matched network request from the fetch() to work
     const timeoutPromise = new Promise(resolve =>
       setTimeout(
-        () => resolve({statusCode: -1, explanation: 'Timed out waiting for fetched start_url'}),
+        () => resolve({statusCode: -1, explanation: 'Timed out waiting for fetched start_url.'}),
         3000
       )
     );
@@ -88,7 +83,7 @@ class StartUrl extends Gatherer {
         if (!response.fromServiceWorker) {
           return resolve({
             statusCode: -1,
-            explanation: 'Unable to fetch start URL via service worker',
+            explanation: 'Unable to fetch start URL via service worker.',
           });
         }
         // Successful SW-served fetch of the start_URL

lighthouse-core/lib/manifest-parser.js

@@ -115,6 +115,7 @@ function checkSameOrigin(url1, url2) {
  * @param {*} jsonInput
  * @param {string} manifestUrl
  * @param {string} documentUrl
+ * @return {{raw: any, value: string, warning?: string}}
  */
 function parseStartUrl(jsonInput, manifestUrl, documentUrl) {
   const raw = jsonInput.start_url;
@@ -127,10 +128,18 @@ function parseStartUrl(jsonInput, manifestUrl, documentUrl) {
       warning: 'ERROR: start_url string empty',
     };
   }
-  const parsedAsString = parseString(raw);
-  if (!parsedAsString.value) {
-    parsedAsString.value = documentUrl;
-    return parsedAsString;
+  if (raw === undefined) {
+    return {
+      raw,
+      value: documentUrl,
+    };
+  }
+  if (typeof raw !== 'string') {
+    return {
+      raw,
+      value: documentUrl,
+      warning: 'ERROR: expected a string.',
+    };
   }
 
   // 8.10(4) - construct URL with raw as input and manifestUrl as the base.