core(image-elements): remove resourceSize property

lighthouse-core/audits/byte-efficiency/offscreen-images.js

@@ -80,7 +80,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
   static computeWaste(image, viewportDimensions, networkRecords) {
     const networkRecord = networkRecords.find(record => record.url === image.src);
     // If we don't know how big it was, we can't really report savings, treat it as passed.
-    if (!image.resourceSize || !networkRecord) return null;
+    if (!networkRecord) return null;
     // If the image had its loading behavior explicitly controlled already, treat it as passed.
     if (image.loading === 'lazy' || image.loading === 'eager') return null;
 
@@ -89,7 +89,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
     const visiblePixels = this.computeVisiblePixels(image.clientRect, viewportDimensions);
     // Treat images with 0 area as if they're offscreen. See https://github.com/GoogleChrome/lighthouse/issues/1914
     const wastedRatio = totalPixels === 0 ? 1 : 1 - visiblePixels / totalPixels;
-    const totalBytes = image.resourceSize;
+    const totalBytes = networkRecord.resourceSize;
     const wastedBytes = Math.round(totalBytes * wastedRatio);
 
     if (!Number.isFinite(wastedRatio)) {

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

@@ -49,11 +49,13 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
   /**
    * @param {LH.Artifacts.ImageElement} image
    * @param {LH.Artifacts.ViewportDimensions} ViewportDimensions
+   * @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
    * @return {null|Error|LH.Audit.ByteEfficiencyItem};
    */
-  static computeWaste(image, ViewportDimensions) {
-    // Nothing can be done without network info.
-    if (!image.resourceSize) {
+  static computeWaste(image, ViewportDimensions, networkRecords) {
+    const networkRecord = networkRecords.find(record => record.url === image.src);
+    // Nothing can be done without network info, ignore images without resource size information.
+    if (!networkRecord) {
       return null;
     }
 
@@ -81,7 +83,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
     const url = URL.elideDataURI(image.src);
     const actualPixels = image.naturalWidth * image.naturalHeight;
     const wastedRatio = 1 - (usedPixels / actualPixels);
-    const totalBytes = image.resourceSize;
+    const totalBytes = networkRecord.resourceSize;
     const wastedBytes = Math.round(totalBytes * wastedRatio);
 
     if (!Number.isFinite(wastedRatio)) {
@@ -98,9 +100,10 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
 
   /**
    * @param {LH.Artifacts} artifacts
+   * @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
    * @return {ByteEfficiencyAudit.ByteEfficiencyProduct}
    */
-  static audit_(artifacts) {
+  static audit_(artifacts, networkRecords) {
     const images = artifacts.ImageElements;
     const ViewportDimensions = artifacts.ViewportDimensions;
 
@@ -109,15 +112,15 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
     /** @type {Map<string, LH.Audit.ByteEfficiencyItem>} */
     const resultsMap = new Map();
     for (const image of images) {
-      // Ignore images without resource size information.
       // Give SVG a free pass because creating a "responsive" SVG is of questionable value.
       // Ignore CSS images because it's difficult to determine what is a spritesheet,
       // and the reward-to-effort ratio for responsive CSS images is quite low https://css-tricks.com/responsive-images-css/.
-      if (!image.resourceSize || image.mimeType === 'image/svg+xml' || image.isCss) {
+      if (image.mimeType === 'image/svg+xml' || image.isCss) {
         continue;
       }
 
-      const processed = UsesResponsiveImages.computeWaste(image, ViewportDimensions);
+      /* eslint-disable max-len */
+      const processed = UsesResponsiveImages.computeWaste(image, ViewportDimensions, networkRecords);
       if (!processed) continue;
 
       if (processed instanceof Error) {

lighthouse-core/gather/gatherers/image-elements.js

@@ -16,7 +16,6 @@ const FontSize = require('./seo/font-size.js');
 
 /* global window, getElementsInDocument, Image, getNodeDetails, ShadowRoot */
 
-
 /** @param {Element} element */
 /* istanbul ignore next */
 function getClientRect(element) {
@@ -77,7 +76,6 @@ function getHTMLImages(allElements) {
       isCss: false,
       isPicture,
       loading: element.loading,
-      resourceSize: 0, // this will get overwritten below
       usesObjectFit: ['cover', 'contain', 'scale-down', 'none'].includes(
         computedStyle.getPropertyValue('object-fit')
       ),
@@ -137,7 +135,6 @@ function getCSSImages(allElements) {
         style.getPropertyValue('image-rendering')
       ),
       usesSrcSetDensityDescriptor: false,
-      resourceSize: 0, // this will get overwritten below
       // @ts-expect-error - getNodeDetails put into scope via stringification
       ...getNodeDetails(element),
     });
@@ -332,13 +329,6 @@ class ImageElements extends Gatherer {
       // Pull some of our information directly off the network record.
       const networkRecord = indexedNetworkRecords[element.src] || {};
       element.mimeType = networkRecord.mimeType;
-      // Resource size is almost always the right one to be using because of the below:
-      //     transferSize = resourceSize + headers.length
-      // HOWEVER, there are some cases where an image is compressed again over the network and transfer size
-      // is smaller (see https://github.com/GoogleChrome/lighthouse/pull/4968).
-      // Use the min of the two numbers to be safe.
-      const {resourceSize = 0, transferSize = 0} = networkRecord;
-      element.resourceSize = Math.min(resourceSize, transferSize);
 
       if (!element.isInShadowDOM && !element.isCss) {
         await this.fetchSourceRules(driver, element.devtoolsNodePath, element);

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

@@ -10,12 +10,13 @@ const UsesResponsiveImagesAudit =
 const assert = require('assert').strict;
 
 /* eslint-env jest */
-function generateRecord(resourceSizeInKb, durationInMs, mimeType = 'image/png') {
+function generateRecord(resourceSizeInKb, durationInMs, url = 'https://google.com/logo.png', mimeType = 'image/png') {
   return {
     mimeType,
     resourceSize: resourceSizeInKb * 1024,
     endTime: durationInMs / 1000,
     responseReceivedTime: 0,
+    url,
   };
 }
 
@@ -26,32 +27,32 @@ function generateSize(width, height, prefix = 'displayed') {
   return size;
 }
 
-function generateImage(clientSize, naturalSize, networkRecord, src = 'https://google.com/logo.png') {
-  Object.assign(networkRecord || {}, {url: src});
-  const image = {src, ...networkRecord};
+function generateImage(clientSize, naturalSize, src = 'https://google.com/logo.png') {
+  Object.assign({url: src});
+  const image = {src};
   Object.assign(image, clientSize, naturalSize);
   return image;
 }
 
 describe('Page uses responsive images', () => {
   function testImage(condition, data) {
     const description = `identifies when an image is ${condition}`;
+    const artifacts = {
+      ViewportDimensions: {
+        innerWidth: 1000,
+        innerHeight: 1000,
+        devicePixelRatio: data.devicePixelRatio || 1,
+      },
+      ImageElements: [
+        generateImage(
+          generateSize(...data.clientSize),
+          generateSize(...data.naturalSize, 'natural')
+        ),
+      ],
+    };
     it(description, () => {
-      const result = UsesResponsiveImagesAudit.audit_({
-        ViewportDimensions: {
-          innerWidth: 1000,
-          innerHeight: 1000,
-          devicePixelRatio: data.devicePixelRatio || 1,
-        },
-        ImageElements: [
-          generateImage(
-            generateSize(...data.clientSize),
-            generateSize(...data.naturalSize, 'natural'),
-            generateRecord(data.sizeInKb, data.durationInMs || 200)
-          ),
-        ],
-      });
-
+      // eslint-disable-next-line max-len
+      const result = UsesResponsiveImagesAudit.audit_(artifacts, [generateRecord(data.sizeInKb, data.durationInMs || 200)]);
       assert.equal(result.items.length, data.listed ? 1 : 0);
       if (data.listed) {
         assert.equal(Math.round(result.items[0].wastedBytes / 1024), data.expectedWaste);
@@ -120,80 +121,84 @@ describe('Page uses responsive images', () => {
           null
         ),
       ],
-    });
+    },
+      []
+    );
 
     assert.equal(auditResult.items.length, 0);
   });
 
   it('identifies when images are not wasteful', () => {
+    const networkRecords = [generateRecord(100, 300, 'https://google.com/logo.png'), generateRecord(90, 500, 'https://google.com/logo2.png'), generateRecord(20, 100, 'data:image/jpeg;base64,foobar')];
     const auditResult = UsesResponsiveImagesAudit.audit_({
       ViewportDimensions: {innerWidth: 1000, innerHeight: 1000, devicePixelRatio: 2},
       ImageElements: [
         generateImage(
           generateSize(200, 200),
           generateSize(450, 450, 'natural'),
-          generateRecord(100, 300),
           'https://google.com/logo.png'
         ),
         generateImage(
           generateSize(100, 100),
           generateSize(210, 210, 'natural'),
-          generateRecord(90, 500),
           'https://google.com/logo2.png'
         ),
         generateImage(
           generateSize(100, 100),
           generateSize(80, 80, 'natural'),
-          generateRecord(20, 100),
           'data:image/jpeg;base64,foobar'
         ),
       ],
-    });
+    },
+      networkRecords
+    );
 
     assert.equal(auditResult.items.length, 2);
   });
 
   it('ignores vectors', () => {
     const urlA = 'https://google.com/logo.svg';
     const naturalSizeA = generateSize(450, 450, 'natural');
-    const recordA = generateRecord(100, 300, 'image/svg+xml');
-
+    // eslint-disable-next-line max-len
+    const image = {...generateImage(generateSize(10, 10), naturalSizeA, urlA), mimeType: 'image/svg+xml'};
     const auditResult = UsesResponsiveImagesAudit.audit_({
       ViewportDimensions: {innerWidth: 1000, innerHeight: 1000, devicePixelRatio: 1},
       ImageElements: [
-        generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA),
+        image,
       ],
-    });
-
+    },
+      [generateRecord(100, 300, urlA, 'image/svg+xml')]
+    );
     assert.equal(auditResult.items.length, 0);
   });
 
   it('ignores CSS', () => {
     const urlA = 'https://google.com/logo.png';
     const naturalSizeA = generateSize(450, 450, 'natural');
-    const recordA = generateRecord(100, 300);
 
     const auditResult = UsesResponsiveImagesAudit.audit_({
       ViewportDimensions: {innerWidth: 1000, innerHeight: 1000, devicePixelRatio: 1},
       ImageElements: [
-        {...generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA), isCss: true},
+        {...generateImage(generateSize(10, 10), naturalSizeA, urlA), isCss: true},
       ],
-    });
+    },
+      [generateRecord(100, 300, urlA)]
+    );
 
     assert.equal(auditResult.items.length, 0);
   });
 
   it('handles failure', () => {
     const urlA = 'https://google.com/logo.png';
     const naturalSizeA = generateSize(NaN, 450, 'natural');
-    const recordA = generateRecord(100, 300);
-
     const auditResult = UsesResponsiveImagesAudit.audit_({
       ViewportDimensions: {innerWidth: 1000, innerHeight: 1000, devicePixelRatio: 1},
       ImageElements: [
-        generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA),
+        generateImage(generateSize(10, 10), naturalSizeA, urlA),
       ],
-    });
+    },
+      [generateRecord(100, 300, urlA)]
+    );
 
     assert.equal(auditResult.items.length, 0);
     assert.equal(auditResult.warnings.length, 1);
@@ -202,21 +207,24 @@ describe('Page uses responsive images', () => {
   it('de-dupes images', () => {
     const urlA = 'https://google.com/logo.png';
     const naturalSizeA = generateSize(450, 450, 'natural');
-    const recordA = generateRecord(100, 300);
+    const recordA = generateRecord(100, 300, urlA);
     const urlB = 'https://google.com/logoB.png';
     const naturalSizeB = generateSize(1000, 1000, 'natural');
-    const recordB = generateRecord(10, 20); // make it small to still test passing
+    const recordB = generateRecord(10, 20, urlB); // make it small to still test passing
+    const networkRecords = [recordA, recordB];
 
     const auditResult = UsesResponsiveImagesAudit.audit_({
       ViewportDimensions: {innerWidth: 1000, innerHeight: 1000, devicePixelRatio: 1},
       ImageElements: [
-        generateImage(generateSize(10, 10), naturalSizeA, recordA, urlA),
-        generateImage(generateSize(450, 450), naturalSizeA, recordA, urlA),
-        generateImage(generateSize(30, 30), naturalSizeA, recordA, urlA),
-        generateImage(generateSize(500, 500), naturalSizeB, recordB, urlB),
-        generateImage(generateSize(100, 100), naturalSizeB, recordB, urlB),
+        generateImage(generateSize(10, 10), naturalSizeA, urlA),
+        generateImage(generateSize(450, 450), naturalSizeA, urlA),
+        generateImage(generateSize(30, 30), naturalSizeA, urlA),
+        generateImage(generateSize(500, 500), naturalSizeB, urlB),
+        generateImage(generateSize(100, 100), naturalSizeB, urlB),
       ],
-    });
+    },
+      networkRecords
+    );
 
     assert.equal(auditResult.items.length, 1);
     assert.equal(auditResult.items[0].wastedPercent, 75, 'correctly computes wastedPercent');