From 93fbc4a1eb3b209fe86f877900c282ded0086aff Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Thu, 18 Jul 2024 15:01:22 +0200 Subject: [PATCH] fix: only auto-attach to a target once --- .../modules/cdp/CdpTargetManager.ts | 13 ++++- tests/session/test_cdp.py | 50 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/bidiMapper/modules/cdp/CdpTargetManager.ts b/src/bidiMapper/modules/cdp/CdpTargetManager.ts index 74c02ce4fa..4d1a84f5a4 100644 --- a/src/bidiMapper/modules/cdp/CdpTargetManager.ts +++ b/src/bidiMapper/modules/cdp/CdpTargetManager.ts @@ -43,6 +43,7 @@ const cdpToBidiTargetTypes = { export class CdpTargetManager { readonly #browserCdpClient: CdpClient; readonly #cdpConnection: CdpConnection; + readonly #targetIdsToBeIgnoredByAutoAttach = new Set(); readonly #selfTargetId: string; readonly #eventManager: EventManager; @@ -70,6 +71,7 @@ export class CdpTargetManager { ) { this.#cdpConnection = cdpConnection; this.#browserCdpClient = browserCdpClient; + this.#targetIdsToBeIgnoredByAutoAttach.add(selfTargetId); this.#selfTargetId = selfTargetId; this.#eventManager = eventManager; this.#browsingContextStorage = browsingContextStorage; @@ -154,9 +156,18 @@ export class CdpTargetManager { switch (targetInfo.type) { case 'page': case 'iframe': { - if (targetInfo.targetId === this.#selfTargetId) { + if (this.#selfTargetId === targetInfo.targetId) { + // break to detach from the self target. break; } + // Mapper only needs one session per target. If we receive additional + // auto-attached sessions, that is very likely coming from custom CDP + // sessions. + if (this.#targetIdsToBeIgnoredByAutoAttach.has(targetInfo.targetId)) { + // Return to leave the session be. + return; + } + this.#targetIdsToBeIgnoredByAutoAttach.add(targetInfo.targetId); const cdpTarget = this.#createCdpTarget(targetCdpClient, targetInfo); const maybeContext = this.#browsingContextStorage.findContext( diff --git a/tests/session/test_cdp.py b/tests/session/test_cdp.py index d4539f196a..0fcbdf8993 100644 --- a/tests/session/test_cdp.py +++ b/tests/session/test_cdp.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio from unittest.mock import ANY import pytest @@ -158,3 +159,52 @@ async def test_cdp_wait_for_event(websocket, get_cdp_session_id, context_id): "session": session_id } }) + + +@pytest.mark.asyncio +async def test_cdp_no_extraneous_events(websocket, get_cdp_session_id, + create_context, url_base): + new_context_id = await create_context() + await execute_command( + websocket, { + "method": "browsingContext.navigate", + "params": { + "url": url_base, + "wait": "complete", + "context": new_context_id + } + }) + + await subscribe(websocket, ["cdp"], [new_context_id]) + + session_id = await get_cdp_session_id(new_context_id) + + id = await send_JSON_command( + websocket, { + "method": "cdp.sendCommand", + "params": { + "method": "Target.attachToTarget", + "params": { + "targetId": new_context_id, + }, + "session": session_id + } + }) + + events = [] + event = await read_JSON_message(websocket) + + session_id = None + with pytest.raises(asyncio.TimeoutError): + while True: + if 'id' in event and event['id'] == id: + session_id = event['result']['result']['sessionId'] + if 'id' not in event: + events.append(event) + event = await asyncio.wait_for(read_JSON_message(websocket), + timeout=1.0) + + for event in events: + if event['method'].startswith( + 'cdp') and event['params']['session'] == session_id: + raise Exception("Unrelated CDP events detected")