-
Notifications
You must be signed in to change notification settings - Fork 190
⚗️ Add canvas change detection for Session Replay #4949
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
Open
BeltranBulbarellaDD
wants to merge
9
commits into
main
from
beltran.bulbarella/canvas_support-2-prefilter
+618
−15
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
c0c0524
Add canvas change detetion
BeltranBulbarellaDD 3b02647
Add canvas change detetion
BeltranBulbarellaDD b76c1ca
filter out unnsupported versions for canvas APIs
BeltranBulbarellaDD e013ae7
Track canvas resizes performed through attributes
BeltranBulbarellaDD 0e9b67a
adapt for init config changes
BeltranBulbarellaDD dc6b3d5
Update config structure
BeltranBulbarellaDD b056d28
⚗️ Add canvas dirty-state prefilter
BeltranBulbarellaDD 07aed32
Make attributeNamespace optional
BeltranBulbarellaDD b97a735
track dirty canvases
BeltranBulbarellaDD 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
74 changes: 74 additions & 0 deletions
74
packages/browser-rum/src/domain/record/canvas/canvasManager.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,74 @@ | ||
| import { registerCleanupTask } from '@datadog/browser-core/test' | ||
| import { createCanvasManager } from './canvasManager' | ||
|
|
||
| describe('createCanvasManager', () => { | ||
| it('tracks whether a canvas is dirty', () => { | ||
| const canvasManager = createCanvasManager() | ||
| const canvas = appendCanvas() | ||
|
|
||
| expect(canvasManager.isCanvasDirty(canvas)).toBeFalse() | ||
|
|
||
| canvasManager.markCanvasDirty(canvas) | ||
| expect(canvasManager.isCanvasDirty(canvas)).toBeTrue() | ||
|
|
||
| canvasManager.markCanvasClean(canvas) | ||
| expect(canvasManager.isCanvasDirty(canvas)).toBeFalse() | ||
|
|
||
| canvasManager.markCanvasDirty(canvas) | ||
| expect(canvasManager.isCanvasDirty(canvas)).toBeTrue() | ||
| }) | ||
|
|
||
| it('tracks canvases independently', () => { | ||
| const canvasManager = createCanvasManager() | ||
| const dirtyCanvas = appendCanvas() | ||
| const cleanCanvas = appendCanvas() | ||
|
|
||
| canvasManager.markCanvasDirty(dirtyCanvas) | ||
|
|
||
| expect(canvasManager.isCanvasDirty(dirtyCanvas)).toBeTrue() | ||
| expect(canvasManager.isCanvasDirty(cleanCanvas)).toBeFalse() | ||
| }) | ||
|
|
||
| it('returns connected dirty canvases', () => { | ||
| const canvasManager = createCanvasManager() | ||
| const canvas = appendCanvas() | ||
|
|
||
| canvasManager.markCanvasDirty(canvas) | ||
|
|
||
| expect(canvasManager.getDirtyCanvases()).toEqual([canvas]) | ||
|
|
||
| canvasManager.markCanvasClean(canvas) | ||
| expect(canvasManager.getDirtyCanvases()).toEqual([]) | ||
| }) | ||
|
|
||
| it('does not retain detached canvases', () => { | ||
| const canvasManager = createCanvasManager() | ||
| const canvas = document.createElement('canvas') | ||
|
|
||
| canvasManager.markCanvasDirty(canvas) | ||
|
|
||
| expect(canvasManager.getDirtyCanvases()).toEqual([]) | ||
|
|
||
| document.body.appendChild(canvas) | ||
| registerCleanupTask(() => canvas.remove()) | ||
| expect(canvasManager.getDirtyCanvases()).toEqual([]) | ||
| }) | ||
|
|
||
| it('clears dirty canvases', () => { | ||
| const canvasManager = createCanvasManager() | ||
| const canvas = appendCanvas() | ||
| canvasManager.markCanvasDirty(canvas) | ||
|
|
||
| canvasManager.clearDirtyCanvases() | ||
|
|
||
| expect(canvasManager.getDirtyCanvases()).toEqual([]) | ||
| expect(canvasManager.isCanvasDirty(canvas)).toBeFalse() | ||
| }) | ||
| }) | ||
|
|
||
| function appendCanvas(): HTMLCanvasElement { | ||
| const canvas = document.createElement('canvas') | ||
| document.body.appendChild(canvas) | ||
| registerCleanupTask(() => canvas.remove()) | ||
| return canvas | ||
| } |
35 changes: 35 additions & 0 deletions
35
packages/browser-rum/src/domain/record/canvas/canvasManager.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,35 @@ | ||
| export interface CanvasManager { | ||
| clearDirtyCanvases: () => void | ||
| getDirtyCanvases: () => HTMLCanvasElement[] | ||
| isCanvasDirty: (canvas: HTMLCanvasElement) => boolean | ||
| markCanvasClean: (canvas: HTMLCanvasElement) => void | ||
| markCanvasDirty: (canvas: HTMLCanvasElement) => void | ||
| } | ||
|
|
||
| export function createCanvasManager(): CanvasManager { | ||
| const dirtyCanvases = new Set<HTMLCanvasElement>() | ||
|
|
||
| return { | ||
| clearDirtyCanvases: () => dirtyCanvases.clear(), | ||
| getDirtyCanvases: () => { | ||
| const connectedCanvases: HTMLCanvasElement[] = [] | ||
|
|
||
| dirtyCanvases.forEach((canvas) => { | ||
| if (canvas.isConnected) { | ||
| connectedCanvases.push(canvas) | ||
| } else { | ||
| dirtyCanvases.delete(canvas) | ||
| } | ||
| }) | ||
|
|
||
| return connectedCanvases | ||
| }, | ||
| isCanvasDirty: (canvas) => dirtyCanvases.has(canvas), | ||
| markCanvasClean: (canvas) => dirtyCanvases.delete(canvas), | ||
| markCanvasDirty: (canvas) => { | ||
| if (canvas.isConnected) { | ||
| dirtyCanvases.add(canvas) | ||
| } | ||
| }, | ||
| } | ||
| } | ||
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
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
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
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
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
74 changes: 74 additions & 0 deletions
74
packages/browser-rum/src/domain/record/trackers/trackCanvas.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,74 @@ | ||
| import { registerCleanupTask } from '@datadog/browser-core/test' | ||
| import type { Tracker } from './tracker.types' | ||
| import { trackCanvas2DMutations } from './trackCanvas' | ||
|
|
||
| describe('trackCanvas2DMutations', () => { | ||
| let canvas: HTMLCanvasElement | ||
| let context: CanvasRenderingContext2D | ||
| let markCanvasDirtySpy: jasmine.Spy<(canvas: HTMLCanvasElement) => void> | ||
| let tracker: Tracker | undefined | ||
|
|
||
| beforeEach(() => { | ||
| canvas = document.createElement('canvas') | ||
| context = canvas.getContext('2d')! | ||
| markCanvasDirtySpy = jasmine.createSpy() | ||
|
|
||
| registerCleanupTask(() => tracker?.stop()) | ||
| }) | ||
|
|
||
| it('marks the canvas dirty after drawing operations', () => { | ||
| tracker = trackCanvas2DMutations(markCanvasDirtySpy) | ||
| const imageData = context.createImageData(1, 1) | ||
| const drawingOperations: Array<{ method: string; draw: () => void }> = [ | ||
| { method: 'clearRect', draw: () => context.clearRect(0, 0, 1, 1) }, | ||
| { method: 'fillRect', draw: () => context.fillRect(0, 0, 1, 1) }, | ||
| { method: 'strokeRect', draw: () => context.strokeRect(0, 0, 1, 1) }, | ||
| { method: 'fill', draw: () => context.fill() }, | ||
| { method: 'stroke', draw: () => context.stroke() }, | ||
| { method: 'fillText', draw: () => context.fillText('foo', 0, 0) }, | ||
| { method: 'strokeText', draw: () => context.strokeText('foo', 0, 0) }, | ||
| { method: 'drawImage', draw: () => context.drawImage(canvas, 0, 0) }, | ||
| { method: 'putImageData', draw: () => context.putImageData(imageData, 0, 0) }, | ||
| { method: 'drawFocusIfNeeded', draw: () => context.drawFocusIfNeeded(canvas) }, | ||
| { method: 'reset', draw: () => context.reset() }, | ||
| ] | ||
|
|
||
| // Skip unsuported APIs per browser version. | ||
| drawingOperations | ||
| .filter( | ||
| ({ method }) => | ||
| typeof CanvasRenderingContext2D.prototype[method as keyof CanvasRenderingContext2D] === 'function' | ||
| ) | ||
| .forEach(({ draw }) => { | ||
| markCanvasDirtySpy.calls.reset() | ||
| draw() | ||
| expect(markCanvasDirtySpy).toHaveBeenCalledOnceWith(canvas) | ||
| }) | ||
| }) | ||
|
|
||
| it('does not mark the canvas dirty for non-drawing operations', () => { | ||
| tracker = trackCanvas2DMutations(markCanvasDirtySpy) | ||
|
|
||
| context.beginPath() | ||
| context.moveTo(0, 0) | ||
| context.lineTo(1, 1) | ||
|
|
||
| expect(markCanvasDirtySpy).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('does not mark the canvas dirty when a drawing operation throws', () => { | ||
| tracker = trackCanvas2DMutations(markCanvasDirtySpy) | ||
|
|
||
| expect(() => context.putImageData(null as unknown as ImageData, 0, 0)).toThrow() | ||
| expect(markCanvasDirtySpy).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('stops tracking canvas mutations', () => { | ||
| tracker = trackCanvas2DMutations(markCanvasDirtySpy) | ||
| tracker.stop() | ||
|
|
||
| context.fillRect(0, 0, 1, 1) | ||
|
|
||
| expect(markCanvasDirtySpy).not.toHaveBeenCalled() | ||
| }) | ||
| }) |
Oops, something went wrong.
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.
issue: When an enabled recording observes canvas churn (for example, charts that create, draw, and remove canvases), this
Setkeeps every removed dirty canvas and its backing bitmap reachable. Removal mutations never delete canvases, and no production code in this commit callsgetDirtyCanvases()orclearDirtyCanvases(), so the pruning inside the getter never runs and memory grows for the lifetime of the recording; detached canvases need weak bookkeeping or cleanup when they are removed.Useful? React with 👍 / 👎.