core(legacy): convert some base artifacts to regular gatherers

build/build-sample-reports.js

@@ -152,9 +152,6 @@ async function generateErrorLHR() {
     HostUserAgent: 'Mozilla/5.0 ErrorUserAgent Chrome/66',
     NetworkUserAgent: 'Mozilla/5.0 ErrorUserAgent Chrome/66',
     BenchmarkIndex: 1000,
-    WebAppManifest: null,
-    InstallabilityErrors: {errors: []},
-    Stacks: [],
     settings: defaultSettings,
     URL: {
       requestedUrl: 'http://fakeurl.com',

core/gather/base-artifacts.js

@@ -39,10 +39,7 @@ async function getBaseArtifacts(resolvedConfig, driver, context) {
     PageLoadError: null,
     GatherContext: context,
     // Artifacts that have been replaced by regular gatherers in Fraggle Rock.
-    Stacks: [],
     NetworkUserAgent: '',
-    WebAppManifest: null,
-    InstallabilityErrors: {errors: []},
     traces: {},
     devtoolsLogs: {},
   };

core/legacy/config/config.js

@@ -41,10 +41,7 @@ const BASE_ARTIFACT_BLANKS = {
   NetworkUserAgent: '',
   BenchmarkIndex: '',
   BenchmarkIndexes: '',
-  WebAppManifest: '',
   GatherContext: '',
-  InstallabilityErrors: '',
-  Stacks: '',
   traces: '',
   devtoolsLogs: '',
   settings: '',
@@ -54,6 +51,14 @@ const BASE_ARTIFACT_BLANKS = {
 };
 const BASE_ARTIFACT_NAMES = Object.keys(BASE_ARTIFACT_BLANKS);
 
+// These were legacy base artifacts, but we need certain gatherers (e.g. bfcache) to run after them.
+// The order is controlled by the config, but still need to force them to run every time.
+const alwaysRunArtifactIds = [
+  'WebAppManifest',
+  'InstallabilityErrors',
+  'Stacks',
+];
+
 /**
  * @param {LegacyResolvedConfig['passes']} passes
  * @param {LegacyResolvedConfig['audits']} audits
@@ -79,7 +84,8 @@ function assertValidPasses(passes, audits) {
       const gatherer = gathererDefn.instance;
       foundGatherers.add(gatherer.name);
       const isGatherRequiredByAudits = requestedGatherers.has(gatherer.name);
-      if (!isGatherRequiredByAudits) {
+      const isAlwaysRunArtifact = alwaysRunArtifactIds.includes(gatherer.name);
+      if (!isGatherRequiredByAudits && !isAlwaysRunArtifact) {
         const msg = `${gatherer.name} gatherer requested, however no audit requires it.`;
         log.warn('config', msg);
       }
@@ -327,6 +333,9 @@ class LegacyResolvedConfig {
 
     // 3. Resolve which gatherers will need to run
     const requestedGathererIds = LegacyResolvedConfig.getGatherersRequestedByAudits(audits);
+    for (const gathererId of alwaysRunArtifactIds) {
+      requestedGathererIds.add(gathererId);
+    }
 
     // 4. Filter to only the neccessary passes
     const passes =

core/legacy/config/legacy-default-config.js

@@ -70,6 +70,9 @@ legacyDefaultConfig.passes = [{
     'trace-elements',
     'inspector-issues',
     'source-maps',
+    'web-app-manifest',
+    'installability-errors',
+    'stacks',
     'full-page-screenshot',
     'bf-cache-failures',
   ],

core/legacy/gather/gather-runner.js

@@ -16,10 +16,7 @@ import * as prepare from '../../gather/driver/prepare.js';
 import * as storage from '../../gather/driver/storage.js';
 import * as navigation from '../../gather/driver/navigation.js';
 import * as serviceWorkers from '../../gather/driver/service-workers.js';
-import WebAppManifest from '../../gather/gatherers/web-app-manifest.js';
-import InstallabilityErrors from '../../gather/gatherers/installability-errors.js';
 import NetworkUserAgent from '../../gather/gatherers/network-user-agent.js';
-import Stacks from '../../gather/gatherers/stacks.js';
 import {finalizeArtifacts} from '../../gather/base-artifacts.js';
 import UrlUtils from '../../lib/url-utils.js';
 
@@ -395,9 +392,6 @@ class GatherRunner {
       HostUserAgent: hostUserAgent,
       NetworkUserAgent: '', // updated later
       BenchmarkIndex: 0, // updated later
-      WebAppManifest: null, // updated later
-      InstallabilityErrors: {errors: []}, // updated later
-      Stacks: [], // updated later
       traces: {},
       devtoolsLogs: {},
       settings: options.settings,
@@ -424,37 +418,6 @@ class GatherRunner {
 
     const baseArtifacts = passContext.baseArtifacts;
 
-    // Fetch the manifest, if it exists.
-    try {
-      baseArtifacts.WebAppManifest = await WebAppManifest.getWebAppManifest(
-        passContext.driver.defaultSession, passContext.url);
-    } catch (err) {
-      log.error('GatherRunner WebAppManifest', err);
-      baseArtifacts.WebAppManifest = null;
-    }
-
-    try {
-      baseArtifacts.InstallabilityErrors = await InstallabilityErrors.getInstallabilityErrors(
-        passContext.driver.defaultSession);
-    } catch (err) {
-      log.error('GatherRunner InstallabilityErrors', err);
-      baseArtifacts.InstallabilityErrors = {
-        errors: [
-          {
-            errorId: 'protocol-timeout',
-            errorArguments: [],
-          },
-        ],
-      };
-    }
-
-    try {
-      baseArtifacts.Stacks = await Stacks.collectStacks(passContext.driver.executionContext);
-    } catch (err) {
-      log.error('GatherRunner Stacks', err);
-      baseArtifacts.Stacks = [];
-    }
-
     // Find the NetworkUserAgent actually used in the devtoolsLogs.
     const devtoolsLog = baseArtifacts.devtoolsLogs[passContext.passConfig.passName];
     baseArtifacts.NetworkUserAgent = NetworkUserAgent.getNetworkUserAgent(devtoolsLog);

core/test/legacy/config/config-test.js

@@ -61,7 +61,8 @@ describe('Config', () => {
       audits: [MyAudit],
     };
     const newConfig = await LegacyResolvedConfig.fromJson(config);
-    assert.equal(MyGatherer, newConfig.passes[0].gatherers[0].implementation);
+    // Gatherer index 3 because of always included artifacts.
+    assert.equal(MyGatherer, newConfig.passes[0].gatherers[3].implementation);
     assert.equal(MyAudit, newConfig.audits[0].implementation);
   });
 
@@ -190,7 +191,12 @@ describe('Config', () => {
       // Trigger filtering logic.
       onlyAudits: ['optional-artifact-audit'],
     });
-    expect(resolvedConfig.passes[0].gatherers.map(g => g.path)).toEqual(['viewport-dimensions']);
+    expect(resolvedConfig.passes[0].gatherers.map(g => g.path)).toEqual([
+      'viewport-dimensions',
+      'web-app-manifest',
+      'installability-errors',
+      'stacks',
+    ]);
   });
 
   it('should keep optional artifacts in gatherers after filter', async () => {
@@ -222,7 +228,13 @@ describe('Config', () => {
       onlyAudits: ['optional-artifact-audit'],
     });
     expect(resolvedConfig.passes[0].gatherers.map(g => g.path))
-      .toEqual(['viewport-dimensions', 'source-maps']);
+      .toEqual([
+        'viewport-dimensions',
+        'source-maps',
+        'web-app-manifest',
+        'installability-errors',
+        'stacks',
+      ]);
   });
 
   it('should keep optional artifacts in gatherers after filter', async () => {
@@ -254,7 +266,13 @@ describe('Config', () => {
       onlyAudits: ['optional-artifact-audit'],
     });
     expect(resolvedConfig.passes[0].gatherers.map(g => g.path))
-      .toEqual(['viewport-dimensions', 'source-maps']);
+      .toEqual([
+        'viewport-dimensions',
+        'source-maps',
+        'web-app-manifest',
+        'installability-errors',
+        'stacks',
+      ]);
   });
 
   it('should keep optional artifacts in gatherers after filter', async () => {
@@ -286,7 +304,13 @@ describe('Config', () => {
       onlyAudits: ['optional-artifact-audit'],
     });
     expect(resolvedConfig.passes[0].gatherers.map(g => g.path))
-      .toEqual(['viewport-dimensions', 'source-maps']);
+      .toEqual([
+        'viewport-dimensions',
+        'source-maps',
+        'web-app-manifest',
+        'installability-errors',
+        'stacks',
+      ]);
   });
 
   it('does not throw when an audit requires only base artifacts', async () => {

types/artifacts.d.ts

@@ -79,12 +79,6 @@ interface ContextualBaseArtifacts {
 interface LegacyBaseArtifacts {
   /** The user agent string that Lighthouse used to load the page. Set to the empty string if unknown. */
   NetworkUserAgent: string;
-  /** Information on detected tech stacks (e.g. JS libraries) used by the page. */
-  Stacks: Artifacts.DetectedStack[];
-  /** Parsed version of the page's Web App Manifest, or null if none found. This moved to a regular artifact in Fraggle Rock. */
-  WebAppManifest: Artifacts.Manifest | null;
-  /** Errors preventing page being installable as PWA. This moved to a regular artifact in Fraggle Rock. */
-  InstallabilityErrors: Artifacts.InstallabilityErrors;
   /** A set of page-load traces, keyed by passName. */
   traces: {[passName: string]: Trace};
   /** A set of DevTools debugger protocol records, keyed by passName. */
@@ -151,6 +145,8 @@ export interface GathererArtifacts extends PublicGathererArtifacts,LegacyBaseArt
   GlobalListeners: Array<Artifacts.GlobalListener>;
   /** The issues surfaced in the devtools Issues panel */
   InspectorIssues: Artifacts.InspectorIssues;
+  /** Errors preventing page being installable as PWA. */
+  InstallabilityErrors: Artifacts.InstallabilityErrors;
   /** JS coverage information for code used during audit. Keyed by script id. */
   // 'url' is excluded because it can be overriden by a magic sourceURL= comment, which makes keeping it a dangerous footgun!
   JsUsage: Record<string, Omit<LH.Crdp.Profiler.ScriptCoverage, 'url'>>;
@@ -172,6 +168,8 @@ export interface GathererArtifacts extends PublicGathererArtifacts,LegacyBaseArt
   ServiceWorker: {versions: LH.Crdp.ServiceWorker.ServiceWorkerVersion[], registrations: LH.Crdp.ServiceWorker.ServiceWorkerRegistration[]};
   /** Source maps of scripts executed in the page. */
   SourceMaps: Array<Artifacts.SourceMap>;
+  /** Information on detected tech stacks (e.g. JS libraries) used by the page. */
+  Stacks: Artifacts.DetectedStack[];
   /** Information on <script> and <link> tags blocking first paint. */
   TagsBlockingFirstPaint: Artifacts.TagBlockingFirstPaint[];
   /** Information about tap targets including their position and size. */
@@ -180,6 +178,8 @@ export interface GathererArtifacts extends PublicGathererArtifacts,LegacyBaseArt
   Trace: Trace;
   /** Elements associated with metrics (ie: Largest Contentful Paint element). */
   TraceElements: Artifacts.TraceElement[];
+  /** Parsed version of the page's Web App Manifest, or null if none found. */
+  WebAppManifest: Artifacts.Manifest | null;
 }
 
 declare module Artifacts {