core(fr): add navigation phase + trace gatherer (#12098)

lighthouse-core/fraggle-rock/config/default-config.js

@@ -12,6 +12,7 @@ const defaultConfig = {
   artifacts: [
     // Artifacts which can be depended on come first.
     {id: 'DevtoolsLog', gatherer: 'devtools-log'},
+    {id: 'Trace', gatherer: 'trace'},
 
     /* eslint-disable max-len */
     {id: 'Accessibility', gatherer: 'accessibility'},
@@ -34,13 +35,15 @@ const defaultConfig = {
 
     // Artifact copies are renamed for compatibility with legacy artifacts.
     {id: 'devtoolsLogs', gatherer: 'devtools-log-compat'},
+    {id: 'traces', gatherer: 'trace-compat'},
   ],
   navigations: [
     {
       id: 'default',
       artifacts: [
         // Artifacts which can be depended on come first.
         'DevtoolsLog',
+        'Trace',
 
         'Accessibility',
         'Appcache',
@@ -61,6 +64,7 @@ const defaultConfig = {
 
         // Compat artifacts come last.
         'devtoolsLogs',
+        'traces',
       ],
     },
   ],

lighthouse-core/fraggle-rock/config/validation.js

@@ -17,7 +17,9 @@ function isFRGathererDefn(gathererDefn) {
  * Determines if the artifact dependency direction is valid.
  * A timespan artifact cannot depend on a snapshot/navigation artifact because snapshot runs after timespan.
  * A snapshot artifact cannot depend on a navigation artifact because it might be run without a navigation.
- * In other words, the dependency's minimum supported mode must be less than or equal to the dependent's.
+ * A navigation artifact also cannot depend on a snapshot artifact because it is collected before snapshot.
+ * In other words, the dependency's minimum supported mode must be less than or equal to the dependent's with
+ * a special exclusion for navigation dependents.
  *
  * @param {LH.Config.FRGathererDefn} dependent The artifact that depends on the other.
  * @param {LH.Config.FRGathererDefn} dependency The artifact that is being depended on by the other.
@@ -27,6 +29,8 @@ function isValidArtifactDependency(dependent, dependency) {
   const levels = {timespan: 0, snapshot: 1, navigation: 2};
   const dependentLevel = Math.min(...dependent.instance.meta.supportedModes.map(l => levels[l]));
   const dependencyLevel = Math.min(...dependency.instance.meta.supportedModes.map(l => levels[l]));
+  // Special case navigation.
+  if (dependentLevel === levels.navigation) return dependencyLevel !== levels.snapshot;
   return dependencyLevel <= dependentLevel;
 }
 
@@ -76,7 +80,7 @@ function throwInvalidArtifactDependency(artifactId, dependencyKey) {
   throw new Error(
     [
       `Dependency "${dependencyKey}" for "${artifactId}" artifact is invalid.`,
-      `A timespan artifact cannot depend on a snapshot artifact.`,
+      `The dependency must be collected before the dependent.`,
     ].join('\n')
   );
 }

lighthouse-core/fraggle-rock/gather/base-gatherer.js

@@ -20,11 +20,11 @@ class FRGatherer {
   meta = {supportedModes: []}
 
   /**
-   * Method to gather results about a page in a particular state.
+   * Method to start observing a page before a navigation.
    * @param {LH.Gatherer.FRTransitionalContext} passContext
-   * @return {LH.Gatherer.PhaseResult}
+   * @return {Promise<void>|void}
    */
-  snapshot(passContext) { }
+  beforeNavigation(passContext) { }
 
   /**
    * Method to start observing a page for an arbitrary period of time.
@@ -40,6 +40,20 @@ class FRGatherer {
    */
   afterTimespan(passContext) { }
 
+  /**
+   * Method to end observing a page after a navigation and return the results.
+   * @param {LH.Gatherer.FRTransitionalContext} passContext
+   * @return {LH.Gatherer.PhaseResult}
+   */
+  afterNavigation(passContext) { }
+
+  /**
+   * Method to gather results about a page in a particular state.
+   * @param {LH.Gatherer.FRTransitionalContext} passContext
+   * @return {LH.Gatherer.PhaseResult}
+   */
+  snapshot(passContext) { }
+
   /**
    * Legacy property used to define the artifact ID. In Fraggle Rock, the artifact ID lives on the config.
    * @return {keyof LH.GathererArtifacts}

lighthouse-core/fraggle-rock/gather/navigation-runner.js

@@ -21,6 +21,15 @@ const {getBaseArtifacts} = require('./base-artifacts.js');
 
 /** @typedef {Record<string, Promise<any>>} IntermediateArtifacts */
 
+/**
+ * @typedef CollectPhaseArtifactOptions
+ * @property {NavigationContext} navigationContext
+ * @property {ArtifactState} artifacts
+ * @property {keyof Omit<LH.Gatherer.FRGathererInstance, 'name'|'meta'>} phase
+ */
+
+/** @typedef {Record<CollectPhaseArtifactOptions['phase'], IntermediateArtifacts>} ArtifactState */
+
 /**
  * @param {{driver: Driver, config: LH.Config.FRConfig, requestedUrl: string}} args
  */
@@ -47,24 +56,59 @@ async function _setupNavigation({driver, navigation}) {
   // TODO(FR-COMPAT): setup network conditions (throttling & cache state)
 }
 
+/** @type {Set<CollectPhaseArtifactOptions['phase']>} */
+const phasesRequiringDependencies = new Set(['afterTimespan', 'afterNavigation', 'snapshot']);
+/** @type {Record<CollectPhaseArtifactOptions['phase'], LH.Gatherer.GatherMode>} */
+const phaseToGatherMode = {
+  beforeNavigation: 'navigation',
+  beforeTimespan: 'timespan',
+  afterTimespan: 'timespan',
+  afterNavigation: 'navigation',
+  snapshot: 'snapshot',
+};
+/** @type {Record<CollectPhaseArtifactOptions['phase'], CollectPhaseArtifactOptions['phase'] | undefined>} */
+const phaseToPriorPhase = {
+  beforeNavigation: undefined,
+  beforeTimespan: undefined,
+  afterTimespan: 'beforeTimespan',
+  afterNavigation: 'beforeNavigation',
+  snapshot: undefined,
+};
+
 /**
- * @param {NavigationContext} navigationContext
- * @param {IntermediateArtifacts} artifacts
+ * Runs the gatherer methods for a particular navigation phase (beforeTimespan/afterNavigation/etc).
+ * All gatherer method return values are stored on the artifact state object, organized by phase.
+ * This method collects required dependencies, runs the applicable gatherer methods, and saves the
+ * result on the artifact state object that was passed as part of `options`.
+ *
+ * @param {CollectPhaseArtifactOptions} options
  */
-async function _beforeTimespanPhase(navigationContext, artifacts) {
+async function _collectPhaseArtifacts({navigationContext, artifacts, phase}) {
+  const gatherMode = phaseToGatherMode[phase];
+  const priorPhase = phaseToPriorPhase[phase];
+  const priorPhaseArtifacts = (priorPhase && artifacts[priorPhase]) || {};
+
   for (const artifactDefn of navigationContext.navigation.artifacts) {
     const gatherer = artifactDefn.gatherer.instance;
-    if (!gatherer.meta.supportedModes.includes('timespan')) continue;
+    if (!gatherer.meta.supportedModes.includes(gatherMode)) continue;
+
+    const priorArtifactPromise = priorPhaseArtifacts[artifactDefn.id] || Promise.resolve();
+    const artifactPromise = priorArtifactPromise.then(async () => {
+      const dependencies = phasesRequiringDependencies.has(phase)
+        ? await collectArtifactDependencies(artifactDefn, await _mergeArtifacts(artifacts))
+        : {};
 
-    const artifactPromise = Promise.resolve().then(() =>
-      gatherer.beforeTimespan({
+      return gatherer[phase]({
         driver: navigationContext.driver,
         gatherMode: 'navigation',
-        dependencies: {},
-      })
-    );
-    artifacts[artifactDefn.id] = artifactPromise;
-    await artifactPromise.catch(() => {});
+        dependencies,
+      });
+    });
+
+    // Do not set the artifact promise if the result was `undefined`.
+    const result = await artifactPromise.catch(err => err);
+    if (result === undefined) continue;
+    artifacts[phase][artifactDefn.id] = artifactPromise;
   }
 }
 
@@ -81,57 +125,32 @@ async function _navigate(navigationContext) {
 }
 
 /**
- * @param {NavigationContext} navigationContext
- * @param {IntermediateArtifacts} artifacts
- */
-async function _afterTimespanPhase(navigationContext, artifacts) {
-  for (const artifactDefn of navigationContext.navigation.artifacts) {
-    const gatherer = artifactDefn.gatherer.instance;
-    if (!gatherer.meta.supportedModes.includes('timespan')) continue;
-
-    const artifactPromise = (artifacts[artifactDefn.id] || Promise.resolve()).then(async () =>
-      gatherer.afterTimespan({
-        driver: navigationContext.driver,
-        gatherMode: 'navigation',
-        dependencies: await collectArtifactDependencies(artifactDefn, artifacts),
-      })
-    );
-    artifacts[artifactDefn.id] = artifactPromise;
-    await artifactPromise.catch(() => {});
-  }
-}
-
-/**
- * @param {NavigationContext} navigationContext
- * @param {IntermediateArtifacts} artifacts
- */
-async function _snapshotPhase(navigationContext, artifacts) {
-  for (const artifactDefn of navigationContext.navigation.artifacts) {
-    const gatherer = artifactDefn.gatherer.instance;
-    if (!gatherer.meta.supportedModes.includes('snapshot')) continue;
-
-    const artifactPromise = Promise.resolve().then(async () =>
-      gatherer.snapshot({
-        driver: navigationContext.driver,
-        gatherMode: 'navigation',
-        dependencies: await collectArtifactDependencies(artifactDefn, artifacts),
-      })
-    );
-    artifacts[artifactDefn.id] = artifactPromise;
-    await artifactPromise.catch(() => {});
-  }
-}
-
-/**
- * @param {IntermediateArtifacts} timespanArtifacts
- * @param {IntermediateArtifacts} snapshotArtifacts
+ * Merges artifact in Lighthouse order of specificity.
+ * If a gatherer method returns `undefined`, the artifact is skipped for that phase (treated as not set).
+ *
+ *  - Navigation artifacts are the most specific. These win over anything.
+ *  - Snapshot artifacts win out next as they have access to all available information.
+ *  - Timespan artifacts win when nothing else is defined.
+ *
+ * @param {ArtifactState} artifactState
  * @return {Promise<Partial<LH.GathererArtifacts>>}
  */
-async function _mergeArtifacts(timespanArtifacts, snapshotArtifacts) {
+async function _mergeArtifacts(artifactState) {
   /** @type {IntermediateArtifacts} */
   const artifacts = {};
-  for (const [id, promise] of Object.entries({...timespanArtifacts, ...snapshotArtifacts})) {
-    artifacts[id] = await promise.catch(err => err);
+
+  const artifactResultsInIncreasingPriority = [
+    artifactState.afterTimespan,
+    artifactState.snapshot,
+    artifactState.afterNavigation,
+  ];
+
+  for (const artifactResults of artifactResultsInIncreasingPriority) {
+    for (const [id, promise] of Object.entries(artifactResults)) {
+      const artifact = await promise.catch(err => err);
+      if (artifact === undefined) continue;
+      artifacts[id] = artifact;
+    }
   }
 
   return artifacts;
@@ -141,18 +160,26 @@ async function _mergeArtifacts(timespanArtifacts, snapshotArtifacts) {
  * @param {NavigationContext} navigationContext
  */
 async function _navigation(navigationContext) {
-  /** @type {IntermediateArtifacts} */
-  const timespanArtifacts = {};
-  /** @type {IntermediateArtifacts} */
-  const snapshotArtifacts = {};
+  /** @type {ArtifactState} */
+  const artifactState = {
+    beforeNavigation: {},
+    beforeTimespan: {},
+    afterTimespan: {},
+    afterNavigation: {},
+    snapshot: {},
+  };
+
+  const options = {navigationContext, artifacts: artifactState};
 
   await _setupNavigation(navigationContext);
-  await _beforeTimespanPhase(navigationContext, timespanArtifacts);
+  await _collectPhaseArtifacts({phase: 'beforeNavigation', ...options});
+  await _collectPhaseArtifacts({phase: 'beforeTimespan', ...options});
   await _navigate(navigationContext);
-  await _afterTimespanPhase(navigationContext, timespanArtifacts);
-  await _snapshotPhase(navigationContext, snapshotArtifacts);
+  await _collectPhaseArtifacts({phase: 'afterTimespan', ...options});
+  await _collectPhaseArtifacts({phase: 'afterNavigation', ...options});
+  await _collectPhaseArtifacts({phase: 'snapshot', ...options});
 
-  const artifacts = await _mergeArtifacts(timespanArtifacts, snapshotArtifacts);
+  const artifacts = await _mergeArtifacts(artifactState);
   return {artifacts};
 }
 
@@ -214,9 +241,7 @@ module.exports = {
   navigation,
   _setup,
   _setupNavigation,
-  _beforeTimespanPhase,
-  _afterTimespanPhase,
-  _snapshotPhase,
+  _collectPhaseArtifacts,
   _navigate,
   _navigation,
   _navigations,

lighthouse-core/fraggle-rock/gather/runner-helpers.js

@@ -5,6 +5,15 @@
  */
 'use strict';
 
+/**
+ *
+ * @param {{id: string}} dependency
+ * @param {Error} error
+ */
+function createDependencyError(dependency, error) {
+  return new Error(`Dependency "${dependency.id}" failed with exception: ${error.message}`);
+}
+
 /**
  * @param {LH.Config.ArtifactDefn} artifact
  * @param {Record<string, LH.Gatherer.PhaseResult>} artifactsById
@@ -17,14 +26,13 @@ async function collectArtifactDependencies(artifact, artifactsById) {
     async ([dependencyName, dependency]) => {
       const dependencyArtifact = artifactsById[dependency.id];
       if (dependencyArtifact === undefined) throw new Error(`"${dependency.id}" did not run`);
+      if (dependencyArtifact instanceof Error) {
+        throw createDependencyError(dependency, dependencyArtifact);
+      }
 
       const dependencyPromise = Promise.resolve()
         .then(() => dependencyArtifact)
-        .catch(err =>
-          Promise.reject(
-            new Error(`Dependency "${dependency.id}" failed with exception: ${err.message}`)
-          )
-        );
+        .catch(err => Promise.reject(createDependencyError(dependency, err)));
 
       return [dependencyName, await dependencyPromise];
     }