Skip to content

Commit

Permalink
chore: fix cross browser leak tests (#32843)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed Sep 27, 2024
1 parent 728b481 commit bcb6860
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 143 deletions.
7 changes: 0 additions & 7 deletions packages/playwright-core/src/client/jsHandle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,6 @@ export class JSHandle<T = any> extends ChannelOwner<channels.JSHandleChannel> im
}
}

async _objectCount() {
return await this._wrapApiCall(async () => {
const { count } = await this._channel.objectCount();
return count;
});
}

override toString(): string {
return this._preview;
}
Expand Down
6 changes: 0 additions & 6 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1843,12 +1843,6 @@ scheme.JSHandleJsonValueResult = tObject({
value: tType('SerializedValue'),
});
scheme.ElementHandleJsonValueResult = tType('JSHandleJsonValueResult');
scheme.JSHandleObjectCountParams = tOptional(tObject({}));
scheme.ElementHandleObjectCountParams = tType('JSHandleObjectCountParams');
scheme.JSHandleObjectCountResult = tObject({
count: tNumber,
});
scheme.ElementHandleObjectCountResult = tType('JSHandleObjectCountResult');
scheme.ElementHandleInitializer = tObject({
preview: tString,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,6 @@ export class BidiExecutionContext implements js.ExecutionContextDelegate {
});
}

objectCount(objectId: js.ObjectId): Promise<number> {
throw new Error('Method not implemented.');
}

async rawCallFunction(functionDeclaration: string, arg: bidi.Script.LocalValue): Promise<bidi.Script.RemoteValue> {
const response = await this._session.send('script.callFunction', {
functionDeclaration,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,6 @@ export class CRExecutionContext implements js.ExecutionContextDelegate {
async releaseHandle(objectId: js.ObjectId): Promise<void> {
await releaseObject(this._client, objectId);
}

async objectCount(objectId: js.ObjectId): Promise<number> {
const result = await this._client.send('Runtime.queryObjects', {
prototypeObjectId: objectId
});
const match = result.objects.description!.match(/Array\((\d+)\)/)!;
return +match[1];
}
}

function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ export class JSHandleDispatcher extends Dispatcher<js.JSHandle, channels.JSHandl
return { value: serializeResult(await this._object.jsonValue()) };
}

async objectCount(params?: channels.JSHandleObjectCountParams | undefined): Promise<channels.JSHandleObjectCountResult> {
return { count: await this._object.objectCount() };
}

async dispose(_: any, metadata: CallMetadata) {
metadata.potentiallyClosesScope = true;
this._object.dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ export class FFExecutionContext implements js.ExecutionContextDelegate {
objectId
});
}

objectCount(objectId: js.ObjectId): Promise<number> {
throw new Error('Method not implemented in Firefox.');
}
}

