[screenshotting]fix resize during capture [7.17] (#132294)

* [screenshotting]fix resize during capture [7.17]

* fix ts in unit test file

* remove unused translations

* fix tests

* fix some tests

* fix more testing

* minimal diff

* do not crash if width has a decimal

x-pack/plugins/reporting/common/constants.ts

@@ -62,6 +62,7 @@ export const LAYOUT_TYPES = {
 export const DEFAULT_VIEWPORT = {
   width: 1950,
   height: 1200,
+  deviceScaleFactor: 1,
 };
 
 // Export Type Definitions

x-pack/plugins/reporting/server/browsers/chromium/driver/chromium_driver.ts

@@ -175,6 +175,7 @@ export class HeadlessChromiumDriver {
         height: boundingClientRect.height,
         width: boundingClientRect.width,
       },
+      captureBeyondViewport: false, // workaround for an internal resize. See: https://github.com/puppeteer/puppeteer/issues/7043
     });
 
     if (Buffer.isBuffer(screenshot)) {
@@ -255,7 +256,7 @@ export class HeadlessChromiumDriver {
     const width = Math.floor(_width);
     const height = Math.floor(_height);
 
-    logger.debug(`Setting viewport to: width=${width} height=${height} zoom=${zoom}`);
+    logger.debug(`Setting viewport to: width=${width} height=${height} scaleFactor=${zoom}`);
 
     await this.page.setViewport({
       width,

x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts

@@ -17,6 +17,7 @@ import { InnerSubscriber } from 'rxjs/internal/InnerSubscriber';
 import { ignoreElements, map, mergeMap, tap } from 'rxjs/operators';
 import { getChromiumDisconnectedError } from '../';
 import { ReportingCore } from '../../..';
+import { DEFAULT_VIEWPORT } from '../../../../common/constants';
 import { durationToNumber } from '../../../../common/schema_utils';
 import { CaptureConfig } from '../../../../server/types';
 import { LevelLogger } from '../../../lib';
@@ -25,6 +26,11 @@ import { HeadlessChromiumDriver } from '../driver';
 import { args } from './args';
 import { getMetrics, Metrics } from './metrics';
 
+interface CreatePageOptions {
+  browserTimezone?: string;
+  defaultViewport: { width?: number };
+}
+
 type BrowserConfig = CaptureConfig['browser']['chromium'];
 
 export class HeadlessChromiumDriverFactory {
@@ -61,14 +67,25 @@ export class HeadlessChromiumDriverFactory {
    * Return an observable to objects which will drive screenshot capture for a page
    */
   createPage(
-    { browserTimezone }: { browserTimezone?: string },
+    { browserTimezone, defaultViewport }: CreatePageOptions,
     pLogger: LevelLogger
   ): Rx.Observable<{ driver: HeadlessChromiumDriver; exit$: Rx.Observable<never> }> {
     // FIXME: 'create' is deprecated
     return Rx.Observable.create(async (observer: InnerSubscriber<unknown, unknown>) => {
       const logger = pLogger.clone(['browser-driver']);
       logger.info(`Creating browser page driver`);
 
+      // We set the viewport width using the client-side layout info to reduce the chances of
+      // browser reflow. Only the window height is expected to be adjusted dramatically
+      // before taking a screenshot, to ensure the elements to capture are contained in the viewport.
+      const viewport = {
+        ...DEFAULT_VIEWPORT,
+        width: defaultViewport.width ?? DEFAULT_VIEWPORT.width,
+      };
+
+      logger.debug(
+        `Launching with viewport: width=${viewport.width} height=${viewport.height} scaleFactor=${viewport.deviceScaleFactor}`
+      );
       const chromiumArgs = this.getChromiumArgs();
       logger.debug(`Chromium launch args set to: ${chromiumArgs}`);
 
@@ -85,6 +102,7 @@ export class HeadlessChromiumDriverFactory {
           ignoreHTTPSErrors: true,
           handleSIGHUP: false,
           args: chromiumArgs,
+          defaultViewport: viewport,
           env: {
             TZ: browserTimezone,
           },

x-pack/plugins/reporting/server/export_types/common/generate_png.ts

@@ -39,6 +39,7 @@ export async function generatePngObservableFactory(reporting: ReportingCore) {
 
     const apmScreenshots = apmTrans?.startSpan('screenshots-pipeline', 'setup');
     let apmBuffer: typeof apm.currentSpan;
+    logger.debug(`Layout: id=${layout.id} width=${layout.width} height=${layout.height}`);
     const screenshots$ = getScreenshots$(captureConfig, browserDriverFactory, {
       logger,
       urlsOrUrlLocatorTuples: [urlOrUrlLocatorTuple],

x-pack/plugins/reporting/server/export_types/printable_pdf_v2/lib/generate_pdf.ts

@@ -48,7 +48,6 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
     tracker.startLayout();
 
     const layout = createLayout(captureConfig, layoutParams);
-    logger.debug(`Layout: width=${layout.width} height=${layout.height}`);
     tracker.endLayout();
 
     tracker.startScreenshots();
@@ -60,6 +59,7 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
       getFullRedirectAppUrl(reporting.getConfig(), job.spaceId, job.forceNow)
     );
 
+    logger.debug(`Layout: id=${layout.id} width=${layout.width} height=${layout.height}`);
     const screenshots$ = getScreenshots$(captureConfig, browserDriverFactory, {
       logger,
       urlsOrUrlLocatorTuples: zip(urls, locatorParams) as UrlOrUrlLocatorTuple[],

x-pack/plugins/reporting/server/lib/layouts/canvas_layout.ts

@@ -38,10 +38,14 @@ export class CanvasLayout extends Layout implements LayoutInstance {
 
   constructor(size: Size) {
     super(LayoutTypes.CANVAS);
-    this.height = size.height;
-    this.width = size.width;
-    this.scaledHeight = size.height * ZOOM;
-    this.scaledWidth = size.width * ZOOM;
+
+    const height = Math.round(size.height);
+    this.height = height;
+    this.scaledHeight = height * ZOOM;
+
+    const width = Math.round(size.width);
+    this.width = width;
+    this.scaledWidth = width * ZOOM;
   }
 
   public getPdfPageOrientation() {

x-pack/plugins/reporting/server/lib/layouts/create_layout.test.ts

@@ -72,6 +72,7 @@ describe('Create Layout', () => {
         },
         "useReportingBranding": true,
         "viewport": Object {
+          "deviceScaleFactor": 1,
           "height": 1200,
           "width": 1950,
         },

x-pack/plugins/reporting/server/lib/layouts/layout.ts

@@ -34,9 +34,16 @@ export abstract class Layout {
     pageSizeParams: PageSizeParams
   ): CustomPageSize | PredefinedPageSize;
 
-  // Return the dimensions unscaled dimensions (before multiplying the zoom factor)
-  // driver.setViewport() Adds a top and left margin to the viewport, and then multiplies by the scaling factor
-  public abstract getViewport(itemsCount: number): ViewZoomWidthHeight | null;
+  /**
+   * Return the unscaled dimensions (before multiplying the zoom factor)
+   *
+   * `itemsCount` is only needed for the `print` layout implementation, where the number of items to capture
+   * affects the viewport size
+   *
+   * @param {number} [itemsCount=1] - The number of items to capture. Default is 1.
+   * @returns ViewZoomWidthHeight - Viewport data
+   */
+  public abstract getViewport(itemsCount?: number): ViewZoomWidthHeight | null;
 
   public abstract getBrowserZoom(): number;
 

x-pack/plugins/reporting/server/lib/layouts/preserve_layout.ts

@@ -24,10 +24,14 @@ export class PreserveLayout extends Layout implements LayoutInstance {
 
   constructor(size: Size, selectors?: Partial<LayoutSelectorDictionary>) {
     super(LAYOUT_TYPES.PRESERVE_LAYOUT);
-    this.height = size.height;
-    this.width = size.width;
-    this.scaledHeight = size.height * ZOOM;
-    this.scaledWidth = size.width * ZOOM;
+
+    const height = Math.round(size.height);
+    this.height = height;
+    this.scaledHeight = height * ZOOM;
+
+    const width = Math.round(size.width);
+    this.width = width;
+    this.scaledWidth = width * ZOOM;
 
     this.selectors = {
       ...getDefaultLayoutSelectors(),

x-pack/plugins/reporting/server/lib/screenshots/constants.ts

@@ -17,3 +17,4 @@ export const CONTEXT_ELEMENTATTRIBUTES = 'ElementPositionAndAttributes';
 export const CONTEXT_WAITFORELEMENTSTOBEINDOM = 'WaitForElementsToBeInDOM';
 export const CONTEXT_SKIPTELEMETRY = 'SkipTelemetry';
 export const CONTEXT_READMETADATA = 'ReadVisualizationsMetadata';
+export const CONTEXT_DEBUG = 'Debug';

x-pack/plugins/reporting/server/lib/screenshots/get_element_position_data.test.ts

@@ -58,8 +58,8 @@ describe('getElementPositionAndAttributes', () => {
           getBoundingClientRect: () => ({
             width: parseFloat(element.style.width),
             height: parseFloat(element.style.height),
-            top: parseFloat(element.style.top),
-            left: parseFloat(element.style.left),
+            y: parseFloat(element.style.top),
+            x: parseFloat(element.style.left),
           }),
         })
       );

x-pack/plugins/reporting/server/lib/screenshots/get_element_position_data.ts

@@ -32,9 +32,8 @@ export const getElementPositionAndAttributes = async (
             results.push({
               position: {
                 boundingClientRect: {
-                  // modern browsers support x/y, but older ones don't
-                  top: boundingClientRect.y || boundingClientRect.top,
-                  left: boundingClientRect.x || boundingClientRect.left,
+                  top: boundingClientRect.y,
+                  left: boundingClientRect.x,
                   width: boundingClientRect.width,
                   height: boundingClientRect.height,
                 },

x-pack/plugins/reporting/server/lib/screenshots/get_screenshots.test.ts

@@ -8,10 +8,13 @@
 import { HeadlessChromiumDriver } from '../../browsers';
 import {
   createMockBrowserDriverFactory,
+  createMockConfig,
   createMockConfigSchema,
+  createMockLayoutInstance,
   createMockLevelLogger,
   createMockReportingCore,
 } from '../../test_helpers';
+import { LayoutInstance } from '../layouts';
 import { getScreenshots } from './get_screenshots';
 
 describe('getScreenshots', () => {
@@ -34,6 +37,7 @@ describe('getScreenshots', () => {
 
   let logger: ReturnType<typeof createMockLevelLogger>;
   let browser: jest.Mocked<HeadlessChromiumDriver>;
+  let layout: LayoutInstance;
 
   beforeEach(async () => {
     const core = await createMockReportingCore(createMockConfigSchema());
@@ -56,14 +60,16 @@ describe('getScreenshots', () => {
         return jest.fn();
       },
     });
+    const config = createMockConfig(createMockConfigSchema());
+    layout = createMockLayoutInstance(config.get('capture'));
   });
 
   afterEach(() => {
     jest.clearAllMocks();
   });
 
   it('should return screenshots', async () => {
-    await expect(getScreenshots(browser, elementsPositionAndAttributes, logger)).resolves
+    await expect(getScreenshots(browser, layout, elementsPositionAndAttributes, logger)).resolves
       .toMatchInlineSnapshot(`
             Array [
               Object {
@@ -109,7 +115,7 @@ describe('getScreenshots', () => {
   });
 
   it('should forward elements positions', async () => {
-    await getScreenshots(browser, elementsPositionAndAttributes, logger);
+    await getScreenshots(browser, layout, elementsPositionAndAttributes, logger);
 
     expect(browser.screenshot).toHaveBeenCalledTimes(2);
     expect(browser.screenshot).toHaveBeenNthCalledWith(
@@ -126,7 +132,7 @@ describe('getScreenshots', () => {
     browser.screenshot.mockResolvedValue(Buffer.from(''));
 
     await expect(
-      getScreenshots(browser, elementsPositionAndAttributes, logger)
+      getScreenshots(browser, layout, elementsPositionAndAttributes, logger)
     ).rejects.toBeInstanceOf(Error);
   });
 });

x-pack/plugins/reporting/server/lib/screenshots/get_screenshots.ts

@@ -5,28 +5,68 @@
  * 2.0.
  */
 
-import { i18n } from '@kbn/i18n';
 import { LevelLogger, startTrace } from '../';
 import { HeadlessChromiumDriver } from '../../browsers';
 import { ElementsPositionAndAttribute, Screenshot } from './';
+import { LayoutInstance } from '../layouts';
 
+/**
+ * Resize the viewport to contain the element to capture.
+ *
+ * @async
+ * @param {HeadlessChromiumDriver} browser - used for its methods to control the page
+ * @param {ElementsPositionAndAttribute['position']} position - position data for the element to capture
+ * @param {Layout} layout - used for client-side layout data from the job params
+ * @param {Logger} logger
+ */
+const resizeViewport = async (
+  browser: HeadlessChromiumDriver,
+  position: ElementsPositionAndAttribute['position'],
+  layout: LayoutInstance,
+  logger: LevelLogger
+) => {
+  const { boundingClientRect, scroll } = position;
+
+  // Using width from the layout is preferred, it avoids the elements moving around horizontally,
+  // which would invalidate the position data that was passed in.
+  const width = layout.width || boundingClientRect.left + scroll.x + boundingClientRect.width;
+
+  await browser.setViewport(
+    {
+      width,
+      height: boundingClientRect.top + scroll.y + boundingClientRect.height,
+      zoom: layout.getBrowserZoom(),
+    },
+    logger
+  );
+};
+
+/**
+ * Get screenshots of multiple areas of the page
+ *
+ * @async
+ * @param {HeadlessChromiumDriver} browser - used for its methods to control the page
+ * @param {Logger} logger
+ * @param {ElementsPositionAndAttribute[]} elementsPositionAndAttributes[] - position data about all the elements to capture
+ * @param {Layout} layout - used for client-side layout data from the job params
+ * @returns {Promise<Screenshot[]>}
+ */
 export const getScreenshots = async (
   browser: HeadlessChromiumDriver,
+  layout: LayoutInstance,
   elementsPositionAndAttributes: ElementsPositionAndAttribute[],
   logger: LevelLogger
 ): Promise<Screenshot[]> => {
-  logger.info(
-    i18n.translate('xpack.reporting.screencapture.takingScreenshots', {
-      defaultMessage: `taking screenshots`,
-    })
-  );
+  logger.info(`taking screenshots`);
 
   const screenshots: Screenshot[] = [];
 
   for (let i = 0; i < elementsPositionAndAttributes.length; i++) {
     const endTrace = startTrace('get_screenshots', 'read');
     const item = elementsPositionAndAttributes[i];
 
+    await resizeViewport(browser, item.position, layout, logger);
+
     const data = await browser.screenshot(item.position);
 
     if (!data?.byteLength) {
@@ -42,14 +82,7 @@ export const getScreenshots = async (
     endTrace();
   }
 
-  logger.info(
-    i18n.translate('xpack.reporting.screencapture.screenshotsTaken', {
-      defaultMessage: `screenshots taken: {numScreenhots}`,
-      values: {
-        numScreenhots: screenshots.length,
-      },
-    })
-  );
+  logger.info(`screenshots taken: ${screenshots.length}`);
 
   return screenshots;
 };

x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts

@@ -450,7 +450,7 @@ describe('Screenshot Observable Pipeline', () => {
                     "height": 1200,
                     "left": 0,
                     "top": 0,
-                    "width": 1800,
+                    "width": 1950,
                   },
                   "scroll": Object {
                     "x": 0,