From b32c8f48123bd872d9a436c17e7bec105edc0a19 Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Thu, 7 Oct 2021 14:40:23 +0000 Subject: [PATCH 1/9] `browsingContext.navigate` --- src/bidiMapper/bidiProtocolTypes.ts | 22 ++++++++++++- src/bidiMapper/commandProcessor.ts | 4 +++ .../context/browsingContextProcessor.ts | 17 ++++++++-- src/bidiMapper/domains/context/context.ts | 23 ++++++++++++- tests/test_bidi.py | 33 ++++++++++++------- 5 files changed, 82 insertions(+), 17 deletions(-) diff --git a/src/bidiMapper/bidiProtocolTypes.ts b/src/bidiMapper/bidiProtocolTypes.ts index 4c1d08248b..425e59852d 100644 --- a/src/bidiMapper/bidiProtocolTypes.ts +++ b/src/bidiMapper/bidiProtocolTypes.ts @@ -95,6 +95,7 @@ export namespace Script { // https://w3c.github.io/webdriver-bidi/#module-browsingContext export namespace BrowsingContext { export type BrowsingContext = string; + export type Navigation = string; export type BrowsingContextGetTreeCommand = { method: 'browsingContext.getTree'; @@ -128,11 +129,30 @@ export namespace BrowsingContext { export type BrowsingContextCreateType = 'tab' | 'window'; export type BrowsingContextCreateParameters = { - type: BrowsingContextCreateType; + type?: BrowsingContextCreateType; }; + export type BrowsingContextCreateResult = { context: BrowsingContext; }; + + export type BrowsingContextNavigateCommand = { + method: 'browsingContext.navigate'; + params: BrowsingContextNavigateParameters; + }; + + export type BrowsingContextNavigateParameters = { + context: BrowsingContext; + url: string; + wait?: ReadinessState; + }; + + export type ReadinessState = 'none'; + // TODO sadym: implement 'interactive' and 'complete' states. + export type BrowsingContextNavigateResult = { + navigation?: Navigation; + url: string; + }; } export namespace Session { diff --git a/src/bidiMapper/commandProcessor.ts b/src/bidiMapper/commandProcessor.ts index ce05442e20..cdb20221e6 100644 --- a/src/bidiMapper/commandProcessor.ts +++ b/src/bidiMapper/commandProcessor.ts @@ -185,6 +185,10 @@ export class CommandProcessor { return await this._contextProcessor.process_createContext( commandData as BrowsingContext.BrowsingContextCreateCommand ); + case 'browsingContext.navigate': + return await this._contextProcessor.process_navigate( + commandData as BrowsingContext.BrowsingContextNavigateCommand + ); case 'PROTO.script.invoke': return await this._contextProcessor.process_PROTO_script_invoke( diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index e895e835ba..31d274157c 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -126,13 +126,14 @@ export class BrowsingContextProcessor { commandData: BrowsingContext.BrowsingContextCreateCommand ): Promise { const params = commandData.params; + return new Promise(async (resolve) => { let targetId: string; const onAttachedToTarget = async ( - params: Protocol.Target.AttachedToTargetEvent + attachToTargetEventParams: Protocol.Target.AttachedToTargetEvent ) => { - if (params.targetInfo.targetId === targetId) { + if (attachToTargetEventParams.targetInfo.targetId === targetId) { browserCdpClient.Target.removeListener( 'attachedToTarget', onAttachedToTarget @@ -140,7 +141,7 @@ export class BrowsingContextProcessor { const context = await this._getOrCreateContext( targetId, - params.sessionId + attachToTargetEventParams.sessionId ); resolve(context.toBidi()); } @@ -151,11 +152,21 @@ export class BrowsingContextProcessor { const result = await browserCdpClient.Target.createTarget({ url: 'about:blank', + newWindow: params.type === 'window', }); targetId = result.targetId; }); } + async process_navigate( + commandData: BrowsingContext.BrowsingContextNavigateCommand + ): Promise { + const params = commandData.params; + const context = this._getKnownContext(params.context); + + return await context.navigate(params.url, params.wait); + } + async process_script_evaluate( commandData: Script.ScriptEvaluateCommand ): Promise { diff --git a/src/bidiMapper/domains/context/context.ts b/src/bidiMapper/domains/context/context.ts index 622c3c352e..16a1141c7d 100644 --- a/src/bidiMapper/domains/context/context.ts +++ b/src/bidiMapper/domains/context/context.ts @@ -17,7 +17,11 @@ import { Protocol } from 'devtools-protocol'; import { CdpClient } from '../../../cdp'; -import { Script, CommonDataTypes } from '../../bidiProtocolTypes'; +import { + BrowsingContext, + CommonDataTypes, + Script, +} from '../../bidiProtocolTypes'; import EVALUATOR_SCRIPT from '../../scripts/eval.es'; @@ -80,6 +84,23 @@ export class Context { }; } + public async navigate( + url: string, + wait: BrowsingContext.ReadinessState = 'none' + ): Promise { + // TODO sadym: implement. + if (wait !== 'none') { + throw new Error(`Not implenented wait '${wait}'`); + } + + const cdpNavigateResult = await this._cdpClient.Page.navigate({ url }); + + return { + navigation: cdpNavigateResult.loaderId, + url: url, + }; + } + /** * Serializes a given CDP object into BiDi, keeping references in the * target's `globalThis`. diff --git a/tests/test_bidi.py b/tests/test_bidi.py index e670d09d75..e7b083c60d 100644 --- a/tests/test_bidi.py +++ b/tests/test_bidi.py @@ -220,30 +220,39 @@ async def _ignore_test_PageClose_browsingContextContextDestroyedEmitted(websocke "url": "about:blank"}} @pytest.mark.asyncio -# Not implemented yet. -async def _ignore_test_navigate_eventPageLoadEmittedAndNavigated(websocket): +async def test_navigateWaitNone_navigated(websocket): contextID = await get_open_context_id(websocket) # Send command. command = { "id": 15, - "method": "PROTO.browsingContext.navigate", + "method": "browsingContext.navigate", "params": { "url": "data:text/html,

test

", - "waitUntil": ["load", "domcontentloaded", "networkidle0", "networkidle2"], + "wait": "none", "context": contextID}} await send_JSON_command(websocket, command) - # Assert "DEBUG.Page.load" event emitted. - resp = await read_JSON_message(websocket) - assert resp == { - "method": "DEBUG.Page.load", - "params": { - "context": contextID}} - # Assert command done. resp = await read_JSON_message(websocket) - assert resp == {"id": 15, "result": {}} + recursiveCompare( + resp, + { + "id": 15, + "result": { + "navigation": "F595626837D41F9A72482D53B0C22F25", + "url": "data:text/html,

test

"}}, + ["navigation"]) + +@pytest.mark.asyncio +# Not implemented yet. +async def _ignore_test_navigateWaitInteractive_navigated(websocket): + ignore = True + +@pytest.mark.asyncio +# Not implemented yet. +async def _ignore_test_navigateWaitComplete_navigated(websocket): + ignore = True @pytest.mark.asyncio # Not implemented yet. From 0d040a00607c6572c83c4f344a350321249a0ff8 Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Thu, 7 Oct 2021 16:02:42 +0000 Subject: [PATCH 2/9] Pass `EVALUATOR_SCRIPT` as a parameter for testibility. --- src/bidiMapper/commandProcessor.ts | 12 ++++++++---- .../context/browsingContextProcessor.ts | 9 +++++++-- src/bidiMapper/domains/context/context.ts | 19 +++++++++++-------- src/bidiMapper/mapper.ts | 9 ++++++++- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/bidiMapper/commandProcessor.ts b/src/bidiMapper/commandProcessor.ts index cdb20221e6..12cbc70561 100644 --- a/src/bidiMapper/commandProcessor.ts +++ b/src/bidiMapper/commandProcessor.ts @@ -29,12 +29,14 @@ export class CommandProcessor { static run( cdpConnection: CdpConnection, bidiServer: IBidiServer, - selfTargetId: string + selfTargetId: string, + EVALUATOR_SCRIPT: string ) { const commandProcessor = new CommandProcessor( cdpConnection, bidiServer, - selfTargetId + selfTargetId, + EVALUATOR_SCRIPT ); commandProcessor.run(); @@ -43,7 +45,8 @@ export class CommandProcessor { private constructor( private _cdpConnection: CdpConnection, private _bidiServer: IBidiServer, - private _selfTargetId: string + private _selfTargetId: string, + EVALUATOR_SCRIPT: string ) { this._browserCdpClient = this._cdpConnection.browserClient(); @@ -55,7 +58,8 @@ export class CommandProcessor { }, (t: Context) => { return this._onContextDestroyed(t); - } + }, + EVALUATOR_SCRIPT ); } diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index 31d274157c..237d89af84 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -38,7 +38,8 @@ export class BrowsingContextProcessor { cdpConnection: CdpConnection, selfTargetId: string, onContextCreated: (t: Context) => Promise, - onContextDestroyed: (t: Context) => Promise + onContextDestroyed: (t: Context) => Promise, + private EVALUATOR_SCRIPT: string ) { this._cdpConnection = cdpConnection; this._selfTargetId = selfTargetId; @@ -53,7 +54,11 @@ export class BrowsingContextProcessor { let context = this._contexts.get(contextId); if (!context) { const sessionCdpClient = this._cdpConnection.sessionClient(cdpSessionId); - context = await Context.create(contextId, sessionCdpClient); + context = await Context.create( + contextId, + sessionCdpClient, + this.EVALUATOR_SCRIPT + ); this._contexts.set(contextId, context); } return context; diff --git a/src/bidiMapper/domains/context/context.ts b/src/bidiMapper/domains/context/context.ts index 16a1141c7d..fa2b6e9c12 100644 --- a/src/bidiMapper/domains/context/context.ts +++ b/src/bidiMapper/domains/context/context.ts @@ -23,8 +23,6 @@ import { Script, } from '../../bidiProtocolTypes'; -import EVALUATOR_SCRIPT from '../../scripts/eval.es'; - export class Context { _targetInfo?: Protocol.Target.TargetInfo; _sessionId?: string; @@ -37,11 +35,16 @@ export class Context { private constructor( private _contextId: string, - private _cdpClient: CdpClient + private _cdpClient: CdpClient, + private EVALUATOR_SCRIPT: string ) {} - public static async create(contextId: string, cdpClient: CdpClient) { - const context = new Context(contextId, cdpClient); + public static async create( + contextId: string, + cdpClient: CdpClient, + EVALUATOR_SCRIPT: string + ) { + const context = new Context(contextId, cdpClient, EVALUATOR_SCRIPT); await context._initialize(); return context; } @@ -110,7 +113,7 @@ export class Context { cdpObject: Protocol.Runtime.RemoteObject ): Promise { const response = await this._cdpClient.Runtime.callFunctionOn({ - functionDeclaration: `${EVALUATOR_SCRIPT}.serialize`, + functionDeclaration: `${this.EVALUATOR_SCRIPT}.serialize`, objectId: this._dummyContextObjectId, arguments: [cdpObject], returnByValue: true, @@ -175,7 +178,7 @@ export class Context { && value instanceof Promise) { value = await value; } - return (${EVALUATOR_SCRIPT}) + return (${this.EVALUATOR_SCRIPT}) .serialize.apply(null, [value]) }`; @@ -208,7 +211,7 @@ export class Context { // https://github.com/GoogleChromeLabs/chromium-bidi/issues/57 const invokeAndSerializeScript = `async (...serializedArgs)=>{ return _invoke(\n${functionDeclaration}\n, serializedArgs); async function _invoke(f, serializedArgs) { - const evaluator = (${EVALUATOR_SCRIPT}); + const evaluator = (${this.EVALUATOR_SCRIPT}); const deserializedArgs = serializedArgs.map(evaluator.deserialize); let resultValue = f.apply(this, deserializedArgs); if(${awaitPromise ? 'true' : 'false'} diff --git a/src/bidiMapper/mapper.ts b/src/bidiMapper/mapper.ts index 86e5158d5a..6f07130d35 100644 --- a/src/bidiMapper/mapper.ts +++ b/src/bidiMapper/mapper.ts @@ -21,6 +21,8 @@ import { CdpClient, CdpConnection } from '../cdp'; import { BidiServer } from './utils/bidiServer'; import { ITransport } from '../utils/transport'; +import EVALUATOR_SCRIPT from './scripts/eval.es'; + import { log } from '../utils/log'; const logSystem = log('system'); @@ -60,7 +62,12 @@ const _waitSelfTargetIdPromise = _waitSelfTargetId(); // The command processor needs to start running before calling _prepareCdp // so that it has a chance to set up event listeners for tracking targets. - CommandProcessor.run(cdpConnection, bidiServer, selfTargetId); + CommandProcessor.run( + cdpConnection, + bidiServer, + selfTargetId, + EVALUATOR_SCRIPT + ); // Needed to get events about new targets. await _prepareCdp(cdpClient); From b8adffc9ddc39280e46c3260c8ef7ab070e60eef Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Thu, 7 Oct 2021 21:36:07 +0000 Subject: [PATCH 3/9] Unit tests + avoid race condition while waiting `Context` creation. --- src/bidiMapper/commandProcessor.ts | 14 +- .../context/browsingContextProcessor.spec.ts | 145 ++++++++++++++++++ .../context/browsingContextProcessor.ts | 101 +++++++----- src/cdp/cdpClient.spec.ts | 47 +++--- src/cdp/cdpConnection.spec.ts | 75 ++++----- src/cdp/cdpConnection.ts | 2 +- src/mapperServer.ts | 2 +- src/tests/stubTransport.spec.ts | 8 +- tests/test_bidi.py | 31 +++- 9 files changed, 295 insertions(+), 130 deletions(-) create mode 100644 src/bidiMapper/domains/context/browsingContextProcessor.spec.ts diff --git a/src/bidiMapper/commandProcessor.ts b/src/bidiMapper/commandProcessor.ts index 12cbc70561..3efe470c04 100644 --- a/src/bidiMapper/commandProcessor.ts +++ b/src/bidiMapper/commandProcessor.ts @@ -64,16 +64,6 @@ export class CommandProcessor { } private run() { - this._browserCdpClient.Target.on('attachedToTarget', async (params) => { - await this._contextProcessor.handleAttachedToTargetEvent(params); - }); - this._browserCdpClient.Target.on('targetInfoChanged', (params) => { - this._contextProcessor.handleInfoChangedEvent(params); - }); - this._browserCdpClient.Target.on('detachedFromTarget', (params) => { - this._contextProcessor.handleDetachedFromTargetEvent(params); - }); - this._bidiServer.on('message', (messageObj) => { return this._onBidiMessage(messageObj); }); @@ -186,11 +176,11 @@ export class CommandProcessor { commandData as Script.ScriptEvaluateCommand ); case 'browsingContext.create': - return await this._contextProcessor.process_createContext( + return await this._contextProcessor.process_browsingContext_create( commandData as BrowsingContext.BrowsingContextCreateCommand ); case 'browsingContext.navigate': - return await this._contextProcessor.process_navigate( + return await this._contextProcessor.process_browsingContext_navigate( commandData as BrowsingContext.BrowsingContextNavigateCommand ); diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts new file mode 100644 index 0000000000..e39045345b --- /dev/null +++ b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts @@ -0,0 +1,145 @@ +/** + * Copyright 2021 Google LLC. + * Copyright (c) Microsoft Corporation. + * + * 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 { StubTransport } from '../../../tests/stubTransport.spec'; + +import * as chai from 'chai'; +import chaiAsPromised from 'chai-as-promised'; +chai.use(chaiAsPromised); + +import * as sinon from 'sinon'; + +import { BrowsingContextProcessor } from './browsingContextProcessor'; +import { CdpConnection, CdpClient } from '../../../cdp'; +import { Context } from './context'; +import { BrowsingContext } from '../../bidiProtocolTypes'; + +describe('BrowsingContextProcessor', function () { + let mockCdpServer: StubTransport; + let browsingContextProcessor: BrowsingContextProcessor; + + beforeEach(async function () { + mockCdpServer = new StubTransport(); + + const onContextCreated = sinon.fake as unknown as ( + t: Context + ) => Promise; + const onContextDestroyed = sinon.fake as unknown as ( + t: Context + ) => Promise; + + browsingContextProcessor = new BrowsingContextProcessor( + new CdpConnection(mockCdpServer), + 'SELF_TARGET_ID', + onContextCreated, + onContextDestroyed, + 'EVALUATOR_SCRIPT' + ); + + // Actual `Context.create` logic involves several CDP calls, so mock it to avoid all the simulations. + Context.create = async ( + contextId: string, + cdpClient: CdpClient, + EVALUATOR_SCRIPT: string + ) => { + return sinon.createStubInstance(Context) as unknown as Context; + }; + }); + + describe('`process_browsingContext_create`', async function () { + const NEW_CONTEXT_ID = 'NEW_CONTEXT_ID'; + + const BROWSING_CONTEXT_CREATE_COMMAND: BrowsingContext.BrowsingContextCreateCommand = + { + method: 'browsingContext.create', + params: {}, + }; + + const EXPECTED_TARGET_CREATE_TARGET_CALL = { + id: 0, + method: 'Target.createTarget', + params: { + url: 'about:blank', + newWindow: false, + }, + }; + + const TARGET_CREATE_TARGET_RESPONSE = { + id: 0, + result: { + targetId: NEW_CONTEXT_ID, + }, + }; + + const TARGET_ATTACHED_TO_TARGET_EVENT = { + method: 'Target.attachedToTarget', + params: { + sessionId: '_ANY_VALUE_', + targetInfo: { + targetId: NEW_CONTEXT_ID, + type: 'page', + title: '', + url: '', + attached: true, + canAccessOpener: false, + browserContextId: '_ANY_ANOTHER_VALUE_', + }, + waitingForDebugger: false, + }, + }; + + it('Target.attachedToTarget before command finished', async function () { + const createResultPromise = + browsingContextProcessor.process_browsingContext_create( + BROWSING_CONTEXT_CREATE_COMMAND + ); + + sinon.assert.calledOnceWithExactly( + mockCdpServer.sendMessage, + JSON.stringify(EXPECTED_TARGET_CREATE_TARGET_CALL) + ); + + await mockCdpServer.emulateIncomingMessage( + TARGET_ATTACHED_TO_TARGET_EVENT + ); + + await mockCdpServer.emulateIncomingMessage(TARGET_CREATE_TARGET_RESPONSE); + + await createResultPromise; + }); + + it('Target.attachedToTarget after command finished', async function () { + const createResultPromise = + browsingContextProcessor.process_browsingContext_create( + BROWSING_CONTEXT_CREATE_COMMAND + ); + + sinon.assert.calledOnceWithExactly( + mockCdpServer.sendMessage, + JSON.stringify(EXPECTED_TARGET_CREATE_TARGET_CALL) + ); + + await mockCdpServer.emulateIncomingMessage(TARGET_CREATE_TARGET_RESPONSE); + + await mockCdpServer.emulateIncomingMessage( + TARGET_ATTACHED_TO_TARGET_EVENT + ); + + await createResultPromise; + }); + }); +}); diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index 237d89af84..be44db64ad 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -16,7 +16,7 @@ */ import { log } from '../../../utils/log'; -import { CdpConnection } from '../../../cdp'; +import { CdpConnection, CdpClient } from '../../../cdp'; import { Context } from './context'; import { BrowsingContext, Script } from '../../bidiProtocolTypes'; import Protocol from 'devtools-protocol'; @@ -24,7 +24,7 @@ import Protocol from 'devtools-protocol'; const logContext = log('context'); export class BrowsingContextProcessor { - private _contexts: Map = new Map(); + private _contexts: Map> = new Map(); private _sessionToTargets: Map = new Map(); // Set from outside. @@ -45,36 +45,56 @@ export class BrowsingContextProcessor { this._selfTargetId = selfTargetId; this._onContextCreated = onContextCreated; this._onContextDestroyed = onContextDestroyed; + + this._setCdpEventListeners(this._cdpConnection.browserClient()); } + private _setCdpEventListeners(browserCdpClient: CdpClient) { + browserCdpClient.Target.on('attachedToTarget', async (params) => { + await this._handleAttachedToTargetEvent(params); + }); + browserCdpClient.Target.on('targetInfoChanged', (params) => { + this._handleInfoChangedEvent(params); + }); + browserCdpClient.Target.on('detachedFromTarget', (params) => { + this._handleDetachedFromTargetEvent(params); + }); + } + + // Creation of `Context` can take quite a while. To avoid race condition, in + // the map kept Pmomise, eventualy resolved with the`Context`. private async _getOrCreateContext( contextId: string, cdpSessionId: string ): Promise { - let context = this._contexts.get(contextId); - if (!context) { - const sessionCdpClient = this._cdpConnection.sessionClient(cdpSessionId); - context = await Context.create( + let contextPromise = this._contexts.get(contextId); + if (!contextPromise) { + const sessionCdpClient = this._cdpConnection.getCdpClient(cdpSessionId); + + // Don't wait for actual creation. Just put the Promise into map. + contextPromise = Context.create( contextId, sessionCdpClient, this.EVALUATOR_SCRIPT ); - this._contexts.set(contextId, context); + this._contexts.set(contextId, contextPromise); } - return context; + return contextPromise; } - private _tryGetContext(contextId: string): Context | undefined { - return this._contexts.get(contextId); + private async _tryGetContext( + contextId: string + ): Promise { + return await this._contexts.get(contextId); } - private _getKnownContext(contextId: string): Context { - const context = this._contexts.get(contextId); + private async _getKnownContext(contextId: string): Promise { + const context = await this._contexts.get(contextId); if (!context) throw new Error('context not found'); return context; } - async handleAttachedToTargetEvent( + private async _handleAttachedToTargetEvent( params: Protocol.Target.AttachedToTargetEvent ) { logContext('AttachedToTarget event received', params); @@ -87,37 +107,40 @@ export class BrowsingContextProcessor { sessionId ); context._updateTargetInfo(targetInfo); - this._sessionToTargets.delete(sessionId); - this._sessionToTargets.set(sessionId, context); - - // context._setSessionId(eventData.params.sessionId); + context._setSessionId(sessionId); this._onContextCreated(context); } - handleInfoChangedEvent(params: Protocol.Target.TargetInfoChangedEvent) { + private async _handleInfoChangedEvent( + params: Protocol.Target.TargetInfoChangedEvent + ) { logContext('infoChangedEvent event received', params); const targetInfo = params.targetInfo; if (!this._isValidTarget(targetInfo)) return; - const context = this._tryGetContext(targetInfo.targetId); + const context = await this._tryGetContext(targetInfo.targetId); if (context) { context._onInfoChangedEvent(targetInfo); } } - // { "method": "Target.detachedFromTarget", "params": { "sessionId": "7EFBFB2A4942A8989B3EADC561BC46E9", "targetId": "19416886405CBA4E03DBB59FA67FF4E8" } } - handleDetachedFromTargetEvent( + // { "method": "Target.detachedFromTarget", + // "params": { + // "sessionId": "7EFBFB2A4942A8989B3EADC561BC46E9", + // "targetId": "19416886405CBA4E03DBB59FA67FF4E8" } } + private async _handleDetachedFromTargetEvent( params: Protocol.Target.DetachedFromTargetEvent ) { logContext('detachedFromTarget event received', params); - // TODO: params.targetId is deprecated. Update this class to track using params.sessionId instead. + // TODO: params.targetId is deprecated. Update this class to track using + // params.sessionId instead. const targetId = params.targetId!; - const context = this._tryGetContext(targetId); + const context = await this._tryGetContext(targetId); if (context) { this._onContextDestroyed(context); @@ -127,13 +150,26 @@ export class BrowsingContextProcessor { } } - async process_createContext( + async process_browsingContext_create( commandData: BrowsingContext.BrowsingContextCreateCommand ): Promise { const params = commandData.params; return new Promise(async (resolve) => { - let targetId: string; + const browserCdpClient = this._cdpConnection.browserClient(); + + const result = await browserCdpClient.Target.createTarget({ + url: 'about:blank', + newWindow: params.type === 'window', + }); + + const targetId = result.targetId; + + const existingContext = await this._tryGetContext(targetId); + if (existingContext) { + resolve(existingContext.toBidi()); + return; + } const onAttachedToTarget = async ( attachToTargetEventParams: Protocol.Target.AttachedToTargetEvent @@ -152,22 +188,15 @@ export class BrowsingContextProcessor { } }; - const browserCdpClient = this._cdpConnection.browserClient(); browserCdpClient.Target.on('attachedToTarget', onAttachedToTarget); - - const result = await browserCdpClient.Target.createTarget({ - url: 'about:blank', - newWindow: params.type === 'window', - }); - targetId = result.targetId; }); } - async process_navigate( + async process_browsingContext_navigate( commandData: BrowsingContext.BrowsingContextNavigateCommand ): Promise { const params = commandData.params; - const context = this._getKnownContext(params.context); + const context = await this._getKnownContext(params.context); return await context.navigate(params.url, params.wait); } @@ -176,7 +205,7 @@ export class BrowsingContextProcessor { commandData: Script.ScriptEvaluateCommand ): Promise { const params = commandData.params; - const context = this._getKnownContext( + const context = await this._getKnownContext( (params.target as Script.ContextTarget).context ); return await context.scriptEvaluate( @@ -189,7 +218,7 @@ export class BrowsingContextProcessor { commandData: Script.PROTO.ScriptInvokeCommand ): Promise { const params = commandData.params; - const context = this._getKnownContext( + const context = await this._getKnownContext( (params.target as Script.ContextTarget).context ); return await context.PROTO_scriptInvoke( diff --git a/src/cdp/cdpClient.spec.ts b/src/cdp/cdpClient.spec.ts index d4fb11fbb5..be12bf1f18 100644 --- a/src/cdp/cdpClient.spec.ts +++ b/src/cdp/cdpClient.spec.ts @@ -59,9 +59,6 @@ describe('CdpClient tests.', function () { const cdpClient = cdpConnection.browserClient(); - // Get handler 'onMessage' to notify 'cdpClient' about new CDP messages. - const onMessage = mockCdpServer.getOnMessage(); - // Send CDP command and store returned promise. const commandPromise = cdpClient.Target.activateTarget({ targetId: TEST_TARGET_ID, @@ -71,7 +68,7 @@ describe('CdpClient tests.', function () { sinon.assert.calledOnce(mockCdpServer.sendMessage); // Notify 'cdpClient' the CDP command is finished. - onMessage(JSON.stringify({ id: 0, result: {} })); + await mockCdpServer.emulateIncomingMessage({ id: 0, result: {} }); // Assert 'cdpClient' resolved message promise. chai.assert.eventually.deepEqual(commandPromise, {}); @@ -92,9 +89,6 @@ describe('CdpClient tests.', function () { const commandResult1 = { id: 0, result: expectedResult1 }; const commandResult2 = { id: 1, result: expectedResult2 }; - // Get handler 'onMessage' to notify 'cdpClient' about new CDP messages. - const onMessage = mockCdpServer.getOnMessage(); - // Send 2 CDP commands and store returned promises. const commandPromise1 = cdpClient.Target.attachToTarget({ targetId: TEST_TARGET_ID, @@ -107,12 +101,12 @@ describe('CdpClient tests.', function () { sinon.assert.calledTwice(mockCdpServer.sendMessage); // Notify 'cdpClient' the command2 is finished. - onMessage(JSON.stringify(commandResult2)); + await mockCdpServer.emulateIncomingMessage(commandResult2); // Assert second message promise is resolved. const actualResult2 = await commandPromise2; // Notify 'cdpClient' the command1 is finished. - onMessage(JSON.stringify(commandResult1)); + await mockCdpServer.emulateIncomingMessage(commandResult1); // Assert first message promise is resolved. const actualResult1 = await commandPromise1; @@ -125,9 +119,6 @@ describe('CdpClient tests.', function () { const cdpConnection = new CdpConnection(mockCdpServer); const cdpClient = cdpConnection.browserClient(); - // Get handler 'onMessage' to notify 'cdpClient' about new CDP messages. - const onMessage = mockCdpServer.getOnMessage(); - // Register event callbacks. const genericCallback = sinon.fake(); cdpClient.on('event', genericCallback); @@ -136,12 +127,10 @@ describe('CdpClient tests.', function () { cdpClient.Target.on('attachedToTarget', typedCallback); // Send a CDP event. - onMessage( - JSON.stringify({ - method: 'Target.attachedToTarget', - params: { targetId: TEST_TARGET_ID }, - }) - ); + await mockCdpServer.emulateIncomingMessage({ + method: 'Target.attachedToTarget', + params: { targetId: TEST_TARGET_ID }, + }); // Verify that callbacks are called. sinon.assert.calledOnceWithExactly( @@ -161,7 +150,9 @@ describe('CdpClient tests.', function () { cdpClient.Target.off('Target.attachedToTarget', typedCallback); // Send another CDP event. - onMessage(JSON.stringify({ params: { targetId: TEST_TARGET_ID } })); + await mockCdpServer.emulateIncomingMessage({ + params: { targetId: TEST_TARGET_ID }, + }); sinon.assert.notCalled(genericCallback); sinon.assert.notCalled(typedCallback); @@ -173,9 +164,6 @@ describe('CdpClient tests.', function () { const cdpConnection = new CdpConnection(mockCdpServer); const cdpClient = cdpConnection.browserClient(); - // Get handler 'onMessage' to notify 'cdpClient' about new CDP messages. - const onMessage = mockCdpServer.getOnMessage(); - // Send CDP command and store returned promise. const commandPromise = cdpClient.sendCommand('Target.attachToTarget', { targetId: TEST_TARGET_ID, @@ -185,9 +173,10 @@ describe('CdpClient tests.', function () { sinon.assert.calledOnce(mockCdpServer.sendMessage); // Notify 'cdpClient' the CDP command is finished. - onMessage( - JSON.stringify({ id: 0, result: { targetId: TEST_TARGET_ID } }) - ); + await mockCdpServer.emulateIncomingMessage({ + id: 0, + result: { targetId: TEST_TARGET_ID }, + }); // Assert sendCommand resolved message promise. chai.assert.eventually.deepEqual(commandPromise, { @@ -205,9 +194,6 @@ describe('CdpClient tests.', function () { message: 'something happened', }; - // Get handler 'onMessage' to notify 'cdpClient' about new CDP messages. - const onMessage = mockCdpServer.getOnMessage(); - // Send CDP command and store returned promise. const commandPromise = cdpClient.sendCommand('Target.attachToTarget', { targetId: TEST_TARGET_ID, @@ -217,7 +203,10 @@ describe('CdpClient tests.', function () { sinon.assert.calledOnce(mockCdpServer.sendMessage); // Notify 'cdpClient' the CDP command is finished. - onMessage(JSON.stringify({ id: 0, error: expectedError })); + await mockCdpServer.emulateIncomingMessage({ + id: 0, + error: expectedError, + }); // Assert sendCommand rejects with error. chai.assert.isRejected(commandPromise, expectedError); diff --git a/src/cdp/cdpConnection.spec.ts b/src/cdp/cdpConnection.spec.ts index 48d268b2f1..feaed0221f 100644 --- a/src/cdp/cdpConnection.spec.ts +++ b/src/cdp/cdpConnection.spec.ts @@ -50,46 +50,38 @@ describe('CdpConnection', function () { const cdpConnection = new CdpConnection(mockCdpServer); chai.assert.throws( - () => cdpConnection.sessionClient(SOME_SESSION_ID), + () => cdpConnection.getCdpClient(SOME_SESSION_ID), 'Unknown CDP session ID' ); - const onMessage = mockCdpServer.getOnMessage(); - onMessage( - JSON.stringify({ - method: 'Target.attachedToTarget', - params: { sessionId: SOME_SESSION_ID }, - }) - ); + await mockCdpServer.emulateIncomingMessage({ + method: 'Target.attachedToTarget', + params: { sessionId: SOME_SESSION_ID }, + }); - const client = cdpConnection.sessionClient(SOME_SESSION_ID); - chai.assert.isNotNull(client); + const cdpClient = cdpConnection.getCdpClient(SOME_SESSION_ID); + chai.assert.isNotNull(cdpClient); }); it('removes the CdpClient for a session when the Target.detachedFromTarget event is received', async function () { const mockCdpServer = new StubTransport(); const cdpConnection = new CdpConnection(mockCdpServer); - const onMessage = mockCdpServer.getOnMessage(); - onMessage( - JSON.stringify({ - method: 'Target.attachedToTarget', - params: { sessionId: SOME_SESSION_ID }, - }) - ); + await mockCdpServer.emulateIncomingMessage({ + method: 'Target.attachedToTarget', + params: { sessionId: SOME_SESSION_ID }, + }); - const cdpClient = cdpConnection.sessionClient(SOME_SESSION_ID); + const cdpClient = cdpConnection.getCdpClient(SOME_SESSION_ID); chai.assert.isNotNull(cdpClient); - onMessage( - JSON.stringify({ - method: 'Target.detachedFromTarget', - params: { sessionId: SOME_SESSION_ID }, - }) - ); + await mockCdpServer.emulateIncomingMessage({ + method: 'Target.detachedFromTarget', + params: { sessionId: SOME_SESSION_ID }, + }); chai.assert.throws( - () => cdpConnection.sessionClient(SOME_SESSION_ID), + () => cdpConnection.getCdpClient(SOME_SESSION_ID), 'Unknown CDP session ID' ); }); @@ -107,7 +99,6 @@ describe('CdpConnection', function () { sessionId: ANOTHER_SESSION_ID, method: 'Page.loadEventFired', }; - const onMessage = mockCdpServer.getOnMessage(); const browserCallback = sinon.fake(); const sessionCallback = sinon.fake(); @@ -118,50 +109,46 @@ describe('CdpConnection', function () { browserClient.Browser.on('downloadWillBegin', browserCallback); // Verify that the browser callback receives the message. - onMessage(JSON.stringify(browserMessage)); + await mockCdpServer.emulateIncomingMessage(browserMessage); sinon.assert.calledOnceWithExactly(browserCallback, {}); browserCallback.resetHistory(); // Attach session A. - onMessage( - JSON.stringify({ - method: 'Target.attachedToTarget', - params: { sessionId: SOME_SESSION_ID }, - }) - ); + await mockCdpServer.emulateIncomingMessage({ + method: 'Target.attachedToTarget', + params: { sessionId: SOME_SESSION_ID }, + }); - const sessionClient = cdpConnection.sessionClient(SOME_SESSION_ID)!; + const sessionClient = cdpConnection.getCdpClient(SOME_SESSION_ID)!; chai.assert.isNotNull(sessionClient); sessionClient.Page.on('frameNavigated', sessionCallback); // Send another message for the browser and verify that only the browser callback receives it. // Verifies that adding another client doesn't affect routing for existing clients. - onMessage(JSON.stringify(browserMessage)); + await mockCdpServer.emulateIncomingMessage(browserMessage); sinon.assert.notCalled(sessionCallback); sinon.assert.calledOnceWithExactly(browserCallback, {}); browserCallback.resetHistory(); // Send a message for session A and verify that it is received. - onMessage(JSON.stringify(sessionMessage)); + await mockCdpServer.emulateIncomingMessage(sessionMessage); sinon.assert.notCalled(browserCallback); sinon.assert.calledOnceWithExactly(sessionCallback, {}); sessionCallback.resetHistory(); // Attach session B. - onMessage( - JSON.stringify({ - method: 'Target.attachedToTarget', - params: { sessionId: ANOTHER_SESSION_ID }, - }) - ); + await mockCdpServer.emulateIncomingMessage({ + method: 'Target.attachedToTarget', + params: { sessionId: ANOTHER_SESSION_ID }, + }); - const otherSessionClient = cdpConnection.sessionClient(ANOTHER_SESSION_ID)!; + const otherSessionClient = cdpConnection.getCdpClient(ANOTHER_SESSION_ID)!; chai.assert.isNotNull(otherSessionClient); otherSessionClient.Page.on('loadEventFired', otherSessionCallback); // Send a message for session B and verify that only the session B callback receives it. // Verifies that a message is sent only to the session client it is intended for. - onMessage(JSON.stringify(othersessionMessage)); + await mockCdpServer.emulateIncomingMessage(othersessionMessage); sinon.assert.notCalled(browserCallback); sinon.assert.notCalled(sessionCallback); sinon.assert.calledOnceWithExactly(otherSessionCallback, {}); diff --git a/src/cdp/cdpConnection.ts b/src/cdp/cdpConnection.ts index 510fd348ca..1a79318f3a 100644 --- a/src/cdp/cdpConnection.ts +++ b/src/cdp/cdpConnection.ts @@ -67,7 +67,7 @@ export class CdpConnection { * @param sessionId The sessionId of the CdpClient to retrieve. * @returns The CdpClient object attached to the given session, or null if the session is not attached. */ - sessionClient(sessionId: string): CdpClient { + getCdpClient(sessionId: string): CdpClient { const cdpClient = this._sessionCdpClients.get(sessionId); if (!cdpClient) { throw new Error('Unknown CDP session ID'); diff --git a/src/mapperServer.ts b/src/mapperServer.ts index 201d3e3fcb..f51b9ace42 100644 --- a/src/mapperServer.ts +++ b/src/mapperServer.ts @@ -130,7 +130,7 @@ export class MapperServer { const { sessionId: mapperSessionId } = await browserClient.Target.attachToTarget({ targetId, flatten: true }); - const mapperCdpClient = cdpConnection.sessionClient(mapperSessionId); + const mapperCdpClient = cdpConnection.getCdpClient(mapperSessionId); if (!mapperCdpClient) { throw new Error('Unable to connect to mapper CDP target'); } diff --git a/src/tests/stubTransport.spec.ts b/src/tests/stubTransport.spec.ts index 3f367ac84c..59cd271b8b 100644 --- a/src/tests/stubTransport.spec.ts +++ b/src/tests/stubTransport.spec.ts @@ -32,11 +32,17 @@ export class StubTransport implements ITransport { sendMessage: TypedSpy; close: TypedSpy; - getOnMessage(): (string: string) => void { + private _getOnMessage(): (string: string) => void { assert.called(this.setOnMessage); return this.setOnMessage.getCall(0).args[0]; } + public async emulateIncomingMessage(messageObject: unknown) { + this._getOnMessage()(JSON.stringify(messageObject)); + // `setTimeout` allows the message to be processed. + await new Promise((resolve) => setTimeout(resolve, 0)); + } + constructor() { this.sendMessage = typedSpy(); this.setOnMessage = typedSpy(); diff --git a/tests/test_bidi.py b/tests/test_bidi.py index e7b083c60d..e6f42eca62 100644 --- a/tests/test_bidi.py +++ b/tests/test_bidi.py @@ -169,9 +169,9 @@ async def ignore_test_getTreeWithNestedContexts_contextReturned(websocket): # TODO sadym: implement @pytest.mark.asyncio -# Not implemented yet. -async def _ignore_test_createContext_eventContextCreatedEmittedAndContextCreated(websocket): - # Send command. +async def test_createContext_eventContextCreatedEmittedAndContextCreated(websocket): + initialContextID = await get_open_context_id(websocket) + command = { "id": 9, "method": "browsingContext.create", @@ -180,11 +180,11 @@ async def _ignore_test_createContext_eventContextCreatedEmittedAndContextCreated # Assert "browsingContext.contextCreated" event emitted. resp = await read_JSON_message(websocket) - contextID = resp['params']['context'] + newContextID = resp['params']['context'] assert resp == { "method": "browsingContext.contextCreated", "params": { - "context":contextID, + "context": newContextID, "parent": None, "url": ""}} @@ -193,10 +193,29 @@ async def _ignore_test_createContext_eventContextCreatedEmittedAndContextCreated assert resp == { "id": 9, "result": { - "context": contextID, + "context": newContextID, "parent": None, "url": ""}} + # Get all contexts and assert new context is created. + command = {"id": 10, "method": "browsingContext.getTree", "params": {}} + await send_JSON_command(websocket, command) + + # Assert "browsingContext.getTree" command done. + # TODO sadym: make order-agnostic. Maybe order by `context`? + resp = await read_JSON_message(websocket) + assert resp == { + "id": 10, + "result": { + "contexts": [{ + "context": newContextID, + "url": "about:blank", + "children": [] + }, { + "context": initialContextID, + "url": "about:blank", + "children": [] }]}} + @pytest.mark.asyncio # Not implemented yet. async def _ignore_test_PageClose_browsingContextContextDestroyedEmitted(websocket): From 5a92cd281bb5d87bfef855be7c34c9c542ab7298 Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Thu, 7 Oct 2021 21:39:31 +0000 Subject: [PATCH 4/9] minor change --- .../context/browsingContextProcessor.spec.ts | 83 ++++++++++--------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts index e39045345b..1a28c9d2cd 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts @@ -31,38 +31,62 @@ import { BrowsingContext } from '../../bidiProtocolTypes'; describe('BrowsingContextProcessor', function () { let mockCdpServer: StubTransport; let browsingContextProcessor: BrowsingContextProcessor; + let cdpConnection: CdpConnection; + + const EVALUATOR_SCRIPT = 'EVALUATOR_SCRIPT'; + const NEW_CONTEXT_ID = 'NEW_CONTEXT_ID'; + const TARGET_ATTACHED_TO_TARGET_EVENT = { + method: 'Target.attachedToTarget', + params: { + sessionId: '_ANY_VALUE_', + targetInfo: { + targetId: NEW_CONTEXT_ID, + type: 'page', + title: '', + url: '', + attached: true, + canAccessOpener: false, + browserContextId: '_ANY_ANOTHER_VALUE_', + }, + waitingForDebugger: false, + }, + }; beforeEach(async function () { mockCdpServer = new StubTransport(); - - const onContextCreated = sinon.fake as unknown as ( - t: Context - ) => Promise; - const onContextDestroyed = sinon.fake as unknown as ( - t: Context - ) => Promise; - + cdpConnection = new CdpConnection(mockCdpServer); browsingContextProcessor = new BrowsingContextProcessor( - new CdpConnection(mockCdpServer), + cdpConnection, 'SELF_TARGET_ID', - onContextCreated, - onContextDestroyed, - 'EVALUATOR_SCRIPT' + sinon.fake as unknown as (t: Context) => Promise, + sinon.fake as unknown as (t: Context) => Promise, + EVALUATOR_SCRIPT ); // Actual `Context.create` logic involves several CDP calls, so mock it to avoid all the simulations. - Context.create = async ( - contextId: string, - cdpClient: CdpClient, - EVALUATOR_SCRIPT: string - ) => { - return sinon.createStubInstance(Context) as unknown as Context; - }; + Context.create = sinon.fake( + async (_1: string, _2: CdpClient, _3: string) => { + return sinon.createStubInstance(Context) as unknown as Context; + } + ); }); - describe('`process_browsingContext_create`', async function () { - const NEW_CONTEXT_ID = 'NEW_CONTEXT_ID'; + describe('handle events', async function () { + it('`Target.attachedToTarget` creates Context', async function () { + sinon.assert.notCalled(Context.create as sinon.SinonSpy); + await mockCdpServer.emulateIncomingMessage( + TARGET_ATTACHED_TO_TARGET_EVENT + ); + sinon.assert.calledOnceWithExactly( + Context.create as sinon.SinonSpy, + NEW_CONTEXT_ID, + sinon.match.any, + EVALUATOR_SCRIPT + ); + }); + }); + describe('handle `process_browsingContext_create`', async function () { const BROWSING_CONTEXT_CREATE_COMMAND: BrowsingContext.BrowsingContextCreateCommand = { method: 'browsingContext.create', @@ -85,23 +109,6 @@ describe('BrowsingContextProcessor', function () { }, }; - const TARGET_ATTACHED_TO_TARGET_EVENT = { - method: 'Target.attachedToTarget', - params: { - sessionId: '_ANY_VALUE_', - targetInfo: { - targetId: NEW_CONTEXT_ID, - type: 'page', - title: '', - url: '', - attached: true, - canAccessOpener: false, - browserContextId: '_ANY_ANOTHER_VALUE_', - }, - waitingForDebugger: false, - }, - }; - it('Target.attachedToTarget before command finished', async function () { const createResultPromise = browsingContextProcessor.process_browsingContext_create( From 74244e398197042a36ec7c382bf7fc09ec157b6d Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Mon, 11 Oct 2021 11:02:27 +0000 Subject: [PATCH 5/9] `browsingContext.navigate` -> `PROTO.browsingContext.navigate` --- src/bidiMapper/bidiProtocolTypes.ts | 32 ++++++++++--------- src/bidiMapper/commandProcessor.ts | 8 ++--- .../context/browsingContextProcessor.ts | 6 ++-- src/bidiMapper/domains/context/context.ts | 4 +-- tests/test_bidi.py | 10 +++--- 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/bidiMapper/bidiProtocolTypes.ts b/src/bidiMapper/bidiProtocolTypes.ts index 425e59852d..dc70a4937e 100644 --- a/src/bidiMapper/bidiProtocolTypes.ts +++ b/src/bidiMapper/bidiProtocolTypes.ts @@ -136,23 +136,25 @@ export namespace BrowsingContext { context: BrowsingContext; }; - export type BrowsingContextNavigateCommand = { - method: 'browsingContext.navigate'; - params: BrowsingContextNavigateParameters; - }; + export namespace PROTO { + export type BrowsingContextNavigateCommand = { + method: 'PROTO.browsingContext.navigate'; + params: BrowsingContextNavigateParameters; + }; - export type BrowsingContextNavigateParameters = { - context: BrowsingContext; - url: string; - wait?: ReadinessState; - }; + export type BrowsingContextNavigateParameters = { + context: BrowsingContext; + url: string; + wait?: ReadinessState; + }; - export type ReadinessState = 'none'; - // TODO sadym: implement 'interactive' and 'complete' states. - export type BrowsingContextNavigateResult = { - navigation?: Navigation; - url: string; - }; + export type ReadinessState = 'none'; + // TODO sadym: implement 'interactive' and 'complete' states. + export type BrowsingContextNavigateResult = { + navigation?: Navigation; + url: string; + }; + } } export namespace Session { diff --git a/src/bidiMapper/commandProcessor.ts b/src/bidiMapper/commandProcessor.ts index 3efe470c04..2ede801b08 100644 --- a/src/bidiMapper/commandProcessor.ts +++ b/src/bidiMapper/commandProcessor.ts @@ -179,11 +179,11 @@ export class CommandProcessor { return await this._contextProcessor.process_browsingContext_create( commandData as BrowsingContext.BrowsingContextCreateCommand ); - case 'browsingContext.navigate': - return await this._contextProcessor.process_browsingContext_navigate( - commandData as BrowsingContext.BrowsingContextNavigateCommand - ); + case 'PROTO.browsingContext.navigate': + return await this._contextProcessor.process_PROTO_browsingContext_navigate( + commandData as BrowsingContext.PROTO.BrowsingContextNavigateCommand + ); case 'PROTO.script.invoke': return await this._contextProcessor.process_PROTO_script_invoke( commandData as Script.PROTO.ScriptInvokeCommand diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index be44db64ad..5c14fb74a2 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -192,9 +192,9 @@ export class BrowsingContextProcessor { }); } - async process_browsingContext_navigate( - commandData: BrowsingContext.BrowsingContextNavigateCommand - ): Promise { + async process_PROTO_browsingContext_navigate( + commandData: BrowsingContext.PROTO.BrowsingContextNavigateCommand + ): Promise { const params = commandData.params; const context = await this._getKnownContext(params.context); diff --git a/src/bidiMapper/domains/context/context.ts b/src/bidiMapper/domains/context/context.ts index fa2b6e9c12..ac585d6f4a 100644 --- a/src/bidiMapper/domains/context/context.ts +++ b/src/bidiMapper/domains/context/context.ts @@ -89,8 +89,8 @@ export class Context { public async navigate( url: string, - wait: BrowsingContext.ReadinessState = 'none' - ): Promise { + wait: BrowsingContext.PROTO.ReadinessState = 'none' + ): Promise { // TODO sadym: implement. if (wait !== 'none') { throw new Error(`Not implenented wait '${wait}'`); diff --git a/tests/test_bidi.py b/tests/test_bidi.py index e6f42eca62..c96b255960 100644 --- a/tests/test_bidi.py +++ b/tests/test_bidi.py @@ -239,13 +239,13 @@ async def _ignore_test_PageClose_browsingContextContextDestroyedEmitted(websocke "url": "about:blank"}} @pytest.mark.asyncio -async def test_navigateWaitNone_navigated(websocket): +async def test_PROTO_navigateWaitNone_navigated(websocket): contextID = await get_open_context_id(websocket) # Send command. command = { "id": 15, - "method": "browsingContext.navigate", + "method": "PROTO.browsingContext.navigate", "params": { "url": "data:text/html,

test

", "wait": "none", @@ -265,17 +265,17 @@ async def test_navigateWaitNone_navigated(websocket): @pytest.mark.asyncio # Not implemented yet. -async def _ignore_test_navigateWaitInteractive_navigated(websocket): +async def _ignore_test_PROTO_navigateWaitInteractive_navigated(websocket): ignore = True @pytest.mark.asyncio # Not implemented yet. -async def _ignore_test_navigateWaitComplete_navigated(websocket): +async def _ignore_test_PROTO_navigateWaitComplete_navigated(websocket): ignore = True @pytest.mark.asyncio # Not implemented yet. -async def _ignore_test_navigateWithShortTimeout_timeoutOccuredAndEventPageLoadEmitted(websocket): +async def _ignore_test_PROTO_navigateWithShortTimeout_timeoutOccuredAndEventPageLoadEmitted(websocket): contextID = await get_open_context_id(websocket) # Send command. From 953a9ab1d2d3d09ddd0f8d4e057fe27e157a39b0 Mon Sep 17 00:00:00 2001 From: Maksim Sadym <69349599+sadym-chromium@users.noreply.github.com> Date: Thu, 14 Oct 2021 11:37:02 +0200 Subject: [PATCH 6/9] Update src/bidiMapper/domains/context/browsingContextProcessor.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Philip Jägenstedt --- src/bidiMapper/domains/context/browsingContextProcessor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index 5c14fb74a2..8270445abe 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -61,8 +61,8 @@ export class BrowsingContextProcessor { }); } - // Creation of `Context` can take quite a while. To avoid race condition, in - // the map kept Pmomise, eventualy resolved with the`Context`. + // Creation of `Context` can take quite a while. To avoid race condition, keep + // a Promise in the map, eventually resolved with the `Context`. private async _getOrCreateContext( contextId: string, cdpSessionId: string From 8f83b582966a5ac54188e714f15b3bda73e274e6 Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Thu, 14 Oct 2021 09:56:04 +0000 Subject: [PATCH 7/9] `PROTO.browsingContext.navigate` -> `browsingContext.navigate` --- src/bidiMapper/bidiProtocolTypes.ts | 34 +++++++++---------- src/bidiMapper/commandProcessor.ts | 8 ++--- .../context/browsingContextProcessor.ts | 6 ++-- src/bidiMapper/domains/context/context.ts | 4 +-- tests/test_bidi.py | 18 +++++----- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/bidiMapper/bidiProtocolTypes.ts b/src/bidiMapper/bidiProtocolTypes.ts index dc70a4937e..64fc7dd3e5 100644 --- a/src/bidiMapper/bidiProtocolTypes.ts +++ b/src/bidiMapper/bidiProtocolTypes.ts @@ -136,25 +136,25 @@ export namespace BrowsingContext { context: BrowsingContext; }; - export namespace PROTO { - export type BrowsingContextNavigateCommand = { - method: 'PROTO.browsingContext.navigate'; - params: BrowsingContextNavigateParameters; - }; + export type BrowsingContextNavigateCommand = { + method: 'browsingContext.navigate'; + params: BrowsingContextNavigateParameters; + }; - export type BrowsingContextNavigateParameters = { - context: BrowsingContext; - url: string; - wait?: ReadinessState; - }; + export type BrowsingContextNavigateParameters = { + context: BrowsingContext; + url: string; + wait?: ReadinessState; + }; - export type ReadinessState = 'none'; - // TODO sadym: implement 'interactive' and 'complete' states. - export type BrowsingContextNavigateResult = { - navigation?: Navigation; - url: string; - }; - } + export type ReadinessState = 'none'; + // TODO sadym: implement 'interactive' and 'complete' states. + export type BrowsingContextNavigateResult = { + navigation?: Navigation; + url: string; + }; + + export namespace PROTO {} } export namespace Session { diff --git a/src/bidiMapper/commandProcessor.ts b/src/bidiMapper/commandProcessor.ts index 2ede801b08..3efe470c04 100644 --- a/src/bidiMapper/commandProcessor.ts +++ b/src/bidiMapper/commandProcessor.ts @@ -179,11 +179,11 @@ export class CommandProcessor { return await this._contextProcessor.process_browsingContext_create( commandData as BrowsingContext.BrowsingContextCreateCommand ); - - case 'PROTO.browsingContext.navigate': - return await this._contextProcessor.process_PROTO_browsingContext_navigate( - commandData as BrowsingContext.PROTO.BrowsingContextNavigateCommand + case 'browsingContext.navigate': + return await this._contextProcessor.process_browsingContext_navigate( + commandData as BrowsingContext.BrowsingContextNavigateCommand ); + case 'PROTO.script.invoke': return await this._contextProcessor.process_PROTO_script_invoke( commandData as Script.PROTO.ScriptInvokeCommand diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index 8270445abe..c5f311c1ff 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -192,9 +192,9 @@ export class BrowsingContextProcessor { }); } - async process_PROTO_browsingContext_navigate( - commandData: BrowsingContext.PROTO.BrowsingContextNavigateCommand - ): Promise { + async process_browsingContext_navigate( + commandData: BrowsingContext.BrowsingContextNavigateCommand + ): Promise { const params = commandData.params; const context = await this._getKnownContext(params.context); diff --git a/src/bidiMapper/domains/context/context.ts b/src/bidiMapper/domains/context/context.ts index ac585d6f4a..fa2b6e9c12 100644 --- a/src/bidiMapper/domains/context/context.ts +++ b/src/bidiMapper/domains/context/context.ts @@ -89,8 +89,8 @@ export class Context { public async navigate( url: string, - wait: BrowsingContext.PROTO.ReadinessState = 'none' - ): Promise { + wait: BrowsingContext.ReadinessState = 'none' + ): Promise { // TODO sadym: implement. if (wait !== 'none') { throw new Error(`Not implenented wait '${wait}'`); diff --git a/tests/test_bidi.py b/tests/test_bidi.py index c96b255960..8405da9484 100644 --- a/tests/test_bidi.py +++ b/tests/test_bidi.py @@ -71,10 +71,10 @@ async def read_JSON_message(websocket): # Open given URL in the given context. async def goto_url(websocket, contextID, url): - # Send "PROTO.browsingContext.navigate" command. + # Send "browsingContext.navigate" command. command = { "id": 9998, - "method": "PROTO.browsingContext.navigate", + "method": "browsingContext.navigate", "params": { "url": url, "context": contextID}} @@ -84,7 +84,7 @@ async def goto_url(websocket, contextID, url): resp = await read_JSON_message(websocket) assert resp["method"] == "DEBUG.Page.load" - # Assert "PROTO.browsingContext.navigate" command done. + # Assert "browsingContext.navigate" command done. resp = await read_JSON_message(websocket) assert resp["id"] == 9998 return contextID @@ -239,13 +239,13 @@ async def _ignore_test_PageClose_browsingContextContextDestroyedEmitted(websocke "url": "about:blank"}} @pytest.mark.asyncio -async def test_PROTO_navigateWaitNone_navigated(websocket): +async def test_navigateWaitNone_navigated(websocket): contextID = await get_open_context_id(websocket) # Send command. command = { "id": 15, - "method": "PROTO.browsingContext.navigate", + "method": "browsingContext.navigate", "params": { "url": "data:text/html,

test

", "wait": "none", @@ -265,23 +265,23 @@ async def test_PROTO_navigateWaitNone_navigated(websocket): @pytest.mark.asyncio # Not implemented yet. -async def _ignore_test_PROTO_navigateWaitInteractive_navigated(websocket): +async def _ignore_test_navigateWaitInteractive_navigated(websocket): ignore = True @pytest.mark.asyncio # Not implemented yet. -async def _ignore_test_PROTO_navigateWaitComplete_navigated(websocket): +async def _ignore_test_navigateWaitComplete_navigated(websocket): ignore = True @pytest.mark.asyncio # Not implemented yet. -async def _ignore_test_PROTO_navigateWithShortTimeout_timeoutOccuredAndEventPageLoadEmitted(websocket): +async def _ignore_test_navigateWithShortTimeout_timeoutOccuredAndEventPageLoadEmitted(websocket): contextID = await get_open_context_id(websocket) # Send command. command = { "id": 16, - "method": "PROTO.browsingContext.navigate", + "method": "browsingContext.navigate", "params": { "url": "data:text/html,

test

", "context": contextID, From 3fd095b8e27c527de9a2fb8ecc803e7469386738 Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Thu, 14 Oct 2021 10:07:50 +0000 Subject: [PATCH 8/9] `browsingContext.create` -> `PROTO.browsingContext.create` --- src/bidiMapper/bidiProtocolTypes.ts | 35 ++++++++++--------- src/bidiMapper/commandProcessor.ts | 6 ++-- .../context/browsingContextProcessor.spec.ts | 10 +++--- .../context/browsingContextProcessor.ts | 6 ++-- tests/test_bidi.py | 4 +-- 5 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/bidiMapper/bidiProtocolTypes.ts b/src/bidiMapper/bidiProtocolTypes.ts index 64fc7dd3e5..fad013ff2b 100644 --- a/src/bidiMapper/bidiProtocolTypes.ts +++ b/src/bidiMapper/bidiProtocolTypes.ts @@ -120,22 +120,6 @@ export namespace BrowsingContext { children: BrowsingContextInfoList; }; - // `browsingContext.create` - export type BrowsingContextCreateCommand = { - method: 'browsingContext.create'; - params: BrowsingContextCreateParameters; - }; - - export type BrowsingContextCreateType = 'tab' | 'window'; - - export type BrowsingContextCreateParameters = { - type?: BrowsingContextCreateType; - }; - - export type BrowsingContextCreateResult = { - context: BrowsingContext; - }; - export type BrowsingContextNavigateCommand = { method: 'browsingContext.navigate'; params: BrowsingContextNavigateParameters; @@ -154,7 +138,24 @@ export namespace BrowsingContext { url: string; }; - export namespace PROTO {} + export namespace PROTO { + // `browsingContext.create`: + // https://github.com/w3c/webdriver-bidi/pull/133 + export type BrowsingContextCreateCommand = { + method: 'PROTO.browsingContext.create'; + params: BrowsingContextCreateParameters; + }; + + export type BrowsingContextCreateType = 'tab' | 'window'; + + export type BrowsingContextCreateParameters = { + type?: BrowsingContextCreateType; + }; + + export type BrowsingContextCreateResult = { + context: BrowsingContext; + }; + } } export namespace Session { diff --git a/src/bidiMapper/commandProcessor.ts b/src/bidiMapper/commandProcessor.ts index 3efe470c04..c08366e02a 100644 --- a/src/bidiMapper/commandProcessor.ts +++ b/src/bidiMapper/commandProcessor.ts @@ -175,9 +175,9 @@ export class CommandProcessor { return await this._contextProcessor.process_script_evaluate( commandData as Script.ScriptEvaluateCommand ); - case 'browsingContext.create': - return await this._contextProcessor.process_browsingContext_create( - commandData as BrowsingContext.BrowsingContextCreateCommand + case 'PROTO.browsingContext.create': + return await this._contextProcessor.process_PROTO_browsingContext_create( + commandData as BrowsingContext.PROTO.BrowsingContextCreateCommand ); case 'browsingContext.navigate': return await this._contextProcessor.process_browsingContext_navigate( diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts index 1a28c9d2cd..4acc49ec80 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts @@ -86,10 +86,10 @@ describe('BrowsingContextProcessor', function () { }); }); - describe('handle `process_browsingContext_create`', async function () { - const BROWSING_CONTEXT_CREATE_COMMAND: BrowsingContext.BrowsingContextCreateCommand = + describe('handle `process_PROTO_browsingContext_create`', async function () { + const BROWSING_CONTEXT_CREATE_COMMAND: BrowsingContext.PROTO.BrowsingContextCreateCommand = { - method: 'browsingContext.create', + method: 'PROTO.browsingContext.create', params: {}, }; @@ -111,7 +111,7 @@ describe('BrowsingContextProcessor', function () { it('Target.attachedToTarget before command finished', async function () { const createResultPromise = - browsingContextProcessor.process_browsingContext_create( + browsingContextProcessor.process_PROTO_browsingContext_create( BROWSING_CONTEXT_CREATE_COMMAND ); @@ -131,7 +131,7 @@ describe('BrowsingContextProcessor', function () { it('Target.attachedToTarget after command finished', async function () { const createResultPromise = - browsingContextProcessor.process_browsingContext_create( + browsingContextProcessor.process_PROTO_browsingContext_create( BROWSING_CONTEXT_CREATE_COMMAND ); diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index c5f311c1ff..393b36653c 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -150,9 +150,9 @@ export class BrowsingContextProcessor { } } - async process_browsingContext_create( - commandData: BrowsingContext.BrowsingContextCreateCommand - ): Promise { + async process_PROTO_browsingContext_create( + commandData: BrowsingContext.PROTO.BrowsingContextCreateCommand + ): Promise { const params = commandData.params; return new Promise(async (resolve) => { diff --git a/tests/test_bidi.py b/tests/test_bidi.py index 8405da9484..d3187754c9 100644 --- a/tests/test_bidi.py +++ b/tests/test_bidi.py @@ -169,12 +169,12 @@ async def ignore_test_getTreeWithNestedContexts_contextReturned(websocket): # TODO sadym: implement @pytest.mark.asyncio -async def test_createContext_eventContextCreatedEmittedAndContextCreated(websocket): +async def test_PROTO_browsingContextCreate_eventContextCreatedEmittedAndContextCreated(websocket): initialContextID = await get_open_context_id(websocket) command = { "id": 9, - "method": "browsingContext.create", + "method": "PROTO.browsingContext.create", "params": {}} await send_JSON_command(websocket, command) From e5155278fbfe10e8a50a85d56daccf358487b09b Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Tue, 19 Oct 2021 09:22:56 +0000 Subject: [PATCH 9/9] PR fixes: * Avoid race condition in waiting for a new target created; * Minor changes. --- .../context/browsingContextProcessor.spec.ts | 4 +-- .../context/browsingContextProcessor.ts | 25 ++++++++++++------- src/bidiMapper/domains/context/context.ts | 2 +- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts index 4acc49ec80..cf7a24f1c0 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.spec.ts @@ -58,8 +58,8 @@ describe('BrowsingContextProcessor', function () { browsingContextProcessor = new BrowsingContextProcessor( cdpConnection, 'SELF_TARGET_ID', - sinon.fake as unknown as (t: Context) => Promise, - sinon.fake as unknown as (t: Context) => Promise, + sinon.fake(), + sinon.fake(), EVALUATOR_SCRIPT ); diff --git a/src/bidiMapper/domains/context/browsingContextProcessor.ts b/src/bidiMapper/domains/context/browsingContextProcessor.ts index 393b36653c..1b7bd5200c 100644 --- a/src/bidiMapper/domains/context/browsingContextProcessor.ts +++ b/src/bidiMapper/domains/context/browsingContextProcessor.ts @@ -82,6 +82,10 @@ export class BrowsingContextProcessor { return contextPromise; } + private _hasKnownContext(contextId: string): boolean { + return this._contexts.has(contextId); + } + private async _tryGetContext( contextId: string ): Promise { @@ -89,9 +93,8 @@ export class BrowsingContextProcessor { } private async _getKnownContext(contextId: string): Promise { - const context = await this._contexts.get(contextId); - if (!context) throw new Error('context not found'); - return context; + if (!this._hasKnownContext(contextId)) throw new Error('context not found'); + return await this._contexts.get(contextId)!; } private async _handleAttachedToTargetEvent( @@ -139,6 +142,7 @@ export class BrowsingContextProcessor { // TODO: params.targetId is deprecated. Update this class to track using // params.sessionId instead. + // https://github.com/GoogleChromeLabs/chromium-bidi/issues/60 const targetId = params.targetId!; const context = await this._tryGetContext(targetId); if (context) { @@ -163,10 +167,10 @@ export class BrowsingContextProcessor { newWindow: params.type === 'window', }); - const targetId = result.targetId; + const contextId = result.targetId; - const existingContext = await this._tryGetContext(targetId); - if (existingContext) { + if (this._hasKnownContext(contextId)) { + const existingContext = await this._getKnownContext(contextId); resolve(existingContext.toBidi()); return; } @@ -174,14 +178,14 @@ export class BrowsingContextProcessor { const onAttachedToTarget = async ( attachToTargetEventParams: Protocol.Target.AttachedToTargetEvent ) => { - if (attachToTargetEventParams.targetInfo.targetId === targetId) { + if (attachToTargetEventParams.targetInfo.targetId === contextId) { browserCdpClient.Target.removeListener( 'attachedToTarget', onAttachedToTarget ); const context = await this._getOrCreateContext( - targetId, + contextId, attachToTargetEventParams.sessionId ); resolve(context.toBidi()); @@ -198,7 +202,10 @@ export class BrowsingContextProcessor { const params = commandData.params; const context = await this._getKnownContext(params.context); - return await context.navigate(params.url, params.wait); + return await context.navigate( + params.url, + params.wait !== undefined ? params.wait : 'none' + ); } async process_script_evaluate( diff --git a/src/bidiMapper/domains/context/context.ts b/src/bidiMapper/domains/context/context.ts index fa2b6e9c12..3753821787 100644 --- a/src/bidiMapper/domains/context/context.ts +++ b/src/bidiMapper/domains/context/context.ts @@ -89,7 +89,7 @@ export class Context { public async navigate( url: string, - wait: BrowsingContext.ReadinessState = 'none' + wait: BrowsingContext.ReadinessState ): Promise { // TODO sadym: implement. if (wait !== 'none') {