function checkException(exceptionDetails?: Protocol.Runtime.ExceptionDetails) {
Expand Down
11 changes: 0 additions & 11 deletions packages/playwright-core/src/server/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ export interface ExecutionContextDelegate {
getProperties(context: ExecutionContext, objectId: ObjectId): Promise<Map<string, JSHandle>>;
createHandle(context: ExecutionContext, remoteObject: RemoteObject): JSHandle;
releaseHandle(objectId: ObjectId): Promise<void>;
objectCount(objectId: ObjectId): Promise<number>;
}

export class ExecutionContext extends SdkObject {
Expand Down Expand Up @@ -126,10 +125,6 @@ export class ExecutionContext extends SdkObject {
return this._utilityScriptPromise;
}

async objectCount(objectId: ObjectId): Promise<number> {
return this._delegate.objectCount(objectId);
}

async doSlowMo() {
// overridden in FrameExecutionContext
}
Expand Down Expand Up @@ -246,12 +241,6 @@ export class JSHandle<T = any> extends SdkObject {
if (this._previewCallback)
this._previewCallback(preview);
}

async objectCount(): Promise<number> {
if (!this._objectId)
throw new Error('Can only count objects for a handle that points to the constructor prototype');
return this._context.objectCount(this._objectId);
}
}

export async function evaluate(context: ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise<any> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ export class WKExecutionContext implements js.ExecutionContextDelegate {
async releaseHandle(objectId: js.ObjectId): Promise<void> {
await this._session.send('Runtime.releaseObject', { objectId });
}

objectCount(objectId: js.ObjectId): Promise<number> {
throw new Error('Method not implemented in WebKit.');
}
}

function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObject): any {
Expand Down
6 changes: 0 additions & 6 deletions packages/protocol/src/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3226,7 +3226,6 @@ export interface JSHandleChannel extends JSHandleEventTarget, Channel {
getPropertyList(params?: JSHandleGetPropertyListParams, metadata?: CallMetadata): Promise<JSHandleGetPropertyListResult>;
getProperty(params: JSHandleGetPropertyParams, metadata?: CallMetadata): Promise<JSHandleGetPropertyResult>;
jsonValue(params?: JSHandleJsonValueParams, metadata?: CallMetadata): Promise<JSHandleJsonValueResult>;
objectCount(params?: JSHandleObjectCountParams, metadata?: CallMetadata): Promise<JSHandleObjectCountResult>;
}
export type JSHandlePreviewUpdatedEvent = {
preview: string,
Expand Down Expand Up @@ -3278,11 +3277,6 @@ export type JSHandleJsonValueOptions = {};
export type JSHandleJsonValueResult = {
value: SerializedValue,
};
export type JSHandleObjectCountParams = {};
export type JSHandleObjectCountOptions = {};
export type JSHandleObjectCountResult = {
count: number,
};

export interface JSHandleEvents {
'previewUpdated': JSHandlePreviewUpdatedEvent;
Expand Down
4 changes: 0 additions & 4 deletions packages/protocol/src/protocol.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2488,10 +2488,6 @@ JSHandle:
returns:
value: SerializedValue

objectCount:
returns:
count: number

events:

previewUpdated:
Expand Down
117 changes: 57 additions & 60 deletions tests/page/page-leaks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,36 @@ function leakedJSHandles(): string {
return lines.join('\n');
}

async function objectCounts(pageImpl, constructorName: string): Promise<{ main: number, utility: number }> {
async function weakRefObjects(pageImpl: any, selector: string) {
for (const world of ['main', 'utility']) {
const context = await pageImpl.mainFrame()._context(world);
await context.evaluate(selector => {
const elements = document.querySelectorAll(selector);
globalThis.weakRefs = globalThis.weakRefs || [];
for (const element of elements)
globalThis.weakRefs.push(new WeakRef(element));
}, selector);
}
}

async function weakRefCount(pageImpl): Promise<{ main: number, utility: number }> {
const result = { main: 0, utility: 0 };
for (const world of ['main', 'utility']) {
await pageImpl.requestGC();
const context = await pageImpl.mainFrame()._context(world);
const prototype = await context.evaluateHandle(name => (window as any)[name].prototype, constructorName);
result[world] = await prototype.objectCount();
result[world] = await context.evaluate(() => globalThis.weakRefs.filter(r => !!r.deref()).length);
}
return result;
}

async function checkWeakRefs(pageImpl, from: number, to: number) {
await expect(async () => {
const counts = await weakRefCount(pageImpl);
expect(counts.main + counts.utility).toBeGreaterThanOrEqual(from);
expect(counts.main + counts.utility).toBeLessThan(to);
}).toPass();
}

test.beforeEach(() => {
(globalThis as any).leakedJSHandles = new Map();
});
Expand All @@ -59,104 +79,86 @@ test.afterEach(() => {
(globalThis as any).leakedJSHandles = null;
});

test('click should not leak', async ({ page, browserName, toImpl }) => {
test('click should not leak', async ({ page, toImpl }) => {
await page.setContent(`
<button>static button 1</button>
<button>static button 2</button>
<div id="buttons"></div>
`);
// Create JS wrappers for static elements.
await page.evaluate(() => document.querySelectorAll('button'));

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').click();
await page.evaluate(() => {
document.getElementById('buttons').textContent = '';
});
await page.locator('#buttons > button').last().click();
}

expect(leakedJSHandles()).toBeFalsy();
await weakRefObjects(toImpl(page), 'button');
await page.evaluate(() => {
document.getElementById('buttons').textContent = '';
});

if (browserName === 'chromium') {
await expect(async () => {
const counts = await objectCounts(toImpl(page), 'HTMLButtonElement');
expect(counts.main + counts.utility).toBeGreaterThanOrEqual(2);
expect(counts.main + counts.utility).toBeLessThan(25);
}).toPass();
}
expect(leakedJSHandles()).toBeFalsy();
await checkWeakRefs(toImpl(page), 2, 25);
});

test('fill should not leak', async ({ page, mode, browserName, toImpl }) => {
test('fill should not leak', async ({ page, mode, toImpl }) => {
test.skip(mode !== 'default');

await page.setContent(`
<input value="static input 1"</input>
<input value="static input 2"</input>
<div id="inputs"></div>
`);
// Create JS wrappers for static elements.
await page.evaluate(() => document.querySelectorAll('input'));

for (let i = 0; i < 25; ++i) {
await page.evaluate(i => {
await page.evaluate(() => {
const element = document.createElement('input');
document.getElementById('inputs').appendChild(element);
}, i);
await page.locator('#inputs > input').fill('input ' + i);
await page.evaluate(() => {
document.getElementById('inputs').textContent = '';
});
await page.locator('#inputs > input').last().fill('input ' + i);
}

expect(leakedJSHandles()).toBeFalsy();
await weakRefObjects(toImpl(page), 'input');
await page.evaluate(() => {
document.getElementById('inputs').textContent = '';
});

if (browserName === 'chromium') {
await expect(async () => {
const counts = await objectCounts(toImpl(page), 'HTMLInputElement');
expect(counts.main + counts.utility).toBeGreaterThanOrEqual(2);
expect(counts.main + counts.utility).toBeLessThan(25);
}).toPass();
}
expect(leakedJSHandles()).toBeFalsy();
await checkWeakRefs(toImpl(page), 2, 25);
});

test('expect should not leak', async ({ page, mode, browserName, toImpl }) => {
test('expect should not leak', async ({ page, mode, toImpl }) => {
test.skip(mode !== 'default');

await page.setContent(`
<button>static button 1</button>
<button>static button 2</button>
<div id="buttons"></div>
`);
await weakRefObjects(toImpl(page), 'button');

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 expect(page.locator('#buttons > button')).toBeVisible();
await page.evaluate(() => {
document.getElementById('buttons').textContent = '';
});
await expect(page.locator('#buttons > button').last()).toBeVisible();
}

expect(leakedJSHandles()).toBeFalsy();
await weakRefObjects(toImpl(page), 'button');
await page.evaluate(() => {
document.getElementById('buttons').textContent = '';
});

if (browserName === 'chromium') {
await expect(async () => {
const counts = await objectCounts(toImpl(page), 'HTMLButtonElement');
expect(counts.main + counts.utility).toBeGreaterThanOrEqual(2);
expect(counts.main + counts.utility).toBeLessThan(25);
}).toPass();
}
expect(leakedJSHandles()).toBeFalsy();
await checkWeakRefs(toImpl(page), 2, 25);
});

test('waitFor should not leak', async ({ page, mode, browserName, toImpl }) => {
test('waitFor should not leak', async ({ page, mode, toImpl }) => {
test.skip(mode !== 'default');

await page.setContent(`
Expand All @@ -171,19 +173,14 @@ test('waitFor should not leak', async ({ page, mode, browserName, toImpl }) => {
element.textContent = 'dynamic ' + i;
document.getElementById('buttons').appendChild(element);
}, i);
await page.locator('#buttons > button').waitFor();
await page.evaluate(() => {
document.getElementById('buttons').textContent = '';
});
await page.locator('#buttons > button').last().waitFor();
}

expect(leakedJSHandles()).toBeFalsy();
await weakRefObjects(toImpl(page), 'button');
await page.evaluate(() => {
document.getElementById('buttons').textContent = '';
});

if (browserName === 'chromium') {
await expect(async () => {
const counts = await objectCounts(toImpl(page), 'HTMLButtonElement');
expect(counts.main + counts.utility).toBeGreaterThanOrEqual(2);
expect(counts.main + counts.utility).toBeLessThan(25);
}).toPass();
}
expect(leakedJSHandles()).toBeFalsy();
await checkWeakRefs(toImpl(page), 2, 25);
});
25 changes: 0 additions & 25 deletions tests/page/page-object-count.spec.ts

This file was deleted.

0 comments on commit bcb6860

Please sign in to comment.