cherry-pick(#33240, #33264): fix(recorder): do not leak when instanti…

…ated in snapshots (#33259)
microsoft · Oct 24, 2024 · f26c6fc · f26c6fc
1 parent ff1932b
commit f26c6fc
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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);