From 993a6b2a2a886d4504ae359ee40cfa1966feffe9 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 23 Oct 2024 10:24:53 -0700 Subject: [PATCH] fix(recorder): do not leak when instantiated in snapshots (#33240) --- .../src/server/injected/injectedScript.ts | 11 ++++++-- .../src/server/injected/recorder/recorder.ts | 2 +- packages/trace-viewer/src/ui/snapshotTab.tsx | 4 +++ tests/library/trace-viewer.spec.ts | 27 +++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 7abc5850bb..6446d16966 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -143,13 +143,19 @@ export class InjectedScript { builtinSetTimeout(callback: Function, timeout: number) { if (this.window.__pwClock?.builtin) return this.window.__pwClock.builtin.setTimeout(callback, timeout); - return setTimeout(callback, timeout); + return this.window.setTimeout(callback, timeout); + } + + builtinClearTimeout(timeout: number | undefined) { + if (this.window.__pwClock?.builtin) + return this.window.__pwClock.builtin.clearTimeout(timeout); + return this.window.clearTimeout(timeout); } builtinRequestAnimationFrame(callback: FrameRequestCallback) { if (this.window.__pwClock?.builtin) return this.window.__pwClock.builtin.requestAnimationFrame(callback); - return requestAnimationFrame(callback); + return this.window.requestAnimationFrame(callback); } eval(expression: string): any { @@ -1558,6 +1564,7 @@ declare global { __pwClock?: { builtin: { setTimeout: Window['setTimeout'], + clearTimeout: Window['clearTimeout'], requestAnimationFrame: Window['requestAnimationFrame'], } } diff --git a/packages/playwright-core/src/server/injected/recorder/recorder.ts b/packages/playwright-core/src/server/injected/recorder/recorder.ts index 9dfa2cd372..1d50495c77 100644 --- a/packages/playwright-core/src/server/injected/recorder/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder/recorder.ts @@ -1092,7 +1092,7 @@ export class Recorder { recreationInterval = this.injectedScript.builtinSetTimeout(recreate, 500); }; recreationInterval = this.injectedScript.builtinSetTimeout(recreate, 500); - this._listeners.push(() => clearInterval(recreationInterval)); + this._listeners.push(() => this.injectedScript.builtinClearTimeout(recreationInterval)); this.highlight.appendChild(createSvgElement(this.document, clipPaths)); this.overlay?.install(); diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index 56d5b0b2e5..bbf0e763da 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -269,6 +269,10 @@ function createRecorders(recorders: { recorder: Recorder, frameSelector: string const recorder = new Recorder(injectedScript); win._injectedScript = injectedScript; win._recorder = { recorder, frameSelector: parentFrameSelector }; + if (isUnderTest) { + (window as any)._weakRecordersForTest = (window as any)._weakRecordersForTest || new Set(); + (window as any)._weakRecordersForTest.add(new WeakRef(recorder)); + } } recorders.push(win._recorder); diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts index 37666fc1c6..eb81fa8b9c 100644 --- a/tests/library/trace-viewer.spec.ts +++ b/tests/library/trace-viewer.spec.ts @@ -1410,6 +1410,33 @@ test('should show baseURL in metadata pane', { await expect(traceViewer.metadataTab).toContainText('baseURL:https://example.com'); }); +test('should not leak recorders', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/33086' }, +}, async ({ showTraceViewer }) => { + const traceViewer = await showTraceViewer([traceFile]); + + const counts = async () => { + return await traceViewer.page.evaluate(() => { + const weakSet = (window as any)._weakRecordersForTest || new Set(); + const weakList = [...weakSet]; + const aliveList = weakList.filter(r => !!r.deref()); + return { total: weakList.length, alive: aliveList.length }; + }); + }; + + await traceViewer.snapshotFrame('page.goto'); + await traceViewer.snapshotFrame('page.evaluate'); + await traceViewer.page.requestGC(); + await expect.poll(() => counts()).toEqual({ total: 4, alive: 1 }); + + await traceViewer.snapshotFrame('page.setContent'); + await traceViewer.snapshotFrame('page.goto'); + await traceViewer.snapshotFrame('page.evaluate'); + await traceViewer.snapshotFrame('page.setContent'); + await traceViewer.page.requestGC(); + await expect.poll(() => counts()).toEqual({ total: 8, alive: 1 }); +}); + test('should serve css without content-type', async ({ page, runAndTrace, server }) => { server.setRoute('/one-style.css', (req, res) => { res.writeHead(200);