From d45efe881f6300d17eaf34ce33a292d12ecf5516 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 18 Apr 2023 14:11:46 -0400 Subject: [PATCH] chore: don't leak from waitFor (#22465) Fixes https://github.com/microsoft/playwright/issues/22458 --- package-lock.json | 1 + packages/playwright-core/src/server/frames.ts | 4 +++ packages/playwright-core/src/server/types.ts | 2 +- tests/page/page-leaks.spec.ts | 30 +++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 41a334edc4..5507c16f42 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6199,6 +6199,7 @@ } }, "packages/playwright-ct-core": { + "name": "@playwright/experimental-ct-core", "version": "1.33.0-next", "license": "Apache-2.0", "dependencies": { diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts index b53b1e6305..7cde76438f 100644 --- a/packages/playwright-core/src/server/frames.ts +++ b/packages/playwright-core/src/server/frames.ts @@ -815,6 +815,10 @@ export class Frame extends SdkObject { result.dispose(); return continuePolling; } + if (options.omitReturnValue) { + result.dispose(); + return null; + } const element = state === 'attached' || state === 'visible' ? await result.evaluateHandle(r => r.element) : null; result.dispose(); if (!element) diff --git a/packages/playwright-core/src/server/types.ts b/packages/playwright-core/src/server/types.ts index f3c9b96c65..1767783ec8 100644 --- a/packages/playwright-core/src/server/types.ts +++ b/packages/playwright-core/src/server/types.ts @@ -25,7 +25,7 @@ export type StrictOptions = { export type QueryOnSelectorOptions = StrictOptions & TimeoutOptions; -export type WaitForElementOptions = TimeoutOptions & StrictOptions & { state?: 'attached' | 'detached' | 'visible' | 'hidden' }; +export type WaitForElementOptions = TimeoutOptions & StrictOptions & { state?: 'attached' | 'detached' | 'visible' | 'hidden' } & { omitReturnValue?: boolean }; export type WaitForFunctionOptions = TimeoutOptions & { pollingInterval?: number }; diff --git a/tests/page/page-leaks.spec.ts b/tests/page/page-leaks.spec.ts index d04bb4dc6d..b33ce8e1d4 100644 --- a/tests/page/page-leaks.spec.ts +++ b/tests/page/page-leaks.spec.ts @@ -149,3 +149,33 @@ test('expect should not leak', async ({ page, mode, browserName, toImpl }) => { expect(counts.main + counts.utility).toBeLessThan(25); } }); + +test('waitFor should not leak', async ({ page, mode, browserName, toImpl }) => { + test.skip(mode !== 'default'); + + await page.setContent(` + + +
+ `); + + for (let i = 0; i < 25; ++i) { + await page.evaluate(i => { + const element = document.createElement('button'); + element.textContent = 'dynamic ' + i; + document.getElementById('buttons').appendChild(element); + }, i); + await page.locator('#buttons > button').waitFor(); + await page.evaluate(() => { + document.getElementById('buttons').textContent = ''; + }); + } + + expect(leakedJSHandles()).toBeFalsy(); + + if (browserName === 'chromium') { + const counts = await objectCounts(toImpl(page), 'HTMLButtonElement'); + expect(counts.main + counts.utility).toBeGreaterThanOrEqual(2); + expect(counts.main + counts.utility).toBeLessThan(25); + } +});