Skip to content

Commit

Permalink
cherry-pick(#33240, #33264): fix(recorder): do not leak when instanti…
Browse files Browse the repository at this point in the history
…ated in snapshots (#33259)
  • Loading branch information
dgozman authored Oct 24, 2024
1 parent ff1932b commit f26c6fc
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 3 deletions.
11 changes: 9 additions & 2 deletions packages/playwright-core/src/server/injected/injectedScript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1543,6 +1549,7 @@ declare global {
__pwClock?: {
builtin: {
setTimeout: Window['setTimeout'],
clearTimeout: Window['clearTimeout'],
requestAnimationFrame: Window['requestAnimationFrame'],
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions packages/trace-viewer/src/ui/snapshotTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
38 changes: 38 additions & 0 deletions tests/library/trace-viewer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit f26c6fc

Please sign in to comment.