feat: associate InProgressRequest with requests (#19648)

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<InProgressRequest>(
+          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

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 <map>
 #include <memory>
 #include <set>
 #include <string>
@@ -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<url::Origin> 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<network::mojom::TrustedURLLoaderHeaderClient>
       url_loader_header_client_binding_;
 
+  // Mapping from our own internally generated request ID to an
+  // InProgressRequest instance.
+  std::map<uint64_t, std::unique_ptr<InProgressRequest>> 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<int32_t, uint64_t> network_request_id_to_web_request_id_;
+
   DISALLOW_COPY_AND_ASSIGN(ProxyingURLLoaderFactory);
 };