From 7a5ef0d157d57b9ebb04f8387956f1ef365badc2 Mon Sep 17 00:00:00 2001 From: Joel Einbinder Date: Fri, 9 Jul 2021 01:55:01 -0500 Subject: [PATCH] chore(eval): merge internal evaluate functions (#7517) --- src/dispatchers/frameDispatcher.ts | 15 ++++++- src/server/browserContext.ts | 13 +++--- src/server/chromium/crPage.ts | 6 +-- src/server/dom.ts | 32 +------------ src/server/electron/electron.ts | 2 +- src/server/frames.ts | 23 ++-------- src/server/javascript.ts | 45 ++++++++++++++----- src/server/page.ts | 4 +- src/server/supplements/har/harTracer.ts | 8 ++-- .../supplements/recorder/recorderApp.ts | 26 +++++------ src/server/supplements/recorderSupplement.ts | 2 +- src/server/webkit/wkPage.ts | 4 +- tests/browsercontext-expose-function.spec.ts | 3 +- tests/page/page-expose-function.spec.ts | 8 ++-- 14 files changed, 89 insertions(+), 102 deletions(-) diff --git a/src/dispatchers/frameDispatcher.ts b/src/dispatchers/frameDispatcher.ts index 843e0da1ab..33157fa1d4 100644 --- a/src/dispatchers/frameDispatcher.ts +++ b/src/dispatchers/frameDispatcher.ts @@ -67,11 +67,22 @@ export class FrameDispatcher extends Dispatcher { - return { value: serializeResult(await this._frame.evaluateExpressionAndWaitForSignals(params.expression, params.isFunction, parseArgument(params.arg), 'main')) }; + return { value: serializeResult(await this._frame.eval(params.expression, { + arg: parseArgument(params.arg), + isFunction: params.isFunction, + waitForSignals: true, + world: 'main', + }))}; } async evaluateExpressionHandle(params: channels.FrameEvaluateExpressionHandleParams, metadata: CallMetadata): Promise { - return { handle: ElementHandleDispatcher.fromJSHandle(this._scope, await this._frame.evaluateExpressionHandleAndWaitForSignals(params.expression, params.isFunction, parseArgument(params.arg), 'main')) }; + return { handle: ElementHandleDispatcher.fromJSHandle(this._scope, await this._frame.eval(params.expression, { + arg: parseArgument(params.arg), + isFunction: params.isFunction, + returnHandle: true, + waitForSignals: true, + world: 'main', + }))}; } async waitForSelector(params: channels.FrameWaitForSelectorParams, metadata: CallMetadata): Promise { diff --git a/src/server/browserContext.ts b/src/server/browserContext.ts index 2fb49fe6d8..870a15e46c 100644 --- a/src/server/browserContext.ts +++ b/src/server/browserContext.ts @@ -337,9 +337,9 @@ export abstract class BrowserContext extends SdkObject { const originStorage: types.OriginStorage = { origin, localStorage: [] }; const frame = page.mainFrame(); await frame.goto(internalMetadata, origin); - const storage = await frame.evaluateExpression(`({ + const storage = await frame.eval(`({ localStorage: Object.keys(localStorage).map(name => ({ name, value: localStorage.getItem(name) })), - })`, false, undefined, 'utility'); + })`, {world: 'utility'}); originStorage.localStorage = storage.localStorage; if (storage.localStorage.length) result.origins.push(originStorage); @@ -361,11 +361,10 @@ export abstract class BrowserContext extends SdkObject { for (const originState of state.origins) { const frame = page.mainFrame(); await frame.goto(metadata, originState.origin); - await frame.evaluateExpression(` - originState => { - for (const { name, value } of (originState.localStorage || [])) - localStorage.setItem(name, value); - }`, true, originState, 'utility'); + await frame.eval((originState: types.OriginStorage) => { + for (const { name, value } of (originState.localStorage || [])) + localStorage.setItem(name, value); + }, {arg: originState, world: 'utility'}); } await page.close(internalMetadata); } diff --git a/src/server/chromium/crPage.ts b/src/server/chromium/crPage.ts index 9eb076a707..c0a2b3daee 100644 --- a/src/server/chromium/crPage.ts +++ b/src/server/chromium/crPage.ts @@ -171,7 +171,7 @@ export class CRPage implements PageDelegate { async exposeBinding(binding: PageBinding) { await this._forAllFrameSessions(frame => frame._initBinding(binding)); - await Promise.all(this._page.frames().map(frame => frame.evaluateExpression(binding.source, false, {}, binding.world).catch(e => {}))); + await Promise.all(this._page.frames().map(frame => frame.eval(binding.source, {world: binding.world}).catch(e => {}))); } async updateExtraHTTPHeaders(): Promise { @@ -473,9 +473,9 @@ class FrameSession { worldName: UTILITY_WORLD_NAME, }); for (const binding of this._crPage._browserContext._pageBindings.values()) - frame.evaluateExpression(binding.source, false, undefined, binding.world).catch(e => {}); + frame.eval(binding.source, {world: binding.world}).catch(e => {}); for (const source of this._crPage._browserContext._evaluateOnNewDocumentSources) - frame.evaluateExpression(source, false, undefined, 'main').catch(e => {}); + frame.eval(source).catch(e => {}); } const isInitialEmptyPage = this._isMainFrame() && this._page.mainFrame().url() === ':'; if (isInitialEmptyPage) { diff --git a/src/server/dom.ts b/src/server/dom.ts index 9dbe8be5c2..f981a1caa7 100644 --- a/src/server/dom.ts +++ b/src/server/dom.ts @@ -50,36 +50,6 @@ export class FrameExecutionContext extends js.ExecutionContext { return null; } - async evaluate(pageFunction: js.Func1, arg?: Arg): Promise { - return js.evaluate(this, true /* returnByValue */, pageFunction, arg); - } - - async evaluateHandle(pageFunction: js.Func1, arg?: Arg): Promise> { - return js.evaluate(this, false /* returnByValue */, pageFunction, arg); - } - - async evaluateExpression(expression: string, isFunction: boolean | undefined, arg?: any): Promise { - return js.evaluateExpression(this, true /* returnByValue */, expression, isFunction, arg); - } - - async evaluateAndWaitForSignals(pageFunction: js.Func1, arg?: Arg): Promise { - return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { - return this.evaluate(pageFunction, arg); - }); - } - - async evaluateExpressionAndWaitForSignals(expression: string, isFunction: boolean | undefined, arg?: any): Promise { - return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { - return this.evaluateExpression(expression, isFunction, arg); - }); - } - - async evaluateExpressionHandleAndWaitForSignals(expression: string, isFunction: boolean | undefined, arg: any): Promise { - return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { - return js.evaluateExpression(this, false /* returnByValue */, expression, isFunction, arg); - }); - } - createHandle(remoteObject: js.RemoteObject): js.JSHandle { if (this.frame._page._delegate.isElementHandle(remoteObject)) return new ElementHandle(this, remoteObject.objectId!); @@ -135,7 +105,7 @@ export class ElementHandle extends js.JSHandle { private async _evaluateInMainAndWaitForSignals(pageFunction: js.Func1<[js.JSHandle, ElementHandle, Arg], R>, arg: Arg): Promise { const main = await this._context.frame._mainContext(); - return main.evaluateAndWaitForSignals(pageFunction, [await main.injectedScript(), this, arg]); + return main.eval(pageFunction, {arg: [await main.injectedScript(), this, arg], waitForSignals: true}); } async evaluateInUtility(pageFunction: js.Func1<[js.JSHandle, ElementHandle, Arg], R>, arg: Arg): Promise { diff --git a/src/server/electron/electron.ts b/src/server/electron/electron.ts index 54e87294c3..a445b4abd9 100644 --- a/src/server/electron/electron.ts +++ b/src/server/electron/electron.ts @@ -68,7 +68,7 @@ export class ElectronApplication extends SdkObject { this._nodeSession.on('Runtime.executionContextCreated', async (event: any) => { if (event.context.auxData && event.context.auxData.isDefault) { this._nodeExecutionContext = new js.ExecutionContext(this, new CRExecutionContext(this._nodeSession, event.context)); - f(await js.evaluate(this._nodeExecutionContext, false /* returnByValue */, `process.mainModule.require('electron')`)); + f(this._nodeExecutionContext.eval(`process.mainModule.require('electron')`, { returnHandle: true })); } }); }); diff --git a/src/server/frames.ts b/src/server/frames.ts index 36a4bfc9e0..cae22534aa 100644 --- a/src/server/frames.ts +++ b/src/server/frames.ts @@ -508,7 +508,7 @@ export class Frame extends SdkObject { this._nonStallingEvaluations.add(callback); try { return await Promise.race([ - context.evaluateExpression(expression, isFunction), + context.eval(expression, { isFunction }), frameInvalidated ]); } finally { @@ -658,25 +658,10 @@ export class Frame extends SdkObject { return this._context('utility'); } - async evaluateExpressionHandleAndWaitForSignals(expression: string, isFunction: boolean | undefined, arg: any, world: types.World = 'main'): Promise { + async eval(pageFunction: string|Function, options: js.EvalOptions & {world?: types.World} = {}) { + const {world = 'main'} = options; const context = await this._context(world); - const handle = await context.evaluateExpressionHandleAndWaitForSignals(expression, isFunction, arg); - if (world === 'main') - await this._page._doSlowMo(); - return handle; - } - - async evaluateExpression(expression: string, isFunction: boolean | undefined, arg: any, world: types.World = 'main'): Promise { - const context = await this._context(world); - const value = await context.evaluateExpression(expression, isFunction, arg); - if (world === 'main') - await this._page._doSlowMo(); - return value; - } - - async evaluateExpressionAndWaitForSignals(expression: string, isFunction: boolean | undefined, arg: any, world: types.World = 'main'): Promise { - const context = await this._context(world); - const value = await context.evaluateExpressionAndWaitForSignals(expression, isFunction, arg); + const value = await context.eval(pageFunction, options); if (world === 'main') await this._page._doSlowMo(); return value; diff --git a/src/server/javascript.ts b/src/server/javascript.ts index 82fc630fa7..6a7cc2a3af 100644 --- a/src/server/javascript.ts +++ b/src/server/javascript.ts @@ -52,6 +52,14 @@ export interface ExecutionContextDelegate { releaseHandle(objectId: ObjectId): Promise; } +export type EvalOptions = { + arg?: any; + args?: any[]; + returnHandle?: boolean; + isFunction?: boolean; + waitForSignals?: boolean; +}; + export class ExecutionContext extends SdkObject { readonly _delegate: ExecutionContextDelegate; private _utilityScriptPromise: Promise | undefined; @@ -89,6 +97,27 @@ export class ExecutionContext extends SdkObject { return await this._delegate.rawEvaluateJSON(expression); } + async eval(pageFunction: Function|string, { + arg, + args = [arg], + returnHandle = false, + isFunction = typeof pageFunction === 'function', + waitForSignals = false + }: EvalOptions): Promise { + const action = () => evaluateExpression(this, !returnHandle, String(pageFunction), isFunction, ...args); + if (waitForSignals) + return this.waitForSignalsCreatedBy(action); + return action(); + } + + async evaluate(pageFunction: Func1, arg?: Arg): Promise { + return this.eval(pageFunction, {arg}); + } + + async evaluateHandle(pageFunction: Func1, arg?: Arg): Promise> { + return this.eval(pageFunction, {arg, returnHandle: true}); + } + async doSlowMo() { // overrided in FrameExecutionContext } @@ -119,15 +148,15 @@ export class JSHandle extends SdkObject { } async evaluate(pageFunction: FuncOn, arg?: Arg): Promise { - return evaluate(this._context, true /* returnByValue */, pageFunction, this, arg); + return this._context.eval(pageFunction, { args: [this, arg]}); } async evaluateHandle(pageFunction: FuncOn, arg?: Arg): Promise> { - return evaluate(this._context, false /* returnByValue */, pageFunction, this, arg); + return this._context.eval(pageFunction, { returnHandle: true, args: [this, arg]}); } async evaluateExpressionAndWaitForSignals(expression: string, isFunction: boolean | undefined, returnByValue: boolean, arg: any) { - const value = await evaluateExpressionAndWaitForSignals(this._context, returnByValue, expression, isFunction, this, arg); + const value = await this._context.eval(expression, {isFunction, returnHandle: !returnByValue, args: [this, arg], waitForSignals: true}); await this._context.doSlowMo(); return value; } @@ -189,11 +218,7 @@ export class JSHandle extends SdkObject { } } -export async function evaluate(context: ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { - return evaluateExpression(context, returnByValue, String(pageFunction), typeof pageFunction === 'function', ...args); -} - -export async function evaluateExpression(context: ExecutionContext, returnByValue: boolean, expression: string, isFunction: boolean | undefined, ...args: any[]): Promise { +async function evaluateExpression(context: ExecutionContext, returnByValue: boolean, expression: string, isFunction: boolean | undefined, ...args: any[]): Promise { const utilityScript = await context.utilityScript(); expression = normalizeEvaluationExpression(expression, isFunction); const handles: (Promise)[] = []; @@ -236,10 +261,6 @@ export async function evaluateExpression(context: ExecutionContext, returnByValu } } -export async function evaluateExpressionAndWaitForSignals(context: ExecutionContext, returnByValue: boolean, expression: string, isFunction?: boolean, ...args: any[]): Promise { - return await context.waitForSignalsCreatedBy(() => evaluateExpression(context, returnByValue, expression, isFunction, ...args)); -} - export function parseUnserializableValue(unserializableValue: string): any { if (unserializableValue === 'NaN') return NaN; diff --git a/src/server/page.ts b/src/server/page.ts index ee4fb9a9d4..e9c39d15cc 100644 --- a/src/server/page.ts +++ b/src/server/page.ts @@ -537,11 +537,11 @@ export class Worker extends SdkObject { } async evaluateExpression(expression: string, isFunction: boolean | undefined, arg: any): Promise { - return js.evaluateExpression(await this._executionContextPromise, true /* returnByValue */, expression, isFunction, arg); + return (await this._executionContextPromise).eval(expression, {isFunction, arg}); } async evaluateExpressionHandle(expression: string, isFunction: boolean | undefined, arg: any): Promise { - return js.evaluateExpression(await this._executionContextPromise, false /* returnByValue */, expression, isFunction, arg); + return (await this._executionContextPromise).eval(expression, {isFunction, arg, returnHandle: true}); } } diff --git a/src/server/supplements/har/harTracer.ts b/src/server/supplements/har/harTracer.ts index 1ce25d8e30..22068c9de5 100644 --- a/src/server/supplements/har/harTracer.ts +++ b/src/server/supplements/har/harTracer.ts @@ -82,12 +82,12 @@ export class HarTracer { private _onDOMContentLoaded(page: Page) { const pageEntry = this._ensurePageEntry(page); - const promise = page.mainFrame().evaluateExpression(String(() => { + const promise = page.mainFrame().eval(() => { return { title: document.title, domContentLoaded: performance.timing.domContentLoadedEventStart, }; - }), true, undefined, 'utility').then(result => { + }, {world: 'utility'}).then(result => { pageEntry.title = result.title; pageEntry.pageTimings.onContentLoad = result.domContentLoaded; }).catch(() => {}); @@ -96,12 +96,12 @@ export class HarTracer { private _onLoad(page: Page) { const pageEntry = this._ensurePageEntry(page); - const promise = page.mainFrame().evaluateExpression(String(() => { + const promise = page.mainFrame().eval(() => { return { title: document.title, loaded: performance.timing.loadEventStart, }; - }), true, undefined, 'utility').then(result => { + }, {world: 'utility'}).then(result => { pageEntry.title = result.title; pageEntry.pageTimings.onLoad = result.loaded; }).catch(() => {}); diff --git a/src/server/supplements/recorder/recorderApp.ts b/src/server/supplements/recorder/recorderApp.ts index ef5cf00884..cdb8a06149 100644 --- a/src/server/supplements/recorder/recorderApp.ts +++ b/src/server/supplements/recorder/recorderApp.ts @@ -122,27 +122,27 @@ export class RecorderApp extends EventEmitter { } async setMode(mode: 'none' | 'recording' | 'inspecting'): Promise { - await this._page.mainFrame().evaluateExpression(((mode: Mode) => { + await this._page.mainFrame().eval((mode: Mode) => { window.playwrightSetMode(mode); - }).toString(), true, mode, 'main').catch(() => {}); + }, {arg: mode}).catch(() => {}); } async setFile(file: string): Promise { - await this._page.mainFrame().evaluateExpression(((file: string) => { + await this._page.mainFrame().eval((file: string) => { window.playwrightSetFile(file); - }).toString(), true, file, 'main').catch(() => {}); + }, {arg: file}).catch(() => {}); } async setPaused(paused: boolean): Promise { - await this._page.mainFrame().evaluateExpression(((paused: boolean) => { + await this._page.mainFrame().eval((paused: boolean) => { window.playwrightSetPaused(paused); - }).toString(), true, paused, 'main').catch(() => {}); + }, {arg: paused}).catch(() => {}); } async setSources(sources: Source[]): Promise { - await this._page.mainFrame().evaluateExpression(((sources: Source[]) => { + await this._page.mainFrame().eval((sources: Source[]) => { window.playwrightSetSources(sources); - }).toString(), true, sources, 'main').catch(() => {}); + }, {arg: sources}).catch(() => {}); // Testing harness for runCLI mode. { @@ -155,15 +155,15 @@ export class RecorderApp extends EventEmitter { } async setSelector(selector: string, focus?: boolean): Promise { - await this._page.mainFrame().evaluateExpression(((arg: any) => { - window.playwrightSetSelector(arg.selector, arg.focus); - }).toString(), true, { selector, focus }, 'main').catch(() => {}); + await this._page.mainFrame().eval((selector: string, focus?: boolean) => { + window.playwrightSetSelector(selector, focus); + }, {args: [selector, focus]}).catch(() => {}); } async updateCallLogs(callLogs: CallLog[]): Promise { - await this._page.mainFrame().evaluateExpression(((callLogs: CallLog[]) => { + await this._page.mainFrame().eval((callLogs: CallLog[]) => { window.playwrightUpdateLogs(callLogs); - }).toString(), true, callLogs, 'main').catch(() => {}); + }, {arg: callLogs}).catch(() => {}); } async bringToFront() { diff --git a/src/server/supplements/recorderSupplement.ts b/src/server/supplements/recorderSupplement.ts index f28207ec29..8789248fc6 100644 --- a/src/server/supplements/recorderSupplement.ts +++ b/src/server/supplements/recorderSupplement.ts @@ -249,7 +249,7 @@ export class RecorderSupplement implements InstrumentationListener { private _refreshOverlay() { for (const page of this._context.pages()) - page.mainFrame().evaluateExpression('window._playwrightRefreshOverlay()', false, undefined, 'main').catch(() => {}); + page.mainFrame().eval('window._playwrightRefreshOverlay()').catch(() => {}); } private async _onPage(page: Page) { diff --git a/src/server/webkit/wkPage.ts b/src/server/webkit/wkPage.ts index 7373307fab..c8ddb242bc 100644 --- a/src/server/webkit/wkPage.ts +++ b/src/server/webkit/wkPage.ts @@ -191,7 +191,7 @@ export class WKPage implements PageDelegate { const bootstrapScript = this._calculateBootstrapScript(world); if (bootstrapScript.length) promises.push(session.send('Page.setBootstrapScript', { source: bootstrapScript, worldName: webkitWorldName(world) })); - this._page.frames().map(frame => frame.evaluateExpression(bootstrapScript, false, undefined, world).catch(e => {})); + this._page.frames().map(frame => frame.eval(bootstrapScript, {world}).catch(e => {})); } if (contextOptions.bypassCSP) promises.push(session.send('Page.setBypassCSP', { enabled: true })); @@ -723,7 +723,7 @@ export class WKPage implements PageDelegate { private async _evaluateBindingScript(binding: PageBinding): Promise { const script = this._bindingToScript(binding); - await Promise.all(this._page.frames().map(frame => frame.evaluateExpression(script, false, {}, binding.world).catch(e => {}))); + await Promise.all(this._page.frames().map(frame => frame.eval(script, {world: binding.world}).catch(e => {}))); } async evaluateOnNewDocument(script: string): Promise { diff --git a/tests/browsercontext-expose-function.spec.ts b/tests/browsercontext-expose-function.spec.ts index bf2e2fc80e..d361fb0215 100644 --- a/tests/browsercontext-expose-function.spec.ts +++ b/tests/browsercontext-expose-function.spec.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +import { JSHandle } from '..'; import { contextTest as it, expect } from './config/browserTest'; it('expose binding should work', async ({context}) => { @@ -73,7 +74,7 @@ it('should be callable from-inside addInitScript', async ({context, server}) => }); it('exposeBindingHandle should work', async ({context}) => { - let target; + let target: JSHandle; await context.exposeBinding('logme', (source, t) => { target = t; return 17; diff --git a/tests/page/page-expose-function.spec.ts b/tests/page/page-expose-function.spec.ts index becd285dcd..f4ebb8b6f1 100644 --- a/tests/page/page-expose-function.spec.ts +++ b/tests/page/page-expose-function.spec.ts @@ -250,22 +250,22 @@ it('should work with internal bindings', async ({page, toImpl, server, mode, bro foo = arg; }, 'utility'); expect(await page.evaluate('!!window.foo')).toBe(false); - expect(await implPage.mainFrame().evaluateExpression('!!window.foo', false, {}, 'utility')).toBe(true); + expect(await implPage.mainFrame().eval('!!window.foo', {world: 'utility'})).toBe(true); expect(foo).toBe(undefined); - await implPage.mainFrame().evaluateExpression('window.foo(123)', false, {}, 'utility'); + await implPage.mainFrame().eval('window.foo(123)', {world: 'utility'}); expect(foo).toBe(123); // should work after reload await page.goto(server.EMPTY_PAGE); expect(await page.evaluate('!!window.foo')).toBe(false); - await implPage.mainFrame().evaluateExpression('window.foo(456)', false, {}, 'utility'); + await implPage.mainFrame().eval('window.foo(456)', {world: 'utility'}); expect(foo).toBe(456); // should work inside frames const frame = await attachFrame(page, 'myframe', server.CROSS_PROCESS_PREFIX + '/empty.html'); expect(await frame.evaluate('!!window.foo')).toBe(false); const implFrame: import('../../src/server/frames').Frame = toImpl(frame); - await implFrame.evaluateExpression('window.foo(789)', false, {}, 'utility'); + await implFrame.eval('window.foo(789)', {world: 'utility'}); expect(foo).toBe(789); });