core: generalize TraceElements gatherer and add animated elements (#1…

…1138) * Add animation events to trace elements * Add trace elements to smoke test * Update smoke test to trace-elements * nits * lint * rename metric name * Fix smoke test * remove node id * lint * Add node test * nit * newline * fix tests * update sample * nits * old code cleanup * height too * set height manually * fix sample * Fix flaky score * switch to layout-shift * Correct element * switch to div * nit
GoogleChrome · Jul 27, 2020 · 43042e1 · 43042e1
1 parent d8d8caa
commit 43042e1
Show file tree

Hide file tree

Showing 11 changed files with 322 additions and 68 deletions.
diff --git a/lighthouse-cli/test/fixtures/perf/delayed-element.js b/lighthouse-cli/test/fixtures/perf/delayed-element.js
@@ -28,8 +28,9 @@ async function rerender(iterations) {
 setTimeout(() => {
   const imgEl = document.createElement('img');
   imgEl.src = '../dobetterweb/lighthouse-480x318.jpg';
-  const textEl = document.createElement('span');
+  const textEl = document.createElement('div');
   textEl.textContent = 'Sorry!';
+  textEl.style.height = '18px' // this height can be flaky so we set it manually
   const top = document.getElementById('late-content');
   top.appendChild(imgEl);
   top.appendChild(textEl);

diff --git a/lighthouse-cli/test/fixtures/perf/trace-elements.html b/lighthouse-cli/test/fixtures/perf/trace-elements.html
@@ -1,6 +1,21 @@
 <html>
 <meta name="viewport" content="width=device-width, initial-scale=1, minimum-scale=1">
+<style>
+ #animate-me {
+   width: 100px;
+   height: 100px;
+   background-color: red;
+   animation-name: anim;
+   animation-duration: 1s;
+ }
+
+ @keyframes anim {
+   from {background-color: red;}
+   from {background-color: blue;}
+ }
+</style>
 <body>
+  <div id="animate-me"></div>
   <div id="late-content"></div>
   <h1>Please don't move me</h1>
   <script src="delayed-element.js"></script>

diff --git a/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/perf/expectations.js
@@ -156,6 +156,68 @@ module.exports = [
     },
   },
   {
+    artifacts: {
+      TraceElements: [
+        {
+          traceEventType: 'largest-contentful-paint',
+          selector: 'body > div#late-content > img',
+          nodeLabel: 'img',
+          snippet: '<img src="../dobetterweb/lighthouse-480x318.jpg">',
+          boundingRect: {
+            top: 108,
+            bottom: 426,
+            left: 8,
+            right: 488,
+            width: 480,
+            height: 318,
+          },
+        },
+        {
+          traceEventType: 'layout-shift',
+          selector: 'body > h1',
+          nodeLabel: 'Please don\'t move me',
+          snippet: '<h1>',
+          boundingRect: {
+            top: 465,
+            bottom: 502,
+            left: 8,
+            right: 352,
+            width: 344,
+            height: 37,
+          },
+          score: '0.058 +/- 0.01',
+        },
+        {
+          traceEventType: 'layout-shift',
+          selector: 'body > div#late-content > div',
+          nodeLabel: 'Sorry!',
+          snippet: '<div style="height: 18px;">',
+          boundingRect: {
+            top: 426,
+            bottom: 444,
+            left: 8,
+            right: 352,
+            width: 344,
+            height: 18,
+          },
+          score: '0.026 +/- 0.01',
+        },
+        {
+          traceEventType: 'animation',
+          selector: 'body > div#animate-me',
+          nodeLabel: 'div',
+          snippet: '<div id="animate-me">',
+          boundingRect: {
+            top: 8,
+            bottom: 108,
+            left: 8,
+            right: 108,
+            width: 100,
+            height: 100,
+          },
+        },
+      ],
+    },
     lhr: {
       requestedUrl: 'http://localhost:10200/perf/trace-elements.html',
       finalUrl: 'http://localhost:10200/perf/trace-elements.html',

diff --git a/lighthouse-core/audits/largest-contentful-paint-element.js b/lighthouse-core/audits/largest-contentful-paint-element.js
@@ -42,8 +42,8 @@ class LargestContentfulPaintElement extends Audit {
    * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
-    const lcpElement =
-      artifacts.TraceElements.find(element => element.metricName === 'largest-contentful-paint');
+    const lcpElement = artifacts.TraceElements
+      .find(element => element.traceEventType === 'largest-contentful-paint');
     const lcpElementDetails = [];
     if (lcpElement) {
       lcpElementDetails.push({

diff --git a/lighthouse-core/audits/layout-shift-elements.js b/lighthouse-core/audits/layout-shift-elements.js
@@ -43,8 +43,8 @@ class LayoutShiftElements extends Audit {
    * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
-    const clsElements =
-      artifacts.TraceElements.filter(element => element.metricName === 'cumulative-layout-shift');
+    const clsElements = artifacts.TraceElements
+      .filter(element => element.traceEventType === 'layout-shift');
 
     const clsElementData = clsElements.map(element => {
       return {

diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js
@@ -20,16 +20,13 @@ const RectHelpers = require('../../lib/rect-helpers.js');
 
 /**
  * @this {HTMLElement}
- * @param {string} metricName
- * @return {LH.Artifacts.TraceElement | undefined}
  */
 /* istanbul ignore next */
-function setAttributeMarker(metricName) {
+function getNodeDetailsData() {
   const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; // eslint-disable-line no-undef
   let traceElement;
   if (elem) {
     traceElement = {
-      metricName,
       // @ts-ignore - put into scope via stringification
       devtoolsNodePath: getNodePath(elem), // eslint-disable-line no-undef
       // @ts-ignore - put into scope via stringification
@@ -137,6 +134,24 @@ class TraceElements extends Gatherer {
     return topFive;
   }
 
+  /**
+   * Find the node ids of elements which are animated using the Animation trace events.
+   * @param {Array<LH.TraceEvent>} mainThreadEvents
+   * @return {Array<TraceElementData>}
+   */
+  static getAnimatedElements(mainThreadEvents) {
+    const animatedElementIds = new Set(mainThreadEvents
+      .filter(e => e.name === 'Animation' && e.ph === 'b')
+      .map(e => this.getNodeIDFromTraceEvent(e)));
+
+    /** @type Array<TraceElementData> */
+    const animatedElementData = [];
+    for (const nodeId of animatedElementIds) {
+      nodeId && animatedElementData.push({nodeId});
+    }
+    return animatedElementData;
+  }
+
   /**
    * @param {LH.Gatherer.PassContext} passContext
    * @param {LH.Gatherer.LoadData} loadData
@@ -150,40 +165,47 @@ class TraceElements extends Gatherer {
 
     const {largestContentfulPaintEvt, mainThreadEvents} =
       TraceProcessor.computeTraceOfTab(loadData.trace);
-    /** @type {Array<TraceElementData>} */
-    const backendNodeData = [];
 
     const lcpNodeId = TraceElements.getNodeIDFromTraceEvent(largestContentfulPaintEvt);
     const clsNodeData = TraceElements.getTopLayoutShiftElements(mainThreadEvents);
-    if (lcpNodeId) {
-      backendNodeData.push({nodeId: lcpNodeId});
-    }
-    backendNodeData.push(...clsNodeData);
+    const animatedElementData = TraceElements.getAnimatedElements(mainThreadEvents);
 
-    const traceElements = [];
-    for (let i = 0; i < backendNodeData.length; i++) {
-      const backendNodeId = backendNodeData[i].nodeId;
-      const metricName =
-        lcpNodeId === backendNodeId ? 'largest-contentful-paint' : 'cumulative-layout-shift';
-      const objectId = await driver.resolveNodeIdToObjectId(backendNodeId);
-      if (!objectId) continue;
-      const response = await driver.sendCommand('Runtime.callFunctionOn', {
-        objectId,
-        functionDeclaration: `function () {
-          ${setAttributeMarker.toString()};
-          ${pageFunctions.getNodePathString};
-          ${pageFunctions.getNodeSelectorString};
-          ${pageFunctions.getNodeLabelString};
-          ${pageFunctions.getOuterHTMLSnippetString};
-          ${pageFunctions.getBoundingClientRectString};
-          return setAttributeMarker.call(this, '${metricName}');
-        }`,
-        returnByValue: true,
-        awaitPromise: true,
-      });
+    /** @type Map<string, {nodeId: number, score?: number}[]> */
+    const backendNodeDataMap = new Map([
+      ['largest-contentful-paint', lcpNodeId ? [{nodeId: lcpNodeId}] : []],
+      ['layout-shift', clsNodeData],
+      ['animation', animatedElementData],
+    ]);
 
-      if (response && response.result && response.result.value) {
-        traceElements.push({...response.result.value, score: backendNodeData[i].score});
+    const traceElements = [];
+    for (const [traceEventType, backendNodeData] of backendNodeDataMap) {
+      for (let i = 0; i < backendNodeData.length; i++) {
+        const backendNodeId = backendNodeData[i].nodeId;
+        const objectId = await driver.resolveNodeIdToObjectId(backendNodeId);
+        if (!objectId) continue;
+        const response = await driver.sendCommand('Runtime.callFunctionOn', {
+          objectId,
+          functionDeclaration: `function () {
+            ${getNodeDetailsData.toString()};
+            ${pageFunctions.getNodePathString};
+            ${pageFunctions.getNodeSelectorString};
+            ${pageFunctions.getNodeLabelString};
+            ${pageFunctions.getOuterHTMLSnippetString};
+            ${pageFunctions.getBoundingClientRectString};
+            return getNodeDetailsData.call(this);
+          }`,
+          returnByValue: true,
+          awaitPromise: true,
+        });
+
+        if (response && response.result && response.result.value) {
+          traceElements.push({
+            traceEventType,
+            ...response.result.value,
+            score: backendNodeData[i].score,
+            nodeId: backendNodeId,
+          });
+        }
       }
     }
 

diff --git a/lighthouse-core/test/audits/largest-contentful-paint-element-test.js b/lighthouse-core/test/audits/largest-contentful-paint-element-test.js
@@ -13,7 +13,7 @@ describe('Performance: largest-contentful-paint-element audit', () => {
   it('correctly surfaces the LCP element', async () => {
     const artifacts = {
       TraceElements: [{
-        metricName: 'largest-contentful-paint',
+        traceEventType: 'largest-contentful-paint',
         devtoolsNodePath: '1,HTML,3,BODY,5,DIV,0,HEADER',
         selector: 'div.l-header > div.chorus-emc__content',
         nodeLabel: 'My Test Label',

diff --git a/lighthouse-core/test/audits/layout-shift-elements-test.js b/lighthouse-core/test/audits/layout-shift-elements-test.js
@@ -14,7 +14,7 @@ describe('Performance: layout-shift-elements audit', () => {
   it('correctly surfaces a single CLS element', async () => {
     const artifacts = {
       TraceElements: [{
-        metricName: 'cumulative-layout-shift',
+        traceEventType: 'layout-shift',
         devtoolsNodePath: '1,HTML,3,BODY,5,DIV,0,HEADER',
         selector: 'div.l-header > div.chorus-emc__content',
         nodeLabel: 'My Test Label',
@@ -34,7 +34,7 @@ describe('Performance: layout-shift-elements audit', () => {
 
   it('correctly surfaces multiple CLS elements', async () => {
     const clsElement = {
-      metricName: 'cumulative-layout-shift',
+      traceEventType: 'layout-shift',
       devtoolsNodePath: '1,HTML,3,BODY,5,DIV,0,HEADER',
       selector: 'div.l-header > div.chorus-emc__content',
       nodeLabel: 'My Test Label',