core(preload-lcp-image): properly calculate the potential savings (#1…

…1612)

lighthouse-cli/test/cli/__snapshots__/index-test.js.snap

@@ -174,6 +174,9 @@ Object {
     Object {
       "path": "valid-source-maps",
     },
+    Object {
+      "path": "preload-lcp-image",
+    },
     Object {
       "path": "manual/pwa-cross-browser",
     },
@@ -938,6 +941,11 @@ Object {
           "id": "legacy-javascript",
           "weight": 0,
         },
+        Object {
+          "group": "load-opportunities",
+          "id": "preload-lcp-image",
+          "weight": 0,
+        },
         Object {
           "group": "diagnostics",
           "id": "total-byte-weight",

lighthouse-core/audits/preload-lcp-image.js

@@ -104,16 +104,83 @@ class PreloadLCPImageAudit extends Audit {
 
   /**
    * Computes the estimated effect of preloading the LCP image.
-   * @param {LH.Gatherer.Simulation.GraphNetworkNode} lcpNode
+   * @param {LH.Gatherer.Simulation.GraphNetworkNode|undefined} lcpNode
    * @param {LH.Gatherer.Simulation.GraphNode} graph
    * @param {LH.Gatherer.Simulation.Simulator} simulator
    * @return {{wastedMs: number, results: Array<{url: string, wastedMs: number}>}}
    */
   static computeWasteWithGraph(lcpNode, graph, simulator) {
-    const simulation = simulator.simulate(graph, {flexibleOrdering: true});
-    // For now, we are simply using the duration of the LCP image request as the wastedMS value
-    const lcpTiming = simulation.nodeTimings.get(lcpNode);
-    const wastedMs = lcpTiming && lcpTiming.duration || 0;
+    if (!lcpNode) {
+      return {
+        wastedMs: 0,
+        results: [],
+      };
+    }
+
+    const modifiedGraph = graph.cloneWithRelationships();
+
+    // Store the IDs of the LCP Node's dependencies for later
+    /** @type {Set<string>} */
+    const dependenciesIds = new Set();
+    for (const node of lcpNode.getDependencies()) {
+      dependenciesIds.add(node.id);
+    }
+
+    /** @type {LH.Gatherer.Simulation.GraphNode|null} */
+    let modifiedLCPNode = null;
+    /** @type {LH.Gatherer.Simulation.GraphNode|null} */
+    let mainDocumentNode = null;
+
+    for (const {node} of modifiedGraph.traverseGenerator()) {
+      if (node.type !== 'network') continue;
+
+      const networkNode = /** @type {LH.Gatherer.Simulation.GraphNetworkNode} */ (node);
+      if (node.isMainDocument()) {
+        mainDocumentNode = networkNode;
+      } else if (networkNode.id === lcpNode.id) {
+        modifiedLCPNode = networkNode;
+      }
+    }
+
+    if (!mainDocumentNode) {
+      // Should always find the main document node
+      throw new Error('Could not find main document node');
+    }
+
+    if (!modifiedLCPNode) {
+      // Should always find the LCP node as well or else this function wouldn't have been called
+      throw new Error('Could not find the LCP node');
+    }
+
+    // Preload will request the resource as soon as its discovered in the main document.
+    // Reflect this change in the dependencies in our modified graph.
+    modifiedLCPNode.removeAllDependencies();
+    modifiedLCPNode.addDependency(mainDocumentNode);
+
+    const simulationBeforeChanges = simulator.simulate(graph, {flexibleOrdering: true});
+    const simulationAfterChanges = simulator.simulate(modifiedGraph, {flexibleOrdering: true});
+    const lcpTimingsBefore = simulationBeforeChanges.nodeTimings.get(lcpNode);
+    if (!lcpTimingsBefore) throw new Error('Impossible - node timings should never be undefined');
+    const lcpTimingsAfter = simulationAfterChanges.nodeTimings.get(modifiedLCPNode);
+    if (!lcpTimingsAfter) throw new Error('Impossible - node timings should never be undefined');
+    /** @type {Map<String, LH.Gatherer.Simulation.GraphNode>} */
+    const modifiedNodesById = Array.from(simulationAfterChanges.nodeTimings.keys())
+      .reduce((map, node) => map.set(node.id, node), new Map());
+
+    // Even with preload, the image can't be painted before it's even inserted into the DOM.
+    // New LCP time will be the max of image download and image in DOM (endTime of its deps).
+    let maxDependencyEndTime = 0;
+    for (const nodeId of Array.from(dependenciesIds)) {
+      const node = modifiedNodesById.get(nodeId);
+      if (!node) throw new Error('Impossible - node should never be undefined');
+      const timings = simulationAfterChanges.nodeTimings.get(node);
+      const endTime = timings && timings.endTime || 0;
+      maxDependencyEndTime = Math.max(maxDependencyEndTime, endTime);
+    }
+
+    const wastedMs = lcpTimingsBefore.endTime -
+      Math.max(lcpTimingsAfter.endTime, maxDependencyEndTime);
+
     return {
       wastedMs,
       results: [{
@@ -145,18 +212,13 @@ class PreloadLCPImageAudit extends Audit {
     const graph = lanternLCP.pessimisticGraph;
     // eslint-disable-next-line max-len
     const lcpNode = PreloadLCPImageAudit.getLCPNodeToPreload(mainResource, graph, lcpElement, artifacts.ImageElements);
-    if (!lcpNode) {
-      return {
-        score: 1,
-        notApplicable: true,
-      };
-    }
 
     const {results, wastedMs} =
       PreloadLCPImageAudit.computeWasteWithGraph(lcpNode, graph, simulator);
 
     /** @type {LH.Audit.Details.Opportunity['headings']} */
     const headings = [
+      {key: 'url', valueType: 'thumbnail', label: ''},
       {key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
       {key: 'wastedMs', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnWastedMs)},
     ];

lighthouse-core/config/default-config.js

@@ -243,6 +243,7 @@ const defaultConfig = {
     'non-composited-animations',
     'unsized-images',
     'valid-source-maps',
+    'preload-lcp-image',
     'manual/pwa-cross-browser',
     'manual/pwa-page-transitions',
     'manual/pwa-each-page-has-url',
@@ -455,6 +456,7 @@ const defaultConfig = {
         {id: 'efficient-animated-content', weight: 0, group: 'load-opportunities'},
         {id: 'duplicated-javascript', weight: 0, group: 'load-opportunities'},
         {id: 'legacy-javascript', weight: 0, group: 'load-opportunities'},
+        {id: 'preload-lcp-image', weight: 0, group: 'load-opportunities'},
         {id: 'total-byte-weight', weight: 0, group: 'diagnostics'},
         {id: 'uses-long-cache-ttl', weight: 0, group: 'diagnostics'},
         {id: 'dom-size', weight: 0, group: 'diagnostics'},

lighthouse-core/config/experimental-config.js

@@ -18,7 +18,6 @@ const config = {
     'full-page-screenshot',
     'large-javascript-libraries',
     'script-treemap-data',
-    'preload-lcp-image',
   ],
   passes: [{
     passName: 'defaultPass',
@@ -32,7 +31,6 @@ const config = {
     'performance': {
       auditRefs: [
         {id: 'large-javascript-libraries', weight: 0, group: 'diagnostics'},
-        {id: 'preload-lcp-image', weight: 0, group: 'load-opportunities'},
       ],
     },
     // @ts-ignore: `title` is required in CategoryJson. setting to the same value as the default

lighthouse-core/test/audits/preload-lcp-image-test.js

@@ -89,26 +89,15 @@ describe('Performance: preload-lcp audit', () => {
     ];
   };
 
-  it('should suggest preloading a lcp image', async () => {
-    const networkRecords = mockNetworkRecords();
-    const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl);
-    const context = {settings: {}, computedCache: new Map()};
-    const results = await PreloadLCPImage.audit(artifacts, context);
-    expect(results.numericValue).toEqual(180);
-    expect(results.details.overallSavingsMs).toEqual(180);
-    expect(results.details.items[0].url).toEqual(imageUrl);
-    expect(results.details.items[0].wastedMs).toEqual(180);
-  });
-
   it('shouldn\'t be applicable if lcp image is not found', async () => {
     const networkRecords = mockNetworkRecords();
     const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl);
     artifacts.ImageElements = [];
     const context = {settings: {}, computedCache: new Map()};
     const results = await PreloadLCPImage.audit(artifacts, context);
     expect(results.score).toEqual(1);
-    expect(results.notApplicable).toBeTruthy();
-    expect(results.details).toBeUndefined();
+    expect(results.details.overallSavingsMs).toEqual(0);
+    expect(results.details.items).toHaveLength(0);
   });
 
   it('shouldn\'t be applicable if the lcp is already preloaded', async () => {
@@ -118,8 +107,8 @@ describe('Performance: preload-lcp audit', () => {
     const context = {settings: {}, computedCache: new Map()};
     const results = await PreloadLCPImage.audit(artifacts, context);
     expect(results.score).toEqual(1);
-    expect(results.notApplicable).toBeTruthy();
-    expect(results.details).toBeUndefined();
+    expect(results.details.overallSavingsMs).toEqual(0);
+    expect(results.details.items).toHaveLength(0);
   });
 
   it('shouldn\'t be applicable if the lcp request is not from over the network', async () => {
@@ -129,7 +118,42 @@ describe('Performance: preload-lcp audit', () => {
     const context = {settings: {}, computedCache: new Map()};
     const results = await PreloadLCPImage.audit(artifacts, context);
     expect(results.score).toEqual(1);
-    expect(results.notApplicable).toBeTruthy();
-    expect(results.details).toBeUndefined();
+    expect(results.details.overallSavingsMs).toEqual(0);
+    expect(results.details.items).toHaveLength(0);
+  });
+
+  it('should suggest preloading a lcp image if all criteria is met', async () => {
+    const networkRecords = mockNetworkRecords();
+    const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl);
+    const context = {settings: {}, computedCache: new Map()};
+    const results = await PreloadLCPImage.audit(artifacts, context);
+    expect(results.numericValue).toEqual(180);
+    expect(results.details.overallSavingsMs).toEqual(180);
+    expect(results.details.items[0].url).toEqual(imageUrl);
+    expect(results.details.items[0].wastedMs).toEqual(180);
+  });
+
+  it('should suggest preloading when LCP is waiting on the image', async () => {
+    const networkRecords = mockNetworkRecords();
+    networkRecords[3].transferSize = 5 * 1000 * 1000;
+    const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl);
+    const context = {settings: {}, computedCache: new Map()};
+    const results = await PreloadLCPImage.audit(artifacts, context);
+    expect(results.numericValue).toEqual(30);
+    expect(results.details.overallSavingsMs).toEqual(30);
+    expect(results.details.items[0].url).toEqual(imageUrl);
+    expect(results.details.items[0].wastedMs).toEqual(30);
+  });
+
+  it('should suggest preloading when LCP is waiting on a dependency', async () => {
+    const networkRecords = mockNetworkRecords();
+    networkRecords[2].transferSize = 2 * 1000 * 1000;
+    const artifacts = mockArtifacts(networkRecords, mainDocumentNodeUrl, imageUrl);
+    const context = {settings: {}, computedCache: new Map()};
+    const results = await PreloadLCPImage.audit(artifacts, context);
+    expect(results.numericValue).toEqual(30);
+    expect(results.details.overallSavingsMs).toEqual(30);
+    expect(results.details.items[0].url).toEqual(imageUrl);
+    expect(results.details.items[0].wastedMs).toEqual(30);
   });
 });

lighthouse-core/test/results/sample_v2.json

@@ -2071,6 +2071,22 @@
         "items": []
       }
     },
+    "preload-lcp-image": {
+      "id": "preload-lcp-image",
+      "title": "Preload Largest Contentful Paint image",
+      "description": "Preload the image used by the LCP element in order to improve your LCP time. [Learn more](https://web.dev/optimize-lcp/#preload-important-resources).",
+      "score": 1,
+      "scoreDisplayMode": "numeric",
+      "numericValue": 0,
+      "numericUnit": "millisecond",
+      "displayValue": "",
+      "details": {
+        "type": "opportunity",
+        "headings": [],
+        "items": [],
+        "overallSavingsMs": 0
+      }
+    },
     "pwa-cross-browser": {
       "id": "pwa-cross-browser",
       "title": "Site works cross-browser",
@@ -4572,6 +4588,11 @@
           "weight": 0,
           "group": "load-opportunities"
         },
+        {
+          "id": "preload-lcp-image",
+          "weight": 0,
+          "group": "load-opportunities"
+        },
         {
           "id": "total-byte-weight",
           "weight": 0,
@@ -5872,6 +5893,24 @@
         "duration": 100,
         "entryType": "measure"
       },
+      {
+        "startTime": 0,
+        "name": "lh:audit:preload-lcp-image",
+        "duration": 100,
+        "entryType": "measure"
+      },
+      {
+        "startTime": 0,
+        "name": "lh:computed:LanternLargestContentfulPaint",
+        "duration": 100,
+        "entryType": "measure"
+      },
+      {
+        "startTime": 0,
+        "name": "lh:computed:LanternFirstContentfulPaint",
+        "duration": 100,
+        "entryType": "measure"
+      },
       {
         "startTime": 0,
         "name": "lh:audit:pwa-cross-browser",
@@ -7224,6 +7263,12 @@
       "lighthouse-core/audits/valid-source-maps.js | description": [
         "audits[valid-source-maps].description"
       ],
+      "lighthouse-core/audits/preload-lcp-image.js | title": [
+        "audits[preload-lcp-image].title"
+      ],
+      "lighthouse-core/audits/preload-lcp-image.js | description": [
+        "audits[preload-lcp-image].description"
+      ],
       "lighthouse-core/audits/manual/pwa-cross-browser.js | title": [
         "audits[pwa-cross-browser].title"
       ],

third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-export-run-expected.txt

@@ -2,11 +2,11 @@ Tests that exporting works.
 
 ++++++++ testExportHtml
 
-# of .lh-audit divs (original): 135
+# of .lh-audit divs (original): 136
 
-# of .lh-audit divs (exported html): 135
+# of .lh-audit divs (exported html): 136
 
 ++++++++ testExportJson
 
-# of audits (json): 151
+# of audits (json): 152
 

third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-successful-run-expected.txt

@@ -248,6 +248,9 @@ Auditing: Avoids `unload` event listeners
 Auditing: Avoid non-composited animations
 Auditing: Image elements have explicit `width` and `height`
 Auditing: Page has valid source maps
+Auditing: Preload Largest Contentful Paint image
+Computing artifact: LanternLargestContentfulPaint
+Computing artifact: LanternFirstContentfulPaint
 Auditing: Site works cross-browser
 Auditing: Page transitions don't feel like they block on the network
 Auditing: Each page has a URL
@@ -512,6 +515,7 @@ password-inputs-can-be-pasted-into: pass
 performance-budget: notApplicable
 plugins: pass
 preload-fonts: notApplicable
+preload-lcp-image: numeric
 pwa-cross-browser: manual
 pwa-each-page-has-url: manual
 pwa-page-transitions: manual
@@ -564,5 +568,5 @@ visual-order-follows-dom: manual
 without-javascript: pass
 works-offline: fail
 
-# of .lh-audit divs: 155
+# of .lh-audit divs: 156
 

third-party/chromium-webtests/webtests/http/tests/devtools/lighthouse/lighthouse-view-trace-run-expected.txt

@@ -43,6 +43,7 @@ no-document-write
 non-composited-animations
 offscreen-images
 performance-budget
+preload-lcp-image
 redirects
 render-blocking-resources
 resource-summary