From dbbdabfd1bbebb3a8fd195a9bbd16f17c7b45d1b Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 24 Feb 2025 12:11:17 -0800 Subject: [PATCH] chore: pass JSHandles instead of ObjectId to/from context delegate (#34895) --- .../src/server/bidi/bidiExecutionContext.ts | 14 ++++--- .../src/server/chromium/crExecutionContext.ts | 14 ++++--- packages/playwright-core/src/server/dom.ts | 8 +++- .../src/server/firefox/ffExecutionContext.ts | 14 ++++--- .../playwright-core/src/server/javascript.ts | 40 ++++++++++--------- .../src/server/webkit/wkExecutionContext.ts | 14 ++++--- 6 files changed, 59 insertions(+), 45 deletions(-) diff --git a/packages/playwright-core/src/server/bidi/bidiExecutionContext.ts b/packages/playwright-core/src/server/bidi/bidiExecutionContext.ts index c9b1890af9..431e1b9841 100644 --- a/packages/playwright-core/src/server/bidi/bidiExecutionContext.ts +++ b/packages/playwright-core/src/server/bidi/bidiExecutionContext.ts @@ -61,7 +61,7 @@ export class BidiExecutionContext implements js.ExecutionContextDelegate { throw new js.JavaScriptErrorInEvaluate('Unexpected response type: ' + JSON.stringify(response)); } - async rawEvaluateHandle(expression: string): Promise { + async rawEvaluateHandle(context: js.ExecutionContext, expression: string): Promise { const response = await this._session.send('script.evaluate', { expression, target: this._target, @@ -72,7 +72,7 @@ export class BidiExecutionContext implements js.ExecutionContextDelegate { }); if (response.type === 'success') { if ('handle' in response.result) - return response.result.handle!; + return createHandle(context, response.result); throw new js.JavaScriptErrorInEvaluate('Cannot get handle: ' + JSON.stringify(response.result)); } if (response.type === 'exception') @@ -80,14 +80,14 @@ export class BidiExecutionContext implements js.ExecutionContextDelegate { throw new js.JavaScriptErrorInEvaluate('Unexpected response type: ' + JSON.stringify(response)); } - async evaluateWithArguments(functionDeclaration: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], objectIds: string[]): Promise { + async evaluateWithArguments(functionDeclaration: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], handles: js.JSHandle[]): Promise { const response = await this._session.send('script.callFunction', { functionDeclaration, target: this._target, arguments: [ { handle: utilityScript._objectId! }, ...values.map(BidiSerializer.serialize), - ...objectIds.map(handle => ({ handle })), + ...handles.map(handle => ({ handle: handle._objectId! })), ], resultOwnership: returnByValue ? undefined : bidi.Script.ResultOwnership.Root, // Necessary for the handle to be returned. serializationOptions: returnByValue ? {} : { maxObjectDepth: 0, maxDomDepth: 0 }, @@ -121,10 +121,12 @@ export class BidiExecutionContext implements js.ExecutionContextDelegate { return map; } - async releaseHandle(objectId: js.ObjectId): Promise { + async releaseHandle(handle: js.JSHandle): Promise { + if (!handle._objectId) + return; await this._session.send('script.disown', { target: this._target, - handles: [objectId], + handles: [handle._objectId], }); } diff --git a/packages/playwright-core/src/server/chromium/crExecutionContext.ts b/packages/playwright-core/src/server/chromium/crExecutionContext.ts index 4f4c505422..da11f1e686 100644 --- a/packages/playwright-core/src/server/chromium/crExecutionContext.ts +++ b/packages/playwright-core/src/server/chromium/crExecutionContext.ts @@ -46,24 +46,24 @@ export class CRExecutionContext implements js.ExecutionContextDelegate { return remoteObject.value; } - async rawEvaluateHandle(expression: string): Promise { + async rawEvaluateHandle(context: js.ExecutionContext, expression: string): Promise { const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.evaluate', { expression, contextId: this._contextId, }).catch(rewriteError); if (exceptionDetails) throw new js.JavaScriptErrorInEvaluate(getExceptionMessage(exceptionDetails)); - return remoteObject.objectId!; + return createHandle(context, remoteObject); } - async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], objectIds: string[]): Promise { + async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], handles: js.JSHandle[]): Promise { const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', { functionDeclaration: expression, objectId: utilityScript._objectId, arguments: [ { objectId: utilityScript._objectId }, ...values.map(value => ({ value })), - ...objectIds.map(objectId => ({ objectId })), + ...handles.map(handle => ({ objectId: handle._objectId! })), ], returnByValue, awaitPromise: true, @@ -88,8 +88,10 @@ export class CRExecutionContext implements js.ExecutionContextDelegate { return result; } - async releaseHandle(objectId: js.ObjectId): Promise { - await releaseObject(this._client, objectId); + async releaseHandle(handle: js.JSHandle): Promise { + if (!handle._objectId) + return; + await releaseObject(this._client, handle._objectId); } } diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts index 3366a1a50d..fbea38b814 100644 --- a/packages/playwright-core/src/server/dom.ts +++ b/packages/playwright-core/src/server/dom.ts @@ -105,7 +105,11 @@ export class FrameExecutionContext extends js.ExecutionContext { ); })(); `; - this._injectedScriptPromise = this.rawEvaluateHandle(source).then(objectId => new js.JSHandle(this, 'object', 'InjectedScript', objectId)); + this._injectedScriptPromise = this.rawEvaluateHandle(source) + .then(handle => { + handle._setPreview('InjectedScript'); + return handle; + }); } return this._injectedScriptPromise; } @@ -118,7 +122,7 @@ export class ElementHandle extends js.JSHandle { declare readonly _objectId: string; readonly _frame: frames.Frame; - constructor(context: FrameExecutionContext, objectId: js.ObjectId) { + constructor(context: FrameExecutionContext, objectId: string) { super(context, 'node', undefined, objectId); this._page = context.frame._page; this._frame = context.frame; diff --git a/packages/playwright-core/src/server/firefox/ffExecutionContext.ts b/packages/playwright-core/src/server/firefox/ffExecutionContext.ts index d0388ae2fe..1b9af6bfaa 100644 --- a/packages/playwright-core/src/server/firefox/ffExecutionContext.ts +++ b/packages/playwright-core/src/server/firefox/ffExecutionContext.ts @@ -44,23 +44,23 @@ export class FFExecutionContext implements js.ExecutionContextDelegate { return payload.result!.value; } - async rawEvaluateHandle(expression: string): Promise { + async rawEvaluateHandle(context: js.ExecutionContext, expression: string): Promise { const payload = await this._session.send('Runtime.evaluate', { expression, returnByValue: false, executionContextId: this._executionContextId, }).catch(rewriteError); checkException(payload.exceptionDetails); - return payload.result!.objectId!; + return createHandle(context, payload.result!); } - async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], objectIds: string[]): Promise { + async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], handles: js.JSHandle[]): Promise { const payload = await this._session.send('Runtime.callFunction', { functionDeclaration: expression, args: [ { objectId: utilityScript._objectId, value: undefined }, ...values.map(value => ({ value })), - ...objectIds.map(objectId => ({ objectId, value: undefined })), + ...handles.map(handle => ({ objectId: handle._objectId!, value: undefined })), ], returnByValue, executionContextId: this._executionContextId @@ -82,10 +82,12 @@ export class FFExecutionContext implements js.ExecutionContextDelegate { return result; } - async releaseHandle(objectId: js.ObjectId): Promise { + async releaseHandle(handle: js.JSHandle): Promise { + if (!handle._objectId) + return; await this._session.send('Runtime.disposeObject', { executionContextId: this._executionContextId, - objectId + objectId: handle._objectId, }); } } diff --git a/packages/playwright-core/src/server/javascript.ts b/packages/playwright-core/src/server/javascript.ts index edb0d95bc9..f01cb88bcf 100644 --- a/packages/playwright-core/src/server/javascript.ts +++ b/packages/playwright-core/src/server/javascript.ts @@ -23,8 +23,6 @@ import { LongStandingScope } from '../utils/isomorphic/manualPromise'; import type * as dom from './dom'; import type { UtilityScript } from './injected/utilityScript'; -export type ObjectId = string; - interface TaggedAsJSHandle { __jshandle: T; } @@ -49,10 +47,10 @@ export type SmartHandle = T extends Node ? dom.ElementHandle : JSHandle export interface ExecutionContextDelegate { rawEvaluateJSON(expression: string): Promise; - rawEvaluateHandle(expression: string): Promise; - evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: JSHandle, values: any[], objectIds: ObjectId[]): Promise; + rawEvaluateHandle(context: ExecutionContext, expression: string): Promise; + evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: JSHandle, values: any[], handles: JSHandle[]): Promise; getProperties(object: JSHandle): Promise>; - releaseHandle(objectId: ObjectId): Promise; + releaseHandle(handle: JSHandle): Promise; } export class ExecutionContext extends SdkObject { @@ -79,21 +77,21 @@ export class ExecutionContext extends SdkObject { return this._raceAgainstContextDestroyed(this.delegate.rawEvaluateJSON(expression)); } - rawEvaluateHandle(expression: string): Promise { - return this._raceAgainstContextDestroyed(this.delegate.rawEvaluateHandle(expression)); + rawEvaluateHandle(expression: string): Promise { + return this._raceAgainstContextDestroyed(this.delegate.rawEvaluateHandle(this, expression)); } - async evaluateWithArguments(expression: string, returnByValue: boolean, values: any[], objectIds: ObjectId[]): Promise { + async evaluateWithArguments(expression: string, returnByValue: boolean, values: any[], handles: JSHandle[]): Promise { const utilityScript = await this._utilityScript(); - return this._raceAgainstContextDestroyed(this.delegate.evaluateWithArguments(expression, returnByValue, utilityScript, values, objectIds)); + return this._raceAgainstContextDestroyed(this.delegate.evaluateWithArguments(expression, returnByValue, utilityScript, values, handles)); } getProperties(object: JSHandle): Promise> { return this._raceAgainstContextDestroyed(this.delegate.getProperties(object)); } - releaseHandle(objectId: ObjectId): Promise { - return this.delegate.releaseHandle(objectId); + releaseHandle(handle: JSHandle): Promise { + return this.delegate.releaseHandle(handle); } adoptIfNeeded(handle: JSHandle): Promise | null { @@ -108,7 +106,11 @@ export class ExecutionContext extends SdkObject { ${utilityScriptSource.source} return new (module.exports.UtilityScript())(${isUnderTest()}); })();`; - this._utilityScriptPromise = this._raceAgainstContextDestroyed(this.delegate.rawEvaluateHandle(source).then(objectId => new JSHandle(this, 'object', 'UtilityScript', objectId))); + this._utilityScriptPromise = this._raceAgainstContextDestroyed(this.delegate.rawEvaluateHandle(this, source)) + .then(handle => { + handle._setPreview('UtilityScript'); + return handle; + }); } return this._utilityScriptPromise; } @@ -122,13 +124,13 @@ export class JSHandle extends SdkObject { __jshandle: T = true as any; readonly _context: ExecutionContext; _disposed = false; - readonly _objectId: ObjectId | undefined; + readonly _objectId: string | undefined; readonly _value: any; private _objectType: string; protected _preview: string; private _previewCallback: ((preview: string) => void) | undefined; - constructor(context: ExecutionContext, type: string, preview: string | undefined, objectId?: ObjectId, value?: any) { + constructor(context: ExecutionContext, type: string, preview: string | undefined, objectId?: string, value?: any) { super(context, 'handle'); this._context = context; this._objectId = objectId; @@ -185,7 +187,7 @@ export class JSHandle extends SdkObject { if (!this._objectId) return this._value; const script = `(utilityScript, ...args) => utilityScript.jsonValue(...args)`; - return this._context.evaluateWithArguments(script, true, [true], [this._objectId]); + return this._context.evaluateWithArguments(script, true, [true], [this]); } asElement(): dom.ElementHandle | null { @@ -197,7 +199,7 @@ export class JSHandle extends SdkObject { return; this._disposed = true; if (this._objectId) { - this._context.releaseHandle(this._objectId).catch(e => {}); + this._context.releaseHandle(this).catch(e => {}); if ((globalThis as any).leakedJSHandles) (globalThis as any).leakedJSHandles.delete(this); } @@ -254,11 +256,11 @@ export async function evaluateExpression(context: ExecutionContext, expression: return { fallThrough: handle }; })); - const utilityScriptObjectIds: ObjectId[] = []; + const utilityScriptObjects: JSHandle[] = []; for (const handle of await Promise.all(handles)) { if (handle._context !== context) throw new JavaScriptErrorInEvaluate('JSHandles can be evaluated only in the context they were created!'); - utilityScriptObjectIds.push(handle._objectId!); + utilityScriptObjects.push(handle); } // See UtilityScript for arguments. @@ -266,7 +268,7 @@ export async function evaluateExpression(context: ExecutionContext, expression: const script = `(utilityScript, ...args) => utilityScript.evaluate(...args)`; try { - return await context.evaluateWithArguments(script, options.returnByValue || false, utilityScriptValues, utilityScriptObjectIds); + return await context.evaluateWithArguments(script, options.returnByValue || false, utilityScriptValues, utilityScriptObjects); } finally { toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose())); } diff --git a/packages/playwright-core/src/server/webkit/wkExecutionContext.ts b/packages/playwright-core/src/server/webkit/wkExecutionContext.ts index f9386d3adc..c7533d1ac5 100644 --- a/packages/playwright-core/src/server/webkit/wkExecutionContext.ts +++ b/packages/playwright-core/src/server/webkit/wkExecutionContext.ts @@ -48,7 +48,7 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { } } - async rawEvaluateHandle(expression: string): Promise { + async rawEvaluateHandle(context: js.ExecutionContext, expression: string): Promise { try { const response = await this._session.send('Runtime.evaluate', { expression, @@ -57,13 +57,13 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { }); if (response.wasThrown) throw new js.JavaScriptErrorInEvaluate(response.result.description); - return response.result.objectId!; + return createHandle(context, response.result); } catch (error) { throw rewriteError(error); } } - async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], objectIds: string[]): Promise { + async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], handles: js.JSHandle[]): Promise { try { const response = await this._session.send('Runtime.callFunctionOn', { functionDeclaration: expression, @@ -71,7 +71,7 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { arguments: [ { objectId: utilityScript._objectId }, ...values.map(value => ({ value })), - ...objectIds.map(objectId => ({ objectId })), + ...handles.map(handle => ({ objectId: handle._objectId! })), ], returnByValue, emulateUserGesture: true, @@ -101,8 +101,10 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { return result; } - async releaseHandle(objectId: js.ObjectId): Promise { - await this._session.send('Runtime.releaseObject', { objectId }); + async releaseHandle(handle: js.JSHandle): Promise { + if (!handle._objectId) + return; + await this._session.send('Runtime.releaseObject', { objectId: handle._objectId }); } }