-
Notifications
You must be signed in to change notification settings - Fork 324
fix(replay): force preserveDrawingBuffer before the recorder chunk loads #4543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
c9167fe
fix(replay): force preserveDrawingBuffer before the recorder chunk loads
ksvat b035272
fix(replay): do not make the canvas patch wait on a persisted remote …
ksvat fd3679b
chore(replay): record the new private property in the terser mangle list
ksvat da13054
test(replay): prove a pre-recorder WebGL canvas captures its pixels
ksvat 5c0d1c4
test(replay): skip the new canvas compat test on array.js without the…
ksvat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'posthog-js': patch | ||
| --- | ||
|
|
||
| Fix WebGL canvases replaying blank when the page creates its rendering context while loading. A WebGL context created with `preserveDrawingBuffer: false` - the spec default, and what most renderers ask for - lets the browser discard the drawn pixels once the frame has been composited, so the frames replay captures come back empty. The recorder already forces the attribute on when it patches `getContext`, but it could only do that once the lazily loaded recorder bundle had arrived, and context attributes cannot be changed after creation - so any renderer that booted with the page had already created a context that could never be captured. That patch now also runs synchronously during `posthog.init()` whenever canvas recording is already known to be on: declaring `session_recording.captureCanvas.recordCanvas` covers the first page load, and enabling canvas recording in project settings alone covers every load after the one that persists the remote config. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
149 changes: 149 additions & 0 deletions
149
packages/browser/playwright/mocked/session-recording/canvas-preserve-drawing-buffer.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| import { expect, test } from '../utils/posthog-playwright-test-base' | ||
| import { start, waitForSessionRecordingToStart } from '../utils/setup' | ||
| import { BrowserContext, Page } from '@playwright/test' | ||
|
|
||
| // A WebGL context created with preserveDrawingBuffer: false lets the browser discard the drawn | ||
| // pixels once the frame has been composited, so canvas capture reads back an empty buffer. rrweb | ||
| // forces the attribute on, but only from the moment the lazily loaded recorder bundle patches | ||
| // getContext - too late for a renderer that boots with the page, because context attributes are | ||
| // fixed at creation time. | ||
| // | ||
| // This drives that exact ordering: the canvas is created from posthog's `loaded` callback, i.e. | ||
| // after init() (where the fix patches getContext) but before remote config has come back and the | ||
| // recorder chunk has loaded. It then draws ONCE and never repaints, like an editor or viewer | ||
| // sitting idle - a canvas that repaints every frame can land fresh pixels in the buffer by luck, | ||
| // which would hide the bug. | ||
| // | ||
| // The drawn content is a red fill, and the context is left with a transparent clear colour, so | ||
| // rrweb's "canvas loaded before rrweb" rescue hack (a bare gl.clear) wipes the canvas rather than | ||
| // happening to repaint it red. Only preserving the drawing buffer keeps the red. | ||
|
|
||
| const CANVAS_SIZE = { width: 300, height: 200 } | ||
|
|
||
| function canvasFrameBase64s(events: any[]): string[] { | ||
| const frames: string[] = [] | ||
| for (const e of events.filter((ev) => ev.event === '$snapshot')) { | ||
| for (const snap of e.properties?.$snapshot_data || []) { | ||
| // rrweb IncrementalSnapshot (3) with CanvasMutation source (9) | ||
| if (snap.type !== 3 || snap.data?.source !== 9) { | ||
| continue | ||
| } | ||
| const drawImage = (snap.data.commands || []).find((c: any) => c.property === 'drawImage') | ||
| const base64 = drawImage?.args?.[0]?.args?.[0]?.data?.[0]?.base64 | ||
| if (base64) { | ||
| frames.push(base64) | ||
| } | ||
| } | ||
| } | ||
| return frames | ||
| } | ||
|
|
||
| test.describe('canvas capture of a context created before the recorder loads', () => { | ||
| test('keeps the drawn pixels instead of capturing a blank frame', async ({ | ||
| page, | ||
| context, | ||
| browserName, | ||
| }: { | ||
| page: Page | ||
| context: BrowserContext | ||
| browserName: string | ||
| }) => { | ||
| // canvas FPS capture emits no canvas mutations under Playwright's headless WebKit, and | ||
| // WebGL under headless Firefox is unreliable - rrweb's own canvas FPS tests are | ||
| // chromium-only for the same reasons | ||
| test.skip(browserName !== 'chromium', 'canvas FPS capture of WebGL is only reliable under chromium') | ||
|
|
||
| await start( | ||
| { | ||
| options: { | ||
| session_recording: { | ||
| compress_events: false, | ||
| captureCanvas: { recordCanvas: true, canvasFps: 8, canvasQuality: 1 }, | ||
| }, | ||
| }, | ||
| flagsResponseOverrides: { | ||
| sessionRecording: { endpoint: '/ses/' }, | ||
| capturePerformance: true, | ||
| autocapture_opt_out: true, | ||
| }, | ||
| url: './playground/cypress/index.html', | ||
| // `loaded` fires at the end of init(), after extensions have initialized and so | ||
| // after the fix has patched getContext, but before remote config has returned and | ||
| // the recorder chunk has loaded. Registering the draw here (rather than driving it | ||
| // from the test) keeps that ordering inside the page, with no round trip to race. | ||
| runBeforePostHogInit: (pg) => { | ||
| void pg.evaluate(({ width, height }) => { | ||
| ;(window as any).__ph_loaded = () => { | ||
| const canvas = document.createElement('canvas') | ||
| canvas.width = width | ||
| canvas.height = height | ||
| canvas.style.width = width + 'px' | ||
| canvas.style.height = height + 'px' | ||
| document.body.appendChild(canvas) | ||
|
|
||
| // deliberately no context attributes: preserveDrawingBuffer defaults to false | ||
| const gl = canvas.getContext('webgl') as WebGLRenderingContext | null | ||
| if (!gl) { | ||
| return | ||
| } | ||
|
|
||
| // fill red through a scissored clear, so no shader plumbing is needed | ||
| gl.enable(gl.SCISSOR_TEST) | ||
| gl.scissor(0, 0, width, height) | ||
| gl.clearColor(1, 0, 0, 1) | ||
| gl.clear(gl.COLOR_BUFFER_BIT) | ||
| gl.disable(gl.SCISSOR_TEST) | ||
|
|
||
| // leave the clear colour transparent: a bare gl.clear() now wipes | ||
| // the canvas rather than happening to repaint it red | ||
| gl.clearColor(0, 0, 0, 0) | ||
| gl.finish() | ||
| ;(window as any).__webglCanvasDrawn = true | ||
| } | ||
| }, CANVAS_SIZE) | ||
| }, | ||
| }, | ||
| page, | ||
| context | ||
| ) | ||
|
|
||
| await waitForSessionRecordingToStart(page) | ||
|
|
||
| // the canvas must have been created before the recorder was ready, not by us afterwards | ||
| expect(await page.evaluate(() => (window as any).__webglCanvasDrawn === true)).toBe(true) | ||
|
|
||
| // let the FPS snapshot loop run, then flush. the canvas is never repainted in this window. | ||
| await page.locator('[data-cy-input]').type('x') | ||
| await page.waitForTimeout(1500) | ||
| await page.evaluate(() => (window as any).posthog?.capture('flush')) | ||
| await page.waitForTimeout(800) | ||
|
|
||
| const frames = canvasFrameBase64s((await page.capturedEvents()) || []) | ||
|
|
||
| // without the fix the buffer is discarded, every frame encodes as fully transparent, and | ||
| // the worker's fingerprint dedup drops them all - so there is nothing here at all | ||
| expect(frames.length).toBeGreaterThan(0) | ||
|
|
||
| const centrePixel = await page.evaluate( | ||
| async (base64: string) => { | ||
| const img = new Image() | ||
| img.src = 'data:image/webp;base64,' + base64 | ||
| await img.decode() | ||
| const readback = document.createElement('canvas') | ||
| readback.width = img.width | ||
| readback.height = img.height | ||
| const ctx = readback.getContext('2d')! | ||
| ctx.drawImage(img, 0, 0) | ||
| const [r, g, b, a] = ctx.getImageData(Math.floor(img.width / 2), Math.floor(img.height / 2), 1, 1).data | ||
| return { r, g, b, a } | ||
| }, | ||
| frames[frames.length - 1] | ||
| ) | ||
|
|
||
| // the captured frame is the red the page drew, not a wiped canvas | ||
| expect(centrePixel.a).toBeGreaterThan(200) | ||
| expect(centrePixel.r).toBeGreaterThan(200) | ||
| expect(centrePixel.g).toBeLessThan(60) | ||
| expect(centrePixel.b).toBeLessThan(60) | ||
| }) | ||
| }) |
64 changes: 64 additions & 0 deletions
64
packages/browser/src/__tests__/extensions/replay/preserve-drawing-buffer.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import { forcePreserveDrawingBuffer } from '../../../extensions/replay/preserve-drawing-buffer' | ||
|
|
||
| describe('forcePreserveDrawingBuffer', () => { | ||
| let originalGetContext: typeof HTMLCanvasElement.prototype.getContext | ||
| let calls: any[][] | ||
|
|
||
| beforeEach(() => { | ||
| jest.resetModules() | ||
| calls = [] | ||
| originalGetContext = HTMLCanvasElement.prototype.getContext | ||
| HTMLCanvasElement.prototype.getContext = function (...args: any[]) { | ||
| calls.push(args) | ||
| return { fake: 'context' } as any | ||
| } as any | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| HTMLCanvasElement.prototype.getContext = originalGetContext | ||
| }) | ||
|
|
||
| // the module guards against double patching, so each assertion runs against a fresh import | ||
| const patch = async () => { | ||
| const module = await import('../../../extensions/replay/preserve-drawing-buffer') | ||
| module.forcePreserveDrawingBuffer() | ||
| } | ||
|
|
||
| it('adds preserveDrawingBuffer when webgl is requested without attributes', async () => { | ||
| await patch() | ||
|
|
||
| document.createElement('canvas').getContext('webgl') | ||
|
|
||
| expect(calls).toEqual([['webgl', { preserveDrawingBuffer: true }]]) | ||
| }) | ||
|
|
||
| it('keeps the caller other attributes when overriding preserveDrawingBuffer', async () => { | ||
| await patch() | ||
|
|
||
| document.createElement('canvas').getContext('webgl2', { antialias: false, preserveDrawingBuffer: false }) | ||
|
|
||
| expect(calls).toEqual([['webgl2', { antialias: false, preserveDrawingBuffer: true }]]) | ||
| }) | ||
|
|
||
| it('leaves non-webgl contexts alone', async () => { | ||
| await patch() | ||
|
|
||
| document.createElement('canvas').getContext('2d') | ||
|
|
||
| expect(calls).toEqual([['2d']]) | ||
| }) | ||
|
|
||
| it('returns whatever the original getContext returned', async () => { | ||
| await patch() | ||
|
|
||
| expect(document.createElement('canvas').getContext('webgl')).toEqual({ fake: 'context' }) | ||
| }) | ||
|
|
||
| it('only patches once', async () => { | ||
| forcePreserveDrawingBuffer() | ||
| const patchedOnce = HTMLCanvasElement.prototype.getContext | ||
| forcePreserveDrawingBuffer() | ||
|
|
||
| expect(HTMLCanvasElement.prototype.getContext).toBe(patchedOnce) | ||
| }) | ||
| }) |
71 changes: 71 additions & 0 deletions
71
...browser/src/__tests__/extensions/replay/session-recording-preserve-drawing-buffer.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { SessionRecording } from '../../../extensions/replay/session-recording' | ||
| import { forcePreserveDrawingBuffer } from '../../../extensions/replay/preserve-drawing-buffer' | ||
| import { SESSION_RECORDING_REMOTE_CONFIG } from '../../../constants' | ||
| import { createMockPostHog } from '../../helpers/posthog-instance' | ||
| import { PostHogConfig } from '../../../types' | ||
| import { isUndefined } from '@posthog/core' | ||
|
|
||
| jest.mock('../../../extensions/replay/preserve-drawing-buffer', () => ({ | ||
| forcePreserveDrawingBuffer: jest.fn(), | ||
| })) | ||
|
|
||
| describe('SessionRecording preserving canvas drawing buffers', () => { | ||
| beforeEach(() => { | ||
| jest.mocked(forcePreserveDrawingBuffer).mockClear() | ||
| }) | ||
|
|
||
| const preserveDrawingBuffersFor = ({ | ||
| clientSide, | ||
| serverSide, | ||
| disableSessionRecording = false, | ||
| optedOut = false, | ||
| }: { | ||
| clientSide?: boolean | ||
| serverSide?: boolean | ||
| disableSessionRecording?: boolean | ||
| optedOut?: boolean | ||
| }): boolean => { | ||
| const instance = createMockPostHog({ | ||
| config: { | ||
| token: 'test-token', | ||
| api_host: 'https://test.com', | ||
| disable_session_recording: disableSessionRecording, | ||
| session_recording: isUndefined(clientSide) ? {} : { captureCanvas: { recordCanvas: clientSide } }, | ||
| } as PostHogConfig, | ||
| sessionManager: {} as any, | ||
| consent: { isOptedOut: () => optedOut } as any, | ||
| get_property: (key: string) => | ||
| key === SESSION_RECORDING_REMOTE_CONFIG ? { canvasRecording: { enabled: serverSide } } : undefined, | ||
| }) | ||
|
|
||
| // exercised through initialize(), the same entry point posthog.init() uses | ||
| new SessionRecording(instance)['_preserveCanvasDrawingBuffers']() | ||
|
|
||
| return jest.mocked(forcePreserveDrawingBuffer).mock.calls.length > 0 | ||
| } | ||
|
|
||
| it('patches when canvas recording is asked for client side, with nothing persisted yet', () => { | ||
| // the first-ever page load: no remote config has been stored, so this must not wait for one | ||
| expect(preserveDrawingBuffersFor({ clientSide: true })).toBe(true) | ||
| }) | ||
|
|
||
| it('patches when a persisted remote config has canvas recording on', () => { | ||
| expect(preserveDrawingBuffersFor({ serverSide: true })).toBe(true) | ||
| }) | ||
|
|
||
| it('does not patch when nothing has canvas recording on', () => { | ||
| expect(preserveDrawingBuffersFor({})).toBe(false) | ||
| expect(preserveDrawingBuffersFor({ clientSide: false })).toBe(false) | ||
| expect(preserveDrawingBuffersFor({ serverSide: false })).toBe(false) | ||
| }) | ||
|
|
||
| it('lets a client side false override a persisted remote config', () => { | ||
| expect(preserveDrawingBuffersFor({ clientSide: false, serverSide: true })).toBe(false) | ||
| }) | ||
|
|
||
| it('does not patch when session recording is disabled or consent was refused', () => { | ||
| expect(preserveDrawingBuffersFor({ clientSide: true, disableSessionRecording: true })).toBe(false) | ||
| expect(preserveDrawingBuffersFor({ serverSide: true, disableSessionRecording: true })).toBe(false) | ||
| expect(preserveDrawingBuffersFor({ clientSide: true, optedOut: true })).toBe(false) | ||
| }) | ||
| }) |
49 changes: 49 additions & 0 deletions
49
packages/browser/src/extensions/replay/preserve-drawing-buffer.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { createLogger } from '@posthog/browser-common/utils/logger' | ||
| import { window } from '@posthog/browser-common/utils/globals' | ||
| import { isObject } from '@posthog/core' | ||
|
|
||
| const logger = createLogger('[SessionRecording][Canvas]') | ||
|
|
||
| const WEBGL_CONTEXT_NAMES = ['webgl', 'webgl2', 'experimental-webgl'] | ||
|
|
||
| let patched = false | ||
|
|
||
| /** | ||
| * Canvas replay reads pixels back out of a canvas some time after the page drew into it. A WebGL | ||
| * context created with `preserveDrawingBuffer: false` - the spec default, and what any | ||
| * performance-minded renderer asks for - lets the browser throw those pixels away as soon as the | ||
| * frame has been composited, so the frames we capture come back blank. | ||
| * | ||
| * The recorder already forces `preserveDrawingBuffer: true` when it patches `getContext`, but it can | ||
| * only do that once the lazily loaded recorder bundle has arrived. Context attributes are fixed at | ||
| * creation time and cannot be changed afterwards, so an app that builds its renderer while the page | ||
| * is loading - WebGL editors, map and 3D canvases, WASM-backed engines - has already created a | ||
| * context we can never capture by the time the recorder patches anything. | ||
| * | ||
| * Patching during `posthog.init()` closes that window. It is deliberately narrow: it only forces the | ||
| * one attribute, and only when canvas recording is already known to be on. | ||
| */ | ||
| export function forcePreserveDrawingBuffer(): void { | ||
| const canvasPrototype = window?.HTMLCanvasElement?.prototype | ||
| if (patched || !canvasPrototype?.getContext) { | ||
| return | ||
| } | ||
| patched = true | ||
|
|
||
| const originalGetContext = canvasPrototype.getContext | ||
| canvasPrototype.getContext = function (this: HTMLCanvasElement, contextType: string, ...args: any[]) { | ||
| try { | ||
| if (WEBGL_CONTEXT_NAMES.indexOf(contextType) !== -1) { | ||
| if (isObject(args[0])) { | ||
| args[0].preserveDrawingBuffer = true | ||
|
Comment on lines
+37
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes an object the page gave us. If the page reuses that object for other canvases, we have changed it for them too. And if the object is frozen, this line throws, the Copying instead of editing avoids both: args[0] = isObject(args[0]) ? { ...args[0], preserveDrawingBuffer: true } : { preserveDrawingBuffer: true } |
||
| } else { | ||
| args[0] = { preserveDrawingBuffer: true } | ||
| } | ||
| } | ||
| } catch (e) { | ||
| logger.error('could not force preserveDrawingBuffer', e) | ||
| } | ||
|
|
||
| return originalGetContext.apply(this, [contextType, ...args] as any) | ||
| } as typeof canvasPrototype.getContext | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blocking: The global patch is never removed
patchedonly transitions to true, and the originalgetContextis not retained for restoration. After stop, remote disablement, opt-out, orshutdown(), newly created contexts therefore keep forcingpreserveDrawingBufferfor the page lifetime. Please add multi-instance-safe, reference-counted cleanup that restores the original when no recorder requires it.