diff --git a/src/chromium/crExecutionContext.ts b/src/chromium/crExecutionContext.ts index 6f5d7e0be8..be26b5b627 100644 --- a/src/chromium/crExecutionContext.ts +++ b/src/chromium/crExecutionContext.ts @@ -16,7 +16,6 @@ */ import { CRSession } from './crConnection'; -import { helper } from '../helper'; import { getExceptionMessage, releaseObject } from './crProtocolHelper'; import { Protocol } from './protocol'; import * as js from '../javascript'; @@ -43,45 +42,22 @@ export class CRExecutionContext implements js.ExecutionContextDelegate { return remoteObject.objectId!; } - async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { - if (helper.isString(pageFunction)) { - return this._callOnUtilityScript(context, - `evaluate`, [ - { value: debugSupport.ensureSourceUrl(pageFunction) }, - ], returnByValue, () => { }); - } - - if (typeof pageFunction !== 'function') - throw new Error(`Expected to get |string| or |function| as the first argument, but got "${pageFunction}" instead.`); - const { functionText, values, handles, dispose } = await js.prepareFunctionCall(pageFunction, context, args); - return this._callOnUtilityScript(context, - 'callFunction', [ - { value: functionText }, - ...values.map(value => ({ value })), - ...handles, - ], returnByValue, dispose); - } - - private async _callOnUtilityScript(context: js.ExecutionContext, method: string, args: Protocol.Runtime.CallArgument[], returnByValue: boolean, dispose: () => void) { - try { - const utilityScript = await context.utilityScript(); - const { exceptionDetails, result: remoteObject } = await this._client.send('Runtime.callFunctionOn', { - functionDeclaration: `function (...args) { return this.${method}(...args) }` + debugSupport.generateSourceUrl(), - objectId: utilityScript._objectId, - arguments: [ - { value: returnByValue }, - ...args - ], - returnByValue, - awaitPromise: true, - userGesture: true - }).catch(rewriteError); - if (exceptionDetails) - throw new Error('Evaluation failed: ' + getExceptionMessage(exceptionDetails)); - return returnByValue ? parseEvaluationResultValue(remoteObject.value) : context.createHandle(remoteObject); - } finally { - dispose(); - } + async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], objectIds: string[]): 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 })), + ], + returnByValue, + awaitPromise: true, + userGesture: true + }).catch(rewriteError); + if (exceptionDetails) + throw new Error('Evaluation failed: ' + getExceptionMessage(exceptionDetails)); + return returnByValue ? parseEvaluationResultValue(remoteObject.value) : utilityScript._context.createHandle(remoteObject); } async getProperties(handle: js.JSHandle): Promise> { @@ -110,16 +86,6 @@ export class CRExecutionContext implements js.ExecutionContextDelegate { return; await releaseObject(this._client, handle._objectId); } - - async handleJSONValue(handle: js.JSHandle): Promise { - if (handle._objectId) { - return this._callOnUtilityScript(handle._context, - `jsonValue`, [ - { objectId: handle._objectId }, - ], true, () => {}); - } - return handle._value; - } } function rewriteError(error: Error): Protocol.Runtime.evaluateReturnValue { diff --git a/src/dom.ts b/src/dom.ts index 492140a8ff..528837873a 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -64,7 +64,7 @@ export class FrameExecutionContext extends js.ExecutionContext { async evaluateInternal(pageFunction: types.Func1, arg: Arg): Promise; async evaluateInternal(pageFunction: never, ...args: never[]): Promise { return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { - return this._delegate.evaluate(this, true /* returnByValue */, pageFunction, ...args); + return js.evaluate(this, true /* returnByValue */, pageFunction, ...args); }); } @@ -72,7 +72,7 @@ export class FrameExecutionContext extends js.ExecutionContext { async evaluateHandleInternal(pageFunction: types.Func1, arg: Arg): Promise>; async evaluateHandleInternal(pageFunction: never, ...args: never[]): Promise { return await this.frame._page._frameManager.waitForSignalsCreatedBy(null, false /* noWaitFor */, async () => { - return this._delegate.evaluate(this, false /* returnByValue */, pageFunction, ...args); + return js.evaluate(this, false /* returnByValue */, pageFunction, ...args); }); } diff --git a/src/firefox/ffExecutionContext.ts b/src/firefox/ffExecutionContext.ts index ead3746455..a271ce580a 100644 --- a/src/firefox/ffExecutionContext.ts +++ b/src/firefox/ffExecutionContext.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { helper } from '../helper'; import * as js from '../javascript'; import { FFSession } from './ffConnection'; import { Protocol } from './protocol'; @@ -42,46 +41,21 @@ export class FFExecutionContext implements js.ExecutionContextDelegate { return payload.result!.objectId!; } - async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { - if (helper.isString(pageFunction)) { - return this._callOnUtilityScript(context, - `evaluate`, [ - { value: debugSupport.ensureSourceUrl(pageFunction) }, - ], returnByValue, () => {}); - } - if (typeof pageFunction !== 'function') - throw new Error(`Expected to get |string| or |function| as the first argument, but got "${pageFunction}" instead.`); - - const { functionText, values, handles, dispose } = await js.prepareFunctionCall(pageFunction, context, args); - - return this._callOnUtilityScript(context, - `callFunction`, [ - { value: functionText }, - ...values.map(value => ({ value })), - ...handles.map(handle => ({ objectId: handle.objectId, value: handle.value })), - ], returnByValue, dispose); - } - - private async _callOnUtilityScript(context: js.ExecutionContext, method: string, args: Protocol.Runtime.CallFunctionArgument[], returnByValue: boolean, dispose: () => void) { - try { - const utilityScript = await context.utilityScript(); - const payload = await this._session.send('Runtime.callFunction', { - functionDeclaration: `(utilityScript, ...args) => utilityScript.${method}(...args)` + debugSupport.generateSourceUrl(), - args: [ - { objectId: utilityScript._objectId, value: undefined }, - { value: returnByValue }, - ...args - ], - returnByValue, - executionContextId: this._executionContextId - }).catch(rewriteError); - checkException(payload.exceptionDetails); - if (returnByValue) - return parseEvaluationResultValue(payload.result!.value); - return context.createHandle(payload.result!); - } finally { - dispose(); - } + async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], objectIds: string[]): 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 })), + ], + returnByValue, + executionContextId: this._executionContextId + }).catch(rewriteError); + checkException(payload.exceptionDetails); + if (returnByValue) + return parseEvaluationResultValue(payload.result!.value); + return utilityScript._context.createHandle(payload.result!); } async getProperties(handle: js.JSHandle): Promise> { @@ -110,16 +84,6 @@ export class FFExecutionContext implements js.ExecutionContextDelegate { objectId: handle._objectId, }).catch(error => {}); } - - async handleJSONValue(handle: js.JSHandle): Promise { - if (handle._objectId) { - return await this._callOnUtilityScript(handle._context, - `jsonValue`, [ - { objectId: handle._objectId, value: undefined }, - ], true, () => {}); - } - return handle._value; - } } function checkException(exceptionDetails?: Protocol.Runtime.ExceptionDetails) { diff --git a/src/javascript.ts b/src/javascript.ts index d07ed913b3..c8d1c35143 100644 --- a/src/javascript.ts +++ b/src/javascript.ts @@ -20,19 +20,20 @@ import * as utilityScriptSource from './generated/utilityScriptSource'; import { InnerLogger } from './logger'; import * as debugSupport from './debug/debugSupport'; import { serializeAsCallArgument } from './utilityScriptSerializers'; +import { helper } from './helper'; +type ObjectId = string; export type RemoteObject = { - objectId?: string, + objectId?: ObjectId, value?: any }; export interface ExecutionContextDelegate { - evaluate(context: ExecutionContext, returnByValue: boolean, pageFunction: string | Function, ...args: any[]): Promise; - rawEvaluate(pageFunction: string): Promise; + rawEvaluate(expression: string): Promise; + evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: JSHandle, values: any[], objectIds: ObjectId[]): Promise; getProperties(handle: JSHandle): Promise>; createHandle(context: ExecutionContext, remoteObject: RemoteObject): JSHandle; releaseHandle(handle: JSHandle): Promise; - handleJSONValue(handle: JSHandle): Promise; } export class ExecutionContext { @@ -65,11 +66,11 @@ export class ExecutionContext { export class JSHandle { readonly _context: ExecutionContext; _disposed = false; - readonly _objectId: string | undefined; + readonly _objectId: ObjectId | undefined; readonly _value: any; private _objectType: string; - constructor(context: ExecutionContext, type: string, objectId?: string, value?: any) { + constructor(context: ExecutionContext, type: string, objectId?: ObjectId, value?: any) { this._context = context; this._objectId = objectId; this._value = value; @@ -79,13 +80,13 @@ export class JSHandle { async evaluate(pageFunction: types.FuncOn, arg: Arg): Promise; async evaluate(pageFunction: types.FuncOn, arg?: any): Promise; async evaluate(pageFunction: types.FuncOn, arg: Arg): Promise { - return this._context._delegate.evaluate(this._context, true /* returnByValue */, pageFunction, this, arg); + return evaluate(this._context, true /* returnByValue */, pageFunction, this, arg); } async evaluateHandle(pageFunction: types.FuncOn, arg: Arg): Promise>; async evaluateHandle(pageFunction: types.FuncOn, arg?: any): Promise>; async evaluateHandle(pageFunction: types.FuncOn, arg: Arg): Promise> { - return this._context._delegate.evaluate(this._context, false /* returnByValue */, pageFunction, this, arg); + return evaluate(this._context, false /* returnByValue */, pageFunction, this, arg); } async getProperty(propertyName: string): Promise { @@ -104,8 +105,12 @@ export class JSHandle { return this._context._delegate.getProperties(this); } - jsonValue(): Promise { - return this._context._delegate.handleJSONValue(this); + async jsonValue(): Promise { + if (!this._objectId) + return this._value; + const utilityScript = await this._context.utilityScript(); + const script = `(utilityScript, ...args) => utilityScript.jsonValue(...args)` + debugSupport.generateSourceUrl(); + return this._context._delegate.evaluateWithArguments(script, true, utilityScript, [true], [this._objectId]); } asElement(): dom.ElementHandle | null { @@ -130,15 +135,14 @@ export class JSHandle { } } -type CallArgument = { - value?: any, - objectId?: string -} - -export async function prepareFunctionCall( - pageFunction: Function, - context: ExecutionContext, - args: any[]): Promise<{ functionText: string, values: any[], handles: CallArgument[], dispose: () => void }> { +export async function evaluate(context: ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { + const utilityScript = await context.utilityScript(); + if (helper.isString(pageFunction)) { + const script = `(utilityScript, ...args) => utilityScript.evaluate(...args)` + debugSupport.generateSourceUrl(); + return context._delegate.evaluateWithArguments(script, returnByValue, utilityScript, [returnByValue, debugSupport.ensureSourceUrl(pageFunction)], []); + } + if (typeof pageFunction !== 'function') + throw new Error(`Expected to get |string| or |function| as the first argument, but got "${pageFunction}" instead.`); const originalText = pageFunction.toString(); let functionText = originalText; @@ -180,18 +184,24 @@ export async function prepareFunctionCall( } return { fallThrough: handle }; })); - const resultHandles: CallArgument[] = []; + + const utilityScriptObjectIds: ObjectId[] = []; for (const handle of await Promise.all(handles)) { if (handle._context !== context) throw new Error('JSHandles can be evaluated only in the context they were created!'); - resultHandles.push({ objectId: handle._objectId }); + utilityScriptObjectIds.push(handle._objectId!); } - const dispose = () => { - toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose())); - }; functionText += await debugSupport.generateSourceMapUrl(originalText, functionText); - return { functionText, values: [ args.length, ...args ], handles: resultHandles, dispose }; + // See UtilityScript for arguments. + const utilityScriptValues = [returnByValue, functionText, args.length, ...args]; + + const script = `(utilityScript, ...args) => utilityScript.callFunction(...args)` + debugSupport.generateSourceUrl(); + try { + return context._delegate.evaluateWithArguments(script, returnByValue, utilityScript, utilityScriptValues, utilityScriptObjectIds); + } finally { + toDispose.map(handlePromise => handlePromise.then(handle => handle.dispose())); + } } export function parseUnserializableValue(unserializableValue: string): any { diff --git a/src/page.ts b/src/page.ts index fed842a2e6..945bca1152 100644 --- a/src/page.ts +++ b/src/page.ts @@ -586,16 +586,14 @@ export class Worker extends EventEmitter { async evaluate(pageFunction: types.Func1, arg?: any): Promise; async evaluate(pageFunction: types.Func1, arg: Arg): Promise { assertMaxArguments(arguments.length, 2); - const context = await this._executionContextPromise; - return context._delegate.evaluate(context, true /* returnByValue */, pageFunction, arg); + return js.evaluate(await this._executionContextPromise, true /* returnByValue */, pageFunction, arg); } async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise>; async evaluateHandle(pageFunction: types.Func1, arg?: any): Promise>; async evaluateHandle(pageFunction: types.Func1, arg: Arg): Promise> { assertMaxArguments(arguments.length, 2); - const context = await this._executionContextPromise; - return context._delegate.evaluate(context, false /* returnByValue */, pageFunction, arg); + return js.evaluate(await this._executionContextPromise, false /* returnByValue */, pageFunction, arg); } } diff --git a/src/server/electron.ts b/src/server/electron.ts index 2e09b87427..b1a68647bf 100644 --- a/src/server/electron.ts +++ b/src/server/electron.ts @@ -133,7 +133,7 @@ export class ElectronApplication extends ExtendedEventEmitter { this._nodeExecutionContext = new js.ExecutionContext(new CRExecutionContext(this._nodeSession, event.context), this._logger); }); await this._nodeSession.send('Runtime.enable', {}).catch(e => {}); - this._nodeElectronHandle = await this._nodeExecutionContext!._delegate.evaluate(this._nodeExecutionContext!, false /* returnByValue */, () => { + this._nodeElectronHandle = await js.evaluate(this._nodeExecutionContext!, false /* returnByValue */, () => { // Resolving the race between the debugger and the boot-time script. if ((global as any)._playwrightRun) return (global as any)._playwrightRun(); diff --git a/src/webkit/wkExecutionContext.ts b/src/webkit/wkExecutionContext.ts index b1d9693577..623cc44168 100644 --- a/src/webkit/wkExecutionContext.ts +++ b/src/webkit/wkExecutionContext.ts @@ -16,7 +16,6 @@ */ import { WKSession, isSwappedOutError } from './wkConnection'; -import { helper } from '../helper'; import { Protocol } from './protocol'; import * as js from '../javascript'; import * as debugSupport from '../debug/debugSupport'; @@ -41,20 +40,33 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { } async rawEvaluate(expression: string): Promise { - const contextId = this._contextId; - const response = await this._session.send('Runtime.evaluate', { - expression: debugSupport.ensureSourceUrl(expression), - contextId, - returnByValue: false - }); - if (response.wasThrown) - throw new Error('Evaluation failed: ' + response.result.description); - return response.result.objectId!; + try { + const response = await this._session.send('Runtime.evaluate', { + expression: debugSupport.ensureSourceUrl(expression), + contextId: this._contextId, + returnByValue: false + }); + if (response.wasThrown) + throw new Error('Evaluation failed: ' + response.result.description); + return response.result.objectId!; + } catch (error) { + throw rewriteError(error); + } } - async evaluate(context: js.ExecutionContext, returnByValue: boolean, pageFunction: Function | string, ...args: any[]): Promise { + async evaluateWithArguments(expression: string, returnByValue: boolean, utilityScript: js.JSHandle, values: any[], objectIds: string[]): Promise { try { - let response = await this._evaluateRemoteObject(context, pageFunction, args, returnByValue); + let response = await this._session.send('Runtime.callFunctionOn', { + functionDeclaration: expression, + objectId: utilityScript._objectId!, + arguments: [ + { objectId: utilityScript._objectId }, + ...values.map(value => ({ value })), + ...objectIds.map(objectId => ({ objectId })), + ], + returnByValue: false, // We need to return real Promise if that is a promise. + emulateUserGesture: true + }); if (response.result.objectId && response.result.className === 'Promise') { response = await Promise.race([ this._executionContextDestroyedPromise.then(() => contextDestroyedResult), @@ -67,53 +79,12 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { if (response.wasThrown) throw new Error('Evaluation failed: ' + response.result.description); if (!returnByValue) - return context.createHandle(response.result); + return utilityScript._context.createHandle(response.result); if (response.result.objectId) - return await this._returnObjectByValue(context, response.result.objectId); + return await this._returnObjectByValue(utilityScript._context, response.result.objectId); return parseEvaluationResultValue(response.result.value); } catch (error) { - if (isSwappedOutError(error) || error.message.includes('Missing injected script for given')) - throw new Error('Execution context was destroyed, most likely because of a navigation.'); - throw error; - } - } - - private async _evaluateRemoteObject(context: js.ExecutionContext, pageFunction: Function | string, args: any[], returnByValue: boolean): Promise { - if (helper.isString(pageFunction)) { - const utilityScript = await context.utilityScript(); - const functionDeclaration = `function (returnByValue, pageFunction) { return this.evaluate(returnByValue, pageFunction); }` + debugSupport.generateSourceUrl(); - return await this._session.send('Runtime.callFunctionOn', { - functionDeclaration, - objectId: utilityScript._objectId!, - arguments: [ - { value: returnByValue }, - { value: debugSupport.ensureSourceUrl(pageFunction) } ], - returnByValue: false, // We need to return real Promise if that is a promise. - emulateUserGesture: true - }); - } - - if (typeof pageFunction !== 'function') - throw new Error(`Expected to get |string| or |function| as the first argument, but got "${pageFunction}" instead.`); - - const { functionText, values, handles, dispose } = await js.prepareFunctionCall(pageFunction, context, args); - - try { - const utilityScript = await context.utilityScript(); - return await this._session.send('Runtime.callFunctionOn', { - functionDeclaration: `function (...args) { return this.callFunction(...args) }` + debugSupport.generateSourceUrl(), - objectId: utilityScript._objectId!, - arguments: [ - { value: returnByValue }, - { value: functionText }, - ...values.map(value => ({ value })), - ...handles, - ], - returnByValue: false, // We need to return real Promise if that is a promise. - emulateUserGesture: true - }); - } finally { - dispose(); + throw rewriteError(error); } } @@ -130,10 +101,11 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { if (serializeResponse.wasThrown) return undefined; return parseEvaluationResultValue(serializeResponse.result.value); - } catch (e) { - if (isSwappedOutError(e)) - return contextDestroyedResult; + } catch (error) { return undefined; + // TODO: we should actually throw an error, but that breaks the common case of undefined + // that is for some reason reported as an object and cannot be accessed after navigation. + // throw rewriteError(error); } } @@ -164,22 +136,6 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { return; await this._session.send('Runtime.releaseObject', {objectId: handle._objectId}).catch(error => {}); } - - async handleJSONValue(handle: js.JSHandle): Promise { - if (handle._objectId) { - const utilityScript = await handle._context.utilityScript(); - const response = await this._session.send('Runtime.callFunctionOn', { - functionDeclaration: 'function (object) { return this.jsonValue(true, object); }' + debugSupport.generateSourceUrl(), - objectId: utilityScript._objectId!, - arguments: [ { objectId: handle._objectId } ], - returnByValue: true - }); - if (response.wasThrown) - throw new Error('Evaluation failed: ' + response.result.description); - return parseEvaluationResultValue(response.result.value); - } - return handle._value; - } } const contextDestroyedResult = { @@ -194,3 +150,9 @@ function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObj const unserializableValue = remoteObject.type === 'number' && value === null ? remoteObject.description : undefined; return unserializableValue ? js.parseUnserializableValue(unserializableValue) : value; } + +function rewriteError(error: Error): Error { + if (isSwappedOutError(error) || error.message.includes('Missing injected script for given')) + return new Error('Execution context was destroyed, most likely because of a navigation.'); + return error; +} diff --git a/test/evaluation.spec.js b/test/evaluation.spec.js index 7ef807f556..bcc2f94961 100644 --- a/test/evaluation.spec.js +++ b/test/evaluation.spec.js @@ -309,6 +309,22 @@ describe('Page.evaluate', function() { }); expect(result).toEqual([42]); }); + it.fail(WEBKIT)('should not throw an error when evaluation does a synchronous navigation and returns an object', async({page, server}) => { + // It is imporant to be on about:blank for sync reload. + const result = await page.evaluate(() => { + window.location.reload(); + return {a: 42}; + }); + expect(result).toEqual({a: 42}); + }); + it('should not throw an error when evaluation does a synchronous navigation and returns undefined', async({page, server}) => { + // It is imporant to be on about:blank for sync reload. + const result = await page.evaluate(() => { + window.location.reload(); + return undefined; + }); + expect(result).toBe(undefined); + }); it.slow()('should transfer 100Mb of data from page to node.js', async({page, server}) => { const a = await page.evaluate(() => Array(100 * 1024 * 1024 + 1).join('a')); expect(a.length).toBe(100 * 1024 * 1024);