fix: only auto-attach to a target once

GoogleChromeLabs · Jul 18, 2024 · 4cfe7c1 · 4cfe7c1
1 parent 0cde2e4
commit 4cfe7c1
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 3 deletions.
diff --git a/src/bidiMapper/modules/cdp/CdpTargetManager.ts b/src/bidiMapper/modules/cdp/CdpTargetManager.ts
@@ -43,7 +43,7 @@ const cdpToBidiTargetTypes = {
 export class CdpTargetManager {
   readonly #browserCdpClient: CdpClient;
   readonly #cdpConnection: CdpConnection;
-  readonly #selfTargetId: string;
+  readonly #targetIdsToBeIgnoredByAutoAttach = new Set<string>();
   readonly #eventManager: EventManager;
 
   readonly #browsingContextStorage: BrowsingContextStorage;
@@ -70,7 +70,7 @@ export class CdpTargetManager {
   ) {
     this.#cdpConnection = cdpConnection;
     this.#browserCdpClient = browserCdpClient;
-    this.#selfTargetId = selfTargetId;
+    this.#targetIdsToBeIgnoredByAutoAttach.add(selfTargetId);
     this.#eventManager = eventManager;
     this.#browsingContextStorage = browsingContextStorage;
     this.#preloadScriptStorage = preloadScriptStorage;
@@ -154,9 +154,13 @@ export class CdpTargetManager {
     switch (targetInfo.type) {
       case 'page':
       case 'iframe': {
-        if (targetInfo.targetId === this.#selfTargetId) {
+        // 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)) {
           break;
         }
+        // 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
@@ -15,6 +15,7 @@
 
 from unittest.mock import ANY
 
+import asyncio
 import pytest
 from anys import ANY_INT
 from test_helpers import (ANY_TIMESTAMP, AnyExtending, execute_command,
@@ -158,3 +159,53 @@ 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:
+          print(event)
+          if 'id' in event and event['id'] == id:
+              print(event)
+              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")