From f26c6fc226d161f9139e7bf590faf404a542ba5f Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 24 Oct 2024 04:14:04 -0700 Subject: [PATCH] cherry-pick(#33240, #33264): fix(recorder): do not leak when instantiated in snapshots (#33259) --- .../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 | 38 +++++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 69fe959f81847..a1f4772e858b1 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -147,13 +147,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 { @@ -1543,6 +1549,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 cdc29a105063e..f2a6e5e8ab6af 100644 --- a/packages/playwright-core/src/server/injected/recorder/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder/recorder.ts @@ -1051,7 +1051,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.overlay?.install(); this.document.adoptedStyleSheets.push(this._stylesheet); diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index 82e4282c6fdd1..48f911a97a659 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -315,6 +315,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 71689fa4c14f8..818abb744f8d2 100644 --- a/tests/library/trace-viewer.spec.ts +++ b/tests/library/trace-viewer.spec.ts @@ -1390,6 +1390,44 @@ 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 aliveCount = 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 aliveList.length; + }); + }; + + await expect(traceViewer.snapshotContainer.contentFrame().locator('body')).toContainText(`Hi, I'm frame`); + + const frame1 = await traceViewer.snapshotFrame('page.goto'); + await expect(frame1.locator('body')).toContainText('Hello world'); + + const frame2 = await traceViewer.snapshotFrame('page.evaluate'); + await expect(frame2.locator('button')).toBeVisible(); + + await traceViewer.page.requestGC(); + await expect.poll(() => aliveCount()).toBeLessThanOrEqual(2); // two snapshot iframes + + const frame3 = await traceViewer.snapshotFrame('page.setViewportSize'); + await expect(frame3.locator('body')).toContainText(`Hi, I'm frame`); + + const frame4 = await traceViewer.snapshotFrame('page.goto'); + await expect(frame4.locator('body')).toContainText('Hello world'); + + const frame5 = await traceViewer.snapshotFrame('page.evaluate'); + await expect(frame5.locator('button')).toBeVisible(); + + await traceViewer.page.requestGC(); + await expect.poll(() => aliveCount()).toBeLessThanOrEqual(2); // two snapshot iframes +}); + test('should serve css without content-type', async ({ page, runAndTrace, server }) => { server.setRoute('/one-style.css', (req, res) => { res.writeHead(200);