RequestServer: Avoid unnecessary IPC::Dictionary wrapper

We already have and use HashMap<DeprecatedString, DeprecatedString>
nearly everywhere, which is equivalent.

Userland/Libraries/LibProtocol/RequestClient.cpp

@@ -22,15 +22,14 @@ void RequestClient::ensure_connection(URL const& url, ::RequestServer::CacheLeve
 template<typename RequestHashMapTraits>
 RefPtr<Request> RequestClient::start_request(DeprecatedString const& method, URL const& url, HashMap<DeprecatedString, DeprecatedString, RequestHashMapTraits> const& request_headers, ReadonlyBytes request_body, Core::ProxyData const& proxy_data)
 {
-    IPC::Dictionary header_dictionary;
+    auto headers_or_error = request_headers.template clone<Traits<DeprecatedString>>();
-    for (auto& it : request_headers)
+    if (headers_or_error.is_error())
-        header_dictionary.add(it.key, it.value);
+        return nullptr;
-
     auto body_result = ByteBuffer::copy(request_body);
     if (body_result.is_error())
         return nullptr;
 
-    auto response = IPCProxy::start_request(method, url, header_dictionary, body_result.release_value(), proxy_data);
+    auto response = IPCProxy::start_request(method, url, headers_or_error.release_value(), body_result.release_value(), proxy_data);
     auto request_id = response.request_id();
     if (request_id < 0 || !response.response_fd().has_value())
         return nullptr;
@@ -71,13 +70,20 @@ void RequestClient::request_progress(i32 request_id, Optional<u32> const& total_
     }
 }
 
-void RequestClient::headers_became_available(i32 request_id, IPC::Dictionary const& response_headers, Optional<u32> const& status_code)
+void RequestClient::headers_became_available(i32 request_id, HashMap<DeprecatedString, DeprecatedString, CaseInsensitiveStringTraits> const& response_headers, Optional<u32> const& status_code)
 {
-    if (auto request = const_cast<Request*>(m_requests.get(request_id).value_or(nullptr))) {
+    auto request = const_cast<Request*>(m_requests.get(request_id).value_or(nullptr));
-        HashMap<DeprecatedString, DeprecatedString, CaseInsensitiveStringTraits> headers;
+    if (!request) {
-        response_headers.for_each_entry([&](auto& name, auto& value) { headers.set(name, value); });
+        warnln("Received headers for non-existent request {}", request_id);
-        request->did_receive_headers({}, headers, status_code);
+        return;
     }
+    auto response_headers_clone_or_error = response_headers.clone();
+    if (response_headers_clone_or_error.is_error()) {
+        warnln("Error while receiving headers for request {}: {}", request_id, response_headers_clone_or_error.error());
+        return;
+    }
+
+    request->did_receive_headers({}, response_headers_clone_or_error.release_value(), status_code);
 }
 
 void RequestClient::certificate_requested(i32 request_id)

Userland/Libraries/LibProtocol/RequestClient.h

@@ -35,7 +35,7 @@ private:
     virtual void request_progress(i32, Optional<u32> const&, u32) override;
     virtual void request_finished(i32, bool, u32) override;
     virtual void certificate_requested(i32) override;
-    virtual void headers_became_available(i32, IPC::Dictionary const&, Optional<u32> const&) override;
+    virtual void headers_became_available(i32, HashMap<DeprecatedString, DeprecatedString, CaseInsensitiveStringTraits> const&, Optional<u32> const&) override;
 
     HashMap<i32, RefPtr<Request>> m_requests;
 };

Userland/Services/RequestServer/ConnectionFromClient.cpp

@@ -36,7 +36,7 @@ Messages::RequestServer::IsSupportedProtocolResponse ConnectionFromClient::is_su
     return supported;
 }
 
