From 31469ee45a449350460dc250d6962628638ce63e Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 10 Nov 2022 21:00:27 -0500 Subject: [PATCH] Browser+WebContent+WebDriver: Move Execute Async Script to WebContent With this, WebDriverEndpoints is unused and removed :^) --- .../Applications/Browser/BrowserWindow.cpp | 4 -- Userland/Applications/Browser/Tab.h | 4 -- .../Browser/WebDriverConnection.cpp | 15 ---- .../Browser/WebDriverConnection.h | 1 - .../Applications/Browser/WebDriverEndpoints.h | 29 -------- .../Browser/WebDriverSessionClient.ipc | 1 - .../LibWebView/OutOfProcessWebView.cpp | 5 -- .../LibWebView/OutOfProcessWebView.h | 2 - .../WebContent/ConnectionFromClient.cpp | 21 ------ .../WebContent/ConnectionFromClient.h | 2 - .../Services/WebContent/WebContentServer.ipc | 2 - .../Services/WebContent/WebDriverClient.ipc | 1 + .../WebContent/WebDriverConnection.cpp | 32 +++++++++ .../Services/WebContent/WebDriverConnection.h | 1 + Userland/Services/WebDriver/Client.cpp | 3 +- Userland/Services/WebDriver/Session.cpp | 72 ------------------- Userland/Services/WebDriver/Session.h | 1 - 17 files changed, 35 insertions(+), 161 deletions(-) delete mode 100644 Userland/Applications/Browser/WebDriverEndpoints.h diff --git a/Userland/Applications/Browser/BrowserWindow.cpp b/Userland/Applications/Browser/BrowserWindow.cpp index 05b1ba5ee4..dc128bcdec 100644 --- a/Userland/Applications/Browser/BrowserWindow.cpp +++ b/Userland/Applications/Browser/BrowserWindow.cpp @@ -607,10 +607,6 @@ void BrowserWindow::create_new_tab(URL url, bool activate) return active_tab().view().take_screenshot(); }; - new_tab.webdriver_endpoints().on_execute_script = [this](String const& body, Vector const& json_arguments, Optional const& timeout, bool async) { - return active_tab().view().webdriver_execute_script(body, json_arguments, timeout, async); - }; - new_tab.load(url); dbgln_if(SPAM_DEBUG, "Added new tab {:p}, loading {}", &new_tab, url); diff --git a/Userland/Applications/Browser/Tab.h b/Userland/Applications/Browser/Tab.h index b2886d0068..c016783209 100644 --- a/Userland/Applications/Browser/Tab.h +++ b/Userland/Applications/Browser/Tab.h @@ -8,7 +8,6 @@ #pragma once #include "History.h" -#include "WebDriverEndpoints.h" #include #include #include @@ -74,7 +73,6 @@ public: Function()> on_get_session_storage_entries; Function on_take_screenshot; - WebDriverEndpoints& webdriver_endpoints() { return m_webdriver_endpoints; } void enable_webdriver_mode(); enum class InspectorTarget { @@ -143,8 +141,6 @@ private: Optional m_navigating_url; - WebDriverEndpoints m_webdriver_endpoints {}; - bool m_loaded { false }; bool m_is_history_navigation { false }; }; diff --git a/Userland/Applications/Browser/WebDriverConnection.cpp b/Userland/Applications/Browser/WebDriverConnection.cpp index 5e0faaf9d1..2785cafe6a 100644 --- a/Userland/Applications/Browser/WebDriverConnection.cpp +++ b/Userland/Applications/Browser/WebDriverConnection.cpp @@ -58,21 +58,6 @@ void WebDriverConnection::forward() browser_window->active_tab().go_forward(); } -Messages::WebDriverSessionClient::ExecuteScriptResponse WebDriverConnection::execute_script(String const& body, Vector const& json_arguments, Optional const& timeout, bool async) -{ - dbgln("WebDriverConnection: execute_script"); - if (auto browser_window = m_browser_window.strong_ref()) { - auto& tab = browser_window->active_tab(); - if (tab.webdriver_endpoints().on_execute_script) { - auto response = tab.webdriver_endpoints().on_execute_script(body, json_arguments, timeout, async); - // WebContentServer's and WebDriverSessionClient's ExecuteScriptResponse have an identical - // structure but are distinct types, so we have to convert here. - return { response.result_type(), response.json_result() }; - } - } - return { {} }; -} - Messages::WebDriverSessionClient::GetAllCookiesResponse WebDriverConnection::get_all_cookies() { dbgln_if(WEBDRIVER_DEBUG, "WebDriverConnection: get_cookies"); diff --git a/Userland/Applications/Browser/WebDriverConnection.h b/Userland/Applications/Browser/WebDriverConnection.h index 9247eac580..e10bbf5bb7 100644 --- a/Userland/Applications/Browser/WebDriverConnection.h +++ b/Userland/Applications/Browser/WebDriverConnection.h @@ -42,7 +42,6 @@ public: virtual void refresh() override; virtual void back() override; virtual void forward() override; - virtual Messages::WebDriverSessionClient::ExecuteScriptResponse execute_script(String const& body, Vector const& json_arguments, Optional const& timeout, bool async) override; virtual Messages::WebDriverSessionClient::GetAllCookiesResponse get_all_cookies() override; virtual Messages::WebDriverSessionClient::GetNamedCookieResponse get_named_cookie(String const& name) override; virtual void add_cookie(Web::Cookie::ParsedCookie const&) override; diff --git a/Userland/Applications/Browser/WebDriverEndpoints.h b/Userland/Applications/Browser/WebDriverEndpoints.h deleted file mode 100644 index 2f5c03ea26..0000000000 --- a/Userland/Applications/Browser/WebDriverEndpoints.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (c) 2022, Tobias Christiansen - * Copyright (c) 2022, Tim Flynn - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include -#include -#include -#include - -namespace Messages::WebContentServer { -class WebdriverExecuteScriptResponse; -} - -namespace Browser { - -class WebDriverEndpoints { -public: - WebDriverEndpoints() = default; - ~WebDriverEndpoints() = default; - - Function const& json_arguments, Optional const& timeout, bool async)> on_execute_script; -}; - -} diff --git a/Userland/Applications/Browser/WebDriverSessionClient.ipc b/Userland/Applications/Browser/WebDriverSessionClient.ipc index 1b38fa717d..ef0d9c6db0 100644 --- a/Userland/Applications/Browser/WebDriverSessionClient.ipc +++ b/Userland/Applications/Browser/WebDriverSessionClient.ipc @@ -18,7 +18,6 @@ endpoint WebDriverSessionClient { refresh() =| back() =| forward() =| - execute_script(String body, Vector json_arguments, Optional timeout, bool async) => (Web::WebDriver::ExecuteScriptResultType result_type, String json_result) get_all_cookies() => (Vector cookies) get_named_cookie(String name) => (Optional cookie) add_cookie(Web::Cookie::ParsedCookie cookie) =| diff --git a/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp b/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp index 09e6d00c57..337a20270d 100644 --- a/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp +++ b/Userland/Libraries/LibWebView/OutOfProcessWebView.cpp @@ -591,11 +591,6 @@ Gfx::ShareableBitmap OutOfProcessWebView::take_document_screenshot() return client().take_document_screenshot(); } -Messages::WebContentServer::WebdriverExecuteScriptResponse OutOfProcessWebView::webdriver_execute_script(String const& body, Vector const& json_arguments, Optional const& timeout, bool async) -{ - return client().webdriver_execute_script(body, json_arguments, timeout, async); -} - void OutOfProcessWebView::focusin_event(GUI::FocusEvent&) { client().async_set_has_focus(true); diff --git a/Userland/Libraries/LibWebView/OutOfProcessWebView.h b/Userland/Libraries/LibWebView/OutOfProcessWebView.h index 9c12a30482..a8e68db925 100644 --- a/Userland/Libraries/LibWebView/OutOfProcessWebView.h +++ b/Userland/Libraries/LibWebView/OutOfProcessWebView.h @@ -75,8 +75,6 @@ public: Gfx::ShareableBitmap take_screenshot() const; Gfx::ShareableBitmap take_document_screenshot(); - Messages::WebContentServer::WebdriverExecuteScriptResponse webdriver_execute_script(String const& body, Vector const& json_arguments, Optional const& timeout, bool async); - Function on_context_menu_request; Function on_link_click; Function on_link_context_menu_request; diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index 587396e251..5dd166adc0 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -574,24 +573,4 @@ void ConnectionFromClient::set_system_visibility_state(bool visible) : Web::HTML::VisibilityState::Hidden); } -Messages::WebContentServer::WebdriverExecuteScriptResponse ConnectionFromClient::webdriver_execute_script(String const& body, Vector const& json_arguments, Optional const& timeout, bool async) -{ - auto& page = m_page_host->page(); - - auto* window = page.top_level_browsing_context().active_window(); - auto& vm = window->vm(); - - auto arguments = JS::MarkedVector { vm.heap() }; - for (auto const& argument_string : json_arguments) { - // NOTE: These are assumed to be valid JSON values. - auto json_value = MUST(JsonValue::from_string(argument_string)); - arguments.append(JS::JSONObject::parse_json_value(vm, json_value)); - } - - auto result = async - ? Web::WebDriver::execute_async_script(page, body, move(arguments), timeout) - : Web::WebDriver::execute_script(page, body, move(arguments), timeout); - return { result.type, result.value.serialized() }; -} - } diff --git a/Userland/Services/WebContent/ConnectionFromClient.h b/Userland/Services/WebContent/ConnectionFromClient.h index 6be1f74d70..ee61d81f53 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.h +++ b/Userland/Services/WebContent/ConnectionFromClient.h @@ -90,8 +90,6 @@ private: virtual Messages::WebContentServer::GetSelectedTextResponse get_selected_text() override; virtual void select_all() override; - virtual Messages::WebContentServer::WebdriverExecuteScriptResponse webdriver_execute_script(String const& body, Vector const& json_arguments, Optional const& timeout, bool async) override; - void flush_pending_paint_requests(); NonnullOwnPtr m_page_host; diff --git a/Userland/Services/WebContent/WebContentServer.ipc b/Userland/Services/WebContent/WebContentServer.ipc index 38c159f02c..4e311de7cb 100644 --- a/Userland/Services/WebContent/WebContentServer.ipc +++ b/Userland/Services/WebContent/WebContentServer.ipc @@ -43,8 +43,6 @@ endpoint WebContentServer take_document_screenshot() => (Gfx::ShareableBitmap data) - webdriver_execute_script(String body, Vector json_arguments, Optional timeout, bool async) => (Web::WebDriver::ExecuteScriptResultType result_type, String json_result) - run_javascript(String js_source) =| dump_layout_tree() => (String dump) diff --git a/Userland/Services/WebContent/WebDriverClient.ipc b/Userland/Services/WebContent/WebDriverClient.ipc index 3862aa2892..b8f803f2f3 100644 --- a/Userland/Services/WebContent/WebDriverClient.ipc +++ b/Userland/Services/WebContent/WebDriverClient.ipc @@ -23,6 +23,7 @@ endpoint WebDriverClient { is_element_enabled(String element_id) => (Web::WebDriver::Response response) get_source() => (Web::WebDriver::Response response) execute_script(JsonValue payload) => (Web::WebDriver::Response response) + execute_async_script(JsonValue payload) => (Web::WebDriver::Response response) take_screenshot() => (Web::WebDriver::Response response) take_element_screenshot(String element_id) => (Web::WebDriver::Response response) } diff --git a/Userland/Services/WebContent/WebDriverConnection.cpp b/Userland/Services/WebContent/WebDriverConnection.cpp index 192694729a..9b4e67098b 100644 --- a/Userland/Services/WebContent/WebDriverConnection.cpp +++ b/Userland/Services/WebContent/WebDriverConnection.cpp @@ -797,6 +797,38 @@ Messages::WebDriverClient::ExecuteScriptResponse WebDriverConnection::execute_sc VERIFY_NOT_REACHED(); } +// 13.2.2 Execute Async Script, https://w3c.github.io/webdriver/#dfn-execute-async-script +Messages::WebDriverClient::ExecuteAsyncScriptResponse WebDriverConnection::execute_async_script(JsonValue const& payload) +{ + // 1. Let body and arguments by the result of trying to extract the script arguments from a request with argument parameters. + auto const& [body, arguments] = TRY(extract_the_script_arguments_from_a_request(payload)); + + // 2. If the current browsing context is no longer open, return error with error code no such window. + TRY(ensure_open_top_level_browsing_context()); + + // FIXME: 3. Handle any user prompts, and return its value if it is an error. + + // 4., 5.1-5.11. + // FIXME: Move timeouts from WebDriver to WebContent and pass the script timeout through here. + auto result = Web::WebDriver::execute_async_script(m_page_host.page(), body, move(arguments), {}); + dbgln_if(WEBDRIVER_DEBUG, "Executing async script returned: {}", result.value); + + switch (result.type) { + // 6. If promise is still pending and the session script timeout is reached, return error with error code script timeout. + case Web::WebDriver::ExecuteScriptResultType::Timeout: + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::ScriptTimeoutError, "Script timed out"); + // 7. Upon fulfillment of promise with value v, let result be a JSON clone of v, and return success with data result. + case Web::WebDriver::ExecuteScriptResultType::PromiseResolved: + return make_success_response(move(result.value)); + // 8. Upon rejection of promise with reason r, let result be a JSON clone of r, and return error with error code javascript error and data result. + case Web::WebDriver::ExecuteScriptResultType::PromiseRejected: + case Web::WebDriver::ExecuteScriptResultType::JavaScriptError: + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::JavascriptError, "Script returned an error", move(result.value)); + } + + VERIFY_NOT_REACHED(); +} + // 17.1 Take Screenshot, https://w3c.github.io/webdriver/#take-screenshot Messages::WebDriverClient::TakeScreenshotResponse WebDriverConnection::take_screenshot() { diff --git a/Userland/Services/WebContent/WebDriverConnection.h b/Userland/Services/WebContent/WebDriverConnection.h index 52f6455681..56e00a907e 100644 --- a/Userland/Services/WebContent/WebDriverConnection.h +++ b/Userland/Services/WebContent/WebDriverConnection.h @@ -55,6 +55,7 @@ private: virtual Messages::WebDriverClient::IsElementEnabledResponse is_element_enabled(String const& element_id) override; virtual Messages::WebDriverClient::GetSourceResponse get_source() override; virtual Messages::WebDriverClient::ExecuteScriptResponse execute_script(JsonValue const& payload) override; + virtual Messages::WebDriverClient::ExecuteAsyncScriptResponse execute_async_script(JsonValue const& payload) override; virtual Messages::WebDriverClient::TakeScreenshotResponse take_screenshot() override; virtual Messages::WebDriverClient::TakeElementScreenshotResponse take_element_screenshot(String const& element_id) override; diff --git a/Userland/Services/WebDriver/Client.cpp b/Userland/Services/WebDriver/Client.cpp index b18e5213e2..0587209c91 100644 --- a/Userland/Services/WebDriver/Client.cpp +++ b/Userland/Services/WebDriver/Client.cpp @@ -744,8 +744,7 @@ Web::WebDriver::Response Client::handle_execute_async_script(Vector { dbgln_if(WEBDRIVER_DEBUG, "Handling POST /session//execute/async"); auto* session = TRY(find_session_with_id(parameters[0])); - auto result = TRY(session->execute_async_script(payload)); - return make_json_value(result); + return session->web_content_connection().execute_async_script(payload); } // 14.1 Get All Cookies, https://w3c.github.io/webdriver/#dfn-get-all-cookies diff --git a/Userland/Services/WebDriver/Session.cpp b/Userland/Services/WebDriver/Session.cpp index c0510a44a2..4732d4fef3 100644 --- a/Userland/Services/WebDriver/Session.cpp +++ b/Userland/Services/WebDriver/Session.cpp @@ -297,78 +297,6 @@ Web::WebDriver::Response Session::get_window_handles() const return JsonValue { handles }; } -struct ScriptArguments { - String script; - JsonArray const& arguments; -}; - -// https://w3c.github.io/webdriver/#dfn-extract-the-script-arguments-from-a-request -static ErrorOr extract_the_script_arguments_from_a_request(JsonValue const& payload) -{ - if (!payload.is_object()) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Payload is not a JSON object"); - - auto const& properties = payload.as_object(); - - // 1. Let script be the result of getting a property named script from the parameters. - // 2. If script is not a String, return error with error code invalid argument. - if (!properties.has_string("script"sv)) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Payload doesn't have a 'script' string property"); - auto script = properties.get("script"sv).as_string(); - - // 3. Let args be the result of getting a property named args from the parameters. - // 4. If args is not an Array return error with error code invalid argument. - if (!properties.has_array("args"sv)) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Payload doesn't have an 'args' string property"); - auto const& args = properties.get("args"sv).as_array(); - - // 5. Let arguments be the result of calling the JSON deserialize algorithm with arguments args. - // NOTE: We forward the JSON array to the Browser and then WebContent process over IPC, so this is not necessary. - - // 6. Return success with data script and arguments. - return ScriptArguments { script, args }; -} - -// 13.2.2 Execute Async Script, https://w3c.github.io/webdriver/#dfn-execute-async-script -Web::WebDriver::Response Session::execute_async_script(JsonValue const& parameters) -{ - // 1. Let body and arguments by the result of trying to extract the script arguments from a request with argument parameters. - auto [body, arguments] = TRY(extract_the_script_arguments_from_a_request(parameters)); - - // 2. If the current browsing context is no longer open, return error with error code no such window. - TRY(check_for_open_top_level_browsing_context_or_return_error()); - - // FIXME: 3. Handle any user prompts, and return its value if it is an error. - - // 4., 5.1-5.11. - Vector json_arguments; - arguments.for_each([&](JsonValue const& json_value) { - // NOTE: serialized() instead of to_string() ensures proper quoting. - json_arguments.append(json_value.serialized()); - }); - - dbgln("Executing async script with 'args': [{}] / 'body':\n{}", String::join(", "sv, json_arguments), body); - auto execute_script_response = m_browser_connection->execute_script(body, json_arguments, m_timeouts_configuration.script_timeout, true); - dbgln("Executing async script returned: {}", execute_script_response.json_result()); - - // NOTE: This is assumed to be a valid JSON value. - auto result = MUST(JsonValue::from_string(execute_script_response.json_result())); - - switch (execute_script_response.result_type()) { - // 6. If promise is still pending and the session script timeout is reached, return error with error code script timeout. - case Web::WebDriver::ExecuteScriptResultType::Timeout: - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::ScriptTimeoutError, "Script timed out"); - // 7. Upon fulfillment of promise with value v, let result be a JSON clone of v, and return success with data result. - case Web::WebDriver::ExecuteScriptResultType::PromiseResolved: - return result; - // 8. Upon rejection of promise with reason r, let result be a JSON clone of r, and return error with error code javascript error and data result. - case Web::WebDriver::ExecuteScriptResultType::PromiseRejected: - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::JavascriptError, "Script returned an error", move(result)); - default: - VERIFY_NOT_REACHED(); - } -} - // https://w3c.github.io/webdriver/#dfn-serialized-cookie static JsonObject serialize_cookie(Web::Cookie::Cookie const& cookie) { diff --git a/Userland/Services/WebDriver/Session.h b/Userland/Services/WebDriver/Session.h index d21b680e61..da590dc1ed 100644 --- a/Userland/Services/WebDriver/Session.h +++ b/Userland/Services/WebDriver/Session.h @@ -58,7 +58,6 @@ public: Web::WebDriver::Response get_window_handle(); ErrorOr> close_window(); Web::WebDriver::Response get_window_handles() const; - Web::WebDriver::Response execute_async_script(JsonValue const& payload); Web::WebDriver::Response get_all_cookies(); Web::WebDriver::Response get_named_cookie(String const& name); Web::WebDriver::Response add_cookie(JsonValue const& payload);