core(fr): add snapshot and timespan support to no-unload-listeners au…

…dit (#12830)
GoogleChrome · Jul 28, 2021 · 233c56e · 233c56e
1 parent fe91d56
commit 233c56e
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 10 deletions.
diff --git a/lighthouse-core/gather/gatherers/global-listeners.js b/lighthouse-core/gather/gatherers/global-listeners.js
@@ -17,7 +17,7 @@ const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
 class GlobalListeners extends FRGatherer {
   /** @type {LH.Gatherer.GathererMeta} */
   meta = {
-    supportedModes: ['snapshot', 'navigation'],
+    supportedModes: ['snapshot', 'timespan', 'navigation'],
   }
 
   /**

diff --git a/lighthouse-core/gather/gatherers/js-usage.js b/lighthouse-core/gather/gatherers/js-usage.js
@@ -13,8 +13,7 @@ const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
 class JsUsage extends FRGatherer {
   /** @type {LH.Gatherer.GathererMeta} */
   meta = {
-    // TODO(FR-COMPAT): special snapshot case for scriptId -> URL mappings.
-    supportedModes: ['timespan', 'navigation'],
+    supportedModes: ['snapshot', 'timespan', 'navigation'],
   }
 
   constructor() {
@@ -60,25 +59,56 @@ class JsUsage extends FRGatherer {
    */
   async startSensitiveInstrumentation(context) {
     const session = context.driver.defaultSession;
+    session.on('Debugger.scriptParsed', this.onScriptParsed);
     await session.sendCommand('Debugger.enable');
-    await session.on('Debugger.scriptParsed', this.onScriptParsed);
   }
 
   /**
    * @param {LH.Gatherer.FRTransitionalContext} context
    */
   async stopSensitiveInstrumentation(context) {
     const session = context.driver.defaultSession;
-    await session.off('Debugger.scriptParsed', this.onScriptParsed);
     await session.sendCommand('Debugger.disable');
+    session.off('Debugger.scriptParsed', this.onScriptParsed);
   }
 
   /**
+   * Usages alone do not always generate an exhaustive list of scripts in timespan and snapshot.
+   * For audits which use this for url/scriptId mappings, we can include an empty usage object.
+   *
+   * @param {Record<string, Array<LH.Crdp.Profiler.ScriptCoverage>>} usageByUrl
+   */
+  _addMissingScriptIds(usageByUrl) {
+    for (const scriptParsedEvent of this._scriptParsedEvents) {
+      const url = scriptParsedEvent.embedderName;
+      if (!url) continue;
+
+      const scripts = usageByUrl[url] || [];
+      if (!scripts.find(s => s.scriptId === scriptParsedEvent.scriptId)) {
+        scripts.push({
+          url,
+          scriptId: scriptParsedEvent.scriptId,
+          functions: [],
+        });
+      }
+      usageByUrl[url] = scripts;
+    }
+  }
+
+  /**
+   * @param {LH.Gatherer.FRTransitionalContext} context
    * @return {Promise<LH.Artifacts['JsUsage']>}
    */
-  async getArtifact() {
+  async getArtifact(context) {
     /** @type {Record<string, Array<LH.Crdp.Profiler.ScriptCoverage>>} */
     const usageByUrl = {};
+
+    // Force `Debugger.scriptParsed` events for url to scriptId mappings in snapshot mode.
+    if (context.gatherMode === 'snapshot') {
+      await this.startSensitiveInstrumentation(context);
+      await this.stopSensitiveInstrumentation(context);
+    }
+
     for (const scriptUsage of this._scriptUsages) {
       // `ScriptCoverage.url` can be overridden by a magic sourceURL comment.
       // Get the associated ScriptParsedEvent and use embedderName, which is the original url.
@@ -104,6 +134,11 @@ class JsUsage extends FRGatherer {
       usageByUrl[url] = scripts;
     }
 
+    // TODO(FR-COMPAT): Enable this logic for legacy and navigation or make a separate artifact for url/scriptId mappings.
+    if (context.gatherMode !== 'navigation') {
+      this._addMissingScriptIds(usageByUrl);
+    }
+
     return usageByUrl;
   }
 }

diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
@@ -99,7 +99,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(`79`);
+      expect(auditResults.length).toMatchInlineSnapshot(`80`);
 
       expect(erroredAudits).toHaveLength(0);
       expect(failedAudits.map(audit => audit.id)).toContain('label');
@@ -130,7 +130,7 @@ describe('Fraggle Rock API', () => {
         notApplicableAudits,
       } = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`49`);
+      expect(auditResults.length).toMatchInlineSnapshot(`50`);
 
       expect(notApplicableAudits.length).toMatchInlineSnapshot(`7`);
       expect(notApplicableAudits.map(audit => audit.id)).not.toContain('server-response-time');
@@ -172,7 +172,7 @@ describe('Fraggle Rock API', () => {
       if (!result) throw new Error('Lighthouse failed to produce a result');
 
       const {auditResults, erroredAudits, notApplicableAudits} = getAuditsBreakdown(result.lhr);
-      expect(auditResults.length).toMatchInlineSnapshot(`49`);
+      expect(auditResults.length).toMatchInlineSnapshot(`50`);
 
       expect(notApplicableAudits.length).toMatchInlineSnapshot(`9`);
       expect(notApplicableAudits.map(audit => audit.id)).toContain('server-response-time');

diff --git a/lighthouse-core/test/gather/gatherers/js-usage-test.js b/lighthouse-core/test/gather/gatherers/js-usage-test.js
@@ -13,6 +13,7 @@ const Driver = require('../../../gather/driver.js');
 const Connection = require('../../../gather/connections/connection.js');
 const JsUsage = require('../../../gather/gatherers/js-usage.js');
 const {createMockSendCommandFn, createMockOnFn} = require('../mock-commands.js');
+const {createMockContext} = require('../../fraggle-rock/gather/mock-driver.js');
 const {flushAllTimersAndMicrotasks} = require('../../test-utils.js');
 
 describe('JsUsage gatherer', () => {
@@ -59,7 +60,7 @@ describe('JsUsage gatherer', () => {
 
     expect(gatherer._scriptUsages).toEqual(coverage);
 
-    return gatherer.getArtifact();
+    return gatherer.getArtifact({gatherMode: 'navigation'});
   }
 
   it('combines coverage data by url', async () => {
@@ -105,4 +106,87 @@ describe('JsUsage gatherer', () => {
       }
     `);
   });
+
+  it('just establishes url to script id mappings in snapshot mode', async () => {
+    const context = createMockContext();
+    context.gatherMode = 'snapshot';
+    context.driver._session.on
+      .mockEvent('Debugger.scriptParsed', {
+        scriptId: '1',
+        embedderName: 'https://www.example.com',
+      });
+    context.driver._session.sendCommand
+      // Events are flushed on domain enable.
+      .mockResponse('Debugger.enable', flushAllTimersAndMicrotasks)
+      .mockResponse('Debugger.disable', {});
+
+    const artifact = await new JsUsage().getArtifact(context.asContext());
+
+    expect(artifact).toEqual({
+      'https://www.example.com': [
+        {
+          scriptId: '1',
+          url: 'https://www.example.com',
+          functions: [],
+        },
+      ],
+    });
+  });
+
+  it('adds script coverages without coverage in timespan', async () => {
+    const context = createMockContext();
+    context.gatherMode = 'timespan';
+    context.driver._session.on
+      .mockEvent('Debugger.scriptParsed', {
+        scriptId: '1',
+        embedderName: 'https://www.example.com',
+      })
+      .mockEvent('Debugger.scriptParsed', {
+        scriptId: '2',
+        embedderName: 'https://www.example.com/script.js',
+      });
+    context.driver._session.sendCommand
+      .mockResponse('Profiler.enable', {})
+      .mockResponse('Profiler.disable', {})
+      .mockResponse('Debugger.enable', {})
+      .mockResponse('Debugger.disable', {})
+      .mockResponse('Profiler.startPreciseCoverage', {})
+      .mockResponse('Profiler.takePreciseCoverage', {
+        result: [{
+          scriptId: '1',
+          url: 'https://www.example.com',
+          functions: [],
+        }],
+      })
+      .mockResponse('Profiler.stopPreciseCoverage', {});
+
+    const gatherer = new JsUsage();
+    await gatherer.startInstrumentation(context);
+    await gatherer.startSensitiveInstrumentation(context);
+
+    // Needed for protocol events to emit.
+    await flushAllTimersAndMicrotasks(1);
+
+    await gatherer.stopSensitiveInstrumentation(context);
+    await gatherer.stopInstrumentation(context);
+
+    const artifact = await gatherer.getArtifact(context);
+
+    expect(artifact).toEqual({
+      'https://www.example.com': [
+        {
+          scriptId: '1',
+          url: 'https://www.example.com',
+          functions: [],
+        },
+      ],
+      'https://www.example.com/script.js': [
+        {
+          scriptId: '2',
+          url: 'https://www.example.com/script.js',
+          functions: [],
+        },
+      ],
+    });
+  });
 });