-Messages::RequestServer::StartRequestResponse ConnectionFromClient::start_request(DeprecatedString const& method, URL const& url, IPC::Dictionary const& request_headers, ByteBuffer const& request_body, Core::ProxyData const& proxy_data)
+Messages::RequestServer::StartRequestResponse ConnectionFromClient::start_request(DeprecatedString const& method, URL const& url, HashMap<DeprecatedString, DeprecatedString> const& request_headers, ByteBuffer const& request_body, Core::ProxyData const& proxy_data)
 {
     if (!url.is_valid()) {
         dbgln("StartRequest: Invalid URL requested: '{}'", url);
@@ -47,7 +47,7 @@ Messages::RequestServer::StartRequestResponse ConnectionFromClient::start_reques
         dbgln("StartRequest: No protocol handler for URL: '{}'", url);
         return { -1, Optional<IPC::File> {} };
     }
-    auto request = protocol->start_request(*this, method, url, request_headers.entries(), request_body, proxy_data);
+    auto request = protocol->start_request(*this, method, url, request_headers, request_body, proxy_data);
     if (!request) {
         dbgln("StartRequest: Protocol handler failed to start request: '{}'", url);
         return { -1, Optional<IPC::File> {} };
@@ -72,10 +72,7 @@ Messages::RequestServer::StopRequestResponse ConnectionFromClient::stop_request(
 
 void ConnectionFromClient::did_receive_headers(Badge<Request>, Request& request)
 {
-    IPC::Dictionary response_headers;
+    auto response_headers = request.response_headers().clone().release_value_but_fixme_should_propagate_errors();
-    for (auto& it : request.response_headers())
-        response_headers.add(it.key, it.value);
-
     async_headers_became_available(request.id(), move(response_headers), request.status_code());
 }
 

Userland/Services/RequestServer/ConnectionFromClient.h

@@ -32,7 +32,7 @@ private:
     explicit ConnectionFromClient(NonnullOwnPtr<Core::LocalSocket>);
 
     virtual Messages::RequestServer::IsSupportedProtocolResponse is_supported_protocol(DeprecatedString const&) override;
-    virtual Messages::RequestServer::StartRequestResponse start_request(DeprecatedString const&, URL const&, IPC::Dictionary const&, ByteBuffer const&, Core::ProxyData const&) override;
+    virtual Messages::RequestServer::StartRequestResponse start_request(DeprecatedString const&, URL const&, HashMap<DeprecatedString, DeprecatedString> const&, ByteBuffer const&, Core::ProxyData const&) override;
     virtual Messages::RequestServer::StopRequestResponse stop_request(i32) override;
     virtual Messages::RequestServer::SetCertificateResponse set_certificate(i32, DeprecatedString const&, DeprecatedString const&) override;
     virtual void ensure_connection(URL const& url, ::RequestServer::CacheLevel const& cache_level) override;

Userland/Services/RequestServer/RequestClient.ipc

@@ -4,7 +4,7 @@ endpoint RequestClient
 {
     request_progress(i32 request_id, Optional<u32> total_size, u32 downloaded_size) =|
     request_finished(i32 request_id, bool success, u32 total_size) =|
-    headers_became_available(i32 request_id, IPC::Dictionary response_headers, Optional<u32> status_code) =|
+    headers_became_available(i32 request_id, HashMap<DeprecatedString,DeprecatedString,CaseInsensitiveStringTraits> response_headers, Optional<u32> status_code) =|
 
     // Certificate requests
     certificate_requested(i32 request_id) =|

Userland/Services/RequestServer/RequestServer.ipc

@@ -6,7 +6,7 @@ endpoint RequestServer
     // Test if a specific protocol is supported, e.g "http"
     is_supported_protocol(DeprecatedString protocol) => (bool supported)
 
-    start_request(DeprecatedString method, URL url, IPC::Dictionary request_headers, ByteBuffer request_body, Core::ProxyData proxy_data) => (i32 request_id, Optional<IPC::File> response_fd)
+    start_request(DeprecatedString method, URL url, HashMap<DeprecatedString,DeprecatedString> request_headers, ByteBuffer request_body, Core::ProxyData proxy_data) => (i32 request_id, Optional<IPC::File> response_fd)
     stop_request(i32 request_id) => (bool success)
     set_certificate(i32 request_id, DeprecatedString certificate, DeprecatedString key) => (bool success)