core(root-causes): grab from trace insights rather than use protocol (#…

…16352)

cli/test/smokehouse/test-definitions/perf-trace-elements.js

@@ -65,12 +65,6 @@ const expectations = {
           nodeLabel: `Please don't move me`,
         },
       },
-      {
-        traceEventType: 'layout-shift',
-        node: {
-          nodeLabel: 'section > img',
-        },
-      },
       {
         traceEventType: 'animation',
         node: {

cli/test/smokehouse/test-definitions/shift-attribution.js

@@ -19,15 +19,11 @@ const expectations = {
             _includes: [
               {
                 node: {selector: 'body > div#blue'},
-                subItems: {items: [{cause: /font/, extra: {value: /Regular\.ttf/}}]},
+                subItems: {items: [
+                  {cause: /Media/, extra: {selector: 'body > img'}},
+                  {cause: /font/, extra: {value: /Regular\.ttf/}},
+                ]},
               },
-              // iframe root causes are disabled due to performance issues
-              // /~https://github.com/GoogleChrome/lighthouse/issues/15869
-              // {
-              //   node: {selector: 'body > div#blue'},
-              //   // TODO: We can't get nodes from non-main frames yet. See runRootCauseAnalysis.
-              //   subItems: {items: [{cause: /iframe/, extra: undefined}]},
-              // },
               {
                 node: {selector: 'body > div#blue'},
                 subItems: {items: [{cause: /Media/, extra: {selector: 'body > img'}}]},

core/audits/layout-shifts.js

@@ -29,8 +29,6 @@ const UIStrings = {
   rootCauseFontChanges: 'Web font loaded',
   /** A possible reason why that the layout shift occured. */
   rootCauseInjectedIframe: 'Injected iframe',
-  /** A possible reason why that the layout shift occured. */
-  rootCauseRenderBlockingRequest: 'A late network request adjusted the page layout',
   /** Label shown per-audit to show how many layout shifts are present. The `{# shifts found}` placeholder will be replaced with the number of layout shifts. */
   displayValueShiftsFound: `{shiftCount, plural, =1 {1 layout shift found} other {# layout shifts found}}`,
 };
@@ -49,7 +47,7 @@ class LayoutShifts extends Audit {
       description: str_(UIStrings.description),
       scoreDisplayMode: Audit.SCORING_MODES.METRIC_SAVINGS,
       guidanceLevel: 2,
-      requiredArtifacts: ['traces', 'RootCauses', 'TraceElements'],
+      requiredArtifacts: ['traces', 'TraceElements'],
     };
   }
 
@@ -67,6 +65,16 @@ class LayoutShifts extends Audit {
     const traceElements = artifacts.TraceElements
       .filter(element => element.traceEventType === 'layout-shift');
 
+    /** @type {LH.Artifacts.TraceEngineRootCauses} */
+    const allRootCauses = {
+      layoutShifts: new Map(),
+    };
+    for (const insightSet of traceEngineResult.insights.values()) {
+      for (const [shift, reasons] of insightSet.model.CLSCulprits.shifts) {
+        allRootCauses.layoutShifts.set(shift, reasons);
+      }
+    }
+
     /** @type {Item[]} */
     const items = [];
     const layoutShiftEvents =
@@ -82,41 +90,30 @@ class LayoutShifts extends Audit {
       const biggestImpactElement = traceElements.find(t => t.nodeId === biggestImpactNodeId);
 
       // Turn root causes into sub-items.
-      const index = layoutShiftEvents.indexOf(event);
-      const rootCauses = artifacts.RootCauses.layoutShifts[index];
+      const rootCauses = allRootCauses.layoutShifts.get(event);
       /** @type {SubItem[]} */
       const subItems = [];
       if (rootCauses) {
-        for (const cause of rootCauses.unsizedMedia) {
+        for (const backendNodeId of rootCauses.unsizedImages) {
           const element = artifacts.TraceElements.find(
-            t => t.traceEventType === 'layout-shift' && t.nodeId === cause.node.backendNodeId);
+            t => t.traceEventType === 'trace-engine' && t.nodeId === backendNodeId);
           subItems.push({
             extra: element ? Audit.makeNodeItem(element.node) : undefined,
             cause: str_(UIStrings.rootCauseUnsizedMedia),
           });
         }
-        for (const cause of rootCauses.fontChanges) {
-          const url = cause.request.args.data.url;
+        for (const request of rootCauses.fontRequests) {
+          const url = request.args.data.url;
           subItems.push({
             extra: {type: 'url', value: url},
             cause: str_(UIStrings.rootCauseFontChanges),
           });
         }
-        for (const cause of rootCauses.iframes) {
-          const element = artifacts.TraceElements.find(
-            t => t.traceEventType === 'layout-shift' && t.nodeId === cause.iframe.backendNodeId);
+        if (rootCauses.iframeIds.length) {
           subItems.push({
-            extra: element ? Audit.makeNodeItem(element.node) : undefined,
             cause: str_(UIStrings.rootCauseInjectedIframe),
           });
         }
-        for (const cause of rootCauses.renderBlockingRequests) {
-          const url = cause.request.args.data.url;
-          subItems.push({
-            extra: {type: 'url', value: url},
-            cause: str_(UIStrings.rootCauseRenderBlockingRequest),
-          });
-        }
       }
 
       items.push({

core/config/default-config.js

@@ -109,7 +109,6 @@ const defaultConfig = {
     // Artifacts which can be depended on come first.
     {id: 'DevtoolsLog', gatherer: 'devtools-log'},
     {id: 'Trace', gatherer: 'trace'},
-    {id: 'RootCauses', gatherer: 'root-causes'},
 
     {id: 'Accessibility', gatherer: 'accessibility'},
     {id: 'AnchorElements', gatherer: 'anchor-elements'},

core/gather/gatherers/trace-elements.js

@@ -23,7 +23,6 @@ import {LighthouseError} from '../../lib/lh-error.js';
 import {Responsiveness} from '../../computed/metrics/responsiveness.js';
 import {CumulativeLayoutShift} from '../../computed/metrics/cumulative-layout-shift.js';
 import {ExecutionContext} from '../driver/execution-context.js';
-import RootCauses from './root-causes.js';
 import {TraceEngineResult} from '../../computed/trace-engine-result.js';
 
 /** @typedef {{nodeId: number, animations?: {name?: string, failureReasonsMask?: number, unsupportedProperties?: string[]}[], type?: string}} TraceElementData */
@@ -46,10 +45,10 @@ function getNodeDetailsData() {
 /* c8 ignore stop */
 
 class TraceElements extends BaseGatherer {
-  /** @type {LH.Gatherer.GathererMeta<'Trace'|'RootCauses'>} */
+  /** @type {LH.Gatherer.GathererMeta<'Trace'>} */
   meta = {
     supportedModes: ['timespan', 'navigation'],
-    dependencies: {Trace: Trace.symbol, RootCauses: RootCauses.symbol},
+    dependencies: {Trace: Trace.symbol},
   };
 
   /** @type {Map<string, string>} */
@@ -103,13 +102,23 @@ class TraceElements extends BaseGatherer {
       seen.add(obj);
 
       if (obj && typeof obj === 'object' && !Array.isArray(obj)) {
-        Object.keys(obj).forEach(key => {
-          if (typeof obj[key] === 'object') {
-            recursiveObjectEnumerate(obj[key], cb, seen);
-          } else {
-            cb(obj, key);
+        if (obj instanceof Map) {
+          for (const [key, val] of obj) {
+            if (typeof val === 'object') {
+              recursiveObjectEnumerate(val, cb, seen);
+            } else {
+              cb(val, key);
+            }
           }
-        });
+        } else {
+          Object.keys(obj).forEach(key => {
+            if (typeof obj[key] === 'object') {
+              recursiveObjectEnumerate(obj[key], cb, seen);
+            } else {
+              cb(obj[key], key);
+            }
+          });
+        }
       } else if (Array.isArray(obj)) {
         obj.forEach(item => {
           if (typeof item === 'object' || Array.isArray(item)) {
@@ -121,12 +130,18 @@ class TraceElements extends BaseGatherer {
 
     /** @type {number[]} */
     const nodeIds = [];
-    recursiveObjectEnumerate(insightSet.model, (obj, key) => {
-      if (typeof obj[key] === 'number' && (key === 'nodeId' || key === 'node_id')) {
-        nodeIds.push(obj[key]);
+    recursiveObjectEnumerate(insightSet.model, (val, key) => {
+      const keys = ['nodeId', 'node_id'];
+      if (typeof val === 'number' && keys.includes(key)) {
+        nodeIds.push(val);
       }
     }, new Set());
 
+    // TODO: would be better if unsizedImages was `Array<{nodeId}>`.
+    for (const shift of insightSet.model.CLSCulprits.shifts.values()) {
+      nodeIds.push(...shift.unsizedImages);
+    }
+
     return [...new Set(nodeIds)].map(id => ({nodeId: id}));
   }
 
@@ -212,14 +227,6 @@ class TraceElements extends BaseGatherer {
           nodeIds.push(biggestImpactedNodeId);
         }
 
-        const index = layoutShiftEvents.indexOf(event);
-        const shiftRootCauses = rootCauses.layoutShifts[index];
-        if (shiftRootCauses) {
-          for (const cause of shiftRootCauses.unsizedMedia) {
-            nodeIds.push(cause.node.backendNodeId);
-          }
-        }
-
         return nodeIds.map(nodeId => ({nodeId}));
       });
   }