From 0e322a0de0902afc723a49aec869fdb596bd9b4a Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Fri, 24 Jul 2026 18:16:20 +0900 Subject: [PATCH] WebDriver: Survive a WebContent crash during a navigation command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: TestWebDriverSessionHistory crash-recovery scenarios failed intermittently in CI, with the WebDriver process aborting (exit status -4) rather than reporting an error. Cause: When a WebContent process is torn down before it replies to a sync WebDriver navigation request, the WebDriver process itself aborts. That happens when a cross-site navigation swaps the WebContent process out, or when the page crashes, before the reply reaches WebDriver. WebDriver forwarded its navigation commands to WebContent with send_sync, whose result is a NonnullOwnPtr guarded by VERIFY(response). When the peer disconnected before replying, that VERIFY aborted the whole WebDriver process — rather than failing only the single command. Fix: Send commands with send_sync_but_allow_failure — and treat a lost response as an empty success. A null reply can only mean the WebContent process is gone — so the recovery path already in place takes over: The window is marked as awaiting replacement, and the caller waits for and adopts the replacement process. perform_async_action pumps the event loop while it spins. So, the connection-closed handler runs and the spin exits rather than hanging. That mirrors load_url_from_ui — which already tolerated a lost reply in this same way. --- Services/WebDriver/Client.cpp | 35 ++++++++++++++++++----- Services/WebDriver/WebContentConnection.h | 3 +- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/Services/WebDriver/Client.cpp b/Services/WebDriver/Client.cpp index 755c7891847ed..8984adbecd1bc 100644 --- a/Services/WebDriver/Client.cpp +++ b/Services/WebDriver/Client.cpp @@ -172,9 +172,11 @@ Web::WebDriver::Response Client::navigate_to(Web::WebDriver::Parameters paramete dbgln_if(WEBDRIVER_DEBUG, "Handling POST /session//url"); auto session = TRY(Session::find_session(parameters[0])); - auto response = TRY(session->perform_async_action([&](auto& connection) { - auto navigate_response = connection.navigate_to(move(payload)); - return navigate_response.response(); + auto response = TRY(session->perform_async_action([&](auto& connection) -> Web::WebDriver::Response { + auto navigate_response = connection.template send_sync_but_allow_failure(move(payload)); + if (!navigate_response) + return JsonValue {}; + return navigate_response->response(); }, Session::WebContentReplacement::Allow)); TRY(session->wait_for_current_window_to_have_web_content_connection()); @@ -205,7 +207,15 @@ Web::WebDriver::Response Client::back(Web::WebDriver::Parameters parameters, Jso auto session = TRY(Session::find_session(parameters[0])); auto response = TRY(session->perform_async_action( - [&](auto& connection) { return connection.back(); }, + [&](auto& connection) -> Web::WebDriver::Response { + // A torn-down WebContent process leaves this reply unanswered. Treat the lost reply as an + // empty success, so only this command fails over to the replacement process instead of + // aborting WebDriver; the wait below then adopts the incoming process. + auto reply = connection.template send_sync_but_allow_failure(); + if (!reply) + return JsonValue {}; + return reply->take_response(); + }, Session::WebContentReplacement::Allow)); TRY(session->wait_for_current_window_to_have_web_content_connection()); response = TRY(session->perform_async_action( @@ -223,7 +233,15 @@ Web::WebDriver::Response Client::forward(Web::WebDriver::Parameters parameters, auto session = TRY(Session::find_session(parameters[0])); auto response = TRY(session->perform_async_action( - [&](auto& connection) { return connection.forward(); }, + [&](auto& connection) -> Web::WebDriver::Response { + // A torn-down WebContent process leaves this reply unanswered. Treat the lost reply as an + // empty success, so only this command fails over to the replacement process instead of + // aborting WebDriver; the wait below then adopts the incoming process. + auto reply = connection.template send_sync_but_allow_failure(); + if (!reply) + return JsonValue {}; + return reply->take_response(); + }, Session::WebContentReplacement::Allow)); TRY(session->wait_for_current_window_to_have_web_content_connection()); response = TRY(session->perform_async_action( @@ -240,8 +258,11 @@ Web::WebDriver::Response Client::refresh(Web::WebDriver::Parameters parameters, dbgln_if(WEBDRIVER_DEBUG, "Handling POST /session//refresh"); auto session = TRY(Session::find_session(parameters[0])); - auto response = TRY(session->perform_async_action([&](auto& connection) { - return connection.refresh(); + auto response = TRY(session->perform_async_action([&](auto& connection) -> Web::WebDriver::Response { + auto refresh_response = connection.template send_sync_but_allow_failure(); + if (!refresh_response) + return JsonValue {}; + return refresh_response->response(); })); if (TRY(session->wait_for_current_window_to_have_web_content_connection())) response = TRY(session->perform_async_action([&](auto& connection) { diff --git a/Services/WebDriver/WebContentConnection.h b/Services/WebDriver/WebContentConnection.h index 32f006d21b59a..54a7f2f8fd5f9 100644 --- a/Services/WebDriver/WebContentConnection.h +++ b/Services/WebDriver/WebContentConnection.h @@ -24,7 +24,8 @@ class WebContentConnection Web::WebDriver::Response wait_for_navigation_completion() { auto response = send_sync_but_allow_failure(); - VERIFY(response); + if (!response) + return JsonValue {}; return response->response(); }