From 9b54365df56914de0c4722233192f2095254da7c Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com> Date: Mon, 20 Feb 2023 13:00:29 +0100 Subject: [PATCH] chore: Add Page.Console event to BiDi (#9700) --- .../puppeteer-core/src/common/JSHandle.ts | 2 +- .../puppeteer-core/src/common/WebWorker.ts | 3 +- .../puppeteer-core/src/common/bidi/Browser.ts | 5 +- .../src/common/bidi/Connection.ts | 14 +++ .../src/common/bidi/JSHandle.ts | 27 +++-- .../puppeteer-core/src/common/bidi/Page.ts | 98 +++++++++++++++++-- .../puppeteer-core/src/common/bidi/types.ts | 6 -- test/TestExpectations.json | 28 +++--- test/src/page.spec.ts | 52 ++++++---- 9 files changed, 176 insertions(+), 59 deletions(-) delete mode 100644 packages/puppeteer-core/src/common/bidi/types.ts diff --git a/packages/puppeteer-core/src/common/JSHandle.ts b/packages/puppeteer-core/src/common/JSHandle.ts index 3cbc752c..1e405916 100644 --- a/packages/puppeteer-core/src/common/JSHandle.ts +++ b/packages/puppeteer-core/src/common/JSHandle.ts @@ -30,7 +30,7 @@ declare const __JSHandleSymbol: unique symbol; /** * @internal */ -export class CDPJSHandle extends JSHandle { +export class CDPJSHandle extends JSHandle { /** * Used for nominally typing {@link JSHandle}. */ diff --git a/packages/puppeteer-core/src/common/WebWorker.ts b/packages/puppeteer-core/src/common/WebWorker.ts index e3a98318..9594e52f 100644 --- a/packages/puppeteer-core/src/common/WebWorker.ts +++ b/packages/puppeteer-core/src/common/WebWorker.ts @@ -15,7 +15,6 @@ */ import {Protocol} from 'devtools-protocol'; -import {JSHandle} from '../api/JSHandle.js'; import {createDeferredPromise} from '../util/DeferredPromise.js'; import {CDPSession} from './Connection.js'; @@ -31,7 +30,7 @@ import {debugError} from './util.js'; */ export type ConsoleAPICalledCallback = ( eventType: ConsoleMessageType, - handles: JSHandle[], + handles: CDPJSHandle[], trace: Protocol.Runtime.StackTrace ) => void; diff --git a/packages/puppeteer-core/src/common/bidi/Browser.ts b/packages/puppeteer-core/src/common/bidi/Browser.ts index 60ccc0be..66f10bb4 100644 --- a/packages/puppeteer-core/src/common/bidi/Browser.ts +++ b/packages/puppeteer-core/src/common/bidi/Browser.ts @@ -36,10 +36,7 @@ export class Browser extends BrowserBase { static async create(opts: Options): Promise { // TODO: await until the connection is established. try { - // TODO: Add 'session.new' to BiDi types - (await opts.connection.send('session.new' as any, {})) as unknown as { - sessionId: string; - }; + await opts.connection.send('session.new', {}); } catch {} return new Browser(opts); } diff --git a/packages/puppeteer-core/src/common/bidi/Connection.ts b/packages/puppeteer-core/src/common/bidi/Connection.ts index 6b89f968..9ec2bde5 100644 --- a/packages/puppeteer-core/src/common/bidi/Connection.ts +++ b/packages/puppeteer-core/src/common/bidi/Connection.ts @@ -42,6 +42,7 @@ interface Commands { params: Bidi.Script.DisownParameters; returnType: Bidi.Script.DisownResult; }; + 'browsingContext.create': { params: Bidi.BrowsingContext.CreateParameters; returnType: Bidi.BrowsingContext.CreateResult; @@ -50,10 +51,23 @@ interface Commands { params: Bidi.BrowsingContext.CloseParameters; returnType: Bidi.BrowsingContext.CloseResult; }; + + 'session.new': { + params: {capabilities?: Record}; // TODO: Update Types in chromium bidi + returnType: {sessionId: string}; + }; 'session.status': { params: {context: string}; // TODO: Update Types in chromium bidi returnType: Bidi.Session.StatusResult; }; + 'session.subscribe': { + params: Bidi.Session.SubscribeParameters; + returnType: Bidi.Session.SubscribeResult; + }; + 'session.unsubscribe': { + params: Bidi.Session.SubscribeParameters; + returnType: Bidi.Session.UnsubscribeResult; + }; } /** diff --git a/packages/puppeteer-core/src/common/bidi/JSHandle.ts b/packages/puppeteer-core/src/common/bidi/JSHandle.ts index 7fe40523..926edeaa 100644 --- a/packages/puppeteer-core/src/common/bidi/JSHandle.ts +++ b/packages/puppeteer-core/src/common/bidi/JSHandle.ts @@ -104,13 +104,9 @@ export class JSHandle extends BaseJSHandle { } override async jsonValue(): Promise { - if (!('handle' in this.#remoteValue)) { - return BidiSerializer.deserialize(this.#remoteValue); - } - const value = await this.evaluate(object => { - return object; - }); - if (value === undefined) { + const value = BidiSerializer.deserialize(this.#remoteValue); + + if (this.#remoteValue.type !== 'undefined' && value === undefined) { throw new Error('Could not serialize referenced object'); } return value; @@ -130,8 +126,23 @@ export class JSHandle extends BaseJSHandle { } } + get isPrimitiveValue(): boolean { + switch (this.#remoteValue.type) { + case 'string': + case 'number': + case 'bigint': + case 'boolean': + case 'undefined': + case 'null': + return true; + + default: + return false; + } + } + override toString(): string { - if (!('handle' in this.#remoteValue)) { + if (this.isPrimitiveValue) { return 'JSHandle:' + BidiSerializer.deserialize(this.#remoteValue); } diff --git a/packages/puppeteer-core/src/common/bidi/Page.ts b/packages/puppeteer-core/src/common/bidi/Page.ts index 8c12c0df..9a2a1eac 100644 --- a/packages/puppeteer-core/src/common/bidi/Page.ts +++ b/packages/puppeteer-core/src/common/bidi/Page.ts @@ -14,33 +14,90 @@ * limitations under the License. */ -import {Page as PageBase} from '../../api/Page.js'; +import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; + +import {Page as PageBase, PageEmittedEvents} from '../../api/Page.js'; import {stringifyFunction} from '../../util/Function.js'; +import {ConsoleMessage, ConsoleMessageLocation} from '../ConsoleMessage.js'; import type {EvaluateFunc, HandleFor} from '../types.js'; import {isString} from '../util.js'; import {Connection} from './Connection.js'; import {JSHandle} from './JSHandle.js'; import {BidiSerializer} from './Serializer.js'; -import {Reference} from './types.js'; /** * @internal */ export class Page extends PageBase { #connection: Connection; + #subscribedEvents = [ + 'log.entryAdded', + ] as Bidi.Session.SubscribeParameters['events']; _contextId: string; constructor(connection: Connection, contextId: string) { super(); this.#connection = connection; this._contextId = contextId; + + // TODO: Investigate an implementation similar to CDPSession + this.connection.send('session.subscribe', { + events: this.#subscribedEvents, + contexts: [this._contextId], + }); + + this.connection.on('log.entryAdded', this.#onLogEntryAdded.bind(this)); + } + + #onLogEntryAdded(event: Bidi.Log.LogEntry): void { + if (isConsoleLogEntry(event)) { + const args = event.args.map(arg => { + return getBidiHandle(this, arg); + }); + + const text = args + .reduce((value, arg) => { + const parsedValue = arg.isPrimitiveValue + ? BidiSerializer.deserialize(arg.bidiObject()) + : arg.toString(); + return `${value} ${parsedValue}`; + }, '') + .slice(1); + + this.emit( + PageEmittedEvents.Console, + new ConsoleMessage( + event.method as any, + text, + args, + getStackTraceLocations(event.stackTrace) + ) + ); + } else if (isJavaScriptLogEntry(event)) { + this.emit( + PageEmittedEvents.Console, + new ConsoleMessage( + event.level as any, + event.text ?? '', + [], + getStackTraceLocations(event.stackTrace) + ) + ); + } } override async close(): Promise { await this.#connection.send('browsingContext.close', { context: this._contextId, }); + + this.connection.send('session.unsubscribe', { + events: this.#subscribedEvents, + contexts: [this._contextId], + }); + + this.connection.off('log.entryAdded', this.#onLogEntryAdded.bind(this)); } get connection(): Connection { @@ -122,20 +179,49 @@ export class Page extends PageBase { return returnByValue ? BidiSerializer.deserialize(result.result) - : getBidiHandle(this, result.result as Reference); + : getBidiHandle(this, result.result); } } /** * @internal */ -export function getBidiHandle(context: Page, result: Reference): JSHandle { - // TODO: | ElementHandle +export function getBidiHandle( + context: Page, + result: Bidi.CommonDataTypes.RemoteValue +): JSHandle { if ( (result.type === 'node' || result.type === 'window') && context._contextId ) { - throw new Error('ElementHandle not implemented'); + // TODO: Implement ElementHandle + return new JSHandle(context, result); } return new JSHandle(context, result); } + +function isConsoleLogEntry( + event: Bidi.Log.LogEntry +): event is Bidi.Log.ConsoleLogEntry { + return event.type === 'console'; +} + +function isJavaScriptLogEntry( + event: Bidi.Log.LogEntry +): event is Bidi.Log.JavascriptLogEntry { + return event.type === 'javascript'; +} + +function getStackTraceLocations(stackTrace?: Bidi.Script.StackTrace) { + const stackTraceLocations: ConsoleMessageLocation[] = []; + if (stackTrace) { + for (const callFrame of stackTrace.callFrames) { + stackTraceLocations.push({ + url: callFrame.url, + lineNumber: callFrame.lineNumber, + columnNumber: callFrame.columnNumber, + }); + } + } + return stackTraceLocations; +} diff --git a/packages/puppeteer-core/src/common/bidi/types.ts b/packages/puppeteer-core/src/common/bidi/types.ts deleted file mode 100644 index 0b3e09ab..00000000 --- a/packages/puppeteer-core/src/common/bidi/types.ts +++ /dev/null @@ -1,6 +0,0 @@ -import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; - -export type Reference = Extract< - Bidi.CommonDataTypes.RemoteValue, - Bidi.CommonDataTypes.RemoteReference ->; diff --git a/test/TestExpectations.json b/test/TestExpectations.json index 64d91e40..077d3c5b 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -1068,7 +1068,7 @@ "expectations": ["FAIL"] }, { - "testIdPattern": "[page.spec] Page Page.Events.Console should work for different console API calls", + "testIdPattern": "[page.spec] Page Page.Events.Console should work for different console API calls with logging functions", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox"], "expectations": ["FAIL"] @@ -1763,24 +1763,12 @@ "parameters": ["webDriverBiDi"], "expectations": ["PASS"] }, - { - "testIdPattern": "[jshandle.spec] JSHandle Page.evaluateHandle should work", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, { "testIdPattern": "[jshandle.spec] JSHandle Page.evaluateHandle should return the RemoteObject", "platforms": ["darwin", "linux", "win32"], "parameters": ["webDriverBiDi"], "expectations": ["FAIL"] }, - { - "testIdPattern": "[jshandle.spec] JSHandle Page.evaluateHandle should use the same JS wrappers", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, { "testIdPattern": "[jshandle.spec] JSHandle JSHandle.jsonValue should not work with dates", "platforms": ["darwin", "linux", "win32"], @@ -1808,7 +1796,7 @@ { "testIdPattern": "[jshandle.spec] JSHandle JSHandle.toString should work with different subtypes", "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], + "parameters": ["firefox", "webDriverBiDi"], "expectations": ["FAIL"] }, { @@ -1822,5 +1810,17 @@ "platforms": ["darwin", "linux", "win32"], "parameters": ["webDriverBiDi"], "expectations": ["SKIP", "FAIL"] + }, + { + "testIdPattern": "[page.spec] Page Page.Events.Console should work", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL"] + }, + { + "testIdPattern": "[page.spec] Page Page.Events.Console should work for different console API calls with timing functions", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL"] } ] diff --git a/test/src/page.spec.ts b/test/src/page.spec.ts index b34d0383..f73ddb32 100644 --- a/test/src/page.spec.ts +++ b/test/src/page.spec.ts @@ -668,7 +668,39 @@ describe('Page', function () { expect(await message.args()[1]!.jsonValue()).toEqual(5); expect(await message.args()[2]!.jsonValue()).toEqual({foo: 'bar'}); }); - it('should work for different console API calls', async () => { + it('should work for different console API calls with logging functions', async () => { + const {page} = getTestState(); + + const messages: any[] = []; + page.on('console', msg => { + return messages.push(msg); + }); + // All console events will be reported before `page.evaluate` is finished. + await page.evaluate(() => { + console.trace('calling console.trace'); + console.dir('calling console.dir'); + console.warn('calling console.warn'); + console.error('calling console.error'); + console.log(Promise.resolve('should not wait until resolved!')); + }); + expect( + messages.map(msg => { + return msg.type(); + }) + ).toEqual(['trace', 'dir', 'warning', 'error', 'log']); + expect( + messages.map(msg => { + return msg.text(); + }) + ).toEqual([ + 'calling console.trace', + 'calling console.dir', + 'calling console.warn', + 'calling console.error', + 'JSHandle@promise', + ]); + }); + it('should work for different console API calls with timing functions', async () => { const {page} = getTestState(); const messages: any[] = []; @@ -680,29 +712,13 @@ describe('Page', function () { // A pair of time/timeEnd generates only one Console API call. console.time('calling console.time'); console.timeEnd('calling console.time'); - console.trace('calling console.trace'); - console.dir('calling console.dir'); - console.warn('calling console.warn'); - console.error('calling console.error'); - console.log(Promise.resolve('should not wait until resolved!')); }); expect( messages.map(msg => { return msg.type(); }) - ).toEqual(['timeEnd', 'trace', 'dir', 'warning', 'error', 'log']); + ).toEqual(['timeEnd']); expect(messages[0]!.text()).toContain('calling console.time'); - expect( - messages.slice(1).map(msg => { - return msg.text(); - }) - ).toEqual([ - 'calling console.trace', - 'calling console.dir', - 'calling console.warn', - 'calling console.error', - 'JSHandle@promise', - ]); }); it('should not fail for window object', async () => { const {page} = getTestState();