core(largest-contentful-paint-element): display LCP value (#15061)

GoogleChrome · Jun 12, 2023 · 1bae933 · 1bae933
1 parent 333a667
commit 1bae933
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 78 deletions.
diff --git a/cli/test/smokehouse/test-definitions/dobetterweb.js b/cli/test/smokehouse/test-definitions/dobetterweb.js
@@ -599,7 +599,7 @@ const expectations = {
       },
       'largest-contentful-paint-element': {
         score: null,
-        displayValue: '1 element found',
+        displayValue: /\d+\xa0ms/,
         details: {
           items: [
             {

diff --git a/cli/test/smokehouse/test-definitions/perf-trace-elements.js b/cli/test/smokehouse/test-definitions/perf-trace-elements.js
@@ -144,7 +144,7 @@ const expectations = {
     audits: {
       'largest-contentful-paint-element': {
         score: null,
-        displayValue: '1 element found',
+        displayValue: /\d+\xa0ms/,
         details: {
           items: {
             0: {

diff --git a/core/audits/largest-contentful-paint-element.js b/core/audits/largest-contentful-paint-element.js
@@ -50,19 +50,44 @@ class LargestContentfulPaintElement extends Audit {
   }
 
   /**
-   * @param {LH.Artifacts} artifacts
+   * @param {LH.Artifacts.MetricComputationDataInput} metricComputationData
    * @param {LH.Audit.Context} context
-   * @return {Promise<LH.Audit.Details.Table|undefined>}
+   * @return {Promise<number|undefined>}
    */
-  static async makePhaseTable(artifacts, context) {
-    const trace = artifacts.traces[Audit.DEFAULT_PASS];
-    const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
-    const gatherContext = artifacts.GatherContext;
-    const metricComputationData = {trace, devtoolsLog, gatherContext,
-      settings: context.settings, URL: artifacts.URL};
+  static async getOptionalLCPMetric(metricComputationData, context) {
+    try {
+      const {timing: metricLcp} =
+        await LargestContentfulPaint.request(metricComputationData, context);
+      return metricLcp;
+    } catch {}
+  }
+
+  /**
+   * @param {LH.Artifacts} artifacts
+   * @return {LH.Audit.Details.Table|undefined}
+   */
+  static makeElementTable(artifacts) {
+    const lcpElement = artifacts.TraceElements
+      .find(element => element.traceEventType === 'largest-contentful-paint');
+    if (!lcpElement) return;
+
+    /** @type {LH.Audit.Details.Table['headings']} */
+    const headings = [
+      {key: 'node', valueType: 'node', label: str_(i18n.UIStrings.columnElement)},
+    ];
 
-    const {timing: metricLcp} =
-      await LargestContentfulPaint.request(metricComputationData, context);
+    const lcpElementDetails = [{node: Audit.makeNodeItem(lcpElement.node)}];
+
+    return Audit.makeTableDetails(headings, lcpElementDetails);
+  }
+
+  /**
+   * @param {number} metricLcp
+   * @param {LH.Artifacts.MetricComputationDataInput} metricComputationData
+   * @param {LH.Audit.Context} context
+   * @return {Promise<LH.Audit.Details.Table>}
+   */
+  static async makePhaseTable(metricLcp, metricComputationData, context) {
     const {ttfb, loadStart, loadEnd} = await LCPBreakdown.request(metricComputationData, context);
 
     let loadDelay = 0;
@@ -102,36 +127,29 @@ class LargestContentfulPaintElement extends Audit {
    * @return {Promise<LH.Audit.Product>}
    */
   static async audit(artifacts, context) {
-    const lcpElement = artifacts.TraceElements
-      .find(element => element.traceEventType === 'largest-contentful-paint');
-    const lcpElementDetails = [];
-    if (lcpElement) {
-      lcpElementDetails.push({
-        node: Audit.makeNodeItem(lcpElement.node),
-      });
-    }
-
-    /** @type {LH.Audit.Details.Table['headings']} */
-    const headings = [
-      {key: 'node', valueType: 'node', label: str_(i18n.UIStrings.columnElement)},
-    ];
+    const trace = artifacts.traces[Audit.DEFAULT_PASS];
+    const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
+    const gatherContext = artifacts.GatherContext;
+    const metricComputationData = {trace, devtoolsLog, gatherContext,
+      settings: context.settings, URL: artifacts.URL};
 
-    const elementTable = Audit.makeTableDetails(headings, lcpElementDetails);
+    const elementTable = this.makeElementTable(artifacts);
+    if (!elementTable) return {score: null, notApplicable: true};
 
     const items = [elementTable];
-    if (elementTable.items.length) {
-      const phaseTable = await this.makePhaseTable(artifacts, context);
-      if (phaseTable) items.push(phaseTable);
+    let displayValue;
+
+    const metricLcp = await this.getOptionalLCPMetric(metricComputationData, context);
+    if (metricLcp) {
+      displayValue = str_(i18n.UIStrings.ms, {timeInMs: metricLcp});
+      const phaseTable = await this.makePhaseTable(metricLcp, metricComputationData, context);
+      items.push(phaseTable);
     }
 
     const details = Audit.makeListDetails(items);
 
-    const displayValue = str_(i18n.UIStrings.displayValueElementsFound,
-      {nodeCount: lcpElementDetails.length});
-
     return {
       score: 1,
-      notApplicable: lcpElementDetails.length === 0,
       displayValue,
       details,
     };

diff --git a/core/test/audits/largest-contentful-paint-element-test.js b/core/test/audits/largest-contentful-paint-element-test.js
@@ -101,8 +101,8 @@ describe('Performance: largest-contentful-paint-element audit', () => {
     const auditResult = await LargestContentfulPaintElementAudit.audit(artifacts, context);
 
     expect(auditResult.score).toEqual(1);
-    expect(auditResult.notApplicable).toEqual(false);
-    expect(auditResult.displayValue).toBeDisplayString('1 element found');
+    expect(auditResult.notApplicable).toBeUndefined();
+    expect(auditResult.displayValue).toBeDisplayString('5,800\xa0ms');
     expect(auditResult.details.items).toHaveLength(2);
     expect(auditResult.details.items[0].items).toHaveLength(1);
     expect(auditResult.details.items[0].items[0].node.path).toEqual('1,HTML,3,BODY,5,DIV,0,HEADER');
@@ -145,10 +145,9 @@ describe('Performance: largest-contentful-paint-element audit', () => {
     const context = {settings: artifacts.settings, computedCache: new Map()};
     const auditResult = await LargestContentfulPaintElementAudit.audit(artifacts, context);
 
-    expect(auditResult.score).toEqual(1);
+    expect(auditResult.score).toEqual(null);
     expect(auditResult.notApplicable).toEqual(true);
-    expect(auditResult.displayValue).toBeDisplayString('0 elements found');
-    expect(auditResult.details.items).toHaveLength(1);
-    expect(auditResult.details.items[0].items).toHaveLength(0);
+    expect(auditResult.displayValue).toBeUndefined();
+    expect(auditResult.details).toBeUndefined();
   });
 });
diff --git a/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json b/core/test/fixtures/fraggle-rock/reports/sample-flow-result.json
@@ -1541,7 +1541,7 @@
             "description": "This is the largest contentful element painted within the viewport. [Learn more about the Largest Contentful Paint element](https://developer.chrome.com/docs/lighthouse/performance/lighthouse-largest-contentful-paint/)",
             "score": null,
             "scoreDisplayMode": "informative",
-            "displayValue": "1 element found",
+            "displayValue": "1,350 ms",
             "details": {
               "type": "list",
               "items": [
@@ -6700,6 +6700,12 @@
                   "timeInMs": 22.397999999999982
                 },
                 "path": "audits[network-server-latency].displayValue"
+              },
+              {
+                "values": {
+                  "timeInMs": 1352.107
+                },
+                "path": "audits[largest-contentful-paint-element].displayValue"
               }
             ],
             "core/lib/i18n/i18n.js | maxPotentialFIDMetric": [
@@ -7016,20 +7022,6 @@
             "core/audits/largest-contentful-paint-element.js | description": [
               "audits[largest-contentful-paint-element].description"
             ],
-            "core/lib/i18n/i18n.js | displayValueElementsFound": [
-              {
-                "values": {
-                  "nodeCount": 1
-                },
-                "path": "audits[largest-contentful-paint-element].displayValue"
-              },
-              {
-                "values": {
-                  "nodeCount": 5
-                },
-                "path": "audits[layout-shift-elements].displayValue"
-              }
-            ],
             "core/lib/i18n/i18n.js | columnElement": [
               "audits[largest-contentful-paint-element].details.items[0].headings[0].label",
               "audits[lcp-lazy-loaded].details.headings[0].label",
@@ -7069,6 +7061,14 @@
             "core/audits/layout-shift-elements.js | description": [
               "audits[layout-shift-elements].description"
             ],
+            "core/lib/i18n/i18n.js | displayValueElementsFound": [
+              {
+                "values": {
+                  "nodeCount": 5
+                },
+                "path": "audits[layout-shift-elements].displayValue"
+              }
+            ],
             "core/audits/layout-shift-elements.js | columnContribution": [
               "audits[layout-shift-elements].details.headings[1].label"
             ],
@@ -18213,7 +18213,7 @@
             "description": "This is the largest contentful element painted within the viewport. [Learn more about the Largest Contentful Paint element](https://developer.chrome.com/docs/lighthouse/performance/lighthouse-largest-contentful-paint/)",
             "score": null,
             "scoreDisplayMode": "informative",
-            "displayValue": "1 element found",
+            "displayValue": "1,800 ms",
             "details": {
               "type": "list",
               "items": [
@@ -23415,6 +23415,12 @@
                   "timeInMs": 3.5377
                 },
                 "path": "audits[network-server-latency].displayValue"
+              },
+              {
+                "values": {
+                  "timeInMs": 1803.0092
+                },
+                "path": "audits[largest-contentful-paint-element].displayValue"
               }
             ],
             "core/lib/i18n/i18n.js | maxPotentialFIDMetric": [
@@ -23733,14 +23739,6 @@
             "core/audits/largest-contentful-paint-element.js | description": [
               "audits[largest-contentful-paint-element].description"
             ],
-            "core/lib/i18n/i18n.js | displayValueElementsFound": [
-              {
-                "values": {
-                  "nodeCount": 1
-                },
-                "path": "audits[largest-contentful-paint-element].displayValue"
-              }
-            ],
             "core/lib/i18n/i18n.js | columnElement": [
               "audits[largest-contentful-paint-element].details.items[0].headings[0].label",
               "audits[lcp-lazy-loaded].details.headings[0].label",

diff --git a/core/test/results/sample_v2.json b/core/test/results/sample_v2.json
@@ -2309,7 +2309,7 @@
       "description": "This is the largest contentful element painted within the viewport. [Learn more about the Largest Contentful Paint element](https://developer.chrome.com/docs/lighthouse/performance/lighthouse-largest-contentful-paint/)",
       "score": null,
       "scoreDisplayMode": "informative",
-      "displayValue": "1 element found",
+      "displayValue": "13,320 ms",
       "details": {
         "type": "list",
         "items": [
@@ -9009,6 +9009,12 @@
             "timeInMs": 582.2760000000001
           },
           "path": "audits[network-server-latency].displayValue"
+        },
+        {
+          "values": {
+            "timeInMs": 13319.961
+          },
+          "path": "audits[largest-contentful-paint-element].displayValue"
         }
       ],
       "core/lib/i18n/i18n.js | maxPotentialFIDMetric": [
@@ -9429,20 +9435,6 @@
       "core/audits/largest-contentful-paint-element.js | description": [
         "audits[largest-contentful-paint-element].description"
       ],
-      "core/lib/i18n/i18n.js | displayValueElementsFound": [
-        {
-          "values": {
-            "nodeCount": 1
-          },
-          "path": "audits[largest-contentful-paint-element].displayValue"
-        },
-        {
-          "values": {
-            "nodeCount": 5
-          },
-          "path": "audits[layout-shift-elements].displayValue"
-        }
-      ],
       "core/lib/i18n/i18n.js | columnElement": [
         "audits[largest-contentful-paint-element].details.items[0].headings[0].label",
         "audits[layout-shift-elements].details.headings[0].label",
@@ -9482,6 +9474,14 @@
       "core/audits/layout-shift-elements.js | description": [
         "audits[layout-shift-elements].description"
       ],
+      "core/lib/i18n/i18n.js | displayValueElementsFound": [
+        {
+          "values": {
+            "nodeCount": 5
+          },
+          "path": "audits[layout-shift-elements].displayValue"
+        }
+      ],
       "core/audits/layout-shift-elements.js | columnContribution": [
         "audits[layout-shift-elements].details.headings[1].label"
       ],