From 8396afeb76b5229c385525d307a8d91efa45d6ff Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 11 Oct 2024 14:52:44 -0400 Subject: [PATCH] LibWeb: Update WebDriver timeout parsing/serializing to the latest spec Namely, all fields in the timeouts object may now be null. There are a few calling AOs that we will want to bring up to date as well. --- .../WebDriver/TimeoutsConfiguration.cpp | 129 ++++++++---------- .../LibWeb/WebDriver/TimeoutsConfiguration.h | 4 +- .../WebContent/WebDriverConnection.cpp | 11 +- 3 files changed, 62 insertions(+), 82 deletions(-) diff --git a/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp b/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp index be7388add9f..c9649c5c5a6 100644 --- a/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp +++ b/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp @@ -5,6 +5,7 @@ */ #include +#include #include namespace Web::WebDriver { @@ -12,95 +13,73 @@ namespace Web::WebDriver { // https://w3c.github.io/webdriver/#dfn-timeouts-object JsonObject timeouts_object(TimeoutsConfiguration const& timeouts) { - // The timeouts object for a timeouts configuration timeouts is an object initialized with the following properties: - auto timeouts_object = JsonObject {}; + // 1. Let serialized be an empty map. + JsonObject serialized; - // "script" - // timeouts' script timeout value, if set, or its default value. - if (timeouts.script_timeout.has_value()) - timeouts_object.set("script", *timeouts.script_timeout); - else - timeouts_object.set("script", JsonValue {}); + // 2. Set serialized["script"] to timeouts' script timeout. + serialized.set("script"sv, timeouts.script_timeout.has_value() ? *timeouts.script_timeout : JsonValue {}); - // "pageLoad" - // timeouts' page load timeout’s value, if set, or its default value. - timeouts_object.set("pageLoad", timeouts.page_load_timeout); + // 3. Set serialized["pageLoad"] to timeouts' page load timeout. + serialized.set("pageLoad"sv, timeouts.page_load_timeout.has_value() ? *timeouts.page_load_timeout : JsonValue {}); - // "implicit" - // timeouts' implicit wait timeout’s value, if set, or its default value. - timeouts_object.set("implicit", timeouts.implicit_wait_timeout); + // 4. Set serialized["implicit"] to timeouts' implicit wait timeout. + serialized.set("implicit"sv, timeouts.implicit_wait_timeout.has_value() ? *timeouts.implicit_wait_timeout : JsonValue {}); - return timeouts_object; + // 5. Return convert an Infra value to a JSON-compatible JavaScript value with serialized. + return serialized; } -// FIXME: Update this to match the newest spec: https://www.w3.org/TR/webdriver2/#dfn-deserialize-as-timeouts-configuration -// https://w3c.github.io/webdriver/#ref-for-dfn-json-deserialize-3 -ErrorOr json_deserialize_as_a_timeouts_configuration(JsonValue const& value) +// https://w3c.github.io/webdriver/#dfn-deserialize-as-timeouts-configuration +ErrorOr json_deserialize_as_a_timeouts_configuration(JsonValue const& timeouts) { - constexpr i64 max_safe_integer = 9007199254740991; - - // 1. Let timeouts be a new timeouts configuration. - auto timeouts = TimeoutsConfiguration {}; - - // 2. If value is not a JSON Object, return error with error code invalid argument. - if (!value.is_object()) + // 1. Set timeouts to the result of converting a JSON-derived JavaScript value to an Infra value with timeouts. + if (!timeouts.is_object()) return Error::from_code(ErrorCode::InvalidArgument, "Payload is not a JSON object"); - // 3. If value has a property with the key "script": - if (value.as_object().has("script"sv)) { - // 1. Let script duration be the value of property "script". - auto script_duration = value.as_object().get("script"sv); + // 2. Let configuration be a new timeouts configuration. + TimeoutsConfiguration configuration {}; - // 2. If script duration is a number and less than 0 or greater than maximum safe integer, or it is not null, return error with error code invalid argument. - Optional script_timeout; - if (script_duration.has_value()) { - bool is_valid; - if (auto duration = script_duration->get_double_with_precision_loss(); duration.has_value()) { - is_valid = *duration >= 0 && *duration <= max_safe_integer; - // FIXME: script_timeout should be double. - script_timeout = static_cast(*duration); - } else if (script_duration->is_null()) { - is_valid = true; - } else { - is_valid = false; - } - if (!is_valid) - return Error::from_code(ErrorCode::InvalidArgument, "Invalid script duration"); + // 3. For each key → value in timeouts: + TRY(timeouts.as_object().try_for_each_member([&](auto const& key, JsonValue const& value) -> ErrorOr { + Optional parsed_value; + + // 1. If «"script", "pageLoad", "implicit"» does not contain key, then continue. + if (!key.is_one_of("script"sv, "pageLoad"sv, "implicit"sv)) + return {}; + + // 2. If value is neither null nor a number greater than or equal to 0 and less than or equal to the maximum + // safe integer return error with error code invalid argument. + if (!value.is_null()) { + auto duration = value.get_integer(); + + if (!duration.has_value() || *duration > JS::MAX_ARRAY_LIKE_INDEX) + return Error::from_code(ErrorCode::InvalidArgument, "Invalid timeout value"); + + parsed_value = static_cast(*duration); } - // 3. Set timeouts’s script timeout to script duration. - timeouts.script_timeout = script_timeout; - } + // 3. Run the substeps matching key: + // -> "script" + if (key == "script"sv) { + // Set configuration's script timeout to value. + configuration.script_timeout = parsed_value; + } + // -> "pageLoad" + else if (key == "pageLoad"sv) { + // Set configuration's page load timeout to value. + configuration.page_load_timeout = parsed_value; + } + // -> "implicit" + else if (key == "implicit"sv) { + // Set configuration's implicit wait timeout to value. + configuration.implicit_wait_timeout = parsed_value; + } - // 4. If value has a property with the key "pageLoad": - if (value.as_object().has("pageLoad"sv)) { - // 1. Let page load duration be the value of property "pageLoad". - // NOTE: We parse this as a double due to WPT sending values such as `{"pageLoad": 300.00000000000006}` - auto page_load_duration = value.as_object().get_double_with_precision_loss("pageLoad"sv); + return {}; + })); - // 2. If page load duration is less than 0 or greater than maximum safe integer, return error with error code invalid argument. - if (!page_load_duration.has_value() || *page_load_duration < 0 || *page_load_duration > max_safe_integer) - return Error::from_code(ErrorCode::InvalidArgument, "Invalid page load duration"); - - // 3. Set timeouts’s page load timeout to page load duration. - timeouts.page_load_timeout = static_cast(*page_load_duration); - } - - // 5. If value has a property with the key "implicit": - if (value.as_object().has("implicit"sv)) { - // 1. Let implicit duration be the value of property "implicit". - auto implicit_duration = value.as_object().get_i64("implicit"sv); - - // 2. If implicit duration is less than 0 or greater than maximum safe integer, return error with error code invalid argument. - if (!implicit_duration.has_value() || *implicit_duration < 0 || *implicit_duration > max_safe_integer) - return Error::from_code(ErrorCode::InvalidArgument, "Invalid implicit duration"); - - // 3. Set timeouts’s implicit wait timeout to implicit duration. - timeouts.implicit_wait_timeout = static_cast(*implicit_duration); - } - - // 6. Return success with data timeouts. - return timeouts; + // 4. Return success with data configuration. + return configuration; } } diff --git a/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.h b/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.h index 0140ea0d44d..244eeb775f2 100644 --- a/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.h +++ b/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.h @@ -15,8 +15,8 @@ namespace Web::WebDriver { // https://w3c.github.io/webdriver/#dfn-timeouts-configuration struct TimeoutsConfiguration { Optional script_timeout { 30'000 }; - u64 page_load_timeout { 300'000 }; - u64 implicit_wait_timeout { 0 }; + Optional page_load_timeout { 300'000 }; + Optional implicit_wait_timeout { 0 }; }; JsonObject timeouts_object(TimeoutsConfiguration const&); diff --git a/Userland/Services/WebContent/WebDriverConnection.cpp b/Userland/Services/WebContent/WebDriverConnection.cpp index 4b81678acf7..fd045316ced 100644 --- a/Userland/Services/WebContent/WebDriverConnection.cpp +++ b/Userland/Services/WebContent/WebDriverConnection.cpp @@ -2356,6 +2356,7 @@ ErrorOr WebDriverConnection::handle_any_user_prompt } // https://w3c.github.io/webdriver/#dfn-waiting-for-the-navigation-to-complete +// FIXME: Update this AO to the latest spec steps. ErrorOr WebDriverConnection::wait_for_navigation_to_complete() { // 1. If the current session has a page loading strategy of none, return success with data null. @@ -2372,11 +2373,10 @@ ErrorOr WebDriverConnection::wait_for_navigation_to // 3. Start a timer. If this algorithm has not completed before timer reaches the session’s session page load timeout in milliseconds, return an error with error code timeout. auto page_load_timeout_fired = false; - auto timer = Core::Timer::create_single_shot(m_timeouts_configuration.page_load_timeout, [&] { + auto timer = Core::Timer::create_single_shot(m_timeouts_configuration.page_load_timeout.value_or(300'000), [&] { page_load_timeout_fired = true; }); timer->start(); - // 4. If there is an ongoing attempt to navigate the current browsing context that has not yet matured, wait for navigation to mature. Web::Platform::EventLoopPlugin::the().spin_until([&] { return page_load_timeout_fired || navigable->ongoing_navigation().has(); @@ -2417,7 +2417,7 @@ void WebDriverConnection::restore_the_window() // Do not return from this operation until the visibility state of the top-level browsing context’s active document has reached the visible state, or until the operation times out. // FIXME: It isn't clear which timeout should be used here. auto page_load_timeout_fired = false; - auto timer = Core::Timer::create_single_shot(m_timeouts_configuration.page_load_timeout, [&] { + auto timer = Core::Timer::create_single_shot(m_timeouts_configuration.page_load_timeout.value_or(300'000), [&] { page_load_timeout_fired = true; }); timer->start(); @@ -2447,7 +2447,7 @@ Gfx::IntRect WebDriverConnection::iconify_the_window() // Do not return from this operation until the visibility state of the top-level browsing context’s active document has reached the hidden state, or until the operation times out. // FIXME: It isn't clear which timeout should be used here. auto page_load_timeout_fired = false; - auto timer = Core::Timer::create_single_shot(m_timeouts_configuration.page_load_timeout, [&] { + auto timer = Core::Timer::create_single_shot(m_timeouts_configuration.page_load_timeout.value_or(300'000), [&] { page_load_timeout_fired = true; }); timer->start(); @@ -2461,10 +2461,11 @@ Gfx::IntRect WebDriverConnection::iconify_the_window() } // https://w3c.github.io/webdriver/#dfn-find +// FIXME: Update this AO to the latest spec steps. ErrorOr WebDriverConnection::find(StartNodeGetter&& start_node_getter, Web::WebDriver::LocationStrategy using_, StringView value) { // 1. Let end time be the current time plus the session implicit wait timeout. - auto end_time = MonotonicTime::now() + AK::Duration::from_milliseconds(static_cast(m_timeouts_configuration.implicit_wait_timeout)); + auto end_time = MonotonicTime::now() + AK::Duration::from_milliseconds(static_cast(m_timeouts_configuration.implicit_wait_timeout.value_or(0))); // 2. Let location strategy be equal to using. auto location_strategy = using_;