WebContent: Track the ongoing script execution with an ID

Executing scripts via WebDriver has a bit of awkwardness around dealing
with user dialogs that open during script execution. When this happens,
we must return control back to the client immediately with a null
response, while allowing the script to continue executing. When the
script completes, we must then ignore its result.

We've previously handled this by tracking a boolean for the ongoing
script execution, set to true when the script begins and false when it
ends (either via normal script completion or the above dialog handling).
However, this failed to handle the following scenario, running two
scripts in a row:

    execute_script("alert('hi'); return 1;")
    execute_script("return 2;")

The first script would execute and open a dialog, and thus return a null
response to the client while the script continued and the dialog remains
open. The second script would "handle any user prompts", which closes
the dialog. This would end the execution of the first script. But since
we're now executing a script again, the boolean flag is true, and we'd
return the result of the first script back to the client. The client
would then think this is the result of the second script.

So we now track script execution with a simple ID. If a script completes
whose execution ID is not the ID of the currently executing script, we
drop the result.
This commit is contained in:
Timothy Flynn 2025-02-07 07:13:56 -05:00 committed by Tim Ledbetter
parent c954d0be27
commit 986fe0f408
Notes: github-actions[bot] 2025-02-10 10:21:36 +00:00
2 changed files with 17 additions and 13 deletions

View file

@ -2067,15 +2067,16 @@ Messages::WebDriverClient::ExecuteScriptResponse WebDriverConnection::execute_sc
// 3. Handle any user prompts, and return its value if it is an error.
handle_any_user_prompts([this, body = move(body), arguments = move(arguments)]() mutable {
m_has_pending_script_execution = true;
auto script_execution_id = m_script_execution_id_counter++;
m_current_script_execution_id = script_execution_id;
// 4. Let timeout be session's session timeouts' script timeout.
auto timeout_ms = m_timeouts_configuration.script_timeout;
// This handles steps 5 to 9 and produces the appropriate result type for the following steps.
Web::WebDriver::execute_script(current_browsing_context(), move(body), move(arguments), timeout_ms, GC::create_function(current_browsing_context().heap(), [this](Web::WebDriver::ExecutionResult result) {
Web::WebDriver::execute_script(current_browsing_context(), move(body), move(arguments), timeout_ms, GC::create_function(current_browsing_context().heap(), [this, script_execution_id](Web::WebDriver::ExecutionResult result) {
dbgln_if(WEBDRIVER_DEBUG, "Executing script returned: {}", result.value);
handle_script_response(result);
handle_script_response(result, script_execution_id);
}));
});
@ -2096,26 +2097,27 @@ Messages::WebDriverClient::ExecuteAsyncScriptResponse WebDriverConnection::execu
// 3. Handle any user prompts, and return its value if it is an error.
handle_any_user_prompts([this, body = move(body), arguments = move(arguments)]() mutable {
m_has_pending_script_execution = true;
auto script_execution_id = m_script_execution_id_counter++;
m_current_script_execution_id = script_execution_id;
// 4. Let timeout be session's session timeouts' script timeout.
auto timeout_ms = m_timeouts_configuration.script_timeout;
// This handles steps 5 to 9 and produces the appropriate result type for the following steps.
Web::WebDriver::execute_async_script(current_browsing_context(), move(body), move(arguments), timeout_ms, GC::create_function(current_browsing_context().heap(), [&](Web::WebDriver::ExecutionResult result) {
Web::WebDriver::execute_async_script(current_browsing_context(), move(body), move(arguments), timeout_ms, GC::create_function(current_browsing_context().heap(), [this, script_execution_id](Web::WebDriver::ExecutionResult result) {
dbgln_if(WEBDRIVER_DEBUG, "Executing async script returned: {}", result.value);
handle_script_response(result);
handle_script_response(result, script_execution_id);
}));
});
return JsonValue {};
}
void WebDriverConnection::handle_script_response(Web::WebDriver::ExecutionResult result)
void WebDriverConnection::handle_script_response(Web::WebDriver::ExecutionResult result, size_t script_execution_id)
{
if (!m_has_pending_script_execution)
if (script_execution_id != m_current_script_execution_id)
return;
m_has_pending_script_execution = false;
m_current_script_execution_id.clear();
auto response = [&]() -> Web::WebDriver::Response {
switch (result.state) {
@ -2838,8 +2840,8 @@ void WebDriverConnection::page_did_open_dialog(Badge<PageClient>)
// https://w3c.github.io/webdriver/#dfn-execute-a-function-body
// If at any point during the algorithm a user prompt appears, immediately return Completion { [[Type]]: normal,
// [[Value]]: null, [[Target]]: empty }, but continue to run the other steps of this algorithm in parallel.
if (m_has_pending_script_execution) {
m_has_pending_script_execution = false;
if (m_current_script_execution_id.has_value()) {
m_current_script_execution_id.clear();
async_driver_execution_complete(JsonValue {});
}
}

View file

@ -149,7 +149,7 @@ private:
GC::RootVector<JS::Value> arguments;
};
ErrorOr<ScriptArguments, Web::WebDriver::Error> extract_the_script_arguments_from_a_request(JS::VM&, JsonValue const& payload);
void handle_script_response(Web::WebDriver::ExecutionResult);
void handle_script_response(Web::WebDriver::ExecutionResult, size_t script_execution_id);
void delete_cookies(Optional<StringView> const& name = {});
@ -172,7 +172,9 @@ private:
GC::Ptr<Web::HTML::BrowsingContext> m_current_top_level_browsing_context;
size_t m_pending_window_rect_requests { 0 };
bool m_has_pending_script_execution { false };
size_t m_script_execution_id_counter { 0 };
Optional<size_t> m_current_script_execution_id;
friend class ElementLocator;
GC::Ptr<ElementLocator> m_element_locator;