core(image-elements): ignore invalid images where possible

core/audits/lcp-lazy-loaded.js

@@ -7,6 +7,8 @@
 
 import {Audit} from './audit.js';
 import * as i18n from '../lib/i18n/i18n.js';
+import ImageRecords from '../computed/image-records.js';
+import NetworkRecords from '../computed/network-records.js';
 
 const UIStrings = {
   /** Title of a Lighthouse audit that provides detail on whether the largest above-the-fold image was loaded with sufficient priority. This descriptive title is shown to users when the image was loaded properly. */
@@ -30,7 +32,7 @@ class LargestContentfulPaintLazyLoaded extends Audit {
       failureTitle: str_(UIStrings.failureTitle),
       description: str_(UIStrings.description),
       supportedModes: ['navigation'],
-      requiredArtifacts: ['TraceElements', 'ViewportDimensions', 'ImageElements'],
+      requiredArtifacts: ['TraceElements', 'ViewportDimensions', 'ImageElements', 'devtoolsLogs'],
     };
   }
 
@@ -47,16 +49,22 @@ class LargestContentfulPaintLazyLoaded extends Audit {
 
   /**
    * @param {LH.Artifacts} artifacts
-   * @return {LH.Audit.Product}
+   * @param {LH.Audit.Context} context
+   * @return {Promise<LH.Audit.Product>}
    */
-  static audit(artifacts) {
+  static async audit(artifacts, context) {
+    const devtoolsLog = artifacts.devtoolsLogs[LargestContentfulPaintLazyLoaded.DEFAULT_PASS];
+    const networkRecords = await NetworkRecords.request(devtoolsLog, context);
+    const images = await ImageRecords.request({
+      ImageElements: artifacts.ImageElements,
+      networkRecords,
+    }, context);
     const lcpElement = artifacts.TraceElements
       .find(element => element.traceEventType === 'largest-contentful-paint');
-    const lcpElementImage = lcpElement ? artifacts.ImageElements.find(elem => {
+    const lcpElementImage = lcpElement ? images.find(elem => {
       return elem.node.devtoolsNodePath === lcpElement.node.devtoolsNodePath;
     }) : undefined;
 
-
     if (!lcpElementImage ||
       !this.isImageInViewport(lcpElementImage, artifacts.ViewportDimensions)) {
       return {score: 1, notApplicable: true};

core/audits/preload-lcp-image.js

@@ -12,6 +12,8 @@ import MainResource from '../computed/main-resource.js';
 import LanternLCP from '../computed/metrics/lantern-largest-contentful-paint.js';
 import LoadSimulator from '../computed/load-simulator.js';
 import {ByteEfficiencyAudit} from './byte-efficiency/byte-efficiency-audit.js';
+import ImageRecords from '../computed/image-records.js';
+import NetworkRecords from '../computed/network-records.js';
 
 const UIStrings = {
   /** Title of a lighthouse audit that tells a user to preload an image in order to improve their LCP time. */
@@ -217,6 +219,11 @@ class PreloadLCPImageAudit extends Audit {
     const gatherContext = artifacts.GatherContext;
     const trace = artifacts.traces[PreloadLCPImageAudit.DEFAULT_PASS];
     const devtoolsLog = artifacts.devtoolsLogs[PreloadLCPImageAudit.DEFAULT_PASS];
+    const networkRecords = await NetworkRecords.request(devtoolsLog, context);
+    const images = await ImageRecords.request({
+      ImageElements: artifacts.ImageElements,
+      networkRecords,
+    }, context);
     const URL = artifacts.URL;
     const metricData = {trace, devtoolsLog, gatherContext, settings: context.settings, URL};
     const lcpElement = artifacts.TraceElements
@@ -230,7 +237,7 @@ class PreloadLCPImageAudit extends Audit {
 
     const graph = lanternLCP.pessimisticGraph;
     // eslint-disable-next-line max-len
-    const {lcpNodeToPreload, initiatorPath} = PreloadLCPImageAudit.getLCPNodeToPreload(mainResource, graph, lcpElement, artifacts.ImageElements);
+    const {lcpNodeToPreload, initiatorPath} = PreloadLCPImageAudit.getLCPNodeToPreload(mainResource, graph, lcpElement, images);
 
     const {results, wastedMs} =
       PreloadLCPImageAudit.computeWasteWithGraph(lcpElement, lcpNodeToPreload, graph, simulator);

core/computed/image-records.js

@@ -5,7 +5,6 @@
  */
 'use strict';
 
-import URL from '../lib/url-shim.js';
 import {makeComputedArtifact} from './computed-artifact.js';
 
 class ImageRecords {
@@ -39,12 +38,13 @@ class ImageRecords {
 
     for (const element of data.ImageElements) {
       const networkRecord = indexedNetworkRecords[element.src];
-      const mimeType = networkRecord?.mimeType;
+      // Ignore if we aren't sure this is really an image.
+      // TODO: we should add something in tracing to get this straight from blink.
+      if (!networkRecord) continue;
 
-      // Don't change the guessed mime type if no mime type was found.
       imageRecords.push({
         ...element,
-        mimeType: mimeType ? mimeType : URL.guessMimeType(element.src),
+        mimeType: networkRecord.mimeType,
       });
     }
 

core/test/audits/byte-efficiency/uses-responsive-images-test.js

@@ -278,6 +278,8 @@ describe('Page uses responsive images', () => {
       mimeType: 'image/png',
       resourceSize: 1024 * 100,
       transferSize: 0,
+      finished: true,
+      statusCode: 200,
       url: 'https://google.com/logo.png',
     };
     const auditResult = await UsesResponsiveImagesAudit.audit_({

core/test/audits/lcp-lazy-loaded-test.js

@@ -5,6 +5,8 @@
  */
 
 import LargestContentfulPaintLazyLoaded from '../../audits/lcp-lazy-loaded.js';
+import {createTestTrace} from '../create-test-trace.js';
+import {networkRecordsToDevtoolsLog} from '../network-records-to-devtools-log.js';
 
 const SAMPLE_NODE = {
   devtoolsNodePath: '1,HTML,1,BODY,3,DIV,2,IMG',
@@ -14,7 +16,7 @@ const SAMPLE_NODE = {
 };
 function generateImage(loading, clientRectTop) {
   return {
-    src: 'test',
+    src: 'http://example.com/test.png',
     loading,
     clientRect: {
       top: clientRectTop,
@@ -25,23 +27,41 @@ function generateImage(loading, clientRectTop) {
     node: SAMPLE_NODE,
   };
 }
+
+/**
+ * @param {ImageElement[]} imageElements
+ */
+function createArtifacts(imageElements) {
+  const networkRecords = imageElements.map(e => ({
+    url: e.src,
+    mimeType: 'image/png',
+  }));
+  return {
+    traces: {
+      [LargestContentfulPaintLazyLoaded.DEFAULT_PASS]: createTestTrace({
+        traceEnd: 6e3,
+        largestContentfulPaint: 45e2,
+      }),
+    },
+    devtoolsLogs: {[LargestContentfulPaintLazyLoaded.DEFAULT_PASS]:
+      networkRecordsToDevtoolsLog(networkRecords)},
+    TraceElements: imageElements.length ? [{
+      traceEventType: 'largest-contentful-paint',
+      node: SAMPLE_NODE,
+    }] : [],
+    ImageElements: imageElements,
+    ViewportDimensions: {
+      innerHeight: 500,
+      innerWidth: 300,
+    },
+  };
+}
+
 describe('Performance: lcp-lazy-loaded audit', () => {
   it('correctly surfaces the lazy loaded LCP element', async () => {
-    const artifacts = {
-      TraceElements: [{
-        traceEventType: 'largest-contentful-paint',
-        node: SAMPLE_NODE,
-      }],
-      ImageElements: [
-        generateImage('lazy', 0),
-      ],
-      ViewportDimensions: {
-        innerHeight: 500,
-        innerWidth: 300,
-      },
-    };
-
-    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts);
+    const artifacts = createArtifacts([generateImage('lazy', 0)]);
+    const context = {settings: {}, computedCache: new Map()};
+    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts, context);
     expect(auditResult.score).toEqual(0);
     expect(auditResult.details.items).toHaveLength(1);
     expect(auditResult.details.items[0].node.path).toEqual('1,HTML,1,BODY,3,DIV,2,IMG');
@@ -50,49 +70,24 @@ describe('Performance: lcp-lazy-loaded audit', () => {
   });
 
   it('eager LCP element scores 1', async () => {
-    const artifacts = {
-      TraceElements: [{
-        traceEventType: 'largest-contentful-paint',
-        node: SAMPLE_NODE,
-      }],
-      ImageElements: [
-        generateImage('eager', 0),
-      ],
-      ViewportDimensions: {
-        innerHeight: 500,
-        innerWidth: 300,
-      },
-    };
-    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts);
+    const artifacts = createArtifacts([generateImage('eager', 0)]);
+    const context = {settings: {}, computedCache: new Map()};
+    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts, context);
     expect(auditResult.score).toEqual(1);
     expect(auditResult.details.items).toHaveLength(1);
   });
 
   it('not applicable when outside of viewport', async () => {
-    const artifacts = {
-      TraceElements: [{
-        traceEventType: 'largest-contentful-paint',
-        node: SAMPLE_NODE,
-      }],
-      ImageElements: [
-        generateImage('lazy', 700),
-      ],
-      ViewportDimensions: {
-        innerHeight: 500,
-        innerWidth: 300,
-      },
-    };
-    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts);
+    const artifacts = createArtifacts([generateImage('lazy', 700)]);
+    const context = {settings: {}, computedCache: new Map()};
+    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts, context);
     expect(auditResult.notApplicable).toEqual(true);
   });
 
   it('doesn\'t throw an error when there is nothing to show', async () => {
-    const artifacts = {
-      TraceElements: [],
-      ImageElements: [],
-    };
-
-    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts);
+    const artifacts = createArtifacts([]);
+    const context = {settings: {}, computedCache: new Map()};
+    const auditResult = await LargestContentfulPaintLazyLoaded.audit(artifacts, context);
     expect(auditResult.score).toEqual(1);
     expect(auditResult.notApplicable).toEqual(true);
   });

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

@@ -82,6 +82,7 @@ describe('Performance: preload-lcp audit', () => {
       {
         requestId: '4',
         resourceType: 'Image',
+        mimeType: 'image/png',
         priority: 'High',
         isLinkPreload: false,
         startTime: 2,

core/test/computed/image-records-test.js

@@ -22,7 +22,7 @@ function mockRequest(partial = {}) {
  */
 function mockElement(partial = {}) {
   return {
-    src: 'https://example.com/img.png',
+    src: partial.src ?? 'https://example.com/img.png',
     srcset: '',
     displayedWidth: 200,
     displayedHeight: 200,
@@ -159,14 +159,28 @@ describe('compute_', () => {
     expect(elements).toEqual([mockElement({mimeType: 'image/png'})]);
   });
 
-  it('guess mime type if no request', async () => {
+  it('ignore image element if no valid record', async () => {
     const elements = await ImageRecords.compute_({
       ImageElements: [
-        mockElement(),
+        mockElement({src: 'https://example.com/img.png'}),
+        mockElement({src: 'https://example.com/img2.png'}),
+      ],
+      networkRecords: [
+        mockRequest({
+          mimeType: 'iam/trash',
+          url: 'https://example.com/img.png',
+          finished: true,
+          statusCode: 200,
+        }),
+        mockRequest({
+          mimeType: 'image/png',
+          url: 'https://example.com/img2.png',
+          finished: true,
+          statusCode: 500,
+        }),
       ],
-      networkRecords: [],
     });
 
-    expect(elements).toEqual([mockElement({mimeType: 'image/png'})]);
+    expect(elements).toEqual([]);
   });
 });