core: fix resource size calculation of cached images #12612

g-yonchev · 2021-06-03T12:02:15Z

Summary
This PR fixes an issue with cached images behaviour

Related Issues/PRs
see #12465 (comment)

google-cla · 2021-06-03T12:02:21Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-06-03T14:10:23Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-06-03T15:01:54Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-06-03T15:26:27Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

google-cla · 2021-06-03T15:37:40Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

g-yonchev · 2021-06-03T17:43:22Z

@googlebot I signed it!

patrickhulce

thanks very much @g-yonchev!

patrickhulce · 2021-06-08T13:49:37Z

lighthouse-core/lib/network-request.js

+   * @param {NetworkRequest} networkRecord
+   * @return {number}
+   */
+  static getActualResourceSize(networkRecord) {


wdyt about getTransferSizeOfResource or getResourceSizeOnNetwork or something?

actual resource size implies to me that we're trying to get the raw size of the resource, but that's not what we're looking for. the point of the transfer size workaround is for cases where the image is compressed over the network again (svg especially). we're just trying to find the size of the image as it was transferred over the network without the headers if we can help it.

@patrickhulce, good point. I renamed the func to getResourceSizeOnNetwork.

patrickhulce · 2021-06-08T13:49:53Z

lighthouse-core/lib/network-request.js

+   * @return {number}
+   */
+  static getActualResourceSize(networkRecord) {
+    // Resource size is almost always the right one to be using because of the below:


move this into the jsdoc description for the function?

patrickhulce · 2021-06-08T13:51:05Z

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

+      [networkRecord]
+    );
+
+    assert.equal(auditResult.items.length, 1);


assert that we're getting the wasted bytes correct too?

patrickhulce · 2021-06-08T13:51:15Z

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

+    };
+
+    return UnusedImages.audit_(artifacts, [networkRecord], context).then(auditResult => {
+      assert.equal(auditResult.items.length, 1);


assert that we're getting the wasted bytes correct too?

patrickhulce

thanks very much @g-yonchev!

Janpot · 2021-07-14T08:01:36Z

@patrickhulce Just noticed that this PR didn't make it into 8.1.0. Are there any other criteria to be met for this PR to be merged?

patrickhulce · 2021-07-14T16:01:36Z

oh shoot, sorry @g-yonchev! No other criteria (other than fixing merge conflict now), just got forgotten in the shuffle

g-yonchev · 2021-07-14T16:11:23Z

I will fix the merge conflict.

g-yonchev · 2021-08-10T13:40:13Z

@patrickhulce I resolved the merge conflicts.
There was also a test failing which I fixed with the commit b96e454

Fix behaviour chached images

946ee6d

g-yonchev requested a review from a team as a code owner June 3, 2021 12:02

g-yonchev requested review from connorjclark and removed request for a team June 3, 2021 12:02

vercel bot deployed to Preview June 3, 2021 12:02 View deployment

google-cla bot added the cla: no label Jun 3, 2021

devtools-bot assigned connorjclark Jun 3, 2021

devtools-bot added the waiting4reviewer label Jun 3, 2021

g-yonchev marked this pull request as draft June 3, 2021 12:04

add tests

283735c

vercel bot deployed to Preview June 3, 2021 14:10 View deployment

g-yonchev changed the title ~~Fix behaviour cached images~~ (core): Fix behaviour cached images Jun 3, 2021

Extract duplicate code into static function

3658936

vercel bot deployed to Preview June 3, 2021 15:01 View deployment

g-yonchev changed the title ~~(core): Fix behaviour cached images~~ core: Fix behaviour cached images Jun 3, 2021

comments

413fa76

vercel bot deployed to Preview June 3, 2021 15:26 View deployment

Janpot approved these changes Jun 3, 2021

View reviewed changes

move comment

e9a419e

vercel bot deployed to Preview June 3, 2021 15:37 View deployment

g-yonchev marked this pull request as ready for review June 3, 2021 16:06

google-cla bot added cla: yes and removed cla: no labels Jun 3, 2021

g-yonchev changed the title ~~core: Fix behaviour cached images~~ core: fix behaviour cached images Jun 3, 2021

Janpot mentioned this pull request Jun 8, 2021

optimized-images calculation incorrect for cached images #12465

Open

patrickhulce reviewed Jun 8, 2021

View reviewed changes

rename func, move comment to jsdoc, and update tests

df968a0

vercel bot deployed to Preview June 16, 2021 09:27 View deployment

g-yonchev requested a review from patrickhulce June 16, 2021 09:29

patrickhulce approved these changes Jun 16, 2021

View reviewed changes

patrickhulce removed the waiting4reviewer label Jun 16, 2021

connorjclark changed the title ~~core: fix behaviour cached images~~ core: fix resource size calculation of cached images Jun 16, 2021

connorjclark approved these changes Jun 16, 2021

View reviewed changes

Merge branch 'master' into fix-behavior-cached-images

36b0432

vercel bot deployed to Preview July 14, 2021 16:14 View deployment

supply test with computedCache and make it async

b96e454

vercel bot deployed to Preview August 10, 2021 11:45 View deployment

patrickhulce merged commit 36454ee into GoogleChrome:master Aug 10, 2021

jamalxx1 approved these changes Sep 6, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: fix resource size calculation of cached images #12612

core: fix resource size calculation of cached images #12612

g-yonchev commented Jun 3, 2021 •

edited

Loading

google-cla bot commented Jun 3, 2021

google-cla bot commented Jun 3, 2021

google-cla bot commented Jun 3, 2021

google-cla bot commented Jun 3, 2021

google-cla bot commented Jun 3, 2021

g-yonchev commented Jun 3, 2021

patrickhulce left a comment

patrickhulce Jun 8, 2021

g-yonchev Jun 16, 2021

patrickhulce Jun 8, 2021

patrickhulce Jun 8, 2021

patrickhulce Jun 8, 2021

patrickhulce left a comment

Janpot commented Jul 14, 2021

patrickhulce commented Jul 14, 2021

g-yonchev commented Jul 14, 2021

g-yonchev commented Aug 10, 2021

core: fix resource size calculation of cached images #12612

core: fix resource size calculation of cached images #12612

Conversation

g-yonchev commented Jun 3, 2021 • edited Loading

google-cla bot commented Jun 3, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

google-cla bot commented Jun 3, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

google-cla bot commented Jun 3, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

google-cla bot commented Jun 3, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

google-cla bot commented Jun 3, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

g-yonchev commented Jun 3, 2021

patrickhulce left a comment

Choose a reason for hiding this comment

patrickhulce Jun 8, 2021

Choose a reason for hiding this comment

g-yonchev Jun 16, 2021

Choose a reason for hiding this comment

patrickhulce Jun 8, 2021

Choose a reason for hiding this comment

patrickhulce Jun 8, 2021

Choose a reason for hiding this comment

patrickhulce Jun 8, 2021

Choose a reason for hiding this comment

patrickhulce left a comment

Choose a reason for hiding this comment

Janpot commented Jul 14, 2021

patrickhulce commented Jul 14, 2021

g-yonchev commented Jul 14, 2021

g-yonchev commented Aug 10, 2021

g-yonchev commented Jun 3, 2021 •

edited

Loading