From 004a99aaeff15f3c24d68423aff661b9d37968b1 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com> Date: Wed, 1 Mar 2023 11:09:17 +0100 Subject: [PATCH] chore: implement simple BiDi ElementHandle (#9753) --- docs/api/puppeteer.elementhandle.aselement.md | 4 +- .../puppeteer-core/src/api/ElementHandle.ts | 107 +++++++++++++++++- .../src/common/ElementHandle.ts | 84 +------------- .../puppeteer-core/src/common/bidi/Context.ts | 8 +- .../src/common/bidi/ElementHandle.ts | 52 +++++++++ .../puppeteer-core/src/common/bidi/Page.ts | 22 +--- .../src/common/bidi/Serializer.ts | 6 +- test/TestExpectations.json | 6 - 8 files changed, 177 insertions(+), 112 deletions(-) create mode 100644 packages/puppeteer-core/src/common/bidi/ElementHandle.ts diff --git a/docs/api/puppeteer.elementhandle.aselement.md b/docs/api/puppeteer.elementhandle.aselement.md index 1d3845215c2..17a9d8a4575 100644 --- a/docs/api/puppeteer.elementhandle.aselement.md +++ b/docs/api/puppeteer.elementhandle.aselement.md @@ -8,10 +8,10 @@ sidebar_label: ElementHandle.asElement ```typescript class ElementHandle { - asElement(): ElementHandle | null; + asElement(): ElementHandle; } ``` **Returns:** -[ElementHandle](./puppeteer.elementhandle.md)<ElementType> \| null +[ElementHandle](./puppeteer.elementhandle.md)<ElementType> diff --git a/packages/puppeteer-core/src/api/ElementHandle.ts b/packages/puppeteer-core/src/api/ElementHandle.ts index be01c31ddd5..a7aad6f60e2 100644 --- a/packages/puppeteer-core/src/api/ElementHandle.ts +++ b/packages/puppeteer-core/src/api/ElementHandle.ts @@ -25,6 +25,7 @@ import { ElementFor, EvaluateFuncWith, HandleFor, + HandleOr, NodeFor, } from '../common/types.js'; import {KeyInput} from '../common/USKeyboardLayout.js'; @@ -158,8 +159,108 @@ export class ElementHandle< /** * @internal */ - constructor() { + protected handle; + + /** + * @internal + */ + constructor(handle: JSHandle) { super(); + this.handle = handle; + } + + /** + * @internal + */ + override get id(): string | undefined { + return this.handle.id; + } + + /** + * @internal + */ + override get disposed(): boolean { + return this.handle.disposed; + } + + /** + * @internal + */ + override async getProperty( + propertyName: HandleOr + ): Promise>; + /** + * @internal + */ + override async getProperty(propertyName: string): Promise>; + override async getProperty( + propertyName: HandleOr + ): Promise> { + return this.handle.getProperty(propertyName); + } + + /** + * @internal + */ + override async getProperties(): Promise> { + return this.handle.getProperties(); + } + + /** + * @internal + */ + override async evaluate< + Params extends unknown[], + Func extends EvaluateFuncWith = EvaluateFuncWith< + ElementType, + Params + > + >( + pageFunction: Func | string, + ...args: Params + ): Promise>> { + return this.handle.evaluate(pageFunction, ...args); + } + + /** + * @internal + */ + override evaluateHandle< + Params extends unknown[], + Func extends EvaluateFuncWith = EvaluateFuncWith< + ElementType, + Params + > + >( + pageFunction: Func | string, + ...args: Params + ): Promise>>> { + return this.handle.evaluateHandle(pageFunction, ...args); + } + + /** + * @internal + */ + override async jsonValue(): Promise { + return this.handle.jsonValue(); + } + + /** + * @internal + */ + override toString(): string { + return this.handle.toString(); + } + + /** + * @internal + */ + override async dispose(): Promise { + return await this.handle.dispose(); + } + + override asElement(): ElementHandle { + return this; } /** @@ -468,10 +569,6 @@ export class ElementHandle< throw new Error('Not implemented'); } - override asElement(): ElementHandle | null { - return this; - } - /** * Resolves to the content frame for element handles referencing * iframe nodes, or null otherwise diff --git a/packages/puppeteer-core/src/common/ElementHandle.ts b/packages/puppeteer-core/src/common/ElementHandle.ts index 5ef8fdf33e6..43313c9366c 100644 --- a/packages/puppeteer-core/src/common/ElementHandle.ts +++ b/packages/puppeteer-core/src/common/ElementHandle.ts @@ -25,7 +25,6 @@ import { Point, PressOptions, } from '../api/ElementHandle.js'; -import {JSHandle} from '../api/JSHandle.js'; import {Page, ScreenshotOptions} from '../api/Page.js'; import {assert} from '../util/assert.js'; import {AsyncIterableUtil} from '../util/AsyncIterableUtil.js'; @@ -38,13 +37,7 @@ import {getQueryHandlerAndSelector} from './GetQueryHandler.js'; import {WaitForSelectorOptions} from './IsolatedWorld.js'; import {CDPJSHandle} from './JSHandle.js'; import {CDPPage} from './Page.js'; -import { - ElementFor, - EvaluateFuncWith, - HandleFor, - HandleOr, - NodeFor, -} from './types.js'; +import {ElementFor, EvaluateFuncWith, HandleFor, NodeFor} from './types.js'; import {KeyInput} from './USKeyboardLayout.js'; import {debugError, isString} from './util.js'; @@ -69,15 +62,14 @@ export class CDPElementHandle< ElementType extends Node = Element > extends ElementHandle { #frame: Frame; - #jsHandle: CDPJSHandle; + declare handle: CDPJSHandle; constructor( context: ExecutionContext, remoteObject: Protocol.Runtime.RemoteObject, frame: Frame ) { - super(); - this.#jsHandle = new CDPJSHandle(context, remoteObject); + super(new CDPJSHandle(context, remoteObject)); this.#frame = frame; } @@ -85,48 +77,18 @@ export class CDPElementHandle< * @internal */ override executionContext(): ExecutionContext { - return this.#jsHandle.executionContext(); + return this.handle.executionContext(); } /** * @internal */ override get client(): CDPSession { - return this.#jsHandle.client; - } - - override get id(): string | undefined { - return this.#jsHandle.id; + return this.handle.client; } override remoteObject(): Protocol.Runtime.RemoteObject { - return this.#jsHandle.remoteObject(); - } - - override async evaluate< - Params extends unknown[], - Func extends EvaluateFuncWith = EvaluateFuncWith< - ElementType, - Params - > - >( - pageFunction: Func | string, - ...args: Params - ): Promise>> { - return this.executionContext().evaluate(pageFunction, this, ...args); - } - - override evaluateHandle< - Params extends unknown[], - Func extends EvaluateFuncWith = EvaluateFuncWith< - ElementType, - Params - > - >( - pageFunction: Func | string, - ...args: Params - ): Promise>>> { - return this.executionContext().evaluateHandle(pageFunction, this, ...args); + return this.handle.remoteObject(); } get #frameManager(): FrameManager { @@ -141,40 +103,6 @@ export class CDPElementHandle< return this.#frame; } - override get disposed(): boolean { - return this.#jsHandle.disposed; - } - - override async getProperty( - propertyName: HandleOr - ): Promise>; - override async getProperty(propertyName: string): Promise>; - override async getProperty( - propertyName: HandleOr - ): Promise> { - return this.#jsHandle.getProperty(propertyName); - } - - override async getProperties(): Promise> { - return this.#jsHandle.getProperties(); - } - - override asElement(): CDPElementHandle | null { - return this; - } - - override async jsonValue(): Promise { - return this.#jsHandle.jsonValue(); - } - - override toString(): string { - return this.#jsHandle.toString(); - } - - override async dispose(): Promise { - return await this.#jsHandle.dispose(); - } - override async $( selector: Selector ): Promise> | null> { diff --git a/packages/puppeteer-core/src/common/bidi/Context.ts b/packages/puppeteer-core/src/common/bidi/Context.ts index a04e696b2ee..73d596a9e84 100644 --- a/packages/puppeteer-core/src/common/bidi/Context.ts +++ b/packages/puppeteer-core/src/common/bidi/Context.ts @@ -22,6 +22,7 @@ import {EvaluateFunc, HandleFor} from '../types.js'; import {isString} from '../util.js'; import {Connection} from './Connection.js'; +import {ElementHandle} from './ElementHandle.js'; import {JSHandle} from './JSHandle.js'; import {BidiSerializer} from './Serializer.js'; @@ -131,10 +132,9 @@ export class Context extends EventEmitter { export function getBidiHandle( context: Context, result: Bidi.CommonDataTypes.RemoteValue -): JSHandle { - if ((result.type === 'node' || result.type === 'window') && context) { - // TODO: Implement ElementHandle - return new JSHandle(context, result); +): JSHandle | ElementHandle { + if (result.type === 'node' || result.type === 'window') { + return new ElementHandle(context, result); } return new JSHandle(context, result); } diff --git a/packages/puppeteer-core/src/common/bidi/ElementHandle.ts b/packages/puppeteer-core/src/common/bidi/ElementHandle.ts new file mode 100644 index 00000000000..21e69e3e9bf --- /dev/null +++ b/packages/puppeteer-core/src/common/bidi/ElementHandle.ts @@ -0,0 +1,52 @@ +/** + * Copyright 2023 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; + +import {ElementHandle as BaseElementHandle} from '../../api/ElementHandle.js'; + +import {Connection} from './Connection.js'; +import {Context} from './Context.js'; +import {JSHandle} from './JSHandle.js'; + +/** + * @internal + */ +export class ElementHandle< + ElementType extends Node = Element +> extends BaseElementHandle { + declare handle: JSHandle; + + constructor(context: Context, remoteValue: Bidi.CommonDataTypes.RemoteValue) { + super(new JSHandle(context, remoteValue)); + } + + context(): Context { + return this.handle.context(); + } + + get connection(): Connection { + return this.handle.connection; + } + + get isPrimitiveValue(): boolean { + return this.handle.isPrimitiveValue; + } + + remoteValue(): Bidi.CommonDataTypes.RemoteValue { + return this.handle.remoteValue(); + } +} diff --git a/packages/puppeteer-core/src/common/bidi/Page.ts b/packages/puppeteer-core/src/common/bidi/Page.ts index c35c6086abe..1d48c01ade9 100644 --- a/packages/puppeteer-core/src/common/bidi/Page.ts +++ b/packages/puppeteer-core/src/common/bidi/Page.ts @@ -20,7 +20,6 @@ import {Page as PageBase, PageEmittedEvents} from '../../api/Page.js'; import {ConsoleMessage, ConsoleMessageLocation} from '../ConsoleMessage.js'; import {EvaluateFunc, HandleFor} from '../types.js'; -import {Connection} from './Connection.js'; import {Context, getBidiHandle} from './Context.js'; import {BidiSerializer} from './Serializer.js'; @@ -38,10 +37,9 @@ export class Page extends PageBase { super(); this.#context = context; - // TODO: Investigate an implementation similar to CDPSession - this.connection.send('session.subscribe', { + this.#context.connection.send('session.subscribe', { events: this.#subscribedEvents, - contexts: [this.contextId], + contexts: [this.#context.id], }); this.#context.on('log.entryAdded', this.#boundOnLogEntryAdded); @@ -85,26 +83,18 @@ export class Page extends PageBase { } override async close(): Promise { - await this.connection.send('session.unsubscribe', { + await this.#context.connection.send('session.unsubscribe', { events: this.#subscribedEvents, - contexts: [this.contextId], + contexts: [this.#context.id], }); - await this.connection.send('browsingContext.close', { - context: this.contextId, + await this.#context.connection.send('browsingContext.close', { + context: this.#context.id, }); this.#context.off('log.entryAdded', this.#boundOnLogEntryAdded); } - get connection(): Connection { - return this.#context.connection; - } - - get contextId(): string { - return this.#context.id; - } - override async evaluateHandle< Params extends unknown[], Func extends EvaluateFunc = EvaluateFunc diff --git a/packages/puppeteer-core/src/common/bidi/Serializer.ts b/packages/puppeteer-core/src/common/bidi/Serializer.ts index d45034112ba..f28b0e73186 100644 --- a/packages/puppeteer-core/src/common/bidi/Serializer.ts +++ b/packages/puppeteer-core/src/common/bidi/Serializer.ts @@ -19,6 +19,7 @@ import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; import {debugError, isDate, isPlainObject, isRegExp} from '../util.js'; import {Context} from './Context.js'; +import {ElementHandle} from './ElementHandle.js'; import {JSHandle} from './JSHandle.js'; /** @@ -149,7 +150,10 @@ export class BidiSerializer { context: Context ): Bidi.CommonDataTypes.LocalOrRemoteValue { // TODO: See use case of LazyArgs - const objectHandle = arg && arg instanceof JSHandle ? arg : null; + const objectHandle = + arg && (arg instanceof JSHandle || arg instanceof ElementHandle) + ? arg + : null; if (objectHandle) { if (objectHandle.context() !== context) { throw new Error( diff --git a/test/TestExpectations.json b/test/TestExpectations.json index 077d3c5bcdc..a9af6c6a1e9 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -1775,12 +1775,6 @@ "parameters": ["webDriverBiDi"], "expectations": ["FAIL"] }, - { - "testIdPattern": "[jshandle.spec] JSHandle JSHandle.asElement should work", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["webDriverBiDi"], - "expectations": ["FAIL"] - }, { "testIdPattern": "[jshandle.spec] JSHandle JSHandle.asElement should return ElementHandle for TextNodes", "platforms": ["darwin", "linux", "win32"],