Repository: GoogleChrome/lighthouse
Pull request #12621 (2021-06-04 19:23:18)

core(fr): convert script-elements gatherer
This would be affected by https://github.com/GoogleChrome/lighthouse/issues/12619 but there are no type errors right now so we don't need to wait.

Ref #11313


Patch:
diff --git a/lighthouse-core/fraggle-rock/config/default-config.js b/lighthouse-core/fraggle-rock/config/default-config.js
index ef55b08ca..c5a601593 100644
--- a/lighthouse-core/fraggle-rock/config/default-config.js
+++ b/lighthouse-core/fraggle-rock/config/default-config.js
@@ -40,6 +40,7 @@ const artifacts = {
   PasswordInputsWithPreventedPaste: '',
   ResponseCompression: '',
   RobotsTxt: '',
+  ScriptElements: '',
   SourceMaps: '',
   Stacks: '',
   TagsBlockingFirstPaint: '',
@@ -91,6 +92,7 @@ const defaultConfig = {
     {id: artifacts.PasswordInputsWithPreventedPaste, gatherer: 'dobetterweb/password-inputs-with-prevented-paste'},
     {id: artifacts.ResponseCompression, gatherer: 'dobetterweb/response-compression'},
     {id: artifacts.RobotsTxt, gatherer: 'seo/robots-txt'},
+    {id: artifacts.ScriptElements, gatherer: 'script-elements'},
     {id: artifacts.SourceMaps, gatherer: 'source-maps'},
     {id: artifacts.Stacks, gatherer: 'stacks'},
     {id: artifacts.TagsBlockingFirstPaint, gatherer: 'dobetterweb/tags-blocking-first-paint'},
@@ -144,6 +146,7 @@ const defaultConfig = {
         artifacts.PasswordInputsWithPreventedPaste,
         artifacts.ResponseCompression,
         artifacts.RobotsTxt,
+        artifacts.ScriptElements,
         artifacts.SourceMaps,
         artifacts.Stacks,
         artifacts.TagsBlockingFirstPaint,
diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js
index a373f40c7..f399cfe4e 100644
--- a/lighthouse-core/gather/gatherers/script-elements.js
+++ b/lighthouse-core/gather/gatherers/script-elements.js
@@ -5,10 +5,14 @@
  */
 'use strict';
 
-const Gatherer = require('./gatherer.js');
+const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
+const NetworkRecords = require('../../computed/network-records.js');
 const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
 const NetworkRequest = require('../../lib/network-request.js');
 const pageFunctions = require('../../lib/page-functions.js');
+const {fetchResponseBodyFromCache} = require('../driver/network.js');
+const DevtoolsLog = require('./devtools-log.js');
+const HostFormFactor = require('./host-form-factor.js');
 
 /* global getNodeDetails */
 
@@ -62,17 +66,25 @@ async function runInSeriesOrParallel(values, promiseMapper, runInSeries) {
 /**
  * @fileoverview Gets JavaScript file contents.
  */
-class ScriptElements extends Gatherer {
+class ScriptElements extends FRGatherer {
+  /** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'|'HostFormFactor'>} */
+  meta = {
+    supportedModes: ['timespan', 'navigation'],
+    dependencies: {DevtoolsLog: DevtoolsLog.symbol, HostFormFactor: HostFormFactor.symbol},
+  }
+
   /**
-   * @param {LH.Gatherer.PassContext} passContext
-   * @param {LH.Gatherer.LoadData} loadData
+   * @param {LH.Gatherer.FRTransitionalContext} context
+   * @param {LH.Artifacts.NetworkRequest[]} networkRecords
+   * @param {LH.Artifacts['HostFormFactor']} formFactor
    * @return {Promise<LH.Artifacts['ScriptElements']>}
    */
-  async afterPass(passContext, loadData) {
-    const driver = passContext.driver;
-    const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url);
+  async _getArtifact(context, networkRecords, formFactor) {
+    const session = context.driver.defaultSession;
+    const executionContext = context.driver.executionContext;
+    const mainResource = NetworkAnalyzer.findMainDocument(networkRecords, context.url);
 
-    const scripts = await driver.executionContext.evaluate(collectAllScriptElements, {
+    const scripts = await executionContext.evaluate(collectAllScriptElements, {
       args: [],
       useIsolation: true,
       deps: [
@@ -85,7 +97,7 @@ class ScriptElements extends Gatherer {
       if (script.content) script.requestId = mainResource.requestId;
     }
 
-    const scriptRecords = loadData.networkRecords
+    const scriptRecords = networkRecords
       // Ignore records from OOPIFs
       .filter(record => !record.sessionId)
       // Only get the content of script requests
@@ -95,8 +107,9 @@ class ScriptElements extends Gatherer {
     // record at a time.
     const scriptRecordContents = await runInSeriesOrParallel(
       scriptRecords,
-      record => driver.getRequestContent(record.requestId).catch(() => ''),
-      passContext.baseArtifacts.HostFormFactor === 'mobile' /* runInSeries*/ );
+      record => fetchResponseBodyFromCache(session, record.requestId).catch(() => ''),
+      formFactor === 'mobile' /* runInSeries */
+    );
 
     for (let i = 0; i < scriptRecords.length; i++) {
       const record = scriptRecords[i];
@@ -123,6 +136,26 @@ class ScriptElements extends Gatherer {
     }
     return scripts;
   }
+
+  /**
+   * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'|'HostFormFactor'>} context
+   */
+  async getArtifact(context) {
+    const devtoolsLog = context.dependencies.DevtoolsLog;
+    const formFactor = context.dependencies.HostFormFactor;
+    const networkRecords = await NetworkRecords.request(devtoolsLog, context);
+    return this._getArtifact(context, networkRecords, formFactor);
+  }
+
+  /**
+   * @param {LH.Gatherer.PassContext} passContext
+   * @param {LH.Gatherer.LoadData} loadData
+   */
+  async afterPass(passContext, loadData) {
+    const networkRecords = loadData.networkRecords;
+    const formFactor = passContext.baseArtifacts.HostFormFactor;
+    return this._getArtifact({...passContext, dependencies: {}}, networkRecords, formFactor);
+  }
 }
 
 module.exports = ScriptElements;
diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts
index 09fc5ba0e..230a1fc6c 100644
--- a/types/artifacts.d.ts
+++ b/types/artifacts.d.ts
@@ -24,7 +24,6 @@ declare global {
       | 'HTTPRedirect'
       | 'Manifest'
       | 'MixedContent'
-      | 'ScriptElements'
       | 'ServiceWorker'
       | keyof FRBaseArtifacts
     >;


Test patch:
diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
index ddcad87fd..7a6f08ba1 100644
--- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js
+++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
@@ -121,7 +121,7 @@ describe('Fraggle Rock API', () => {
 
       const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`35`);
+      expect(auditResults.length).toMatchInlineSnapshot(`36`);
 
       expect(erroredAudits).toHaveLength(0);
       expect(failedAudits.map(audit => audit.id)).toContain('errors-in-console');
@@ -159,7 +159,7 @@ describe('Fraggle Rock API', () => {
       const {lhr} = result;
       const {auditResults, failedAudits, erroredAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`145`);
+      expect(auditResults.length).toMatchInlineSnapshot(`151`);
       expect(erroredAudits).toHaveLength(1); // FIXME: MainDocumentContent broken from merge
 
       const failedAuditIds = failedAudits.map(audit => audit.id);
diff --git a/lighthouse-core/test/fraggle-rock/config/config-test.js b/lighthouse-core/test/fraggle-rock/config/config-test.js
index 4d4437657..9e3af077f 100644
--- a/lighthouse-core/test/fraggle-rock/config/config-test.js
+++ b/lighthouse-core/test/fraggle-rock/config/config-test.js
@@ -63,8 +63,8 @@ describe('Fraggle Rock Config', () => {
   });
 
   it('should throw on invalid artifact definitions', () => {
-    const configJson = {artifacts: [{id: 'ScriptElements', gatherer: 'script-elements'}]};
-    expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/ScriptElements gatherer/);
+    const configJson = {artifacts: [{id: 'ServiceWorker', gatherer: 'service-worker'}]};
+    expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/ServiceWorker gatherer/);
   });
 
   it('should resolve navigation definitions', () => {
diff --git a/lighthouse-core/test/gather/gatherers/script-elements-test.js b/lighthouse-core/test/gather/gatherers/script-elements-test.js
new file mode 100644
index 000000000..973b6435d
--- /dev/null
+++ b/lighthouse-core/test/gather/gatherers/script-elements-test.js
@@ -0,0 +1,169 @@
+/**
+ * @license Copyright 2021 The Lighthouse Authors. 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';
+
+/* eslint-env jest */
+
+const {
+  createMockContext,
+  mockDriverSubmodules,
+} = require('../../fraggle-rock/gather/mock-driver.js');
+const mocks = mockDriverSubmodules();
+const ScriptElements = require('../../../gather/gatherers/script-elements.js');
+const NetworkRequest = require('../../../lib/network-request.js');
+
+/**
+ * @param {Partial<LH.Artifacts.NetworkRequest>=} partial
+ * @return {LH.Artifacts.NetworkRequest}
+ */
+function mockRecord(partial) {
+  const request = new NetworkRequest();
+  request.resourceType = NetworkRequest.TYPES.Script;
+  return Object.assign(request, partial);
+}
+
+/**
+ * @param {Partial<LH.Artifacts.ScriptElement>=} partial
+ * @return {LH.Artifacts.ScriptElement}
+ */
+function mockElement(partial) {
+  return {
+    type: null,
+    src: null,
+    id: null,
+    async: false,
+    defer: false,
+    source: 'head',
+    content: null,
+    requestId: null,
+    node: null,
+    ...partial,
+  };
+}
+
+describe('_getArtifact', () => {
+  let mockContext = createMockContext();
+  /** @type {ScriptElements} */
+  let gatherer;
+  /** @type {LH.Artifacts.ScriptElement[]} */
+  let scriptElements;
+  /** @type {LH.Artifacts.NetworkRequest[]} */
+  let networkRecords;
+  /** @type {Record<string, string>} */
+  let scriptRecordContents;
+  /** @type {LH.Artifacts.NetworkRequest} */
+  let mainDocument;
+
+  beforeEach(() => {
+    mocks.reset();
+    mockContext = createMockContext();
+    gatherer = new ScriptElements();
+    scriptElements = [];
+    mainDocument = mockRecord({resourceType: NetworkRequest.TYPES.Document, requestId: '0'});
+    networkRecords = [mainDocument];
+    scriptRecordContents = {};
+    mockContext.driver._executionContext.evaluate.mockImplementation(() => scriptElements);
+    mocks.networkMock.fetchResponseBodyFromCache
+      .mockImplementation((_, id) => Promise.resolve(scriptRecordContents[id]));
+  });
+
+  it('collects script elements', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+    ];
+    scriptRecordContents = {
+      '1': '// SOURCED',
+    };
+    scriptElements = [
+      mockElement({src: 'https://example.com/script.js'}),
+      mockElement({content: '// INLINE'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED'}),
+      mockElement({content: '// INLINE', requestId: '0'}),
+    ]);
+  });
+
+  it('ignore OOPIF records', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+      mockRecord({url: 'https://oopif.com/script.js', requestId: '2', sessionId: 'OOPIF'}),
+    ];
+    scriptRecordContents = {
+      '1': '// SOURCED',
+      '2': '// OOPIF',
+    };
+    // OOPIF would not produce script element
+    scriptElements = [
+      mockElement({src: 'https://example.com/script.js'}),
+      mockElement({content: '// INLINE'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED'}),
+      mockElement({content: '// INLINE', requestId: '0'}),
+    ]);
+  });
+
+  it('null content for sourced script with empty content', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/empty.js', requestId: '1'}),
+    ];
+    scriptRecordContents = {
+      '1': '',
+    };
+    scriptElements = [
+      mockElement({src: 'https://example.com/empty.js'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/empty.js'}),
+    ]);
+  });
+
+  it('handle erroneous network content', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+    ];
+    mocks.networkMock.fetchResponseBodyFromCache.mockRejectedValue('Error');
+    scriptElements = [
+      mockElement({src: 'https://example.com/script.js'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js'}),
+    ]);
+  });
+
+  it('create element if none found', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+    ];
+    scriptRecordContents = {
+      '1': '// SOURCED',
+    };
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED', source: 'network'}),
+    ]);
+  });
+});