core(css-usage): ignore removed stylesheets

[adamraine]

CSS.getStyleSheetText throws an error if the style sheet was removed. This causes issues in FR timespan mode because style sheets discovered during the timespan can be removed later in the timespan. Example:

const puppeteer = require('puppeteer');
const fs = require('fs');
const open = require('open');
const defaultConfig = require('./lighthouse-core/fraggle-rock/config/default-config.js');
const lighthouse = require('./lighthouse-core/fraggle-rock/api.js');

async function run() {
  const browser = await puppeteer.launch({
    headless: false,
  })
  const page = await browser.newPage();
  const url = 'https://espn.com';

  const config = {
    ...defaultConfig,
    // audits: defaultConfig.audits.filter(a => a === 'byte-efficiency/unminified-css'),
    settings: {
      output: 'html'
    },
  };
  const run = await lighthouse.startTimespan({page, config});

  await page.goto(url);

  const {artifacts, report, lhr} = await run.endTimespan();
  fs.writeFileSync('lhr.json', JSON.stringify(lhr));
  fs.writeFileSync('artifacts.json', JSON.stringify(artifacts));
  fs.writeFileSync('fr-report.html', report);
  open('fr-report.html');

  await page.close();
  await browser.close();
}
run();

Simplest solution is to ignore any style sheet that was removed, which is implemented here. This does mean that styles are only tracked if they persist until the end of the timespan.

I experimented with a solution that ran CSS.getStyleSheetText as CSS.styleSheetAdded events rolled in rather than doing a batch run in getArtifact. Unfortunately, this didn't completely eliminate the risk of the style sheet being removed by the time CSS.getStyleSheetText is run.

[brendankenny]

Simplest solution is to ignore any style sheet that was removed, which is implemented here. This does mean that styles are only tracked if they persist until the end of the timespan.

it makes sense to ignore to get it working (what else are we going to do?). Could also catch on CSS.getStyleSheetText, which is in the style of what we do elsewhere, but maybe explicitly handling sheet removal here is better.

What does this mean for the larger timespan experience, though? It seems like there's going to be multiple subtleties about the results you get from a few of our gatherers, and I don't feel like I have a good handle on the overall picture. For navigation currently, we just do our best and if something's not available, it's just not available. Sometimes we issue a warning, but not consistently. It seems like this might be more of a fundamental issue with snapshots and timespans, though?

[patrickhulce]

What does this mean for the larger timespan experience, though? It seems like there's going to be multiple subtleties about the results you get from a few of our gatherers, and I don't feel like I have a good handle on the overall picture.

It seems like this might be more of a fundamental issue with snapshots and timespans, though?

Are you referring to the fact that our traditional data isn't always available and the data we collect is very run-dependent? That is the fundamental issue with timespans and snapshots that all of @adamraine 's other work has been about slowly fixing 😄 We aren't getting there overnight, or even by the MVP. Immediate goal is an e2e working experience and permanent work thereafter to increase the scope of artifacts we can collect in every environment.

[patrickhulce]

SGTM for now! Long-term, not every audit that uses CSSUsage actually needs the content itself and the header is plenty good enough for its purposes. We can revisit breaking up this artifact at a later date, but this addresses the immediate failures quite elegantly 👍