From 761a4deab39ebdbb21079d0b0ea117a7c2c50f2a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 7 Aug 2019 09:21:53 +0900 Subject: [PATCH] feat: associate InProgressRequest with requests (#19648) --- .../net/proxying_url_loader_factory.cc | 66 +++++++++++++++++-- .../browser/net/proxying_url_loader_factory.h | 13 ++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/shell/browser/net/proxying_url_loader_factory.cc b/shell/browser/net/proxying_url_loader_factory.cc index ceda0ee51563..7dee03edcf67 100644 --- a/shell/browser/net/proxying_url_loader_factory.cc +++ b/shell/browser/net/proxying_url_loader_factory.cc @@ -14,6 +14,12 @@ namespace electron { +namespace { + +int64_t g_request_id = 0; + +} // namespace + ProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams:: FollowRedirectParams() = default; ProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams:: @@ -21,6 +27,7 @@ ProxyingURLLoaderFactory::InProgressRequest::FollowRedirectParams:: ProxyingURLLoaderFactory::InProgressRequest::InProgressRequest( ProxyingURLLoaderFactory* factory, + int64_t web_request_id, int32_t routing_id, int32_t network_service_request_id, uint32_t options, @@ -31,6 +38,7 @@ ProxyingURLLoaderFactory::InProgressRequest::InProgressRequest( : factory_(factory), request_(request), original_initiator_(request.request_initiator), + request_id_(web_request_id), routing_id_(routing_id), network_service_request_id_(network_service_request_id), options_(options), @@ -236,8 +244,8 @@ void ProxyingURLLoaderFactory::InProgressRequest::OnComplete( target_client_->OnComplete(status); // TODO(zcbenz): Call webRequest.onCompleted. - // TODO(zcbenz): Disassociate from factory. - delete this; + // Deletes |this|. + factory_->RemoveRequest(network_service_request_id_, request_id_); } void ProxyingURLLoaderFactory::InProgressRequest::OnLoaderCreated( @@ -663,7 +671,25 @@ void ProxyingURLLoaderFactory::CreateLoaderAndStart( request_id, options, request, std::move(client), traffic_annotation); - // TODO(zcbenz): Create InProgressRequest. + // TODO(zcbenz): Remove the |CreateLoaderAndStart| call and create + // InProgressRequest when the webRequest API is used. + return; + + // The request ID doesn't really matter. It just needs to be unique + // per-BrowserContext so extensions can make sense of it. Note that + // |network_service_request_id_| by contrast is not necessarily unique, so we + // don't use it for identity here. + const uint64_t web_request_id = ++g_request_id; + + if (request_id) + network_request_id_to_web_request_id_.emplace(request_id, web_request_id); + + auto result = requests_.emplace( + web_request_id, + std::make_unique( + this, web_request_id, routing_id, request_id, options, request, + traffic_annotation, std::move(loader), std::move(client))); + result.first->second->Restart(); } void ProxyingURLLoaderFactory::Clone( @@ -674,16 +700,44 @@ void ProxyingURLLoaderFactory::Clone( void ProxyingURLLoaderFactory::OnLoaderCreated( int32_t request_id, network::mojom::TrustedHeaderClientRequest request) { - // TODO(zcbenz): Call |OnLoaderCreated| for |InProgressRequest|. + auto it = network_request_id_to_web_request_id_.find(request_id); + if (it == network_request_id_to_web_request_id_.end()) + return; + + auto request_it = requests_.find(it->second); + DCHECK(request_it != requests_.end()); + request_it->second->OnLoaderCreated(std::move(request)); } void ProxyingURLLoaderFactory::OnTargetFactoryError() { - delete this; + target_factory_.reset(); + proxy_bindings_.CloseAllBindings(); + + MaybeDeleteThis(); } void ProxyingURLLoaderFactory::OnProxyBindingError() { if (proxy_bindings_.empty()) - delete this; + target_factory_.reset(); + + MaybeDeleteThis(); +} + +void ProxyingURLLoaderFactory::RemoveRequest(int32_t network_service_request_id, + uint64_t request_id) { + network_request_id_to_web_request_id_.erase(network_service_request_id); + requests_.erase(request_id); + + MaybeDeleteThis(); +} + +void ProxyingURLLoaderFactory::MaybeDeleteThis() { + // Even if all URLLoaderFactory pipes connected to this object have been + // closed it has to stay alive until all active requests have completed. + if (target_factory_.is_bound() || !requests_.empty()) + return; + + delete this; } } // namespace electron diff --git a/shell/browser/net/proxying_url_loader_factory.h b/shell/browser/net/proxying_url_loader_factory.h index 9d43fab8877d..8ac2285bd3dc 100644 --- a/shell/browser/net/proxying_url_loader_factory.h +++ b/shell/browser/net/proxying_url_loader_factory.h @@ -5,6 +5,7 @@ #ifndef SHELL_BROWSER_NET_PROXYING_URL_LOADER_FACTORY_H_ #define SHELL_BROWSER_NET_PROXYING_URL_LOADER_FACTORY_H_ +#include #include #include #include @@ -35,6 +36,7 @@ class ProxyingURLLoaderFactory public: InProgressRequest( ProxyingURLLoaderFactory* factory, + int64_t web_request_id, int32_t routing_id, int32_t network_service_request_id, uint32_t options, @@ -98,6 +100,7 @@ class ProxyingURLLoaderFactory ProxyingURLLoaderFactory* factory_; network::ResourceRequest request_; const base::Optional original_initiator_; + const uint64_t request_id_; const int32_t routing_id_; const int32_t network_service_request_id_; const uint32_t options_; @@ -173,6 +176,8 @@ class ProxyingURLLoaderFactory private: void OnTargetFactoryError(); void OnProxyBindingError(); + void RemoveRequest(int32_t network_service_request_id, uint64_t request_id); + void MaybeDeleteThis(); // This is passed from api::ProtocolNS. // @@ -188,6 +193,14 @@ class ProxyingURLLoaderFactory mojo::Binding url_loader_header_client_binding_; + // Mapping from our own internally generated request ID to an + // InProgressRequest instance. + std::map> requests_; + + // A mapping from the network stack's notion of request ID to our own + // internally generated request ID for the same request. + std::map network_request_id_to_web_request_id_; + DISALLOW_COPY_AND_ASSIGN(ProxyingURLLoaderFactory); };