perf(gatherers): skip optimization of cross origin images (#2154)

GoogleChrome · May 9, 2017 · d0cb646 · d0cb646
1 parent 5dc01be
commit d0cb646
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 23 deletions.
diff --git a/lighthouse-cli/test/fixtures/byte-efficiency/tester.html b/lighthouse-cli/test/fixtures/byte-efficiency/tester.html
@@ -69,9 +69,10 @@ <h2>Byte efficiency tester page</h2>
   <!-- FAIL(offscreen): image is offscreen -->
   <img style="position: absolute; top: -10000px;" src="lighthouse-unoptimized.jpg">
 
+  <!-- Image is cross-origin, which are disabled for now -->
   <!-- FAIL(optimized): image is not optimized -->
   <!-- PASS(responsive): image is used at full size -->
-  <img src="http://localhost:10503/byte-efficiency/lighthouse-unoptimized.jpg">
+  <!--<img src="http://localhost:10503/byte-efficiency/lighthouse-unoptimized.jpg">-->
 
   <!-- PASSWARN(optimized): image is JPEG optimized but not WebP -->
   <!-- FAIL(responsive): image is 25% used at DPR 2 -->

diff --git a/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js b/lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
@@ -50,7 +50,7 @@ module.exports = [
         extendedInfo: {
           value: {
             results: {
-              length: 5
+              length: 4
             }
           }
         }

diff --git a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
@@ -100,26 +100,23 @@ class OptimizedImages extends Gatherer {
   /**
    * @param {!Object} driver
    * @param {{url: string, isBase64DataUri: boolean, resourceSize: number}} networkRecord
-   * @return {!Promise<{originalSize: number, jpegSize: number, webpSize: number}>}
+   * @return {!Promise<?{originalSize: number, jpegSize: number, webpSize: number}>}
    */
   calculateImageStats(driver, networkRecord) {
-    let uriPromise = Promise.resolve(networkRecord.url);
-
-    // For cross-origin images, circumvent canvas CORS policy by getting image directly from protocol
     if (!networkRecord.isSameOrigin && !networkRecord.isBase64DataUri) {
-      const requestId = networkRecord.requestId;
-      uriPromise = driver.sendCommand('Network.getResponseBody', {requestId}).then(resp => {
-        if (!resp.base64Encoded) {
-          throw new Error('Unable to fetch cross-origin image body');
-        }
-
-        return `data:${networkRecord.mimeType};base64,${resp.body}`;
-      });
+      // Processing of cross-origin images is buggy and very slow, disable for now
+      // See https://github.com/GoogleChrome/lighthouse/issues/1853
+      // See https://github.com/GoogleChrome/lighthouse/issues/2146
+      return Promise.resolve(null);
     }
 
-    return uriPromise.then(uri => {
+    return Promise.resolve(networkRecord.url).then(uri => {
       const script = `(${getOptimizedNumBytes.toString()})(${JSON.stringify(uri)})`;
       return driver.evaluateAsync(script).then(stats => {
+        if (!stats) {
+          return null;
+        }
+
         const isBase64DataUri = networkRecord.isBase64DataUri;
         const base64Length = networkRecord.url.length - networkRecord.url.indexOf(',') - 1;
         return {
@@ -141,7 +138,13 @@ class OptimizedImages extends Gatherer {
       return promise.then(results => {
         return this.calculateImageStats(driver, record)
           .catch(err => ({failed: true, err}))
-          .then(stats => results.concat(Object.assign(stats, record)));
+          .then(stats => {
+            if (!stats) {
+              return results;
+            }
+
+            return results.concat(Object.assign(stats, record));
+          });
       });
     }, Promise.resolve([]));
   }

diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js
@@ -86,12 +86,13 @@ describe('Optimized images', () => {
 
   it('returns all images', () => {
     return optimizedImages.afterPass(options, traceData).then(artifact => {
-      assert.equal(artifact.length, 5);
+      assert.equal(artifact.length, 4);
       assert.ok(/image.jpg/.test(artifact[0].url));
       assert.ok(/transparent.png/.test(artifact[1].url));
       assert.ok(/image.bmp/.test(artifact[2].url));
-      assert.ok(/gmail.*image.jpg/.test(artifact[3].url));
-      assert.ok(/data: image/.test(artifact[4].url));
+      // skip cross-origin for now
+      // assert.ok(/gmail.*image.jpg/.test(artifact[3].url));
+      assert.ok(/data: image/.test(artifact[3].url));
     });
   });
 
@@ -103,12 +104,13 @@ describe('Optimized images', () => {
     };
 
     return optimizedImages.afterPass(options, traceData).then(artifact => {
-      assert.equal(artifact.length, 5);
+      assert.equal(artifact.length, 4);
       checkSizes(artifact[0], 10, 60, 80);
       checkSizes(artifact[1], 11, 60, 80);
       checkSizes(artifact[2], 12, 60, 80);
-      checkSizes(artifact[3], 15, 60, 80); // uses base64 data
-      checkSizes(artifact[4], 20, 80, 100); // uses base64 data
+      // skip cross-origin for now
+      // checkSizes(artifact[3], 15, 60, 80);
+      checkSizes(artifact[3], 20, 80, 100); // uses base64 data
     });
   });
 
@@ -126,7 +128,7 @@ describe('Optimized images', () => {
     return optimizedImages.afterPass(options, traceData).then(artifact => {
       const failed = artifact.find(record => record.failed);
 
-      assert.equal(artifact.length, 5);
+      assert.equal(artifact.length, 4);
       assert.ok(failed, 'passed along failure');
       assert.ok(/whoops/.test(failed.err.message), 'passed along error message');
     });