Explorar el Código

LibWeb: Update (not replace) timeout values in WebDriver's Set Timeouts

Contradictory to the spec, the Set Timeouts endpoint should update the
existing timeouts configuration in-place, rather than replacing it. WPT
expects this, and other browsers already implement this endpoint this
way.
Timothy Flynn hace 9 meses
padre
commit
dae6200c1d

+ 13 - 5
Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp

@@ -31,14 +31,23 @@ JsonObject timeouts_object(TimeoutsConfiguration const& timeouts)
 
 
 // https://w3c.github.io/webdriver/#dfn-deserialize-as-timeouts-configuration
 // https://w3c.github.io/webdriver/#dfn-deserialize-as-timeouts-configuration
 ErrorOr<TimeoutsConfiguration, Error> json_deserialize_as_a_timeouts_configuration(JsonValue const& timeouts)
 ErrorOr<TimeoutsConfiguration, Error> json_deserialize_as_a_timeouts_configuration(JsonValue const& timeouts)
+{
+    // 2. Let configuration be a new timeouts configuration.
+    TimeoutsConfiguration configuration {};
+
+    TRY(json_deserialize_as_a_timeouts_configuration_into(timeouts, configuration));
+
+    // 4. Return success with data configuration.
+    return configuration;
+}
+
+// https://w3c.github.io/webdriver/#dfn-deserialize-as-timeouts-configuration
+ErrorOr<void, Error> json_deserialize_as_a_timeouts_configuration_into(JsonValue const& timeouts, TimeoutsConfiguration& configuration)
 {
 {
     // 1. Set timeouts to the result of converting a JSON-derived JavaScript value to an Infra value with timeouts.
     // 1. Set timeouts to the result of converting a JSON-derived JavaScript value to an Infra value with timeouts.
     if (!timeouts.is_object())
     if (!timeouts.is_object())
         return Error::from_code(ErrorCode::InvalidArgument, "Payload is not a JSON object");
         return Error::from_code(ErrorCode::InvalidArgument, "Payload is not a JSON object");
 
 
-    // 2. Let configuration be a new timeouts configuration.
-    TimeoutsConfiguration configuration {};
-
     // 3. For each key → value in timeouts:
     // 3. For each key → value in timeouts:
     TRY(timeouts.as_object().try_for_each_member([&](auto const& key, JsonValue const& value) -> ErrorOr<void, Error> {
     TRY(timeouts.as_object().try_for_each_member([&](auto const& key, JsonValue const& value) -> ErrorOr<void, Error> {
         Optional<u64> parsed_value;
         Optional<u64> parsed_value;
@@ -78,8 +87,7 @@ ErrorOr<TimeoutsConfiguration, Error> json_deserialize_as_a_timeouts_configurati
         return {};
         return {};
     }));
     }));
 
 
-    // 4. Return success with data configuration.
-    return configuration;
+    return {};
 }
 }
 
 
 }
 }

+ 1 - 0
Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.h

@@ -21,5 +21,6 @@ struct TimeoutsConfiguration {
 
 
 JsonObject timeouts_object(TimeoutsConfiguration const&);
 JsonObject timeouts_object(TimeoutsConfiguration const&);
 ErrorOr<TimeoutsConfiguration, Error> json_deserialize_as_a_timeouts_configuration(JsonValue const&);
 ErrorOr<TimeoutsConfiguration, Error> json_deserialize_as_a_timeouts_configuration(JsonValue const&);
+ErrorOr<void, Error> json_deserialize_as_a_timeouts_configuration_into(JsonValue const&, TimeoutsConfiguration&);
 
 
 }
 }

+ 6 - 2
Userland/Services/WebContent/WebDriverConnection.cpp

@@ -243,11 +243,15 @@ Messages::WebDriverClient::GetTimeoutsResponse WebDriverConnection::get_timeouts
 // 9.2 Set Timeouts, https://w3c.github.io/webdriver/#dfn-set-timeouts
 // 9.2 Set Timeouts, https://w3c.github.io/webdriver/#dfn-set-timeouts
 Messages::WebDriverClient::SetTimeoutsResponse WebDriverConnection::set_timeouts(JsonValue const& payload)
 Messages::WebDriverClient::SetTimeoutsResponse WebDriverConnection::set_timeouts(JsonValue const& payload)
 {
 {
+    // FIXME: Spec issue: As written, the spec replaces the timeouts configuration with the newly provided values. But
+    //        all other implementations update the existing configuration with any new values instead. WPT relies on
+    //        this behavior, and sends us one timeout value at time.
+    //        https://github.com/w3c/webdriver/issues/1596
+
     // 1. Let timeouts be the result of trying to JSON deserialize as a timeouts configuration the request’s parameters.
     // 1. Let timeouts be the result of trying to JSON deserialize as a timeouts configuration the request’s parameters.
-    auto timeouts = TRY(Web::WebDriver::json_deserialize_as_a_timeouts_configuration(payload));
+    TRY(Web::WebDriver::json_deserialize_as_a_timeouts_configuration_into(payload, m_timeouts_configuration));
 
 
     // 2. Make the session timeouts the new timeouts.
     // 2. Make the session timeouts the new timeouts.
-    m_timeouts_configuration = move(timeouts);
 
 
     // 3. Return success with data null.
     // 3. Return success with data null.
     return JsonValue {};
     return JsonValue {};