Skip to content

Commit

Permalink
Merge 97fbdae into 0240849
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Oct 7, 2020
2 parents 0240849 + 97fbdae commit 75dd95e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 21 deletions.
7 changes: 2 additions & 5 deletions lighthouse-core/gather/gatherers/full-page-screenshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class FullPageScreenshot extends Gatherer {
async _takeScreenshot(passContext, maxScreenshotHeight) {
const driver = passContext.driver;
const metrics = await driver.sendCommand('Page.getLayoutMetrics');
const deviceScaleFactor = await driver.evaluateAsync('window.devicePixelRatio');

// Width should match emulated width, without considering content overhang.
// Both layoutViewport and visualViewport capture this. visualViewport accounts
Expand All @@ -48,7 +47,7 @@ class FullPageScreenshot extends Gatherer {
screenHeight: height,
width,
screenWidth: width,
deviceScaleFactor,
deviceScaleFactor: 1,
scale: 1,
positionX: 0,
positionY: 0,
Expand Down Expand Up @@ -77,9 +76,7 @@ class FullPageScreenshot extends Gatherer {
* @return {Promise<LH.Artifacts.FullPageScreenshot | null>}
*/
async afterPass_(passContext) {
const deviceScaleFactor = await passContext.driver.evaluateAsync('window.devicePixelRatio');
const maxScreenshotHeight = Math.floor(MAX_SCREENSHOT_HEIGHT / deviceScaleFactor);
let screenshot = await this._takeScreenshot(passContext, maxScreenshotHeight);
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
Expand Down
25 changes: 9 additions & 16 deletions lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
/* eslint-env jest */

const FullPageScreenshotGatherer = require('../../../gather/gatherers/full-page-screenshot.js');
const MAX_SCREENSHOT_HEIGHT = 16384;

/**
* @param {{contentSize: {width: number, height: number}, screenSize: {width?: number, height?: number, dpr: number}, screenshotData: string[]}}
Expand All @@ -19,9 +18,6 @@ function createMockDriver({contentSize, screenSize, screenshotData}) {
if (code === 'window.innerWidth') {
return contentSize.width;
}
if (code === 'window.devicePixelRatio') {
return screenSize ? screenSize.dpr : 1;
}
if (code.includes('document.documentElement.clientWidth')) {
return {
width: screenSize.width,
Expand Down Expand Up @@ -131,7 +127,7 @@ describe('Full-page screenshot gatherer', () => {
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
mobile: true,
deviceScaleFactor: 2,
deviceScaleFactor: 1,
height: 1500,
width: 500,
screenHeight: 1500,
Expand Down Expand Up @@ -195,9 +191,6 @@ describe('Full-page screenshot gatherer', () => {
width: 412,
height: 15000,
},
screenSize: {
dpr: 2,
},
screenshotData: [
new Array(3 * 1024 * 1024).join('a'),
new Array(1 * 1024 * 1024).join('a'),
Expand All @@ -216,15 +209,15 @@ describe('Full-page screenshot gatherer', () => {
expect(driver.sendCommand).toHaveBeenCalledWith(
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
deviceScaleFactor: 2,
height: Math.floor(MAX_SCREENSHOT_HEIGHT / 2),
screenHeight: Math.floor(MAX_SCREENSHOT_HEIGHT / 2),
deviceScaleFactor: 1,
height: Math.floor(15000),
screenHeight: Math.floor(15000),
})
);
expect(driver.sendCommand).toHaveBeenCalledWith(
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
deviceScaleFactor: 2,
deviceScaleFactor: 1,
height: 5000,
screenHeight: 5000,
})
Expand Down Expand Up @@ -262,15 +255,15 @@ describe('Full-page screenshot gatherer', () => {
expect(driver.sendCommand).toHaveBeenCalledWith(
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
deviceScaleFactor: 3,
height: Math.floor(MAX_SCREENSHOT_HEIGHT / 3),
screenHeight: Math.floor(MAX_SCREENSHOT_HEIGHT / 3),
deviceScaleFactor: 1,
height: Math.floor(15000),
screenHeight: Math.floor(15000),
})
);
expect(driver.sendCommand).toHaveBeenCalledWith(
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
deviceScaleFactor: 3,
deviceScaleFactor: 1,
height: 5000,
screenHeight: 5000,
})
Expand Down

0 comments on commit 75dd95e

Please sign in to comment.