Skip to content

WebDriver: Keep the window alive when a process swap loses its signal #10730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sideshowbarker wants to merge 2 commits into
base: master
Choose a base branch
from
+199 −37
Open

WebDriver: Keep the window alive when a process swap loses its signal #10730

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
61e5862
WebDriver: Keep the window alive when a process swap loses its signal
sideshowbarker Jul 17, 2026
ee6d603
WebDriver: Make both process-replacement waits always terminate
sideshowbarker Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Services/WebDriver/Client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ Web::WebDriver::Response Client::traverse_history_from_ui(Web::WebDriver::Parame
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);

Expand Down
29 changes: 22 additions & 7 deletions Services/WebDriver/Session.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ ErrorOr<void> Session::accept_web_content_transport(NonnullOwnPtr<IPC::Transport

if (auto window = m_windows.find(window_handle); window != m_windows.end()) {
window->value.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) });
}
Expand All @@ -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;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Expand Down Expand Up @@ -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));
}
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -560,6 +566,9 @@ ErrorOr<bool, Web::WebDriver::Error> 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<u64> 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()) {
Expand All @@ -569,6 +578,8 @@ ErrorOr<bool, Web::WebDriver::Error> Session::wait_for_current_window_to_have_we
page_load_timeout = value->get_integer<u64>().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<Core::Timer> timer;
Expand All @@ -588,8 +599,12 @@ ErrorOr<bool, Web::WebDriver::Error> Session::wait_for_current_window_to_have_we
if (timer)
timer->stop();

if (timed_out)
if (timed_out) {
if (replacement_was_inferred)
return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::Timeout,
MUST(String::formatted("WebContent process was not replaced within {} ms", *page_load_timeout)));
return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::Timeout, "Timed out waiting for replacement WebContent process"sv);
}

TRY(ensure_current_window_handle_is_valid());
return true;
Expand Down
23 changes: 20 additions & 3 deletions Services/WebDriver/Session.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,17 @@ class Session : public RefCounted<Session> {
static void close_all();

struct Window {
enum class AwaitingReplacement {
No,
Announced,
InferredFromClosedConnection,
};

String handle;
RefPtr<WebContentConnection> 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
Expand Down Expand Up @@ -88,7 +96,14 @@ class Session : public RefCounted<Session> {
Optional<Web::WebDriver::Response> 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));
Expand All @@ -101,7 +116,7 @@ class Session : public RefCounted<Session> {
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);
return !current_window.has_value() || (current_window->is_awaiting_replacement() && !current_window->web_content_connection);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
});

if (response.has_value())
Expand Down Expand Up @@ -137,6 +152,8 @@ class Session : public RefCounted<Session> {
HashMap<String, Window> m_windows;
String m_current_window_handle;

WebContentConnection const* m_connection_awaiting_possible_replacement { nullptr };

HashMap<u64, NonnullRefPtr<WebContentConnection>> m_pending_connections;
u64 m_next_pending_connection_id { 0 };

Expand Down