1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 09:17:35 +00:00

WebDriver: Inline Sesssion::stop() in session destructor

Previously it was possible to have following sequence of calls
while destroying a session:
1. `WebContentConnection::die()` calls `Client::close_session()`
2. `Client::close_session()` removes a session from active sessions
map which causes session destructor call.
3. Session destructor calls `Client::close_session()` to remove a
session from active sessions.

With `stop()` method inlined into destructor `close_session()` need
to be called just once while destroying a session.
This commit is contained in:
Aliaksandr Kalenik 2023-03-19 13:36:32 +03:00 committed by Jelle Raaijmakers
parent b79cd5cf6e
commit 63c16ff41a
3 changed files with 23 additions and 34 deletions

View file

@ -185,7 +185,7 @@ Web::WebDriver::Response Client::delete_session(Web::WebDriver::Parameters param
// 1. If the current session is an active session, try to close the session. // 1. If the current session is an active session, try to close the session.
if (auto session = find_session_with_id(parameters[0]); !session.is_error()) if (auto session = find_session_with_id(parameters[0]); !session.is_error())
TRY(session.value()->stop()); close_session(session.value()->session_id());
// 2. Return success with data null. // 2. Return success with data null.
return JsonValue {}; return JsonValue {};

View file

@ -26,10 +26,29 @@ Session::Session(unsigned session_id, NonnullRefPtr<Client> client, Web::WebDriv
{ {
} }
// https://w3c.github.io/webdriver/#dfn-close-the-session
Session::~Session() Session::~Session()
{ {
if (auto error = stop(); error.is_error()) if (!m_started)
warnln("Failed to stop session {}: {}", m_id, error.error()); return;
// 1. Perform the following substeps based on the remote ends type:
// NOTE: We perform the "Remote end is an endpoint node" steps in the WebContent process.
web_content_connection().close_session();
// 2. Remove the current session from active sessions.
// NOTE: We are in a session destruction which means it is already removed
// from active sessions
// 3. Perform any implementation-specific cleanup steps.
if (m_browser_pid.has_value()) {
MUST(Core::System::kill(*m_browser_pid, SIGTERM));
m_browser_pid = {};
}
if (m_web_content_socket_path.has_value()) {
MUST(Core::System::unlink(*m_web_content_socket_path));
m_web_content_socket_path = {};
}
} }
ErrorOr<NonnullRefPtr<Core::LocalServer>> Session::create_server(NonnullRefPtr<ServerPromise> promise) ErrorOr<NonnullRefPtr<Core::LocalServer>> Session::create_server(NonnullRefPtr<ServerPromise> promise)
@ -86,35 +105,6 @@ ErrorOr<void> Session::start(LaunchBrowserCallbacks const& callbacks)
return {}; return {};
} }
// https://w3c.github.io/webdriver/#dfn-close-the-session
Web::WebDriver::Response Session::stop()
{
if (!m_started)
return JsonValue {};
// 1. Perform the following substeps based on the remote ends type:
// NOTE: We perform the "Remote end is an endpoint node" steps in the WebContent process.
web_content_connection().close_session();
// 2. Remove the current session from active sessions.
m_client->close_session(session_id());
// 3. Perform any implementation-specific cleanup steps.
if (m_browser_pid.has_value()) {
MUST(Core::System::kill(*m_browser_pid, SIGTERM));
m_browser_pid = {};
}
if (m_web_content_socket_path.has_value()) {
MUST(Core::System::unlink(*m_web_content_socket_path));
m_web_content_socket_path = {};
}
m_started = false;
// 4. If an error has occurred in any of the steps above, return the error, otherwise return success with data null.
return JsonValue {};
}
// 11.2 Close Window, https://w3c.github.io/webdriver/#dfn-close-window // 11.2 Close Window, https://w3c.github.io/webdriver/#dfn-close-window
Web::WebDriver::Response Session::close_window() Web::WebDriver::Response Session::close_window()
{ {
@ -127,7 +117,7 @@ Web::WebDriver::Response Session::close_window()
// 4. If there are no more open top-level browsing contexts, then close the session. // 4. If there are no more open top-level browsing contexts, then close the session.
if (m_windows.size() == 1) if (m_windows.size() == 1)
TRY(stop()); m_client->close_session(session_id());
} }
// 5. Return the result of running the remote end steps for the Get Window Handles command. // 5. Return the result of running the remote end steps for the Get Window Handles command.

View file

@ -49,7 +49,6 @@ public:
} }
ErrorOr<void> start(LaunchBrowserCallbacks const&); ErrorOr<void> start(LaunchBrowserCallbacks const&);
Web::WebDriver::Response stop();
Web::WebDriver::Response close_window(); Web::WebDriver::Response close_window();
Web::WebDriver::Response switch_to_window(StringView); Web::WebDriver::Response switch_to_window(StringView);
Web::WebDriver::Response get_window_handles() const; Web::WebDriver::Response get_window_handles() const;