From 4e830f71f0112e8b83788c8fb983ab13d5387a5a Mon Sep 17 00:00:00 2001 From: "Alex N. Jose" Date: Tue, 21 Mar 2023 23:15:14 -0700 Subject: [PATCH 01/13] core: updated fullpage screenshot calculations. --- core/gather/gatherers/full-page-screenshot.js | 42 ++++++------------- .../gatherers/full-page-screenshot-test.js | 22 +++------- 2 files changed, 18 insertions(+), 46 deletions(-) diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js index f7c6113db75c..5142f9f65940 100644 --- a/core/gather/gatherers/full-page-screenshot.js +++ b/core/gather/gatherers/full-page-screenshot.js @@ -46,17 +46,6 @@ function getObservedDeviceMetrics() { }; } -/** - * The screenshot dimensions are sized to `window.outerHeight` / `window.outerWidth`, - * however the bounding boxes of the elements are relative to `window.innerHeight` / `window.innerWidth`. - */ -function getScreenshotAreaSize() { - return { - width: window.innerWidth, - height: window.innerHeight, - }; -} - function waitForDoubleRaf() { return new Promise((resolve) => { requestAnimationFrame(() => requestAnimationFrame(resolve)); @@ -79,14 +68,13 @@ class FullPageScreenshot extends FRGatherer { const session = context.driver.defaultSession; const metrics = await session.sendCommand('Page.getLayoutMetrics'); - // Height should be as tall as the content. - // Scale the emulated height to reach the content height. - const fullHeight = Math.round( - deviceMetrics.height * - metrics.cssContentSize.height / - metrics.cssLayoutViewport.clientHeight - ); + // To obtain a full page screenshot, we resize the emulated viewport to + // (1) a height equal to the maximum of visual-viewport height and document height. + // (2) a width equal to emulated visual-viewport width (we choose to clip overflow on x-axis). + // Finally, we cap the viewport to a maximum size allowance of WebP format. + const fullHeight = Math.max(deviceMetrics.height, metrics.cssContentSize.height); const height = Math.min(fullHeight, MAX_WEBP_SIZE); + const width = Math.min(deviceMetrics.width, MAX_WEBP_SIZE); // Setup network monitor before we change the viewport. const networkMonitor = new NetworkMonitor(context.driver.targetManager); @@ -103,7 +91,7 @@ class FullPageScreenshot extends FRGatherer { mobile: deviceMetrics.mobile, deviceScaleFactor: 1, height, - width: 0, // Leave width unchanged + width, }); // Now that the viewport is taller, give the page some time to fetch new resources that @@ -128,18 +116,14 @@ class FullPageScreenshot extends FRGatherer { format: 'webp', quality: FULL_PAGE_SCREENSHOT_QUALITY, }); + const metrics = await context.driver.defaultSession.sendCommand('Page.getLayoutMetrics'); const data = 'data:image/webp;base64,' + result.data; - - const screenshotAreaSize = - await context.driver.executionContext.evaluate(getScreenshotAreaSize, { - args: [], - useIsolation: true, - }); - return { data, - width: screenshotAreaSize.width, - height: screenshotAreaSize.height, + // Since we resized emulated viewport to match the desired screenshot size, + // it is safe to rely on visual viewport css dimensions. + width: metrics.cssVisualViewport.clientWidth, + height: metrics.cssVisualViewport.clientHeight, }; } @@ -247,7 +231,7 @@ class FullPageScreenshot extends FRGatherer { mobile: deviceMetrics.mobile, deviceScaleFactor: deviceMetrics.deviceScaleFactor, height: deviceMetrics.height, - width: 0, // Leave width unchanged + width: deviceMetrics.width, }); } } diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index beb9864f6b7c..feb78a205e6d 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -11,8 +11,6 @@ import FullPageScreenshotGatherer from '../../../gather/gatherers/full-page-scre let contentSize; /** @type {{width?: number, height?: number, dpr: number}} */ let screenSize; -/** @type {{width?: number, height?: number}} */ -let screenshotAreaSize; /** @type {string[]} */ let screenshotData; let mockContext = createMockContext(); @@ -20,7 +18,6 @@ let mockContext = createMockContext(); beforeEach(() => { contentSize = {width: 100, height: 100}; screenSize = {width: 100, height: 100, dpr: 1}; - screenshotAreaSize = contentSize; screenshotData = []; mockContext = createMockContext(); mockContext.driver.defaultSession.sendCommand.mockImplementation((method) => { @@ -28,7 +25,7 @@ beforeEach(() => { return { cssContentSize: contentSize, // See comment within _takeScreenshot() implementation - cssLayoutViewport: {clientWidth: screenSize.width, clientHeight: screenSize.height}, + cssVisualViewport: {clientWidth: contentSize.width, clientHeight: contentSize.height}, }; } if (method === 'Page.captureScreenshot') { @@ -50,11 +47,6 @@ beforeEach(() => { }, deviceScaleFactor: screenSize.dpr, }; - } else if (fn.name === 'getScreenshotAreaSize') { - return { - width: screenshotAreaSize.width, - height: screenshotAreaSize.height, - }; } else if (fn.name === 'waitForDoubleRaf') { return {}; } else { @@ -68,7 +60,6 @@ describe('FullPageScreenshot gatherer', () => { const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 412, height: 2000}; screenSize = {width: 412, height: 412}; - screenshotAreaSize = contentSize; mockContext.settings = { ...mockContext.settings, @@ -97,7 +88,6 @@ describe('FullPageScreenshot gatherer', () => { const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 412, height: 2000}; screenSize = {width: 412, height: 412}; - screenshotAreaSize = contentSize; mockContext.settings = { ...mockContext.settings, @@ -131,7 +121,6 @@ describe('FullPageScreenshot gatherer', () => { const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 500, height: 1500}; screenSize = {width: 500, height: 500, dpr: 2}; - screenshotAreaSize = contentSize; mockContext.settings = { ...mockContext.settings, screenEmulation: { @@ -157,7 +146,7 @@ describe('FullPageScreenshot gatherer', () => { mobile: true, deviceScaleFactor: 1, height: 1500, - width: 0, + width: 500, }) ); @@ -168,7 +157,7 @@ describe('FullPageScreenshot gatherer', () => { mobile: true, deviceScaleFactor: 2, height: 500, - width: 0, + width: 500, }) ); }); @@ -176,9 +165,8 @@ describe('FullPageScreenshot gatherer', () => { it('limits the screenshot height to the max Chrome can capture', async () => { const fpsGatherer = new FullPageScreenshotGatherer(); - contentSize = {width: 412, height: 100000}; + contentSize = {width: 100000, height: 100000}; screenSize = {width: 412, height: 412, dpr: 1}; - screenshotAreaSize = contentSize; mockContext.settings = { ...mockContext.settings, formFactor: 'mobile', @@ -197,7 +185,7 @@ describe('FullPageScreenshot gatherer', () => { { mobile: true, deviceScaleFactor: 1, - width: 0, + width: 412, // Horizontal overflow will be clipped to screen size. height: 16383, } ); From 82f9aba88bb7df68acc7f6f83bdbdf99e4ffdbbb Mon Sep 17 00:00:00 2001 From: "Alex N. Jose" Date: Wed, 22 Mar 2023 13:59:29 -0700 Subject: [PATCH 02/13] Update calculations for accounting viewport initial-scale --- core/gather/gatherers/full-page-screenshot.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js index 5142f9f65940..cc19136976f7 100644 --- a/core/gather/gatherers/full-page-screenshot.js +++ b/core/gather/gatherers/full-page-screenshot.js @@ -69,10 +69,12 @@ class FullPageScreenshot extends FRGatherer { const metrics = await session.sendCommand('Page.getLayoutMetrics'); // To obtain a full page screenshot, we resize the emulated viewport to - // (1) a height equal to the maximum of visual-viewport height and document height. + // (1) a height equal to the maximum between visual-viewport height and scaled document height. // (2) a width equal to emulated visual-viewport width (we choose to clip overflow on x-axis). - // Finally, we cap the viewport to a maximum size allowance of WebP format. - const fullHeight = Math.max(deviceMetrics.height, metrics.cssContentSize.height); + // Finally, we cap the viewport to the maximum size allowance of WebP format. + const fullHeight = Math.max( + deviceMetrics.height, metrics.cssContentSize.height * metrics.cssVisualViewport.scale + ); const height = Math.min(fullHeight, MAX_WEBP_SIZE); const width = Math.min(deviceMetrics.width, MAX_WEBP_SIZE); From 7d87db9f16e23bb0afd06fe16572a4b3b3e98ce9 Mon Sep 17 00:00:00 2001 From: "Alex N. Jose" Date: Wed, 22 Mar 2023 14:50:37 -0700 Subject: [PATCH 03/13] Update test to match TS definition of scale. --- core/test/gather/gatherers/full-page-screenshot-test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index feb78a205e6d..a50efcc646b0 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -25,7 +25,8 @@ beforeEach(() => { return { cssContentSize: contentSize, // See comment within _takeScreenshot() implementation - cssVisualViewport: {clientWidth: contentSize.width, clientHeight: contentSize.height}, + cssVisualViewport: {clientWidth: contentSize.width, clientHeight: contentSize.height, + scale: 1.0}, }; } if (method === 'Page.captureScreenshot') { From e582a12b8c7ae20c9c802ddb8964a7a1081912dc Mon Sep 17 00:00:00 2001 From: "Alex N. Jose" Date: Wed, 22 Mar 2023 15:20:56 -0700 Subject: [PATCH 04/13] Update review changes (Math.round + leaving width: 0) Apply scaling to output + overflow smoke tests. --- .../fixtures/scaled-overflow-content.html | 12 ++++ cli/test/smokehouse/core-tests.js | 2 + .../test-definitions/fps-overflow-x.js | 61 +++++++++++++++++++ .../smokehouse/test-definitions/fps-scaled.js | 4 +- core/gather/gatherers/full-page-screenshot.js | 23 ++++--- .../gatherers/full-page-screenshot-test.js | 8 +-- 6 files changed, 92 insertions(+), 18 deletions(-) create mode 100644 cli/test/fixtures/scaled-overflow-content.html create mode 100644 cli/test/smokehouse/test-definitions/fps-overflow-x.js diff --git a/cli/test/fixtures/scaled-overflow-content.html b/cli/test/fixtures/scaled-overflow-content.html new file mode 100644 index 000000000000..0717ea64d68d --- /dev/null +++ b/cli/test/fixtures/scaled-overflow-content.html @@ -0,0 +1,12 @@ + + + + + + + Document + + +

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

+ + \ No newline at end of file diff --git a/cli/test/smokehouse/core-tests.js b/cli/test/smokehouse/core-tests.js index 8cbd54e8778d..7b857a9d5333 100644 --- a/cli/test/smokehouse/core-tests.js +++ b/cli/test/smokehouse/core-tests.js @@ -17,6 +17,7 @@ import formsAutoComplete from './test-definitions/forms-autocomplete.js'; import fpsMax from './test-definitions/fps-max.js'; import fpsMaxPassive from './test-definitions/fps-max-passive.js'; import fpsScaled from './test-definitions/fps-scaled.js'; +import fpsOverflowX from './test-definitions/fps-overflow-x.js'; import issuesMixedContent from './test-definitions/issues-mixed-content.js'; import lanternFetch from './test-definitions/lantern-fetch.js'; import lanternIdleCallbackLong from './test-definitions/lantern-idle-callback-long.js'; @@ -80,6 +81,7 @@ const smokeTests = [ formsAutoComplete, fpsMax, fpsScaled, + fpsOverflowX, fpsMaxPassive, issuesMixedContent, lanternFetch, diff --git a/cli/test/smokehouse/test-definitions/fps-overflow-x.js b/cli/test/smokehouse/test-definitions/fps-overflow-x.js new file mode 100644 index 000000000000..b72552ef633f --- /dev/null +++ b/cli/test/smokehouse/test-definitions/fps-overflow-x.js @@ -0,0 +1,61 @@ +/** + * @license Copyright 2023 The Lighthouse Authors. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ + +/** @type {LH.Config} */ +const config = { + extends: 'lighthouse:default', + settings: { + onlyCategories: ['performance'], + }, +}; + +/** + * @type {Smokehouse.ExpectedRunnerResult} + */ +const expectations = { + artifacts: { + ViewportDimensions: { + innerWidth: 1325, + innerHeight: 2647, + outerWidth: 412, + outerHeight: 823, + devicePixelRatio: 1.75, + }, + }, + lhr: { + requestedUrl: 'http://localhost:10200/scaled-overflow-content.html', + finalDisplayedUrl: 'http://localhost:10200/scaled-overflow-content.html', + audits: {}, + fullPageScreenshot: { + nodes: { + _includes: [ + [ + /-BODY$/, + { + top: 21, + bottom: 58, + left: 8, + right: 816, + width: 808, + height: 37, + }, + ], + ], + }, + screenshot: { + height: 1324, + width: 412, + }, + }, + }, +}; + +export default { + id: 'fps-overflow-x', + expectations, + config, +}; + diff --git a/cli/test/smokehouse/test-definitions/fps-scaled.js b/cli/test/smokehouse/test-definitions/fps-scaled.js index 5e037e1a4795..3b19cbb84261 100644 --- a/cli/test/smokehouse/test-definitions/fps-scaled.js +++ b/cli/test/smokehouse/test-definitions/fps-scaled.js @@ -46,8 +46,8 @@ const expectations = { ], }, screenshot: { - height: 2000, - width: 824, + height: 1000, + width: 412, }, }, }, diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js index cc19136976f7..88b913ebbdd2 100644 --- a/core/gather/gatherers/full-page-screenshot.js +++ b/core/gather/gatherers/full-page-screenshot.js @@ -68,15 +68,14 @@ class FullPageScreenshot extends FRGatherer { const session = context.driver.defaultSession; const metrics = await session.sendCommand('Page.getLayoutMetrics'); - // To obtain a full page screenshot, we resize the emulated viewport to - // (1) a height equal to the maximum between visual-viewport height and scaled document height. - // (2) a width equal to emulated visual-viewport width (we choose to clip overflow on x-axis). - // Finally, we cap the viewport to the maximum size allowance of WebP format. - const fullHeight = Math.max( + // To obtain a full page screenshot, we resize the emulated viewport height to + // the maximum between visual-viewport height and the scaled document height. + // Final height is capped to the maximum size allowance of WebP format. + // Height needs to be rounded to an integer for Emulation.setDeviceMetricOverrides. + const fullHeight = Math.round(Math.max( deviceMetrics.height, metrics.cssContentSize.height * metrics.cssVisualViewport.scale - ); + )); const height = Math.min(fullHeight, MAX_WEBP_SIZE); - const width = Math.min(deviceMetrics.width, MAX_WEBP_SIZE); // Setup network monitor before we change the viewport. const networkMonitor = new NetworkMonitor(context.driver.targetManager); @@ -93,7 +92,7 @@ class FullPageScreenshot extends FRGatherer { mobile: deviceMetrics.mobile, deviceScaleFactor: 1, height, - width, + width: 0, // Leave width unchanged }); // Now that the viewport is taller, give the page some time to fetch new resources that @@ -123,9 +122,9 @@ class FullPageScreenshot extends FRGatherer { return { data, // Since we resized emulated viewport to match the desired screenshot size, - // it is safe to rely on visual viewport css dimensions. - width: metrics.cssVisualViewport.clientWidth, - height: metrics.cssVisualViewport.clientHeight, + // it is safe to rely on scaled visual viewport css dimensions. + width: Math.round(metrics.cssVisualViewport.clientWidth * metrics.cssVisualViewport.scale), + height: Math.round(metrics.cssVisualViewport.clientHeight * metrics.cssVisualViewport.scale), }; } @@ -233,7 +232,7 @@ class FullPageScreenshot extends FRGatherer { mobile: deviceMetrics.mobile, deviceScaleFactor: deviceMetrics.deviceScaleFactor, height: deviceMetrics.height, - width: deviceMetrics.width, + width: 0, // Leave width unchanged }); } } diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index a50efcc646b0..8ce9cffbd800 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -147,7 +147,7 @@ describe('FullPageScreenshot gatherer', () => { mobile: true, deviceScaleFactor: 1, height: 1500, - width: 500, + width: 0, }) ); @@ -158,7 +158,7 @@ describe('FullPageScreenshot gatherer', () => { mobile: true, deviceScaleFactor: 2, height: 500, - width: 500, + width: 0, }) ); }); @@ -166,7 +166,7 @@ describe('FullPageScreenshot gatherer', () => { it('limits the screenshot height to the max Chrome can capture', async () => { const fpsGatherer = new FullPageScreenshotGatherer(); - contentSize = {width: 100000, height: 100000}; + contentSize = {width: 412, height: 100000}; screenSize = {width: 412, height: 412, dpr: 1}; mockContext.settings = { ...mockContext.settings, @@ -186,7 +186,7 @@ describe('FullPageScreenshot gatherer', () => { { mobile: true, deviceScaleFactor: 1, - width: 412, // Horizontal overflow will be clipped to screen size. + width: 0, height: 16383, } ); From 189e7a3118e0dc8da19796ce224602f22b9e85f0 Mon Sep 17 00:00:00 2001 From: "Alex N. Jose" Date: Wed, 26 Jul 2023 16:10:29 -0700 Subject: [PATCH 05/13] account for macOS scrollbar --- core/gather/gatherers/full-page-screenshot.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js index bfc2dfea0c0e..1f82e9eb92ac 100644 --- a/core/gather/gatherers/full-page-screenshot.js +++ b/core/gather/gatherers/full-page-screenshot.js @@ -11,6 +11,8 @@ import * as emulation from '../../lib/emulation.js'; import {pageFunctions} from '../../lib/page-functions.js'; import {waitForNetworkIdle} from '../driver/wait-for-condition.js'; +/** @typedef {{width: number, height: number, deviceScaleFactor: number, mobile: boolean}} DeviceMetrics */ + // JPEG quality setting // Exploration and examples of reports using different quality settings: https://docs.google.com/document/d/1ZSffucIca9XDW2eEwfoevrk-OTl7WQFeMf0CgeJAA8M/edit# // Note: this analysis was done for JPEG, but now we use WEBP. @@ -37,6 +39,8 @@ function getObservedDeviceMetrics() { return { width: window.outerWidth, height: window.outerHeight, + innerWidth: window.innerWidth, + innerHeight: window.innerHeight, screenOrientation: { type: screenOrientationType, angle: window.screen.orientation.angle, @@ -61,7 +65,7 @@ class FullPageScreenshot extends BaseGatherer { /** * @param {LH.Gatherer.Context} context - * @param {{height: number, width: number, mobile: boolean}} deviceMetrics + * @param {DeviceMetrics} deviceMetrics */ async _resizeViewport(context, deviceMetrics) { const session = context.driver.defaultSession; @@ -107,9 +111,10 @@ class FullPageScreenshot extends BaseGatherer { /** * @param {LH.Gatherer.Context} context + * @param {DeviceMetrics} deviceMetrics * @return {Promise} */ - async _takeScreenshot(context) { + async _takeScreenshot(context, deviceMetrics) { const result = await context.driver.defaultSession.sendCommand('Page.captureScreenshot', { format: 'webp', quality: FULL_PAGE_SCREENSHOT_QUALITY, @@ -120,7 +125,8 @@ class FullPageScreenshot extends BaseGatherer { data, // Since we resized emulated viewport to match the desired screenshot size, // it is safe to rely on scaled visual viewport css dimensions. - width: Math.round(metrics.cssVisualViewport.clientWidth * metrics.cssVisualViewport.scale), + // Rely on device metrics width to account for scrollbar on macOS. + width: Math.round(deviceMetrics.width * metrics.cssVisualViewport.scale), height: Math.round(metrics.cssVisualViewport.clientHeight * metrics.cssVisualViewport.scale), }; } @@ -207,7 +213,10 @@ class FullPageScreenshot extends BaseGatherer { // Issue both commands at once, to reduce the chance that the page changes between capturing // a screenshot and resolving the nodes. https://github.com/GoogleChrome/lighthouse/pull/14763 const [screenshot, nodes] = - await Promise.all([this._takeScreenshot(context), this._resolveNodes(context)]); + await Promise.all([ + this._takeScreenshot(context, deviceMetrics), + this._resolveNodes(context), + ]); return { screenshot, nodes, From ffe0f783b92b3938a9c0c89b5fd5dbc8677fb659 Mon Sep 17 00:00:00 2001 From: "Alex N. Jose" Date: Wed, 26 Jul 2023 21:13:18 -0700 Subject: [PATCH 06/13] Dont scale deviceWidth that is already scaled --- core/gather/gatherers/full-page-screenshot.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js index 1f82e9eb92ac..791ef2344a90 100644 --- a/core/gather/gatherers/full-page-screenshot.js +++ b/core/gather/gatherers/full-page-screenshot.js @@ -123,10 +123,10 @@ class FullPageScreenshot extends BaseGatherer { const data = 'data:image/webp;base64,' + result.data; return { data, + // Rely on device metrics width that is already in scaled units to account for scrollbar on macOS. + width: deviceMetrics.width, // Since we resized emulated viewport to match the desired screenshot size, // it is safe to rely on scaled visual viewport css dimensions. - // Rely on device metrics width to account for scrollbar on macOS. - width: Math.round(deviceMetrics.width * metrics.cssVisualViewport.scale), height: Math.round(metrics.cssVisualViewport.clientHeight * metrics.cssVisualViewport.scale), }; } From 0af54f1fc53dd2a381f96ffba30e36f762a9a7b5 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Mar 2024 17:50:00 -0800 Subject: [PATCH 07/13] update --- cli/test/fixtures/scaled-content.html | 2 +- .../fixtures/scaled-overflow-content.html | 4 +- .../test-definitions/fps-overflow-x.js | 15 +++++++- .../smokehouse/test-definitions/fps-scaled.js | 15 +++++++- core/gather/gatherers/full-page-screenshot.js | 38 +++++++------------ 5 files changed, 43 insertions(+), 31 deletions(-) diff --git a/cli/test/fixtures/scaled-content.html b/cli/test/fixtures/scaled-content.html index 8213ff7783bd..df29587a641f 100644 --- a/cli/test/fixtures/scaled-content.html +++ b/cli/test/fixtures/scaled-content.html @@ -4,7 +4,7 @@
-

This content is not 1:1 with device pixels.

+

This content is not 1:1 with device pixels.

diff --git a/cli/test/fixtures/scaled-overflow-content.html b/cli/test/fixtures/scaled-overflow-content.html index 0717ea64d68d..97f1a3cbdd26 100644 --- a/cli/test/fixtures/scaled-overflow-content.html +++ b/cli/test/fixtures/scaled-overflow-content.html @@ -7,6 +7,6 @@ Document -

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

+

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

- \ No newline at end of file + diff --git a/cli/test/smokehouse/test-definitions/fps-overflow-x.js b/cli/test/smokehouse/test-definitions/fps-overflow-x.js index b72552ef633f..5da933f38f54 100644 --- a/cli/test/smokehouse/test-definitions/fps-overflow-x.js +++ b/cli/test/smokehouse/test-definitions/fps-overflow-x.js @@ -43,11 +43,22 @@ const expectations = { height: 37, }, ], + [ + /-H1$/, + { + top: 21, + bottom: 58, + left: 8, + right: 816, + width: 808, + height: 37, + }, + ], ], }, screenshot: { - height: 1324, - width: 412, + height: 1646, + width: 824, }, }, }, diff --git a/cli/test/smokehouse/test-definitions/fps-scaled.js b/cli/test/smokehouse/test-definitions/fps-scaled.js index f62ad1461d49..24abcaa17522 100644 --- a/cli/test/smokehouse/test-definitions/fps-scaled.js +++ b/cli/test/smokehouse/test-definitions/fps-scaled.js @@ -43,11 +43,22 @@ const expectations = { height: 2000, }, ], + [ + /-H1$/, + { + top: 0, + bottom: 37, + left: 0, + right: 824, + width: 824, + height: 37, + }, + ], ], }, screenshot: { - height: 1000, - width: 412, + height: 2000, + width: 824, }, }, }, diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js index ec27a1ae7c76..ad0861ee723e 100644 --- a/core/gather/gatherers/full-page-screenshot.js +++ b/core/gather/gatherers/full-page-screenshot.js @@ -10,8 +10,6 @@ import * as emulation from '../../lib/emulation.js'; import {pageFunctions} from '../../lib/page-functions.js'; import {waitForNetworkIdle} from '../driver/wait-for-condition.js'; -/** @typedef {{width: number, height: number, deviceScaleFactor: number, mobile: boolean}} DeviceMetrics */ - // JPEG quality setting // Exploration and examples of reports using different quality settings: https://docs.google.com/document/d/1ZSffucIca9XDW2eEwfoevrk-OTl7WQFeMf0CgeJAA8M/edit# // Note: this analysis was done for JPEG, but now we use WEBP. @@ -38,8 +36,6 @@ function getObservedDeviceMetrics() { return { width: window.outerWidth, height: window.outerHeight, - innerWidth: window.innerWidth, - innerHeight: window.innerHeight, screenOrientation: { type: screenOrientationType, angle: window.screen.orientation.angle, @@ -64,19 +60,19 @@ class FullPageScreenshot extends BaseGatherer { /** * @param {LH.Gatherer.Context} context - * @param {DeviceMetrics} deviceMetrics + * @param {{height: number, width: number, mobile: boolean}} deviceMetrics */ async _resizeViewport(context, deviceMetrics) { const session = context.driver.defaultSession; const metrics = await session.sendCommand('Page.getLayoutMetrics'); - // To obtain a full page screenshot, we resize the emulated viewport height to - // the maximum between visual-viewport height and the scaled document height. - // Final height is capped to the maximum size allowance of WebP format. - // Height needs to be rounded to an integer for Emulation.setDeviceMetricOverrides. - const fullHeight = Math.round(Math.max( - deviceMetrics.height, metrics.cssContentSize.height * metrics.cssVisualViewport.scale - )); + // Height should be as tall as the content. + // Scale the emulated height to reach the content height. + const fullHeight = Math.round( + deviceMetrics.height * + metrics.cssContentSize.height / + metrics.cssLayoutViewport.clientHeight + ); const height = Math.min(fullHeight, MAX_WEBP_SIZE); // Setup network monitor before we change the viewport. @@ -110,23 +106,20 @@ class FullPageScreenshot extends BaseGatherer { /** * @param {LH.Gatherer.Context} context - * @param {DeviceMetrics} deviceMetrics * @return {Promise} */ - async _takeScreenshot(context, deviceMetrics) { + async _takeScreenshot(context) { + const metrics = await context.driver.defaultSession.sendCommand('Page.getLayoutMetrics'); const result = await context.driver.defaultSession.sendCommand('Page.captureScreenshot', { format: 'webp', quality: FULL_PAGE_SCREENSHOT_QUALITY, }); - const metrics = await context.driver.defaultSession.sendCommand('Page.getLayoutMetrics'); const data = 'data:image/webp;base64,' + result.data; + return { data, - // Rely on device metrics width that is already in scaled units to account for scrollbar on macOS. - width: deviceMetrics.width, - // Since we resized emulated viewport to match the desired screenshot size, - // it is safe to rely on scaled visual viewport css dimensions. - height: Math.round(metrics.cssVisualViewport.clientHeight * metrics.cssVisualViewport.scale), + width: metrics.cssVisualViewport.clientWidth, + height: metrics.cssVisualViewport.clientHeight, }; } @@ -212,10 +205,7 @@ class FullPageScreenshot extends BaseGatherer { // Issue both commands at once, to reduce the chance that the page changes between capturing // a screenshot and resolving the nodes. https://github.com/GoogleChrome/lighthouse/pull/14763 const [screenshot, nodes] = - await Promise.all([ - this._takeScreenshot(context, deviceMetrics), - this._resolveNodes(context), - ]); + await Promise.all([this._takeScreenshot(context), this._resolveNodes(context)]); return { screenshot, nodes, From 0ad4c7ea76cd1108f77bd535c9d200168c18a25a Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Mar 2024 18:03:29 -0800 Subject: [PATCH 08/13] unit --- .../test/gather/gatherers/full-page-screenshot-test.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index 772ebd4d1981..f88acc5b0701 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -23,9 +23,15 @@ beforeEach(() => { if (method === 'Page.getLayoutMetrics') { return { cssContentSize: contentSize, + // This is only accessed on the first call to Page.getLayoutMetrics + // At that time the width and height should match the screen size. + cssLayoutViewport: {clientWidth: screenSize.width, clientHeight: screenSize.height}, + // This is only accessed on the second call to Page.getLayoutMetrics + // At that time the width should be the same as the layout width but the height will + // have been resized to reach the content height. + // This will represent the final screenshot area size. // See comment within _takeScreenshot() implementation - cssVisualViewport: {clientWidth: contentSize.width, clientHeight: contentSize.height, - scale: 1.0}, + cssVisualViewport: {clientWidth: screenSize.width, clientHeight: contentSize.height}, }; } if (method === 'Page.captureScreenshot') { From 59da4f238895cf5c2dcbbd14fde349e011f67286 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Mar 2024 18:07:57 -0800 Subject: [PATCH 09/13] fix 2 --- .../gatherers/full-page-screenshot-test.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index f88acc5b0701..e358fe8ac813 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -10,6 +10,8 @@ import FullPageScreenshotGatherer from '../../../gather/gatherers/full-page-scre let contentSize; /** @type {{width?: number, height?: number, dpr: number}} */ let screenSize; +/** @type {{width?: number, height?: number}} */ +let screenshotSize; /** @type {string[]} */ let screenshotData; let mockContext = createMockContext(); @@ -17,6 +19,7 @@ let mockContext = createMockContext(); beforeEach(() => { contentSize = {width: 100, height: 100}; screenSize = {width: 100, height: 100, dpr: 1}; + screenshotSize = contentSize; screenshotData = []; mockContext = createMockContext(); mockContext.driver.defaultSession.sendCommand.mockImplementation((method) => { @@ -27,11 +30,8 @@ beforeEach(() => { // At that time the width and height should match the screen size. cssLayoutViewport: {clientWidth: screenSize.width, clientHeight: screenSize.height}, // This is only accessed on the second call to Page.getLayoutMetrics - // At that time the width should be the same as the layout width but the height will - // have been resized to reach the content height. - // This will represent the final screenshot area size. - // See comment within _takeScreenshot() implementation - cssVisualViewport: {clientWidth: screenSize.width, clientHeight: contentSize.height}, + // At that time the width and height should match the screenshot size. + cssVisualViewport: {clientWidth: screenshotSize.width, clientHeight: screenshotSize.height}, }; } if (method === 'Page.captureScreenshot') { @@ -53,6 +53,11 @@ beforeEach(() => { }, deviceScaleFactor: screenSize.dpr, }; + } else if (fn.name === 'getScreenshotAreaSize') { + return { + width: screenshotSize.width, + height: screenshotSize.height, + }; } else if (fn.name === 'waitForDoubleRaf') { return {}; } else { @@ -66,6 +71,7 @@ describe('FullPageScreenshot gatherer', () => { const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 412, height: 2000}; screenSize = {width: 412, height: 412}; + screenshotSize = contentSize; mockContext.settings = { ...mockContext.settings, @@ -94,6 +100,7 @@ describe('FullPageScreenshot gatherer', () => { const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 412, height: 2000}; screenSize = {width: 412, height: 412}; + screenshotSize = contentSize; mockContext.settings = { ...mockContext.settings, @@ -127,6 +134,7 @@ describe('FullPageScreenshot gatherer', () => { const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 500, height: 1500}; screenSize = {width: 500, height: 500, dpr: 2}; + screenshotSize = contentSize; mockContext.settings = { ...mockContext.settings, screenEmulation: { @@ -173,6 +181,7 @@ describe('FullPageScreenshot gatherer', () => { contentSize = {width: 412, height: 100000}; screenSize = {width: 412, height: 412, dpr: 1}; + screenshotSize = contentSize; mockContext.settings = { ...mockContext.settings, formFactor: 'mobile', From d2fabd1a0f88adc4d986a1a6e9ecfe3a16fd31cf Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Mar 2024 18:09:17 -0800 Subject: [PATCH 10/13] ope --- core/test/gather/gatherers/full-page-screenshot-test.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index e358fe8ac813..9a6675b4c814 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -53,11 +53,6 @@ beforeEach(() => { }, deviceScaleFactor: screenSize.dpr, }; - } else if (fn.name === 'getScreenshotAreaSize') { - return { - width: screenshotSize.width, - height: screenshotSize.height, - }; } else if (fn.name === 'waitForDoubleRaf') { return {}; } else { From 10cc44236a81983878b6d1176f60843cd1ec64ff Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Mar 2024 18:12:40 -0800 Subject: [PATCH 11/13] ope --- core/test/gather/gatherers/full-page-screenshot-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index 9a6675b4c814..5eed053adecb 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -56,6 +56,7 @@ beforeEach(() => { } else if (fn.name === 'waitForDoubleRaf') { return {}; } else { + throw new Error(`unexpected fn ${fn.name}`); } }); From 74e8015c9b9302bf042c3c549b0995e711bdbcc5 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Mar 2024 18:12:50 -0800 Subject: [PATCH 12/13] ope --- core/test/gather/gatherers/full-page-screenshot-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index 5eed053adecb..9a6675b4c814 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -56,7 +56,6 @@ beforeEach(() => { } else if (fn.name === 'waitForDoubleRaf') { return {}; } else { - throw new Error(`unexpected fn ${fn.name}`); } }); From 468c7f93705a4343a6575e16fee6a199d038c322 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Mar 2024 18:26:19 -0800 Subject: [PATCH 13/13] faster test --- core/gather/gatherers/full-page-screenshot.js | 25 ++++++++++++------- .../gatherers/full-page-screenshot-test.js | 15 +++++++---- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/core/gather/gatherers/full-page-screenshot.js b/core/gather/gatherers/full-page-screenshot.js index ad0861ee723e..1d8ca13deb73 100644 --- a/core/gather/gatherers/full-page-screenshot.js +++ b/core/gather/gatherers/full-page-screenshot.js @@ -58,6 +58,21 @@ class FullPageScreenshot extends BaseGatherer { supportedModes: ['snapshot', 'timespan', 'navigation'], }; + /** + * @param {LH.Gatherer.Context} context + */ + waitForNetworkIdle(context) { + const session = context.driver.defaultSession; + const networkMonitor = context.driver.networkMonitor; + return waitForNetworkIdle(session, networkMonitor, { + pretendDCLAlreadyFired: true, + networkQuietThresholdMs: 1000, + busyEvent: 'network-critical-busy', + idleEvent: 'network-critical-idle', + isIdle: recorder => recorder.isCriticalIdle(), + }); + } + /** * @param {LH.Gatherer.Context} context * @param {{height: number, width: number, mobile: boolean}} deviceMetrics @@ -75,15 +90,7 @@ class FullPageScreenshot extends BaseGatherer { ); const height = Math.min(fullHeight, MAX_WEBP_SIZE); - // Setup network monitor before we change the viewport. - const networkMonitor = context.driver.networkMonitor; - const waitForNetworkIdleResult = waitForNetworkIdle(session, networkMonitor, { - pretendDCLAlreadyFired: true, - networkQuietThresholdMs: 1000, - busyEvent: 'network-critical-busy', - idleEvent: 'network-critical-idle', - isIdle: recorder => recorder.isCriticalIdle(), - }); + const waitForNetworkIdleResult = this.waitForNetworkIdle(context); await session.sendCommand('Emulation.setDeviceMetricsOverride', { mobile: deviceMetrics.mobile, diff --git a/core/test/gather/gatherers/full-page-screenshot-test.js b/core/test/gather/gatherers/full-page-screenshot-test.js index 9a6675b4c814..bc0f5924d47f 100644 --- a/core/test/gather/gatherers/full-page-screenshot-test.js +++ b/core/test/gather/gatherers/full-page-screenshot-test.js @@ -5,6 +5,7 @@ import {createMockContext} from '../../gather/mock-driver.js'; import FullPageScreenshotGatherer from '../../../gather/gatherers/full-page-screenshot.js'; +import {fnAny} from '../../test-utils.js'; /** @type {{width: number, height: number}} */ let contentSize; @@ -15,8 +16,17 @@ let screenshotSize; /** @type {string[]} */ let screenshotData; let mockContext = createMockContext(); +let fpsGatherer = new FullPageScreenshotGatherer(); beforeEach(() => { + fpsGatherer = new FullPageScreenshotGatherer(); + + // Prevent `waitForNetworkIdle` from stalling the tests + fpsGatherer.waitForNetworkIdle = fnAny().mockImplementation(() => ({ + promise: Promise.resolve(), + cancel: fnAny(), + })); + contentSize = {width: 100, height: 100}; screenSize = {width: 100, height: 100, dpr: 1}; screenshotSize = contentSize; @@ -63,7 +73,6 @@ beforeEach(() => { describe('FullPageScreenshot gatherer', () => { it('captures a full-page screenshot', async () => { - const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 412, height: 2000}; screenSize = {width: 412, height: 412}; screenshotSize = contentSize; @@ -92,7 +101,6 @@ describe('FullPageScreenshot gatherer', () => { }); it('resets the emulation correctly when Lighthouse controls it', async () => { - const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 412, height: 2000}; screenSize = {width: 412, height: 412}; screenshotSize = contentSize; @@ -126,7 +134,6 @@ describe('FullPageScreenshot gatherer', () => { }); it('resets the emulation correctly when Lighthouse does not control it', async () => { - const fpsGatherer = new FullPageScreenshotGatherer(); contentSize = {width: 500, height: 1500}; screenSize = {width: 500, height: 500, dpr: 2}; screenshotSize = contentSize; @@ -172,8 +179,6 @@ describe('FullPageScreenshot gatherer', () => { }); it('limits the screenshot height to the max Chrome can capture', async () => { - const fpsGatherer = new FullPageScreenshotGatherer(); - contentSize = {width: 412, height: 100000}; screenSize = {width: 412, height: 412, dpr: 1}; screenshotSize = contentSize;