Skip to content

Commit

Permalink
Merge dc1e1e7 into 5608cbd
Browse files Browse the repository at this point in the history
  • Loading branch information
paulirish committed Nov 21, 2020
2 parents 5608cbd + dc1e1e7 commit 23110b7
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 33 deletions.
13 changes: 2 additions & 11 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 @@ -45,13 +44,9 @@ class FullPageScreenshot extends Gatherer {
await driver.sendCommand('Emulation.setDeviceMetricsOverride', {
mobile: passContext.baseArtifacts.TestedAsMobileDevice,
height,
screenHeight: height,
width,
screenWidth: width,
deviceScaleFactor,
deviceScaleFactor: 1,
scale: 1,
positionX: 0,
positionY: 0,
screenOrientation: {angle: 0, type: 'portraitPrimary'},
});

Expand All @@ -77,9 +72,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 Expand Up @@ -126,8 +119,6 @@ class FullPageScreenshot extends Gatherer {
return {
width: document.documentElement.clientWidth,
height: document.documentElement.clientHeight,
screenWidth: window.screen.width,
screenHeight: window.screen.height,
screenOrientation: {
type: window.screen.orientation.type,
angle: window.screen.orientation.angle,
Expand Down
37 changes: 15 additions & 22 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,11 +127,9 @@ describe('Full-page screenshot gatherer', () => {
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
mobile: true,
deviceScaleFactor: 2,
deviceScaleFactor: 1,
height: 1500,
width: 500,
screenHeight: 1500,
screenWidth: 500,
})
);

Expand All @@ -147,8 +141,6 @@ describe('Full-page screenshot gatherer', () => {
deviceScaleFactor: 2,
height: 500,
width: 500,
screenHeight: 500,
screenWidth: 500,
screenOrientation: {
type: 'landscapePrimary',
angle: 30,
Expand Down Expand Up @@ -183,17 +175,18 @@ describe('Full-page screenshot gatherer', () => {
expect.objectContaining({
deviceScaleFactor: 1,
height: FullPageScreenshotGatherer.MAX_SCREENSHOT_HEIGHT,
screenHeight: FullPageScreenshotGatherer.MAX_SCREENSHOT_HEIGHT,
})
);
});

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: 15000,
height: pageContentHeight,
},
screenSize: {
dpr: 2,
Expand All @@ -216,17 +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: pageContentHeight,
})
);
expect(driver.sendCommand).toHaveBeenCalledWith(
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
deviceScaleFactor: 2,
deviceScaleFactor: 1,
height: 5000,
screenHeight: 5000,
})
);

Expand All @@ -235,10 +226,12 @@ describe('Full-page screenshot gatherer', () => {

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: 15000,
height: pageContentHeight,
},
screenSize: {
dpr: 3,
Expand All @@ -259,20 +252,20 @@ describe('Full-page screenshot gatherer', () => {

const result = await fpsGatherer.afterPass(passContext);

// first attempt
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: pageContentHeight,
})
);
// second attempt
expect(driver.sendCommand).toHaveBeenCalledWith(
'Emulation.setDeviceMetricsOverride',
expect.objectContaining({
deviceScaleFactor: 3,
deviceScaleFactor: 1,
height: 5000,
screenHeight: 5000,
})
);

Expand Down

0 comments on commit 23110b7

Please sign in to comment.