core(css-usage): fetch stylesheet contents immediately after discovery (

#13887)

lighthouse-core/gather/gatherers/css-usage.js

@@ -5,56 +5,76 @@
  */
 'use strict';
 
+const log = require('lighthouse-logger');
 const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
+const Sentry = require('../../lib/sentry.js');
 
 /**
  * @fileoverview Tracks unused CSS rules.
  */
 class CSSUsage extends FRGatherer {
   constructor() {
     super();
-    /** @type {Array<LH.Crdp.CSS.StyleSheetAddedEvent>} */
-    this._stylesheets = [];
-    /** @param {LH.Crdp.CSS.StyleSheetAddedEvent} sheet */
-    this._onStylesheetAdded = sheet => this._stylesheets.push(sheet);
-    /** @param {LH.Crdp.CSS.StyleSheetRemovedEvent} sheet */
-    this._onStylesheetRemoved = sheet => {
-      // We can't fetch the content of removed stylesheets, so we ignore them completely.
-      const styleSheetId = sheet.styleSheetId;
-      this._stylesheets = this._stylesheets.filter(s => s.header.styleSheetId !== styleSheetId);
-    };
+    /** @type {LH.Gatherer.FRProtocolSession|undefined} */
+    this._session = undefined;
+    /** @type {Map<string, Promise<LH.Artifacts.CSSStyleSheetInfo|Error>>} */
+    this._sheetPromises = new Map();
     /**
      * Initialize as undefined so we can assert results are fetched.
      * @type {LH.Crdp.CSS.RuleUsage[]|undefined}
      */
     this._ruleUsage = undefined;
+    this._onStylesheetAdded = this._onStylesheetAdded.bind(this);
   }
 
   /** @type {LH.Gatherer.GathererMeta} */
   meta = {
     supportedModes: ['snapshot', 'timespan', 'navigation'],
   };
 
+  /**
+   * @param {LH.Crdp.CSS.StyleSheetAddedEvent} event
+   */
+  async _onStylesheetAdded(event) {
+    if (!this._session) throw new Error('Session not initialized');
+    const styleSheetId = event.header.styleSheetId;
+    const sheetPromise = this._session.sendCommand('CSS.getStyleSheetText', {styleSheetId})
+      .then(content => ({
+        header: event.header,
+        content: content.text,
+      }))
+      .catch(/** @param {Error} err */ (err) => {
+        log.warn(
+          'CSSUsage',
+          `Error fetching content of stylesheet with URL "${event.header.sourceURL}"`
+        );
+        Sentry.captureException(err, {
+          tags: {
+            gatherer: this.name,
+          },
+          extra: {
+            url: event.header.sourceURL,
+          },
+          level: 'error',
+        });
+        return err;
+      });
+    this._sheetPromises.set(styleSheetId, sheetPromise);
+  }
+
   /**
    * @param {LH.Gatherer.FRTransitionalContext} context
    */
   async startCSSUsageTracking(context) {
     const session = context.driver.defaultSession;
+    this._session = session;
     session.on('CSS.styleSheetAdded', this._onStylesheetAdded);
-    session.on('CSS.styleSheetRemoved', this._onStylesheetRemoved);
 
     await session.sendCommand('DOM.enable');
     await session.sendCommand('CSS.enable');
     await session.sendCommand('CSS.startRuleUsageTracking');
   }
 
-  /**
-   * @param {LH.Gatherer.FRTransitionalContext} context
-   */
-  async startInstrumentation(context) {
-    if (context.gatherMode !== 'timespan') return;
-    await this.startCSSUsageTracking(context);
-  }
 
   /**
    * @param {LH.Gatherer.FRTransitionalContext} context
@@ -64,7 +84,14 @@ class CSSUsage extends FRGatherer {
     const coverageResponse = await session.sendCommand('CSS.stopRuleUsageTracking');
     this._ruleUsage = coverageResponse.ruleUsage;
     session.off('CSS.styleSheetAdded', this._onStylesheetAdded);
-    session.off('CSS.styleSheetRemoved', this._onStylesheetRemoved);
+  }
+
+  /**
+   * @param {LH.Gatherer.FRTransitionalContext} context
+   */
+  async startInstrumentation(context) {
+    if (context.gatherMode !== 'timespan') return;
+    await this.startCSSUsageTracking(context);
   }
 
   /**
@@ -93,25 +120,23 @@ class CSSUsage extends FRGatherer {
       await this.stopCSSUsageTracking(context);
     }
 
-    // Fetch style sheet content in parallel.
-    const promises = this._stylesheets.map(sheet => {
-      const styleSheetId = sheet.header.styleSheetId;
-      return session.sendCommand('CSS.getStyleSheetText', {styleSheetId}).then(content => {
-        return {
-          header: sheet.header,
-          content: content.text,
-        };
-      });
-    });
-    const styleSheetInfo = await Promise.all(promises);
+    /** @type {Map<string, LH.Artifacts.CSSStyleSheetInfo>} */
+    const dedupedStylesheets = new Map();
+    const sheets = await Promise.all(this._sheetPromises.values());
+
+    for (const sheet of sheets) {
+      // Erroneous sheets will be reported via sentry and the log.
+      // We can ignore them here without throwing a fatal error.
+      if (sheet instanceof Error) {
+        continue;
+      }
+
+      dedupedStylesheets.set(sheet.content, sheet);
+    }
 
     await session.sendCommand('CSS.disable');
     await session.sendCommand('DOM.disable');
 
-    const dedupedStylesheets = new Map(styleSheetInfo.map(sheet => {
-      return [sheet.content, sheet];
-    }));
-
     if (!this._ruleUsage) throw new Error('Issue collecting rule usages');
 
     return {

lighthouse-core/test/fixtures/fraggle-rock/css-change/end.css

@@ -0,0 +1,3 @@
+body {
+  border: 5px solid red;
+}

lighthouse-core/test/fixtures/fraggle-rock/css-change/end.html

@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <link rel="stylesheet" href="/end.css">
+</head>
+<body>
+  <style>h1 {color: red}</style>
+  <h1>Should have different styles than the start page</h1>
+</body>
+</html>

lighthouse-core/test/fixtures/fraggle-rock/css-change/start.css

@@ -0,0 +1,3 @@
+body {
+  border: 5px solid black;
+}

lighthouse-core/test/fixtures/fraggle-rock/css-change/start.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <link rel="stylesheet" href="/start.css">
+</head>
+<body>
+  <style>h1 {color: gray}</style>
+  <h1>Start page</h1>
+  <h2>Stylesheets should change after navigating to the next page</h2>
+  <a href="/end.html">Next</a>
+</body>
+</html>

lighthouse-core/test/fraggle-rock/scenarios/cross-origin-test-pptr.js

@@ -0,0 +1,40 @@
+/**
+ * @license Copyright 2022 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.
+ */
+
+import {jest} from '@jest/globals';
+
+import * as lighthouse from '../../../fraggle-rock/api.js';
+import {createTestState} from './pptr-test-utils.js';
+import {LH_ROOT} from '../../../../root.js';
+
+/* eslint-env jest */
+/* eslint-env browser */
+
+jest.setTimeout(90_000);
+
+describe('Cross origin timespan', () => {
+  const state = createTestState();
+
+  state.installSetupAndTeardownHooks();
+
+  beforeAll(() => {
+    state.server.baseDir = state.secondaryServer.baseDir =
+      `${LH_ROOT}/lighthouse-core/test/fixtures/fraggle-rock/css-change`;
+  });
+
+  it('should resolve all stylesheets', async () => {
+    await state.page.goto(`${state.serverBaseUrl}/start.html`, {waitUntil: ['networkidle0']});
+
+    const timespan = await lighthouse.startTimespan({page: state.page});
+    await state.page.goto(`${state.secondaryServerBaseUrl}/end.html`);
+    const result = await timespan.endTimespan();
+    if (!result) throw new Error('Lighthouse did not return a result');
+
+    // Ensure CSS usage didn't error.
+    expect(result.artifacts.CSSUsage.stylesheets).toHaveLength(4);
+    expect(result.lhr.audits['unused-css-rules'].score).not.toBeNull();
+  });
+});

lighthouse-core/test/fraggle-rock/scenarios/pptr-test-utils.js

@@ -32,16 +32,22 @@ function createTestState() {
     browser: /** @type {LH.Puppeteer.Browser} */ (any('browser')),
     page: /** @type {LH.Puppeteer.Page} */ (any('page')),
     server: /** @type {StaticServer} */ (any('server')),
+    secondaryServer: /** @type {StaticServer} */ (any('server')),
     serverBaseUrl: '',
+    secondaryServerBaseUrl: '',
 
     installSetupAndTeardownHooks() {
       beforeAll(async () => {
         this.server = new Server();
+        this.secondaryServer = new Server();
         await this.server.listen(0, '127.0.0.1');
+        await this.secondaryServer.listen(0, '127.0.0.1');
         this.serverBaseUrl = `http://localhost:${this.server.getPort()}`;
+        this.secondaryServerBaseUrl = `http://localhost:${this.secondaryServer.getPort()}`;
         this.browser = await puppeteer.launch({
           headless: true,
           executablePath: getChromePath(),
+          ignoreDefaultArgs: ['--enable-automation'],
         });
       });
 
@@ -56,6 +62,7 @@ function createTestState() {
       afterAll(async () => {
         await this.browser.close();
         await this.server.close();
+        await this.secondaryServer.close();
       });
     },
   };

lighthouse-core/test/gather/gatherers/css-usage-test.js

@@ -72,16 +72,18 @@ describe('.getArtifact', () => {
     });
   });
 
-  it('ignores removed stylesheets', async () => {
+  it('ignores sheet if there was an error fetching content', async () => {
     const driver = createMockDriver();
     driver.defaultSession.on
       .mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '1'}})
-      .mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '2'}})
-      .mockEvent('CSS.styleSheetRemoved', {styleSheetId: '1'});
+      .mockEvent('CSS.styleSheetAdded', {header: {styleSheetId: '2'}});
     driver.defaultSession.sendCommand
       .mockResponse('DOM.enable')
       .mockResponse('CSS.enable')
       .mockResponse('CSS.startRuleUsageTracking')
+      .mockResponse('CSS.getStyleSheetText', () => {
+        throw new Error('Sheet not found');
+      })
       .mockResponse('CSS.getStyleSheetText', {text: 'CSS text 2'})
       .mockResponse('CSS.stopRuleUsageTracking', {
         ruleUsage: [