diff --git a/Services/WebDriver/Client.cpp b/Services/WebDriver/Client.cpp index 755c7891847ed..0ab39d4ebed7d 100644 --- a/Services/WebDriver/Client.cpp +++ b/Services/WebDriver/Client.cpp @@ -311,9 +311,12 @@ Web::WebDriver::Response Client::traverse_history_from_ui(Web::WebDriver::Parame if (payload.is_object()) wait_for_navigation_completion = payload.as_object().get_bool("waitForNavigationCompletion"sv).value_or(true); + RefPtr previous_connection { &session->web_content_connection() }; auto response = TRY(session->perform_async_action( [&](auto& connection) { return connection.traverse_history_from_ui(move(payload)); }, Session::WebContentReplacement::Allow)); + if (response.is_object() && response.as_object().get_bool("willReplaceWebContentProcess"sv).value_or(false)) + session->mark_current_window_as_awaiting_replacement(*previous_connection); TRY(session->wait_for_current_window_to_have_web_content_connection()); if (!wait_for_navigation_completion) diff --git a/Services/WebDriver/Session.cpp b/Services/WebDriver/Session.cpp index 9ee8d8afbb4e9..7c68e79f0ccfa 100644 --- a/Services/WebDriver/Session.cpp +++ b/Services/WebDriver/Session.cpp @@ -288,7 +288,7 @@ ErrorOr Session::accept_web_content_transport(NonnullOwnPtrvalue.web_content_connection = move(pending_connection); - window->value.is_awaiting_replacement = false; + window->value.awaiting_replacement = Window::AwaitingReplacement::No; } else { m_windows.set(window_handle, Session::Window { window_handle, move(pending_connection) }); } @@ -308,7 +308,13 @@ void Session::web_content_connection_closed(WebContentConnection const& connecti if (window.value.web_content_connection.ptr() != &connection) continue; - if (window.value.is_awaiting_replacement) { + if (window.value.is_awaiting_replacement()) { + window.value.web_content_connection = nullptr; + return; + } + + if (&connection == m_connection_awaiting_possible_replacement) { + window.value.awaiting_replacement = Window::AwaitingReplacement::InferredFromClosedConnection; window.value.web_content_connection = nullptr; return; } @@ -340,11 +346,11 @@ void Session::did_update_window_handle(String window_handle, WebContentConnectio auto window = maybe_window.release_value(); window.handle = window_handle; - window.is_awaiting_replacement = false; + window.awaiting_replacement = Window::AwaitingReplacement::No; if (auto existing_window = m_windows.find(window_handle); existing_window != m_windows.end()) { existing_window->value.web_content_connection = move(window.web_content_connection); - existing_window->value.is_awaiting_replacement = false; + existing_window->value.awaiting_replacement = Window::AwaitingReplacement::No; } else { m_windows.set(window_handle, move(window)); } @@ -359,7 +365,7 @@ void Session::did_start_window_replacement(String const& window_handle, WebConte if (window == m_windows.end() || window->value.web_content_connection.ptr() != &connection) return; - window->value.is_awaiting_replacement = true; + window->value.awaiting_replacement = Window::AwaitingReplacement::Announced; window->value.web_content_connection = nullptr; } @@ -369,7 +375,7 @@ void Session::mark_current_window_as_awaiting_replacement(WebContentConnection c if (window == m_windows.end() || window->value.web_content_connection.ptr() != &connection) return; - window->value.is_awaiting_replacement = true; + window->value.awaiting_replacement = Window::AwaitingReplacement::Announced; window->value.web_content_connection = nullptr; } @@ -537,18 +543,6 @@ Web::WebDriver::Response Session::get_window_handles() const return JsonValue { move(handles) }; } -ErrorOr Session::ensure_current_window_handle_is_valid() const -{ - auto current_window = m_windows.get(m_current_window_handle); - if (!current_window.has_value()) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchWindow, "Window not found"sv); - - if (!current_window->web_content_connection) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::UnknownError, "Window is waiting for a replacement WebContent process"sv); - - return {}; -} - ErrorOr Session::wait_for_current_window_to_have_web_content_connection() { m_event_loop.pump(Core::EventLoop::WaitMode::PollForEvents); @@ -560,6 +554,9 @@ ErrorOr Session::wait_for_current_window_to_have_we if (current_window->web_content_connection) return false; + static constexpr u64 INFERRED_REPLACEMENT_TIMEOUT_MS = 5000; + auto replacement_was_inferred = current_window->awaiting_replacement == Window::AwaitingReplacement::InferredFromClosedConnection; + Optional page_load_timeout = Web::WebDriver::TimeoutsConfiguration {}.page_load_timeout; if (m_timeouts_configuration.has_value() && m_timeouts_configuration->is_object()) { if (auto value = m_timeouts_configuration->as_object().get("pageLoad"sv); value.has_value()) { @@ -569,6 +566,8 @@ ErrorOr Session::wait_for_current_window_to_have_we page_load_timeout = value->get_integer().value_or(*page_load_timeout); } } + if (replacement_was_inferred) + page_load_timeout = min(page_load_timeout.value_or(INFERRED_REPLACEMENT_TIMEOUT_MS), INFERRED_REPLACEMENT_TIMEOUT_MS); bool timed_out = false; RefPtr timer; @@ -588,11 +587,30 @@ ErrorOr Session::wait_for_current_window_to_have_we if (timer) timer->stop(); - if (timed_out) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::Timeout, "Timed out waiting for replacement WebContent process"sv); + // Refetch the window — rather than trusting timed_out: If the replacement registered in the same event-loop batch + // that fired the timer, its arrival wins over the timeout. + current_window = m_windows.get(m_current_window_handle); + if (!current_window.has_value()) + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchWindow, "Window not found"sv); + if (current_window->web_content_connection) + return true; + VERIFY(timed_out); + + // The replacement this window was waiting for never arrived — and nothing else will ever connect a WebContent + // process to this window. So, without a transition here, every later command would reach this same wait, and repeat + // this same timeout — for the life of the session. This timeout is the sole owner of that failure transition: + // Remove the window — converging on the end state an unannounced connection close has. Later commands then observe + // an absent window — for which the WebDriver spec prescribes the error in every command's step 1; e.g., from + // https://w3c.github.io/webdriver/#get-current-url: + // 1. If the current top-level browsing context is no longer open, return error with error code no such window. + auto window_handle = m_current_window_handle; + remove_window(window_handle); - TRY(ensure_current_window_handle_is_valid()); - return true; + if (replacement_was_inferred) + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchWindow, + MUST(String::formatted("The window's WebContent process disconnected and was not replaced within {} ms", *page_load_timeout))); + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchWindow, + MUST(String::formatted("The window's replacement WebContent process did not connect within {} ms", *page_load_timeout))); } } diff --git a/Services/WebDriver/Session.h b/Services/WebDriver/Session.h index 8b77642bbe6ce..633728cb7837c 100644 --- a/Services/WebDriver/Session.h +++ b/Services/WebDriver/Session.h @@ -46,9 +46,17 @@ class Session : public RefCounted { static void close_all(); struct Window { + enum class AwaitingReplacement { + No, + Announced, + InferredFromClosedConnection, + }; + String handle; RefPtr web_content_connection; - bool is_awaiting_replacement { false }; + AwaitingReplacement awaiting_replacement { AwaitingReplacement::No }; + + bool is_awaiting_replacement() const { return awaiting_replacement != AwaitingReplacement::No; } }; WebContentConnection& web_content_connection() const @@ -73,7 +81,6 @@ class Session : public RefCounted { Web::WebDriver::Response close_window(); Web::WebDriver::Response switch_to_window(StringView); Web::WebDriver::Response get_window_handles() const; - ErrorOr ensure_current_window_handle_is_valid() const; ErrorOr wait_for_current_window_to_have_web_content_connection(); void mark_current_window_as_awaiting_replacement(WebContentConnection const&); @@ -88,7 +95,14 @@ class Session : public RefCounted { Optional response; RefPtr connection { &web_content_connection() }; - ScopeGuard guard { [&]() { connection->on_driver_execution_complete = nullptr; } }; + auto previous_connection_awaiting_replacement = m_connection_awaiting_possible_replacement; + if (web_content_replacement == WebContentReplacement::Allow) + m_connection_awaiting_possible_replacement = connection.ptr(); + + ScopeGuard guard { [&]() { + connection->on_driver_execution_complete = nullptr; + m_connection_awaiting_possible_replacement = previous_connection_awaiting_replacement; + } }; connection->on_driver_execution_complete = [&](auto result) { response = move(result); }; TRY(action(*connection)); @@ -101,7 +115,17 @@ class Session : public RefCounted { return false; auto current_window = m_windows.get(m_current_window_handle); - return !current_window.has_value() || (current_window->is_awaiting_replacement && !current_window->web_content_connection); + if (!current_window.has_value()) + return true; + + // A replacement WebContent process can register itself with this session before the event loop dispatches + // the closing of the connection this action was sent to. That close matches no window – so, the awaiting- + // replacement state this wait watches for is never entered. The current window being connected to another + // connection is equally proof that no response will ever arrive from the connection this was sent to. + if (current_window->web_content_connection) + return current_window->web_content_connection.ptr() != connection.ptr(); + + return current_window->is_awaiting_replacement(); }); if (response.has_value()) @@ -137,6 +161,8 @@ class Session : public RefCounted { HashMap m_windows; String m_current_window_handle; + WebContentConnection const* m_connection_awaiting_possible_replacement { nullptr }; + HashMap> m_pending_connections; u64 m_next_pending_connection_id { 0 }; diff --git a/Tests/LibWebView/CMakeLists.txt b/Tests/LibWebView/CMakeLists.txt index 68817531af772..efa0100064bc0 100644 --- a/Tests/LibWebView/CMakeLists.txt +++ b/Tests/LibWebView/CMakeLists.txt @@ -59,7 +59,7 @@ if (BUILD_TESTING AND NOT WIN32) NAME TestWebDriverSessionHistory COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test-webdriver-session-history.py $ ) - set_tests_properties(TestWebDriverSessionHistory PROPERTIES TIMEOUT 120) + set_tests_properties(TestWebDriverSessionHistory PROPERTIES TIMEOUT 240) add_test(NAME TestAutocomplete COMMAND $) set_tests_properties(TestAutocomplete PROPERTIES TIMEOUT 60 ENVIRONMENT LADYBIRD_SOURCE_DIR=${LADYBIRD_SOURCE_DIR}) diff --git a/Tests/LibWebView/test-webdriver-session-history.py b/Tests/LibWebView/test-webdriver-session-history.py index 8c4b4e6dae823..ddfeecc920147 100755 --- a/Tests/LibWebView/test-webdriver-session-history.py +++ b/Tests/LibWebView/test-webdriver-session-history.py @@ -9,6 +9,7 @@ import http.server import json import os +import signal import socket import subprocess import sys @@ -73,6 +74,7 @@ def __init__(self, server_address, request_handler_class): self.same_site_post_result_document_ran = threading.Event() self.same_url_post_result_document_ran = threading.Event() self.reload_blocked_document_ran = threading.Event() + self.actions_probe_pointer_event_ran = threading.Event() def handle_error(self, request, client_address): if isinstance(sys.exc_info()[1], BrokenPipeError): @@ -599,6 +601,24 @@ def do_GET(self): ) return + if self.path == "/actions-probe": + self.send_response(200) + self.send_header("Content-Type", "text/html") + self.end_headers() + self.wfile.write( + """ +Actions Probe + +

Actions Probe

""".encode() + ) + return + + if self.path == "/document-ran?actions-probe": + server.actions_probe_pointer_event_ran.set() + self.send_response(204) + self.end_headers() + return + if self.path == "/post-result": self.send_response(200) self.send_header("Content-Type", "text/html") @@ -2398,6 +2418,145 @@ def request_post_crash_recovery(): expect_current_entry_resource(webdriver_port, session_id, "after POST crash recovery", "post", log) +def find_browser_pid_for_session(session_id): + for _ in range(50): + pgrep = subprocess.run(["pgrep", "-f", session_id], capture_output=True, text=True) + pids = [int(pid) for pid in pgrep.stdout.split()] + if len(pids) > 1: + raise AssertionError(f"Expected one browser process for session {session_id}, found pids: {pids}") + if len(pids) == 1: + return pids[0] + time.sleep(0.1) + raise AssertionError(f"Did not find a browser process for session {session_id}") + + +def find_web_content_pid(browser_pid): + for _ in range(50): + pgrep = subprocess.run(["pgrep", "-P", str(browser_pid), "-f", "WebContent"], capture_output=True, text=True) + pids = [int(pid) for pid in pgrep.stdout.split()] + if len(pids) > 1: + raise AssertionError(f"Expected one WebContent child of browser {browser_pid}, found pids: {pids}") + if len(pids) == 1: + return pids[0] + time.sleep(0.1) + raise AssertionError(f"Did not find a WebContent child of browser process {browser_pid}") + + +def pointer_event_then_long_pause_actions_payload(): + return { + "actions": [ + { + "type": "pointer", + "id": "death-probe-pointer", + "parameters": {"pointerType": "mouse"}, + "actions": [ + {"type": "pointerMove", "origin": "viewport", "x": 5, "y": 5}, + {"type": "pointerDown", "button": 0}, + {"type": "pointerUp", "button": 0}, + {"type": "pause", "duration": 20000}, + ], + } + ] + } + + +def post_actions_in_thread(webdriver_port, session_id): + result = {} + + def post_actions(): + try: + result["response"] = request_raw( + webdriver_port, + "POST", + f"/session/{session_id}/actions", + pointer_event_then_long_pause_actions_payload(), + ) + except Exception as error: + result["error"] = error + + actions_thread = threading.Thread(target=post_actions, daemon=True) + actions_thread.start() + return actions_thread, result + + +def run_second_ui_forward_during_pending_forward_test(webdriver_port, page_server, url_a, url_forward_blocked, url_c): + session_id = create_session(webdriver_port) + expect_second_ui_forward_supersedes_pending_forward( + webdriver_port, + session_id, + page_server, + url_a, + url_forward_blocked, + url_c, + ) + request(webdriver_port, "DELETE", f"/session/{session_id}") + + +def run_unannounced_web_content_death_tests(webdriver_port, page_server, url_actions_probe): + run_unannounced_web_content_death_recovery_test(webdriver_port, page_server, url_actions_probe) + run_unannounced_web_content_death_without_replacement_test(webdriver_port, page_server, url_actions_probe) + + +def run_unannounced_web_content_death_recovery_test(webdriver_port, page_server, url_actions_probe): + session_id = create_session(webdriver_port) + log = [f"unannounced WebContent death recovery initial: {current_url(webdriver_port, session_id)}"] + request(webdriver_port, "POST", f"/session/{session_id}/url", {"url": url_actions_probe}) + browser_pid = find_browser_pid_for_session(session_id) + web_content_pid = find_web_content_pid(browser_pid) + + page_server.actions_probe_pointer_event_ran.clear() + actions_thread, result = post_actions_in_thread(webdriver_port, session_id) + wait_for_event(page_server.actions_probe_pointer_event_ran, "pointer event before unannounced WebContent death") + os.kill(web_content_pid, signal.SIGKILL) + actions_thread.join(timeout=45) + if actions_thread.is_alive(): + raise AssertionError("Perform Actions never returned after its WebContent process was killed") + if "error" in result: + raise AssertionError(f"Perform Actions failed after its WebContent process was killed: {result['error']}") + status, _, response_body = result["response"] + if status != 200: + raise AssertionError(f"Perform Actions after WebContent death returned HTTP {status}: {response_body}") + + wait_for_url(webdriver_port, session_id, "after unannounced WebContent death recovery", url_actions_probe, log) + request(webdriver_port, "DELETE", f"/session/{session_id}") + + +def run_unannounced_web_content_death_without_replacement_test(webdriver_port, page_server, url_actions_probe): + session_id = create_session(webdriver_port) + request(webdriver_port, "POST", f"/session/{session_id}/url", {"url": url_actions_probe}) + browser_pid = find_browser_pid_for_session(session_id) + + # WebDriver waits min(pageLoad, 5000) ms for a replacement WebContent process before + # failing the in-flight command; no replacement can arrive once the browser is dead, + # so a short pageLoad keeps that wait brief. + request(webdriver_port, "POST", f"/session/{session_id}/timeouts", {"pageLoad": 1000}) + + page_server.actions_probe_pointer_event_ran.clear() + actions_thread, result = post_actions_in_thread(webdriver_port, session_id) + wait_for_event(page_server.actions_probe_pointer_event_ran, "pointer event before unannounced browser death") + os.kill(browser_pid, signal.SIGKILL) + actions_thread.join(timeout=45) + if actions_thread.is_alive(): + raise AssertionError("Perform Actions never returned after the browser was killed") + if "error" in result: + raise AssertionError(f"Perform Actions failed after the browser was killed: {result['error']}") + status, payload, response_body = result["response"] + value = payload.get("value") + error = value.get("error") if isinstance(value, dict) else None + if error != "no such window": + raise AssertionError( + f"Expected 'no such window' after the browser was killed mid-command, got HTTP {status}: {response_body}" + ) + + status, payload, response_body = request_raw(webdriver_port, "GET", f"/session/{session_id}/url") + value = payload.get("value") + error = value.get("error") if isinstance(value, dict) else None + if error != "invalid session id": + raise AssertionError( + f"Expected 'invalid session id' after the session's browser died, got HTTP {status}: {response_body}" + ) + + def run_test(webdriver_binary): page_server = TestPageServer(("0.0.0.0", 0), TestPageHandler) page_server_thread = threading.Thread(target=page_server.serve_forever, daemon=True) @@ -2446,6 +2605,7 @@ def run_test(webdriver_binary): url_reload_blocked = f"http://localhost:{page_port}/reload-blocked" url_process_swap_back_blocked = f"http://localhost:{page_port}/process-swap-back-blocked" url_forward_blocked = f"http://127.0.0.1:{page_port}/forward-blocked" + url_actions_probe = f"http://localhost:{page_port}/actions-probe" url_frame_a = f"http://localhost:{page_port}/frame-a" url_frame_b_blocked = f"http://localhost:{page_port}/frame-b-blocked" url_iframe_deferred_fragment_first = f"http://localhost:{page_port}/iframe-deferred-fragment-first" @@ -2482,17 +2642,11 @@ def run_test(webdriver_binary): url_cross_site_navigation_blocked, ) - session_id = create_session(webdriver_port) - expect_second_ui_forward_supersedes_pending_forward( - webdriver_port, - session_id, - page_server, - url_a, - url_forward_blocked, - url_c, + run_second_ui_forward_during_pending_forward_test( + webdriver_port, page_server, url_a, url_forward_blocked, url_c ) - request(webdriver_port, "DELETE", f"/session/{session_id}") - session_id = None + + run_unannounced_web_content_death_tests(webdriver_port, page_server, url_actions_probe) session_id = create_session(webdriver_port) log = [f"first-entry replace initial: {current_url(webdriver_port, session_id)}"]