core: special case AppManifest as baseArtifact

lighthouse-core/gather/driver.js

@@ -403,19 +403,28 @@ class Driver {
   /**
    * @return {Promise<{url: string, data: string}|null>}
    */
-  getAppManifest() {
-    return this.sendCommand('Page.getAppManifest')
-      .then(response => {
-        // We're not reading `response.errors` however it may contain critical and noncritical
-        // errors from Blink's manifest parser:
-        //   https://chromedevtools.github.io/debugger-protocol-viewer/tot/Page/#type-AppManifestError
-        if (!response.data) {
-          // If the data is empty, the page had no manifest.
-          return null;
-        }
+  async getAppManifest() {
+    this.setNextProtocolTimeout(3000);
+    const response = await this.sendCommand('Page.getAppManifest');
+    let data = response.data;
+
+    // We're not reading `response.errors` however it may contain critical and noncritical
+    // errors from Blink's manifest parser:
+    //   https://chromedevtools.github.io/debugger-protocol-viewer/tot/Page/#type-AppManifestError
+    if (!data) {
+      // If the data is empty, the page had no manifest.
+      return null;
+    }
 
-        return /** @type {Required<LH.Crdp.Page.GetAppManifestResponse>} */ (response);
-      });
+    const BOM_LENGTH = 3;
+    const BOM_FIRSTCHAR = 65279;
+    const isBomEncoded = data.charCodeAt(0) === BOM_FIRSTCHAR;
+
+    if (isBomEncoded) {
+      data = Buffer.from(data).slice(BOM_LENGTH).toString();
+    }
+
+    return {...response, data};
   }
 
   /**

lighthouse-core/gather/gather-runner.js

@@ -6,10 +6,11 @@
 'use strict';
 
 const log = require('lighthouse-logger');
-const LHError = require('../lib/lh-error');
-const URL = require('../lib/url-shim');
+const manifestParser = require('../lib/manifest-parser.js');
+const LHError = require('../lib/lh-error.js');
+const URL = require('../lib/url-shim.js');
 const NetworkRecorder = require('../lib/network-recorder.js');
-const constants = require('../config/constants');
+const constants = require('../config/constants.js');
 
 const Driver = require('../gather/driver.js'); // eslint-disable-line no-unused-vars
 
@@ -400,6 +401,7 @@ class GatherRunner {
       HostUserAgent: (await options.driver.getBrowserVersion()).userAgent,
       NetworkUserAgent: '', // updated later
       BenchmarkIndex: 0, // updated later
+      WebAppManifest: null, // updated later
       traces: {},
       devtoolsLogs: {},
       settings: options.settings,
@@ -408,6 +410,24 @@ class GatherRunner {
     };
   }
 
+  /**
+   * Uses the debugger protocol to fetch the manifest from within the context of
+   * the target page, reusing any credentials, emulation, etc, already established
+   * there.
+   *
+   * Returns the parsed manifest or null if the page had no manifest. If the manifest
+   * was unparseable as JSON, manifest.value will be undefined and manifest.warning
+   * will have the reason. See manifest-parser.js for more information.
+   *
+   * @param {LH.Gatherer.PassContext} passContext
+   * @return {Promise<LH.Artifacts.Manifest|null>}
+   */
+  static async getWebAppManifest(passContext) {
+    const response = await passContext.driver.getAppManifest();
+    if (!response) return null;
+    return manifestParser(response.data, response.url, passContext.url);
+  }
+
   /**
    * @param {Array<LH.Config.Pass>} passes
    * @param {{driver: Driver, requestedUrl: string, settings: LH.Config.Settings}} options
@@ -437,6 +457,7 @@ class GatherRunner {
           url: options.requestedUrl,
           settings: options.settings,
           passConfig,
+          baseArtifacts,
           // *pass() functions and gatherers can push to this warnings array.
           LighthouseRunWarnings: baseArtifacts.LighthouseRunWarnings,
         };
@@ -448,6 +469,9 @@ class GatherRunner {
         }
         await GatherRunner.beforePass(passContext, gathererResults);
         await GatherRunner.pass(passContext, gathererResults);
+        if (isFirstPass) {
+          baseArtifacts.WebAppManifest = await GatherRunner.getWebAppManifest(passContext);
+        }
         const passData = await GatherRunner.afterPass(passContext, gathererResults);
 
         // Save devtoolsLog, but networkRecords are discarded and not added onto artifacts.

lighthouse-core/gather/gatherers/manifest.js

@@ -6,40 +6,17 @@
 'use strict';
 
 const Gatherer = require('./gatherer');
-const manifestParser = require('../../lib/manifest-parser');
-const BOM_LENGTH = 3;
-const BOM_FIRSTCHAR = 65279;
 
 /**
- * Uses the debugger protocol to fetch the manifest from within the context of
- * the target page, reusing any credentials, emulation, etc, already established
- * there. The artifact produced is the fetched string, if any, passed through
- * the manifest parser.
+ * TODO(phulce): delete this file and move usages over to WebAppManifest
  */
 class Manifest extends Gatherer {
   /**
-   * Returns the parsed manifest or null if the page had no manifest. If the manifest
-   * was unparseable as JSON, manifest.value will be undefined and manifest.warning
-   * will have the reason. See manifest-parser.js for more information.
    * @param {LH.Gatherer.PassContext} passContext
    * @return {Promise<LH.Artifacts['Manifest']>}
    */
   async afterPass(passContext) {
-    const manifestPromise = passContext.driver.getAppManifest();
-    /** @type {Promise<void>} */
-    const timeoutPromise = new Promise(resolve => setTimeout(resolve, 3000));
-
-    const response = await Promise.race([manifestPromise, timeoutPromise]);
-    if (!response) {
-      return null;
-    }
-
-    const isBomEncoded = response.data.charCodeAt(0) === BOM_FIRSTCHAR;
-    if (isBomEncoded) {
-      response.data = Buffer.from(response.data).slice(BOM_LENGTH).toString();
-    }
-
-    return manifestParser(response.data, response.url, passContext.url);
+    return passContext.baseArtifacts.WebAppManifest;
   }
 }
 

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

@@ -6,7 +6,6 @@
 'use strict';
 
 const Gatherer = require('./gatherer');
-const manifestParser = require('../../lib/manifest-parser');
 
 /** @typedef {import('../driver.js')} Driver */
 
@@ -16,27 +15,21 @@ class StartUrl extends Gatherer {
    * @param {LH.Gatherer.PassContext} passContext
    * @return {Promise<LH.Artifacts['StartUrl']>}
    */
-  afterPass(passContext) {
-    const driver = passContext.driver;
-    return driver.goOnline(passContext)
-      .then(() => driver.getAppManifest())
-      .then(response => driver.goOffline().then(() => response))
-      .then(response => response && manifestParser(response.data, response.url, passContext.url))
-      .then(manifest => {
-        const startUrlInfo = this._readManifestStartUrl(manifest);
-        if (startUrlInfo.isReadFailure) {
-          return {statusCode: -1, explanation: startUrlInfo.reason};
-        }
+  async afterPass(passContext) {
+    const manifest = passContext.baseArtifacts.WebAppManifest;
+    const startUrlInfo = this._readManifestStartUrl(manifest);
+    if (startUrlInfo.isReadFailure) {
+      return {statusCode: -1, explanation: startUrlInfo.reason};
+    }
 
-        return this._attemptManifestFetch(passContext.driver, startUrlInfo.startUrl);
-      }).catch(() => {
-        return {statusCode: -1, explanation: 'Unable to fetch start URL via service worker.'};
-      });
+    return this._attemptStartURLFetch(passContext.driver, startUrlInfo.startUrl).catch(() => {
+      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 {LH.Artifacts.Manifest|null} manifest
    * @return {{isReadFailure: true, reason: string}|{isReadFailure: false, startUrl: string}}
    */
   _readManifestStartUrl(manifest) {
@@ -61,7 +54,8 @@ class StartUrl extends Gatherer {
    * @param {string} startUrl
    * @return {Promise<{statusCode: number, explanation: string}>}
    */
-  _attemptManifestFetch(driver, startUrl) {
+  _attemptStartURLFetch(driver, startUrl) {
+    // TODO(phulce): clean up this setTimeout once the response has happened
     // Wait up to 3s to get a matched network request from the fetch() to work
     const timeoutPromise = new Promise(resolve =>
       setTimeout(

lighthouse-core/test/gather/driver-test.js

@@ -117,10 +117,11 @@ function sendCommandStub(command, params) {
 /* eslint-env jest */
 
 let driverStub;
+let connectionStub;
 beforeEach(() => {
-  const connection = new Connection();
-  connection.sendCommand = sendCommandStub;
-  driverStub = new Driver(connection);
+  connectionStub = new Connection();
+  connectionStub.sendCommand = sendCommandStub;
+  driverStub = new Driver(connectionStub);
 });
 
 describe('Browser Driver', () => {
@@ -329,11 +330,42 @@ describe('Browser Driver', () => {
       assert.equal(sendCommandParams[0], undefined);
     });
   });
+
+  describe('.getAppManifest', () => {
+    it('should return null when no manifest', async () => {
+      connectionStub.sendCommand = jest.fn()
+        .mockResolvedValueOnce({data: undefined, url: '/manifest'});
+      const result = await driverStub.getAppManifest();
+      expect(result).toEqual(null);
+    });
+
+    it('should return the manifest', async () => {
+      const manifest = {name: 'The App'};
+      connectionStub.sendCommand = jest.fn()
+        .mockResolvedValueOnce({data: JSON.stringify(manifest), url: '/manifest'});
+      const result = await driverStub.getAppManifest();
+      expect(result).toEqual({data: JSON.stringify(manifest), url: '/manifest'});
+    });
+
+    it('should handle BOM-encoded manifest', async () => {
+      const fs = require('fs');
+      const manifestWithoutBOM = fs.readFileSync(__dirname + '/../fixtures/manifest.json')
+        .toString();
+      const manifestWithBOM = fs.readFileSync(__dirname + '/../fixtures/manifest-bom.json')
+        .toString();
+
+      connectionStub.sendCommand = jest.fn()
+        .mockResolvedValueOnce({data: manifestWithBOM, url: '/manifest'});
+      const result = await driverStub.getAppManifest();
+      expect(result).toEqual({data: manifestWithoutBOM, url: '/manifest'});
+    });
+  });
 });
 
 describe('Multiple tab check', () => {
   beforeEach(() => {
     sendCommandParams = [];
+    sendCommandMockResponses.clear();
   });
 
   it('will pass if there are no current service workers', () => {

lighthouse-core/test/gather/fake-driver.js

@@ -22,6 +22,9 @@ const fakeDriver = {
   getBenchmarkIndex() {
     return Promise.resolve(125.2);
   },
+  getAppManifest() {
+    return Promise.resolve(null);
+  },
   connect() {
     return Promise.resolve();
   },

lighthouse-core/test/gather/gather-runner-test.js

@@ -1070,4 +1070,36 @@ describe('GatherRunner', function() {
         });
     });
   });
+
+  describe('.getWebAppManifest', () => {
+    const MANIFEST_URL = 'https://example.com/manifest.json';
+    let passContext;
+
+    beforeEach(() => {
+      passContext = {
+        url: 'https://example.com/index.html',
+        baseArtifacts: {},
+        driver: fakeDriver,
+      };
+    });
+
+    it('should pass through manifest when null', async () => {
+      const getAppManifest = jest.spyOn(fakeDriver, 'getAppManifest');
+      getAppManifest.mockResolvedValueOnce(null);
+      const result = await GatherRunner.getWebAppManifest(passContext);
+      expect(result).toEqual(null);
+    });
+
+    it('should parse the manifest when found', async () => {
+      const manifest = {name: 'App'};
+      const getAppManifest = jest.spyOn(fakeDriver, 'getAppManifest');
+      getAppManifest.mockResolvedValueOnce({data: JSON.stringify(manifest), url: MANIFEST_URL});
+      const result = await GatherRunner.getWebAppManifest(passContext);
+      expect(result).toHaveProperty('raw', JSON.stringify(manifest));
+      expect(result.value).toMatchObject({
+        name: {value: 'App', raw: 'App'},
+        start_url: {value: passContext.url, raw: undefined},
+      });
+    });
+  });
 });