Skip to content

Commit

Permalink
fix: only auto-attach to a target once
Browse files Browse the repository at this point in the history
  • Loading branch information
OrKoN committed Jul 18, 2024
1 parent 37fece5 commit efaf4bb
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 3 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/puppeteer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ jobs:
uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4.1.6
with:
path: 'bidi'
- name: Set up FFmpeg
uses: FedericoCarboni/setup-ffmpeg@36c6454b5a2348e7794ba2d82a21506605921e3d # v3.0.0
- name: Set up Node.js
uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
with:
Expand Down
15 changes: 14 additions & 1 deletion src/bidiMapper/modules/cdp/CdpTargetManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const cdpToBidiTargetTypes = {
export class CdpTargetManager {
readonly #browserCdpClient: CdpClient;
readonly #cdpConnection: CdpConnection;
readonly #targetIdsToBeIgnoredByAutoAttach = new Set<string>();
readonly #selfTargetId: string;
readonly #eventManager: EventManager;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -149,12 +151,23 @@ export class CdpTargetManager {
parentSessionCdpClient: CdpClient
) {
const {sessionId, targetInfo} = params;

// 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 targetCdpClient = this.#cdpConnection.getCdpClient(sessionId);

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;
}

Expand Down
50 changes: 50 additions & 0 deletions tests/session/test_cdp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")

0 comments on commit efaf4bb

Please sign in to comment.