perf(optimized-images): use Audits.getEncodedResponse #3087
Conversation
| @@ -88,24 +93,49 @@ class OptimizedImages extends Gatherer { | |||
|
|
|||
| /** | |||
| * @param {!Object} driver | |||
| * @param {string} requestId | |||
| * @param {string} encoding | |||
There was a problem hiding this comment.
maybe a note that this will either be jpeg or webp?
There was a problem hiding this comment.
good call, done
| }; | ||
| }); | ||
| }).catch(err => { | ||
| if (/wasn't found/.test(err.message)) { |
There was a problem hiding this comment.
it still seemed reasonable to try the fallback path while we have it if this failed, but I'd rather know about failures for now at least until we release so sgtm :)
| }).catch(err => { | ||
| if (/wasn't found/.test(err.message)) { | ||
| // Mark non-support so we don't keep attempting the protocol method over and over | ||
| this._getEncodedResponseUnsupported = true; |
There was a problem hiding this comment.
can we include something like _getEncodedResponseUnsupported on the extendedInfo? Not the most elegant thing, but when looking at old reports (e.g. http archive) it would be good to have an easy way to know when the results include all images, including cross origin ones. Other ways of doing that would also be good (lighthouse version would work in a pinch, but nice to have something audit-local)
There was a problem hiding this comment.
yeah sure seem good to track this
There was a problem hiding this comment.
added a fromProtocol property to the image result
| }).then(result => { | ||
| if (result) return result; | ||
|
|
||
| // Take the slower/same-origin-limited fallback path if getEncodedResponse isn't available yet |
There was a problem hiding this comment.
maybe an additional note that cross-origin images aren't supported so it's clear until we delete?
There was a problem hiding this comment.
split the comment to make it more explicit
| @@ -22,49 +22,49 @@ const traceData = { | |||
| { | |||
| _url: 'http://google.com/image.jpg', | |||
| _mimeType: 'image/jpeg', | |||
| _resourceSize: 10, | |||
| _resourceSize: 10000, | |||
There was a problem hiding this comment.
what's the deal with this change? Just more realistic sizes/getting above the 4k cutoff?
There was a problem hiding this comment.
yeah just needed to go above the minimum image size
Uses the new Audits.getEncodedResponse method to determine the size savings. Time savings varies since the canvas method was more variable than outright slow but some runs went from ~6s to ~500ms. Also enables fast checking of cross-origin images which was previously impossible.