core(full-page-screenshot): drop max datauri size constraints (#11689)

GoogleChrome · Nov 25, 2020 · 6880512 · 6880512
1 parent ac85955
commit 6880512
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 123 deletions.
diff --git a/lighthouse-core/gather/gatherers/full-page-screenshot.js b/lighthouse-core/gather/gatherers/full-page-screenshot.js
@@ -12,8 +12,6 @@ const Gatherer = require('./gatherer.js');
 const FULL_PAGE_SCREENSHOT_QUALITY = 30;
 // Maximum screenshot height in Chrome https://bugs.chromium.org/p/chromium/issues/detail?id=770769
 const MAX_SCREENSHOT_HEIGHT = 16384;
-// Maximum data URL size in Chrome https://bugs.chromium.org/p/chromium/issues/detail?id=69227
-const MAX_DATA_URL_SIZE = 2 * 1024 * 1024 - 1;
 
 /**
  * @param {string} str
@@ -25,10 +23,9 @@ function snakeCaseToCamelCase(str) {
 class FullPageScreenshot extends Gatherer {
   /**
    * @param {LH.Gatherer.PassContext} passContext
-   * @param {number} maxScreenshotHeight
    * @return {Promise<LH.Artifacts.FullPageScreenshot>}
    */
-  async _takeScreenshot(passContext, maxScreenshotHeight) {
+  async _takeScreenshot(passContext) {
     const driver = passContext.driver;
     const metrics = await driver.sendCommand('Page.getLayoutMetrics');
 
@@ -38,8 +35,8 @@ class FullPageScreenshot extends Gatherer {
     // Note: If the page is zoomed, many assumptions fail.
     //
     // Height should be as tall as the content. So we use contentSize.height
-    const width = Math.min(metrics.layoutViewport.clientWidth, maxScreenshotHeight);
-    const height = Math.min(metrics.contentSize.height, maxScreenshotHeight);
+    const width = Math.min(metrics.layoutViewport.clientWidth, MAX_SCREENSHOT_HEIGHT);
+    const height = Math.min(metrics.contentSize.height, MAX_SCREENSHOT_HEIGHT);
 
     await driver.sendCommand('Emulation.setDeviceMetricsOverride', {
       mobile: passContext.baseArtifacts.TestedAsMobileDevice,
@@ -67,28 +64,6 @@ class FullPageScreenshot extends Gatherer {
     };
   }
 
-  /**
-   * @param {LH.Gatherer.PassContext} passContext
-   * @return {Promise<LH.Artifacts.FullPageScreenshot | null>}
-   */
-  async afterPass_(passContext) {
-    let screenshot = await this._takeScreenshot(passContext, MAX_SCREENSHOT_HEIGHT);
-
-    if (screenshot.data.length > MAX_DATA_URL_SIZE) {
-      // Hitting the data URL size limit is rare, it only happens for pages on tall
-      // desktop sites with lots of images.
-      // So just cutting down the height a bit usually fixes the issue.
-      screenshot = await this._takeScreenshot(passContext, 5000);
-      if (screenshot.data.length > MAX_DATA_URL_SIZE) {
-        passContext.LighthouseRunWarnings.push(
-          'Full page screenshot is too big–report won\'t show element screenshots.');
-        return null;
-      }
-    }
-
-    return screenshot;
-  }
-
   /**
    * @param {LH.Gatherer.PassContext} passContext
    * @return {Promise<LH.Artifacts['FullPageScreenshot']>}
@@ -102,7 +77,8 @@ class FullPageScreenshot extends Gatherer {
       !passContext.settings.internalDisableDeviceScreenEmulation;
 
     try {
-      return await this.afterPass_(passContext);
+      const screenshot = await this._takeScreenshot(passContext);
+      return screenshot;
     } finally {
       // Revert resized page.
       if (lighthouseControlsEmulation) {

diff --git a/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js b/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js
@@ -178,98 +178,4 @@ describe('Full-page screenshot gatherer', () => {
       })
     );
   });
-
-  it('captures a smaller screenshot if the captured data URL is too large', async () => {
-    const fpsGatherer = new FullPageScreenshotGatherer();
-    const pageContentHeight = 15000;
-
-    const driver = createMockDriver({
-      contentSize: {
-        width: 412,
-        height: pageContentHeight,
-      },
-      screenSize: {
-        dpr: 2,
-      },
-      screenshotData: [
-        new Array(3 * 1024 * 1024).join('a'),
-        new Array(1 * 1024 * 1024).join('a'),
-      ],
-    });
-    const passContext = {
-      settings: {
-        emulatedFormFactor: 'mobile',
-      },
-      driver,
-      baseArtifacts: {},
-    };
-
-    const result = await fpsGatherer.afterPass(passContext);
-
-    expect(driver.sendCommand).toHaveBeenCalledWith(
-      'Emulation.setDeviceMetricsOverride',
-      expect.objectContaining({
-        deviceScaleFactor: 1,
-        height: pageContentHeight,
-      })
-    );
-    expect(driver.sendCommand).toHaveBeenCalledWith(
-      'Emulation.setDeviceMetricsOverride',
-      expect.objectContaining({
-        deviceScaleFactor: 1,
-        height: 5000,
-      })
-    );
-
-    expect(result).not.toBeNull();
-  });
-
-  it('returns null if the captured data URL is way too large', async () => {
-    const fpsGatherer = new FullPageScreenshotGatherer();
-    const pageContentHeight = 15000;
-
-    const driver = createMockDriver({
-      contentSize: {
-        width: 412,
-        height: pageContentHeight,
-      },
-      screenSize: {
-        dpr: 3,
-      },
-      screenshotData: [
-        new Array(3 * 1024 * 1024).join('a'),
-        new Array(2 * 1024 * 1024).join('a'),
-      ],
-    });
-    const passContext = {
-      settings: {
-        emulatedFormFactor: 'mobile',
-      },
-      driver,
-      baseArtifacts: {},
-      LighthouseRunWarnings: [],
-    };
-
-    const result = await fpsGatherer.afterPass(passContext);
-
-    // first attempt
-    expect(driver.sendCommand).toHaveBeenCalledWith(
-      'Emulation.setDeviceMetricsOverride',
-      expect.objectContaining({
-        deviceScaleFactor: 1,
-        height: pageContentHeight,
-      })
-    );
-    // second attempt
-    expect(driver.sendCommand).toHaveBeenCalledWith(
-      'Emulation.setDeviceMetricsOverride',
-      expect.objectContaining({
-        deviceScaleFactor: 1,
-        height: 5000,
-      })
-    );
-
-    expect(result).toBeNull();
-    expect(passContext.LighthouseRunWarnings[0]).toMatch('Full page screenshot is too big');
-  });
 });