Skip to content

Commit

Permalink
feat: throw a proper error if the browsing context was destroyed duri…
Browse files Browse the repository at this point in the history
…ng action dispatching (#2908)

Get browsing context each time from the storage to guarantee it is not
disposed.

+ e2e test
  • Loading branch information
sadym-chromium authored Dec 18, 2024
1 parent 8cac4fe commit 62c3005
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 6 deletions.
19 changes: 16 additions & 3 deletions src/bidiMapper/modules/input/ActionDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
isSingleGrapheme,
} from '../../../utils/GraphemeTools.js';
import type {BrowsingContextImpl} from '../context/BrowsingContextImpl.js';
import type {BrowsingContextStorage} from '../context/BrowsingContextStorage.js';

import type {ActionOption} from './ActionOption.js';
import {
Expand Down Expand Up @@ -90,21 +91,33 @@ export class ActionDispatcher {
return result.result.value;
};

readonly #browsingContextStorage: BrowsingContextStorage;

#tickStart = 0;
#tickDuration = 0;
#inputState: InputState;
#context: BrowsingContextImpl;
#contextId: string;
#isMacOS: boolean;

constructor(
inputState: InputState,
context: BrowsingContextImpl,
browsingContextStorage: BrowsingContextStorage,
contextId: string,
isMacOS: boolean,
) {
this.#browsingContextStorage = browsingContextStorage;
this.#inputState = inputState;
this.#context = context;
this.#contextId = contextId;
this.#isMacOS = isMacOS;
}

/**
* The context can be disposed between action ticks, so need to get it each time.
*/
get #context() {
return this.#browsingContextStorage.getContext(this.#contextId);
}

async dispatchActions(
optionsByTick: readonly (readonly Readonly<ActionOption>[])[],
) {
Expand Down
6 changes: 4 additions & 2 deletions src/bidiMapper/modules/input/InputProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export class InputProcessor {
const actionsByTick = this.#getActionsByTick(params, inputState);
const dispatcher = new ActionDispatcher(
inputState,
context,
this.#browsingContextStorage,
params.context,
await ActionDispatcher.isMacOS(context).catch(() => false),
);
await dispatcher.dispatchActions(actionsByTick);
Expand All @@ -63,7 +64,8 @@ export class InputProcessor {
const inputState = this.#inputStateManager.get(topContext);
const dispatcher = new ActionDispatcher(
inputState,
context,
this.#browsingContextStorage,
params.context,
await ActionDispatcher.isMacOS(context).catch(() => false),
);
await dispatcher.dispatchTickActions(inputState.cancelList.reverse());
Expand Down
63 changes: 62 additions & 1 deletion tests/input/test_input.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
import pytest
from syrupy.filters import props
from test_helpers import (AnyExtending, execute_command, goto_url,
send_JSON_command, subscribe, wait_for_event)
read_JSON_message, send_JSON_command, subscribe,
wait_for_event)

SET_FILES_HTML = """
<input id=input type=file>
Expand Down Expand Up @@ -811,3 +812,63 @@ async def test_input_setFiles_unableToSetFileInput(websocket, context_id, html,
message = exception.args[0]['error']

assert message == snapshot


@pytest.mark.asyncio
async def test_input_keyDown_closes_browsing_context(websocket, context_id,
html):
url = html("""
<input onkeydown="window.close()">close</input>
<script>document.querySelector("input").focus();</script>
""")

await subscribe(websocket, ["browsingContext.load"])
on_load = wait_for_event(websocket, "browsingContext.load")

# Open new window via script. Required for script to be able to close it.
resp = await execute_command(
websocket, {
"method": "script.evaluate",
"params": {
"expression": f"window.open('{url}')",
"awaitPromise": False,
"target": {
"context": context_id
}
}
})

# Wait for the new context to load.
await on_load

new_context = resp['result']['value']['context']

command_id = await send_JSON_command(
websocket, {
"method": "input.performActions",
"params": {
"context": new_context,
"actions": [{
"type": "key",
"id": "main_keyboard",
"actions": [{
"type": "keyDown",
"value": "a",
}, {
"type": "pause",
"duration": 250,
}, {
"type": "keyUp",
"value": "a",
}]
}]
}
})

resp = await read_JSON_message(websocket)
assert resp == {
'error': 'no such frame',
'id': command_id,
'message': f'Context {new_context} not found',
'type': 'error',
}

0 comments on commit 62c3005

Please sign in to comment.