From 2e63c591571a2da799c95e632e10d3ec0f80e8af Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 6 Aug 2021 11:37:36 -0700 Subject: [PATCH] feat(trace-viewer): show remote object previews in console (#8024) --- src/server/chromium/crExecutionContext.ts | 25 +++++++++- src/server/console.ts | 2 +- src/server/dom.ts | 4 +- src/server/firefox/ffExecutionContext.ts | 21 ++++++++- src/server/javascript.ts | 14 +++--- src/server/webkit/wkExecutionContext.ts | 23 +++++++++- tests/page/jshandle-to-string.spec.ts | 56 +++++++++++++---------- tests/page/locator-convenience.spec.ts | 44 ------------------ tests/page/page-event-console.spec.ts | 43 +++++++++++++++-- tests/page/workers.spec.ts | 7 ++- 10 files changed, 151 insertions(+), 88 deletions(-) diff --git a/src/server/chromium/crExecutionContext.ts b/src/server/chromium/crExecutionContext.ts index 8bb8c87260..e2d16f93d5 100644 --- a/src/server/chromium/crExecutionContext.ts +++ b/src/server/chromium/crExecutionContext.ts @@ -95,7 +95,7 @@ export class CRExecutionContext implements js.ExecutionContextDelegate { } createHandle(context: js.ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): js.JSHandle { - return new js.JSHandle(context, remoteObject.subtype || remoteObject.type, remoteObject.objectId, potentiallyUnserializableValue(remoteObject)); + return new js.JSHandle(context, remoteObject.subtype || remoteObject.type, renderPreview(remoteObject), remoteObject.objectId, potentiallyUnserializableValue(remoteObject)); } async releaseHandle(objectId: js.ObjectId): Promise { @@ -121,3 +121,26 @@ function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObj const unserializableValue = remoteObject.unserializableValue; return unserializableValue ? js.parseUnserializableValue(unserializableValue) : value; } + +function renderPreview(object: Protocol.Runtime.RemoteObject): string | undefined { + if (object.type === 'undefined') + return 'undefined'; + if ('value' in object) + return String(object.value); + if (object.unserializableValue) + return String(object.unserializableValue); + + if (object.description === 'Object' && object.preview) { + const tokens = []; + for (const { name, value } of object.preview.properties) + tokens.push(`${name}: ${value}`); + return `{${tokens.join(', ')}}`; + } + if (object.subtype === 'array' && object.preview) { + const result = []; + for (const { name, value } of object.preview.properties) + result[+name] = value; + return '[' + String(result) + ']'; + } + return object.description; +} diff --git a/src/server/console.ts b/src/server/console.ts index 95f4791224..3d3b85bc79 100644 --- a/src/server/console.ts +++ b/src/server/console.ts @@ -38,7 +38,7 @@ export class ConsoleMessage extends SdkObject { text(): string { if (this._text === undefined) - this._text = this._args.map(arg => arg._value).join(' '); + this._text = this._args.map(arg => arg.preview()).join(' '); return this._text; } diff --git a/src/server/dom.ts b/src/server/dom.ts index dde2c6685d..e3fc075b75 100644 --- a/src/server/dom.ts +++ b/src/server/dom.ts @@ -101,7 +101,7 @@ export class FrameExecutionContext extends js.ExecutionContext { ); })(); `; - this._injectedScriptPromise = this._delegate.rawEvaluateHandle(source).then(objectId => new js.JSHandle(this, 'object', objectId)); + this._injectedScriptPromise = this._delegate.rawEvaluateHandle(source).then(objectId => new js.JSHandle(this, 'object', undefined, objectId)); } return this._injectedScriptPromise; } @@ -117,7 +117,7 @@ export class ElementHandle extends js.JSHandle { declare readonly _objectId: string; constructor(context: FrameExecutionContext, objectId: string) { - super(context, 'node', objectId); + super(context, 'node', undefined, objectId); this._page = context.frame._page; this._initializePreview().catch(e => {}); } diff --git a/src/server/firefox/ffExecutionContext.ts b/src/server/firefox/ffExecutionContext.ts index c7aa23d131..6f055f5c73 100644 --- a/src/server/firefox/ffExecutionContext.ts +++ b/src/server/firefox/ffExecutionContext.ts @@ -88,7 +88,7 @@ export class FFExecutionContext implements js.ExecutionContextDelegate { } createHandle(context: js.ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): js.JSHandle { - return new js.JSHandle(context, remoteObject.subtype || remoteObject.type || '', remoteObject.objectId, potentiallyUnserializableValue(remoteObject)); + return new js.JSHandle(context, remoteObject.subtype || remoteObject.type || '', renderPreview(remoteObject), remoteObject.objectId, potentiallyUnserializableValue(remoteObject)); } async releaseHandle(objectId: js.ObjectId): Promise { @@ -123,3 +123,22 @@ function potentiallyUnserializableValue(remoteObject: Protocol.Runtime.RemoteObj const unserializableValue = remoteObject.unserializableValue; return unserializableValue ? js.parseUnserializableValue(unserializableValue) : value; } + +function renderPreview(object: Protocol.Runtime.RemoteObject): string | undefined { + if (object.type === 'undefined') + return 'undefined'; + if (object.unserializableValue) + return String(object.unserializableValue); + if (object.type === 'symbol') + return 'Symbol()'; + if (object.subtype === 'regexp') + return 'RegExp'; + if (object.subtype === 'weakmap') + return 'WeakMap'; + if (object.subtype === 'weakset') + return 'WeakSet'; + if (object.subtype) + return object.subtype[0].toUpperCase() + object.subtype.slice(1); + if ('value' in object) + return String(object.value); +} diff --git a/src/server/javascript.ts b/src/server/javascript.ts index 82fc630fa7..9142a51c1a 100644 --- a/src/server/javascript.ts +++ b/src/server/javascript.ts @@ -76,7 +76,7 @@ export class ExecutionContext extends SdkObject { ${utilityScriptSource.source} return new pwExport(); })();`; - this._utilityScriptPromise = this._delegate.rawEvaluateHandle(source).then(objectId => new JSHandle(this, 'object', objectId)); + this._utilityScriptPromise = this._delegate.rawEvaluateHandle(source).then(objectId => new JSHandle(this, 'object', undefined, objectId)); } return this._utilityScriptPromise; } @@ -90,7 +90,7 @@ export class ExecutionContext extends SdkObject { } async doSlowMo() { - // overrided in FrameExecutionContext + // overridden in FrameExecutionContext } } @@ -103,15 +103,13 @@ export class JSHandle extends SdkObject { protected _preview: string; private _previewCallback: ((preview: string) => void) | undefined; - constructor(context: ExecutionContext, type: string, objectId?: ObjectId, value?: any) { + constructor(context: ExecutionContext, type: string, preview: string | undefined, objectId?: ObjectId, value?: any) { super(context, 'handle'); this._context = context; this._objectId = objectId; this._value = value; this._objectType = type; - if (this._objectId) - this._value = 'JSHandle@' + this._objectType; - this._preview = 'JSHandle@' + String(this._objectId ? this._objectType : this._value); + this._preview = this._objectId ? preview || `JSHandle@${this._objectType}` : String(value); } callFunctionNoReply(func: Function, arg: any) { @@ -182,6 +180,10 @@ export class JSHandle extends SdkObject { this._previewCallback = callback; } + preview(): string { + return this._preview; + } + _setPreview(preview: string) { this._preview = preview; if (this._previewCallback) diff --git a/src/server/webkit/wkExecutionContext.ts b/src/server/webkit/wkExecutionContext.ts index 4f0656ccc2..aac2685116 100644 --- a/src/server/webkit/wkExecutionContext.ts +++ b/src/server/webkit/wkExecutionContext.ts @@ -121,7 +121,7 @@ export class WKExecutionContext implements js.ExecutionContextDelegate { createHandle(context: js.ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject): js.JSHandle { const isPromise = remoteObject.className === 'Promise'; - return new js.JSHandle(context, isPromise ? 'promise' : remoteObject.subtype || remoteObject.type, remoteObject.objectId, potentiallyUnserializableValue(remoteObject)); + return new js.JSHandle(context, isPromise ? 'promise' : remoteObject.subtype || remoteObject.type, renderPreview(remoteObject), remoteObject.objectId, potentiallyUnserializableValue(remoteObject)); } async releaseHandle(objectId: js.ObjectId): Promise { @@ -147,3 +147,24 @@ function rewriteError(error: Error): Error { return new Error('Execution context was destroyed, most likely because of a navigation.'); return error; } + +function renderPreview(object: Protocol.Runtime.RemoteObject): string | undefined { + if (object.type === 'undefined') + return 'undefined'; + if ('value' in object) + return String(object.value); + + if (object.description === 'Object' && object.preview) { + const tokens = []; + for (const { name, value } of object.preview.properties!) + tokens.push(`${name}: ${value}`); + return `{${tokens.join(', ')}}`; + } + if (object.subtype === 'array' && object.preview) { + const result = []; + for (const { name, value } of object.preview.properties!) + result[+name] = value; + return '[' + String(result) + ']'; + } + return object.description; +} diff --git a/tests/page/jshandle-to-string.spec.ts b/tests/page/jshandle-to-string.spec.ts index c3c2b7b6a0..39d5f4890c 100644 --- a/tests/page/jshandle-to-string.spec.ts +++ b/tests/page/jshandle-to-string.spec.ts @@ -19,41 +19,47 @@ import { test as it, expect } from './pageTest'; it('should work for primitives', async ({page}) => { const numberHandle = await page.evaluateHandle(() => 2); - expect(numberHandle.toString()).toBe('JSHandle@2'); + expect(numberHandle.toString()).toBe('2'); const stringHandle = await page.evaluateHandle(() => 'a'); - expect(stringHandle.toString()).toBe('JSHandle@a'); + expect(stringHandle.toString()).toBe('a'); }); -it('should work for complicated objects', async ({page}) => { +it('should work for complicated objects', async ({ page, browserName }) => { const aHandle = await page.evaluateHandle(() => window); - expect(aHandle.toString()).toBe('JSHandle@object'); + if (browserName !== 'firefox') + expect(aHandle.toString()).toBe('Window'); + else + expect(aHandle.toString()).toBe('JSHandle@object'); }); -it('should work for promises', async ({page}) => { +it('should work for promises', async ({ page }) => { // wrap the promise in an object, otherwise we will await. const wrapperHandle = await page.evaluateHandle(() => ({b: Promise.resolve(123)})); const bHandle = await wrapperHandle.getProperty('b'); - expect(bHandle.toString()).toBe('JSHandle@promise'); + expect(bHandle.toString()).toBe('Promise'); }); -it('should work with different subtypes', async ({page, browserName}) => { - expect((await page.evaluateHandle('(function(){})')).toString()).toBe('JSHandle@function'); - expect((await page.evaluateHandle('12')).toString()).toBe('JSHandle@12'); - expect((await page.evaluateHandle('true')).toString()).toBe('JSHandle@true'); - expect((await page.evaluateHandle('undefined')).toString()).toBe('JSHandle@undefined'); - expect((await page.evaluateHandle('"foo"')).toString()).toBe('JSHandle@foo'); - expect((await page.evaluateHandle('Symbol()')).toString()).toBe('JSHandle@symbol'); - expect((await page.evaluateHandle('new Map()')).toString()).toBe('JSHandle@map'); - expect((await page.evaluateHandle('new Set()')).toString()).toBe('JSHandle@set'); - expect((await page.evaluateHandle('[]')).toString()).toBe('JSHandle@array'); - expect((await page.evaluateHandle('null')).toString()).toBe('JSHandle@null'); - expect((await page.evaluateHandle('/foo/')).toString()).toBe('JSHandle@regexp'); +it('should work with different subtypes', async ({ page, browserName }) => { + expect((await page.evaluateHandle('(function(){})')).toString()).toContain('function'); + expect((await page.evaluateHandle('12')).toString()).toBe('12'); + expect((await page.evaluateHandle('true')).toString()).toBe('true'); + expect((await page.evaluateHandle('undefined')).toString()).toBe('undefined'); + expect((await page.evaluateHandle('"foo"')).toString()).toBe('foo'); + expect((await page.evaluateHandle('Symbol()')).toString()).toBe('Symbol()'); + expect((await page.evaluateHandle('new Map()')).toString()).toContain('Map'); + expect((await page.evaluateHandle('new Set()')).toString()).toContain('Set'); + expect((await page.evaluateHandle('[]')).toString()).toContain('Array'); + expect((await page.evaluateHandle('null')).toString()).toBe('null'); expect((await page.evaluateHandle('document.body')).toString()).toBe('JSHandle@node'); - expect((await page.evaluateHandle('new Date()')).toString()).toBe('JSHandle@date'); - expect((await page.evaluateHandle('new WeakMap()')).toString()).toBe('JSHandle@weakmap'); - expect((await page.evaluateHandle('new WeakSet()')).toString()).toBe('JSHandle@weakset'); - expect((await page.evaluateHandle('new Error()')).toString()).toBe('JSHandle@error'); - // TODO(yurys): change subtype from array to typedarray in WebKit. - expect((await page.evaluateHandle('new Int32Array()')).toString()).toBe(browserName === 'webkit' ? 'JSHandle@array' : 'JSHandle@typedarray'); - expect((await page.evaluateHandle('new Proxy({}, {})')).toString()).toBe('JSHandle@proxy'); + expect((await page.evaluateHandle('new WeakMap()')).toString()).toBe('WeakMap'); + expect((await page.evaluateHandle('new WeakSet()')).toString()).toBe('WeakSet'); + expect((await page.evaluateHandle('new Error()')).toString()).toContain('Error'); + expect((await page.evaluateHandle('new Proxy({}, {})')).toString()).toBe('Proxy'); +}); + +it('should work with previewable subtypes', async ({ page, browserName }) => { + it.skip(browserName === 'firefox'); + expect((await page.evaluateHandle('/foo/')).toString()).toBe('/foo/'); + expect((await page.evaluateHandle('new Date(0)')).toString()).toContain('GMT'); + expect((await page.evaluateHandle('new Int32Array()')).toString()).toContain('Int32Array'); }); diff --git a/tests/page/locator-convenience.spec.ts b/tests/page/locator-convenience.spec.ts index 07c14139ca..ddc96fc01d 100644 --- a/tests/page/locator-convenience.spec.ts +++ b/tests/page/locator-convenience.spec.ts @@ -88,50 +88,6 @@ it('textContent should work', async ({ page, server }) => { expect(await page.textContent('#inner')).toBe('Text,\nmore text'); }); -it('textContent should be atomic', async ({ playwright, page }) => { - const createDummySelector = () => ({ - query(root, selector) { - const result = root.querySelector(selector); - if (result) - Promise.resolve().then(() => result.textContent = 'modified'); - return result; - }, - queryAll(root: HTMLElement, selector: string) { - const result = Array.from(root.querySelectorAll(selector)); - for (const e of result) - Promise.resolve().then(() => e.textContent = 'modified'); - return result; - } - }); - await playwright.selectors.register('textContentFromLocators', createDummySelector); - await page.setContent(`
Hello
`); - const tc = await page.textContent('textContentFromLocators=div'); - expect(tc).toBe('Hello'); - expect(await page.evaluate(() => document.querySelector('div').textContent)).toBe('modified'); -}); - -it('innerText should be atomic', async ({ playwright, page }) => { - const createDummySelector = () => ({ - query(root: HTMLElement, selector: string) { - const result = root.querySelector(selector); - if (result) - Promise.resolve().then(() => result.textContent = 'modified'); - return result; - }, - queryAll(root: HTMLElement, selector: string) { - const result = Array.from(root.querySelectorAll(selector)); - for (const e of result) - Promise.resolve().then(() => e.textContent = 'modified'); - return result; - } - }); - await playwright.selectors.register('innerTextFromLocators', createDummySelector); - await page.setContent(`
Hello
`); - const tc = await page.innerText('innerTextFromLocators=div'); - expect(tc).toBe('Hello'); - expect(await page.evaluate(() => document.querySelector('div').innerText)).toBe('modified'); -}); - it('isVisible and isHidden should work', async ({ page }) => { await page.setContent(`
Hi
`); diff --git a/tests/page/page-event-console.spec.ts b/tests/page/page-event-console.spec.ts index f0ee53e7a5..141b487f5a 100644 --- a/tests/page/page-event-console.spec.ts +++ b/tests/page/page-event-console.spec.ts @@ -18,14 +18,17 @@ import { test as it, expect } from './pageTest'; import util from 'util'; -it('should work', async ({page}) => { +it('should work', async ({page, browserName}) => { let message = null; page.once('console', m => message = m); await Promise.all([ page.evaluate(() => console.log('hello', 5, {foo: 'bar'})), page.waitForEvent('console') ]); - expect(message.text()).toEqual('hello 5 JSHandle@object'); + if (browserName !== 'firefox') + expect(message.text()).toEqual('hello 5 {foo: bar}'); + else + expect(message.text()).toEqual('hello 5 JSHandle@object'); expect(message.type()).toEqual('log'); expect(await message.args()[0].jsonValue()).toEqual('hello'); expect(await message.args()[1].jsonValue()).toEqual(5); @@ -72,18 +75,21 @@ it('should work for different console API calls', async ({page}) => { 'calling console.dir', 'calling console.warn', 'calling console.error', - 'JSHandle@promise', + 'Promise', ]); }); -it('should not fail for window object', async ({page}) => { +it('should not fail for window object', async ({ page, browserName }) => { let message = null; page.once('console', msg => message = msg); await Promise.all([ page.evaluate(() => console.error(window)), page.waitForEvent('console') ]); - expect(message.text()).toBe('JSHandle@object'); + if (browserName !== 'firefox') + expect(message.text()).toEqual('Window'); + else + expect(message.text()).toEqual('JSHandle@object'); }); it('should trigger correct Log', async ({page, server}) => { @@ -135,3 +141,30 @@ it('should not throw when there are console messages in detached iframes', async // 4. Connect to the popup and make sure it doesn't throw. expect(await popup.evaluate('1 + 1')).toBe(2); }); + +it('should use object previews for arrays and objects', async ({page, browserName}) => { + let text: string; + page.on('console', message => { + text = message.text(); + }); + await page.evaluate(() => console.log([1, 2, 3], {a: 1}, window)); + + if (browserName !== 'firefox') + expect(text).toEqual('[1,2,3] {a: 1} Window'); + else + expect(text).toEqual('Array JSHandle@object JSHandle@object'); +}); + +it('should use object previews for errors', async ({page, browserName}) => { + let text: string; + page.on('console', message => { + text = message.text(); + }); + await page.evaluate(() => console.log(new Error('Exception'))); + if (browserName === 'chromium') + expect(text).toContain('.evaluate'); + if (browserName === 'webkit') + expect(text).toEqual('Error: Exception'); + if (browserName === 'firefox') + expect(text).toEqual('Error'); +}); diff --git a/tests/page/workers.spec.ts b/tests/page/workers.spec.ts index ba14b38797..002d677494 100644 --- a/tests/page/workers.spec.ts +++ b/tests/page/workers.spec.ts @@ -67,11 +67,14 @@ it('should not report console logs from workers twice', async function({page}) { expect(page.url()).not.toContain('blob'); }); -it('should have JSHandles for console logs', async function({page}) { +it('should have JSHandles for console logs', async function({ page, browserName }) { const logPromise = new Promise(x => page.on('console', x)); await page.evaluate(() => new Worker(URL.createObjectURL(new Blob(['console.log(1,2,3,this)'], {type: 'application/javascript'})))); const log = await logPromise; - expect(log.text()).toBe('1 2 3 JSHandle@object'); + if (browserName !== 'firefox') + expect(log.text()).toBe('1 2 3 DedicatedWorkerGlobalScope'); + else + expect(log.text()).toBe('1 2 3 JSHandle@object'); expect(log.args().length).toBe(4); expect(await (await log.args()[3].getProperty('origin')).jsonValue()).toBe('null'); });