core: include inline scripts in Scripts artifact (#7065)

lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js

@@ -32,6 +32,16 @@ module.exports = [
               wastedBytes: '46481 +/- 100',
               wastedPercent: '87 +/- 5',
             },
+            {
+              url: 'inline: \n  function unusedFunction() {\n    // Un...',
+              wastedBytes: '6581 +/- 100',
+              wastedPercent: '99.6 +/- 0.1',
+            },
+            {
+              url: 'inline: \n  // Used block #1\n  // FILLER DATA JUS...',
+              wastedBytes: '6559 +/- 100',
+              wastedPercent: 100,
+            },
           ],
         },
       },

lighthouse-core/audits/byte-efficiency/unminified-javascript.js

@@ -48,10 +48,11 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
 
   /**
    * @param {string} scriptContent
-   * @param {LH.Artifacts.NetworkRequest} networkRecord
+   * @param {string} displayUrl
+   * @param {LH.Artifacts.NetworkRequest|undefined} networkRecord
    * @return {{url: string, totalBytes: number, wastedBytes: number, wastedPercent: number}}
    */
-  static computeWaste(scriptContent, networkRecord) {
+  static computeWaste(scriptContent, displayUrl, networkRecord) {
     const contentLength = scriptContent.length;
     const totalTokenLength = computeTokenLength(scriptContent);
 
@@ -61,7 +62,7 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
     const wastedBytes = Math.round(totalBytes * wastedRatio);
 
     return {
-      url: networkRecord.url,
+      url: displayUrl,
       totalBytes,
       wastedBytes,
       wastedPercent: 100 * wastedRatio,
@@ -77,21 +78,23 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
     /** @type {Array<LH.Audit.ByteEfficiencyItem>} */
     const items = [];
     const warnings = [];
-    for (const requestId of Object.keys(artifacts.Scripts)) {
-      const scriptContent = artifacts.Scripts[requestId];
+    for (const {requestId, inline, content} of artifacts.Scripts) {
+      if (!content) continue;
       const networkRecord = networkRecords.find(record => record.requestId === requestId);
-      if (!networkRecord || !scriptContent) continue;
-
+      const displayUrl = inline || !networkRecord ?
+        `inline: ${content.substr(0, 40)}...` :
+        networkRecord.url;
       try {
-        const result = UnminifiedJavaScript.computeWaste(scriptContent, networkRecord);
+        const result = UnminifiedJavaScript.computeWaste(content, displayUrl, networkRecord);
         // If the ratio is minimal, the file is likely already minified, so ignore it.
         // If the total number of bytes to be saved is quite small, it's also safe to ignore.
         if (result.wastedPercent < IGNORE_THRESHOLD_IN_PERCENT ||
           result.wastedBytes < IGNORE_THRESHOLD_IN_BYTES ||
           !Number.isFinite(result.wastedBytes)) continue;
         items.push(result);
       } catch (err) {
-        warnings.push(`Unable to process ${networkRecord.url}: ${err.message}`);
+        const url = networkRecord ? networkRecord.url : '?';
+        warnings.push(`Unable to process script ${url}: ${err.message}`);
       }
     }
 

lighthouse-core/gather/gatherers/scripts.js

@@ -5,8 +5,11 @@
  */
 'use strict';
 
+const log = require('lighthouse-logger');
 const Gatherer = require('./gatherer');
 const NetworkRequest = require('../../lib/network-request');
+const getElementsInDocumentString = require('../../lib/page-functions.js').getElementsInDocumentString; // eslint-disable-line max-len
+const URL = require('../../lib/url-shim.js');
 
 /**
  * @fileoverview Gets JavaScript file contents.
@@ -20,21 +23,57 @@ class Scripts extends Gatherer {
   async afterPass(passContext, loadData) {
     const driver = passContext.driver;
 
-    /** @type {Object<string, string>} */
-    const scriptContentMap = {};
+    /** @type {LH.Artifacts['Scripts']} */
+    const scripts = [];
+
+    /** @type {string[]} */
+    const inlineScripts = await driver.evaluateAsync(`(() => {
+      ${getElementsInDocumentString};
+
+      return getElementsInDocument('script')
+        .filter(script => !script.src && script.text.trim())
+        .map(script => script.text);
+    })()`, {useIsolation: true});
+
+    if (inlineScripts.length) {
+      // TODO: use NetworkAnalyzer.findMainDocument() when
+      // this PR lands: https://github.com/GoogleChrome/lighthouse/pull/7080
+      // Then make requestId not optional.
+      const mainResource = loadData.networkRecords.find(request =>
+        passContext.url.startsWith(request.url) &&
+          URL.equalWithExcludedFragments(request.url, passContext.url));
+      if (!mainResource) {
+        log.warn('Scripts', 'could not locate mainResource');
+      }
+      const requestId = mainResource ? mainResource.requestId : undefined;
+      scripts.push(
+        ...inlineScripts.map(content => {
+          return {
+            content,
+            inline: true,
+            requestId,
+          };
+        })
+      );
+    }
+
     const scriptRecords = loadData.networkRecords
       .filter(record => record.resourceType === NetworkRequest.TYPES.Script);
 
     for (const record of scriptRecords) {
       try {
         const content = await driver.getRequestContent(record.requestId);
         if (content) {
-          scriptContentMap[record.requestId] = content;
+          scripts.push({
+            content,
+            inline: false,
+            requestId: record.requestId,
+          });
         }
       } catch (e) {}
     }
 
-    return scriptContentMap;
+    return scripts;
   }
 }
 

lighthouse-core/test/audits/byte-efficiency/unminified-javascript-test.js

@@ -16,35 +16,44 @@ const resourceType = 'Script';
 describe('Page uses optimized responses', () => {
   it('fails when given unminified scripts', () => {
     const auditResult = UnminifiedJavascriptAudit.audit_({
-      Scripts: {
-        '123.1':
-          `
+      Scripts: [
+        {
+          requestId: '123.1',
+          content: `
             var foo = new Set();
             foo.add(1);
             foo.add(2);
 
             if (foo.has(2)) {
               console.log('hello!')
             }
-          `,
-        '123.2':
-          `
+        `,
+        },
+        {
+          requestId: '123.2',
+          content: `
             const foo = new Set();
             foo.add(1);
 
             async function go() {
               await foo.has(1)
               console.log('yay esnext!')
             }
-          `,
-        '123.3':
-          /* eslint-disable no-useless-escape */
+        `,
+        },
+        {
+          requestId: '123.3',
+          content: /* eslint-disable no-useless-escape */
           `
             const foo = 1
             /Edge\/\d*\.\d*/.exec('foo')
           `,
-        '123.4': '#$*%dense',
-      },
+        },
+        {
+          requestId: '123.4',
+          content: '#$*%dense',
+        },
+      ],
     }, [
       {requestId: '123.1', url: 'foo.js', transferSize: 20 * KB, resourceType},
       {requestId: '123.2', url: 'other.js', transferSize: 50 * KB, resourceType},
@@ -58,30 +67,67 @@ describe('Page uses optimized responses', () => {
     }));
 
     expect(results).toMatchObject([
-      {url: 'foo.js', wastedPercent: 57, wastedKB: 11},
-      {url: 'other.js', wastedPercent: 53, wastedKB: 27},
+      {url: 'foo.js', wastedPercent: 56, wastedKB: 11},
+      {url: 'other.js', wastedPercent: 53, wastedKB: 26},
       {url: 'valid-ish.js', wastedPercent: 39, wastedKB: 39},
     ]);
   });
 
-  it('passes when scripts are already minified', () => {
+  it('fails when given unminified scripts even with missing network record', () => {
     const auditResult = UnminifiedJavascriptAudit.audit_({
-      Scripts: {
-        '123.1':
-          'var f=new Set();f.add(1);f.add(2);if(f.has(2))console.log(1234)',
-        '123.2':
-          `
-            const foo = new Set();
+      Scripts: [
+        {
+          requestId: '123.1',
+          content: `
+            var foo = new Set();
             foo.add(1);
+            foo.add(2);
 
-            async function go() {
-              await foo.has(1)
-              console.log('yay esnext!')
+            if (foo.has(2)) {
+              console.log('hello!')
             }
-          `,
-        '123.3':
-          'for{(wtf',
-      },
+            // we can't fake the size to get over the threshold w/o a network record,
+            // so make some really big code instead
+            var a = 0;
+            // ${'a++;'.repeat(2000)}
+        `,
+        },
+      ],
+    }, []);
+
+    assert.strictEqual(auditResult.items.length, 1);
+    const item = auditResult.items[0];
+    if (!item.url.startsWith('inline: ')) {
+      assert.fail('url should start with "inline: "');
+    }
+    assert.strictEqual(Math.round(item.wastedBytes / 1024), 3);
+    assert.strictEqual(Math.round(item.wastedPercent), 99);
+  });
+
+  it('passes when scripts are already minified', () => {
+    const auditResult = UnminifiedJavascriptAudit.audit_({
+      Scripts: [
+        {
+          requestId: '123.1',
+          content: 'var f=new Set();f.add(1);f.add(2);if(f.has(2))console.log(1234)',
+        },
+        {
+          requestId: '123.2',
+          content: `
+          const foo = new Set();
+          foo.add(1);
+
+          async function go() {
+            await foo.has(1)
+            console.log('yay esnext!')
+          }
+        `,
+        },
+        {
+          requestId: '123.3',
+          content: 'for{(wtf',
+        },
+      ],
     }, [
       {requestId: '123.1', url: 'foo.js', transferSize: 20 * KB, resourceType},
       {requestId: '123.2', url: 'other.js', transferSize: 3 * KB, resourceType}, // too small

types/artifacts.d.ts

@@ -105,8 +105,8 @@ declare global {
       RobotsTxt: {status: number|null, content: string|null};
       /** Set of exceptions thrown during page load. */
       RuntimeExceptions: Crdp.Runtime.ExceptionThrownEvent[];
-      /** The content of all scripts loaded by the page, keyed by networkRecord requestId. */
-      Scripts: Record<string, string>;
+      /** The content of all scripts loaded by the page, and the networkRecord requestId that contained their content. Note, HTML documents will have one entry per script tag, all with the same requestId. */
+      Scripts: Array<{content: string, inline: boolean, requestId?: string}>;
       /** Version information for all ServiceWorkers active after the first page load. */
       ServiceWorker: {versions: Crdp.ServiceWorker.ServiceWorkerVersion[], registrations: Crdp.ServiceWorker.ServiceWorkerRegistration[]};
       /** The status of an offline fetch of the page's start_url. -1 and a explanation if missing or there was an error. */