diff --git a/components/brave_ads/core/browser/service/network_client.cc b/components/brave_ads/core/browser/service/network_client.cc index feffb6b2633..2ff580ac713 100644 --- a/components/brave_ads/core/browser/service/network_client.cc +++ b/components/brave_ads/core/browser/service/network_client.cc @@ -12,7 +12,6 @@ #include "base/strings/string_util.h" #include "brave/components/brave_ads/core/browser/service/network_client_util.h" #include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h" -#include "net/base/net_errors.h" #include "services/network/public/cpp/resource_request.h" #include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/simple_url_loader.h" @@ -45,18 +44,21 @@ NetworkClient::~NetworkClient() = default; void NetworkClient::SendRequest(mojom::UrlRequestInfoPtr mojom_url_request, SendRequestCallback callback) { + CHECK(mojom_url_request); + return HttpRequest(std::move(mojom_url_request), std::move(callback)); } void NetworkClient::CancelRequests() { weak_ptr_factory_.InvalidateWeakPtrs(); - url_loaders_.clear(); } /////////////////////////////////////////////////////////////////////////////// void NetworkClient::HttpRequest(mojom::UrlRequestInfoPtr mojom_url_request, SendRequestCallback callback) { + CHECK(mojom_url_request); + auto resource_request = std::make_unique(); resource_request->url = mojom_url_request->url; resource_request->method = ToString(mojom_url_request->method); @@ -68,49 +70,43 @@ void NetworkClient::HttpRequest(mojom::UrlRequestInfoPtr mojom_url_request, auto url_loader = network::SimpleURLLoader::Create( std::move(resource_request), GetNetworkTrafficAnnotationTag()); + auto* raw_url_loader = url_loader.get(); - url_loader->SetAllowHttpErrorResults(true); + raw_url_loader->SetAllowHttpErrorResults(true); if (!mojom_url_request->content.empty()) { - url_loader->AttachStringForUpload(mojom_url_request->content, - mojom_url_request->content_type); + raw_url_loader->AttachStringForUpload(mojom_url_request->content, + mojom_url_request->content_type); } - auto* url_loader_copy = url_loader.get(); - url_loaders_.insert(std::move(url_loader)); - - url_loader_copy->DownloadToStringOfUnboundedSizeUntilCrashAndDie( + raw_url_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie( url_loader_factory_.get(), base::BindOnce(&NetworkClient::HttpRequestCallback, - weak_ptr_factory_.GetWeakPtr(), url_loader_copy, + weak_ptr_factory_.GetWeakPtr(), std::move(url_loader), std::move(callback))); } void NetworkClient::HttpRequestCallback( - network::SimpleURLLoader* url_loader, + std::unique_ptr url_loader, SendRequestCallback callback, std::optional response_body) { CHECK(url_loader); - auto iter = url_loaders_.find(url_loader); - CHECK(iter != url_loaders_.cend()); - auto owned_url_loader = std::move(*iter); - url_loaders_.erase(iter); + const GURL& url = url_loader->GetFinalURL(); - const GURL& url = owned_url_loader->GetFinalURL(); - - const auto* response = owned_url_loader->ResponseInfo(); + const auto* response = url_loader->ResponseInfo(); if (!response || !response->headers) { - return ReportError(url, net::ERR_FAILED, std::move(callback)); + // DNS failure, connection error, timeout etc. + return ReportError(url, url_loader->NetError(), std::move(callback)); } - const auto response_headers = response->headers; - auto mojom_url_response = mojom::UrlResponseInfo::New(); mojom_url_response->url = url; - mojom_url_response->code = response_headers->response_code(); + mojom_url_response->code = response->headers->response_code(); mojom_url_response->body = response_body.value_or(""); - mojom_url_response->headers = ExtractHttpResponseHeaders(response_headers); + mojom_url_response->headers = ExtractHttpResponseHeaders(response->headers); + + // Forward the response to the original caller for handling. std::move(callback).Run(std::move(mojom_url_response)); } diff --git a/components/brave_ads/core/browser/service/network_client.h b/components/brave_ads/core/browser/service/network_client.h index 418cb3f51d9..9ea44587342 100644 --- a/components/brave_ads/core/browser/service/network_client.h +++ b/components/brave_ads/core/browser/service/network_client.h @@ -10,8 +10,6 @@ #include #include -#include "base/containers/flat_set.h" -#include "base/containers/unique_ptr_adapters.h" #include "base/memory/scoped_refptr.h" #include "base/memory/weak_ptr.h" #include "brave/components/brave_ads/core/browser/service/network_client_callback.h" @@ -25,7 +23,8 @@ class SharedURLLoaderFactory; namespace brave_ads { -// This class is responsible for sending HTTP network requests. +// Sends network requests, supporting standard HTTP. Standard HTTP requests are +// issued via `SimpleURLLoader`. class NetworkClient { public: NetworkClient( @@ -37,28 +36,26 @@ class NetworkClient { ~NetworkClient(); - // Starts a network request for the given `mojom::UrlRequestInfo`. The - // provided `callback` will be invoked with a `mojom::UrlResponseInfo` unless - // the request is canceled or the `NetworkClient` is destroyed. + // Issues a network request described by `mojom::UrlRequestInfo`. When the + // request completes, `callback` is invoked with a `mojom::UrlResponseInfo`. + // The callback will not run if the request is canceled or if this instance + // is destroyed. void SendRequest(mojom::UrlRequestInfoPtr mojom_url_request, SendRequestCallback callback); - // Cancels all ongoing network requests. Pending callbacks will not be - // invoked. + // Cancels all active requests. Any pending callbacks will be dropped. void CancelRequests(); private: + // Sends the request using standard HTTP. void HttpRequest(mojom::UrlRequestInfoPtr mojom_url_request, SendRequestCallback callback); - void HttpRequestCallback(network::SimpleURLLoader* url_loader, + void HttpRequestCallback(std::unique_ptr url_loader, SendRequestCallback callback, std::optional response_body); const scoped_refptr url_loader_factory_; // Not owned. - base::flat_set, - base::UniquePtrComparator> - url_loaders_; const network::NetworkContextGetter network_context_getter_; diff --git a/components/brave_ads/core/browser/service/network_client_util.h b/components/brave_ads/core/browser/service/network_client_util.h index 29a02b68ff2..24a61c13f07 100644 --- a/components/brave_ads/core/browser/service/network_client_util.h +++ b/components/brave_ads/core/browser/service/network_client_util.h @@ -22,11 +22,12 @@ namespace brave_ads { // Converts `mojom::UrlRequestMethodType` to its string representation. std::string ToString(mojom::UrlRequestMethodType value); -// Returns the network traffic annotation tag for network requests. +// Returns the network traffic annotation tag used to identify and audit network +// requests. net::NetworkTrafficAnnotationTag GetNetworkTrafficAnnotationTag(); -// Extracts all HTTP response headers from a `net::HttpResponseHeaders` object -// and returns them as a flat map with lowercased keys. +// Extracts all HTTP response headers from `net::HttpResponseHeaders` and +// returns them as a flat map with lowercased keys. base::flat_map ExtractHttpResponseHeaders( const scoped_refptr& http_response_headers);