From 986fe0f408f08efd65c94e715f56f211d1678867 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 7 Feb 2025 07:13:56 -0500 Subject: [PATCH] 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. --- Services/WebContent/WebDriverConnection.cpp | 24 +++++++++++---------- Services/WebContent/WebDriverConnection.h | 6 ++++-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/Services/WebContent/WebDriverConnection.cpp b/Services/WebContent/WebDriverConnection.cpp index a6b7ca975f8..4d58d1f61e1 100644 --- a/Services/WebContent/WebDriverConnection.cpp +++ b/Services/WebContent/WebDriverConnection.cpp @@ -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) // 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 {}); } } diff --git a/Services/WebContent/WebDriverConnection.h b/Services/WebContent/WebDriverConnection.h index 44aba6d6cd4..b333f8b50c0 100644 --- a/Services/WebContent/WebDriverConnection.h +++ b/Services/WebContent/WebDriverConnection.h @@ -149,7 +149,7 @@ private: GC::RootVector arguments; }; ErrorOr 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 const& name = {}); @@ -172,7 +172,9 @@ private: GC::Ptr 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 m_current_script_execution_id; friend class ElementLocator; GC::Ptr m_element_locator;