From be963bc8c5c325b5adcd24b805c607a34b4656bc Mon Sep 17 00:00:00 2001
From: Ming-Ying Chung <[email protected]>
Date: Tue, 04 Aug 2026 23:11:53 -0700
Subject: [PATCH] [keepalive] Don't forward responses after initiator is gone

KeepAliveURLLoaderService keeps fetch keepalive requests alive in the
browser process when the initiating document is unloaded so that network
requests can complete. However, when a renderer created a clone of a
factory handle via URLLoaderFactory::Clone(), the browser instantiated a
separate FactoryContext copy for the cloned Mojo receiver. Because these
cloned contexts were independent heap objects, the browser could not
track or enforce document destruction status across cloned factory
receivers. Consequently, a renderer holding a stashed factory clone
could issue new requests post-unloading and still receive full response
data via URLLoaderClient.

This CL converts FactoryContext into a reference-counted object
`scoped_refptr<FactoryContext>`. When BindFactory() is called for a new
document, a distinct FactoryContext instance is allocated for that
document. Subsequent calls to Clone() share the existing FactoryContext
instance across all cloned Mojo receivers of that factory.

If a keepalive request is initiated after the document has been
destroyed, KeepAliveURLLoaderService detects the document teardown via
the shared FactoryContext and drops the renderer-side URLLoaderClient.

The network request still executes in the browser to maintain keepalive
semantics for racing requests, but response data is handled entirely in
the browser process without forwarding data back to the renderer.

Bug: 497869284
Change-Id: I0d25f4945ec7a18bd59e3fb0e5ce0f599c07aff4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8173827
Commit-Queue: Ming-Ying Chung <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1673918}
---

diff --git a/content/browser/loader/keep_alive_url_loader_service.cc b/content/browser/loader/keep_alive_url_loader_service.cc
index 7c9a556d..83768c3 100644
--- a/content/browser/loader/keep_alive_url_loader_service.cc
+++ b/content/browser/loader/keep_alive_url_loader_service.cc
@@ -54,19 +54,12 @@
   CHECK(policy_container_host);
 }
 
-KeepAliveURLLoaderService::FactoryContext::FactoryContext(
-    const std::unique_ptr<FactoryContext>& other)
-    : factory(other->factory),
-      weak_document_ptr(other->weak_document_ptr),
-      ukm_source_id(other->ukm_source_id),
-      policy_container_host(other->policy_container_host),
-      network_isolation_key(other->network_isolation_key) {}
-
 KeepAliveURLLoaderService::FactoryContext::~FactoryContext() = default;
 
 void KeepAliveURLLoaderService::FactoryContext::OnDidCommitNavigation(
     NavigationHandle* navigation_handle) {
   CHECK(navigation_handle);
+  did_commit_navigation = true;
   weak_document_ptr =
       navigation_handle->GetRenderFrameHost()->GetWeakDocumentPtr();
   network_isolation_key =
@@ -82,6 +75,11 @@
   CHECK(policy_container_host);
 }
 
+bool KeepAliveURLLoaderService::FactoryContext::WasInitiatorDocumentDestroyed()
+    const {
+  return did_commit_navigation && !weak_document_ptr.AsRenderFrameHostIfValid();
+}
+
 void KeepAliveURLLoaderService::FactoryContext::
     OnBeforeKeepAliveURLLoaderCreated(
         const network::ResourceRequest& resource_request) {
@@ -203,7 +201,7 @@
   // loader is ensured to exist.
   raw_ptr<KeepAliveURLLoader> CreateKeepAliveURLLoader(
       PendingReceiverType<Interface> receiver,
-      const std::unique_ptr<FactoryContext>& context,
+      const scoped_refptr<FactoryContext>& context,
       int32_t request_id,
       uint32_t options,
       const network::ResourceRequest& resource_request,
@@ -232,7 +230,13 @@
     context->OnBeforeKeepAliveURLLoaderCreated(resource_request);
 
     // Passes in the pending remote of `client` from a renderer so that `loader`
-    // can forward response back to the renderer.
+    // can forward response back to the renderer. If the initiator document has
+    // already been destroyed, drop `client` so that the response is handled
+    // entirely in the browser, the same as for a request whose renderer
+    // disconnects after starting it.
+    if (context->WasInitiatorDocumentDestroyed()) {
+      client.reset();
+    }
     CHECK(context->policy_container_host);
     auto loader = std::make_unique<KeepAliveURLLoader>(
         request_id, options, resource_request, std::move(client),
@@ -400,7 +404,7 @@
     // Adds a new factory receiver to the set, binding the pending `receiver`
     // from to `this` with a new context that has frame-specific data and keeps
     // reference to `subresource_proxying_factory_bundle`.
-    auto context = std::make_unique<FactoryContext>(
+    auto context = base::MakeRefCounted<FactoryContext>(
         std::move(subresource_proxying_factory_bundle),
         std::move(policy_container_host));
     auto weak_context = context->weak_ptr_factory.GetWeakPtr();
@@ -452,10 +456,8 @@
       override {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-    loader_factory_receivers_.Add(
-        this, std::move(receiver),
-        std::make_unique<FactoryContext>(
-            loader_factory_receivers_.current_context()));
+    loader_factory_receivers_.Add(this, std::move(receiver),
+                                  loader_factory_receivers_.current_context());
   }
 
  private:
@@ -467,7 +469,7 @@
   // be removed once it is disconnected from the corresponding remote (usually
   // in a renderer).
   mojo::ReceiverSet<network::mojom::URLLoaderFactory,
-                    std::unique_ptr<FactoryContext>>
+                    scoped_refptr<FactoryContext>>
       loader_factory_receivers_;
 };
 
@@ -501,7 +503,7 @@
     // Adds a new factory receiver to the set, binding the pending `receiver`
     // from to `this` with a new context that has frame-specific data and keeps
     // reference to `shared_url_loader_factory`.
-    auto context = std::make_unique<FactoryContext>(
+    auto context = base::MakeRefCounted<FactoryContext>(
         std::move(shared_url_loader_factory), std::move(policy_container_host));
     auto weak_context = context->weak_ptr_factory.GetWeakPtr();
     loader_factory_receivers_.Add(this, std::move(receiver),
@@ -545,10 +547,8 @@
           receiver) override {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-    loader_factory_receivers_.Add(
-        this, std::move(receiver),
-        std::make_unique<FactoryContext>(
-            loader_factory_receivers_.current_context()));
+    loader_factory_receivers_.Add(this, std::move(receiver),
+                                  loader_factory_receivers_.current_context());
   }
 
  private:
@@ -557,7 +557,7 @@
   // be removed once it is disconnected from the corresponding remote in a
   // renderer.
   mojo::AssociatedReceiverSet<blink::mojom::FetchLaterLoaderFactory,
-                              std::unique_ptr<FactoryContext>>
+                              scoped_refptr<FactoryContext>>
       loader_factory_receivers_;
 };
 
diff --git a/content/browser/loader/keep_alive_url_loader_service.h b/content/browser/loader/keep_alive_url_loader_service.h
index 0a0ddea..e6f7010 100644
--- a/content/browser/loader/keep_alive_url_loader_service.h
+++ b/content/browser/loader/keep_alive_url_loader_service.h
@@ -9,6 +9,7 @@
 #include <optional>
 
 #include "base/containers/lru_cache.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_refptr.h"
 #include "content/browser/loader/keep_alive_url_loader.h"
 #include "content/common/content_export.h"
@@ -58,17 +59,16 @@
   //
   // A FactoryContext is created whenever `BindFactory()` or
   // `BindFetchLaterLoaderFactory()` is called by
-  // RenderFrameHostImpl::CommitNavigation(). It can also be cloned by the same
-  // corresponding renderer, or when new window or new child frame is created.
+  // RenderFrameHostImpl::CommitNavigation(). It can also be "cloned" using an
+  // additional reference by the same corresponding renderer, or when new window
+  // or new child frame is created.
   //
   // See `mojo::ReceiverSetBase` for more details.
-  struct CONTENT_EXPORT FactoryContext {
+  struct CONTENT_EXPORT FactoryContext
+      : public base::RefCounted<FactoryContext> {
     FactoryContext(
         scoped_refptr<network::SharedURLLoaderFactory> factory,
         scoped_refptr<PolicyContainerHost> frame_policy_container_host);
-    // Called when a factory is cloned by URLLoaderFactory::Clone().
-    explicit FactoryContext(const std::unique_ptr<FactoryContext>& other);
-    ~FactoryContext();
     // Not Copyable.
     FactoryContext(const FactoryContext&) = delete;
     FactoryContext& operator=(const FactoryContext&) = delete;
@@ -76,6 +76,10 @@
     // Updates `weak_document_ptr` and other document-related fields.
     void OnDidCommitNavigation(NavigationHandle* navigation_handle);
 

diff --git a/content/browser/loader/keep_alive_url_loader_service_unittest.cc b/content/browser/loader/keep_alive_url_loader_service_unittest.cc
index c6528d5..c74d69a 100644
--- a/content/browser/loader/keep_alive_url_loader_service_unittest.cc
+++ b/content/browser/loader/keep_alive_url_loader_service_unittest.cc
@@ -144,6 +144,10 @@
     return remote_url_loader.is_connected();
   }
   void reset_remote_url_loader() { remote_url_loader.reset(); }
+  void Clone(mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver) {
+    remote_url_loader_factory->Clone(std::move(receiver));
+  }
+  void FlushForTesting() { remote_url_loader_factory.FlushForTesting(); }
 
  private:
   mojo::Remote<network::mojom::URLLoaderFactory> remote_url_loader_factory;
@@ -182,6 +186,18 @@
     return remote_fetch_later_loader_.is_connected();
   }
   void reset_remote_fetch_later_loader() { remote_fetch_later_loader_.reset(); }
+  mojo::PendingAssociatedReceiver<blink::mojom::FetchLaterLoaderFactory>
+  BindNewEndpointAndPassReceiver() {
+    return remote_fetch_later_loader_factory_.BindNewEndpointAndPassReceiver();
+  }
+  void Clone(
+      mojo::PendingAssociatedReceiver<blink::mojom::FetchLaterLoaderFactory>
+          receiver) {
+    remote_fetch_later_loader_factory_->Clone(std::move(receiver));
+  }
+  void FlushForTesting() {
+    remote_fetch_later_loader_factory_.FlushForTesting();
+  }
 
  private:
   mojo::AssociatedRemote<blink::mojom::FetchLaterLoaderFactory>
@@ -565,6 +581,77 @@
   EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
 }
 
+// Verifies that keepalive requests initiated via a cloned `URLLoaderFactory`
+// after the initiating document has been unloaded will start the network
+// load, but will not forward response data back to the renderer
+// `URLLoaderClient`.
+TEST_F(KeepAliveURLLoaderServiceTest,
+       LoadRequestFromClonedFactoryAfterPageIsUnloaded) {
+  FakeRemoteURLLoaderFactory renderer_loader_factory;
+  MockReceiverURLLoaderClient renderer_loader_client;
+  BindKeepAliveURLLoaderFactory(renderer_loader_factory);
+
+  FakeRemoteURLLoaderFactory cloned_factory;
+  renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver());
+  renderer_loader_factory.FlushForTesting();
+
+  // Deletes the current RenderFrameHost and then loads a keepalive request
+  // from the cloned factory.
+  DeleteContents();
+  cloned_factory.CreateLoaderAndStart(
+      CreateResourceRequest(GURL(kTestRequestUrl)),
+      renderer_loader_client.BindNewPipeAndPassRemote(),
+      /*expect_success=*/true);
+
+  EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
+  EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+
+  // When a response is received, it should not be forwarded to the client
+  // since the initiator document has been unloaded.
+  EXPECT_CALL(renderer_loader_client, OnReceiveResponse(_, _, _)).Times(0);
+  GetLastPendingRequest()->client->OnReceiveResponse(
+      CreateResponseHead({{kTestResponseHeaderName, kTestResponseHeaderValue}}),
+      /*body=*/{}, std::nullopt);
+  EXPECT_TRUE(base::test::RunUntil(
+      [&]() { return loader_service().NumLoadersForTesting() == 0u; }));
+  EXPECT_EQ(loader_service().NumLoadersForTesting(), 0u);
+}
+
+// Verifies that cloning a `URLLoaderFactory` after the initiating document
+// has been unloaded correctly binds the new factory receiver to the shared
+// `FactoryContext`.
+TEST_F(KeepAliveURLLoaderServiceTest, CloneFactoryAfterPageIsUnloaded) {
+  FakeRemoteURLLoaderFactory renderer_loader_factory;
+  MockReceiverURLLoaderClient renderer_loader_client;
+  BindKeepAliveURLLoaderFactory(renderer_loader_factory);
+
+  // Deletes the current RenderFrameHost first before cloning the factory.
+  DeleteContents();
+
+  FakeRemoteURLLoaderFactory cloned_factory;
+  renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver());
+  renderer_loader_factory.FlushForTesting();
+
+  cloned_factory.CreateLoaderAndStart(
+      CreateResourceRequest(GURL(kTestRequestUrl)),
+      renderer_loader_client.BindNewPipeAndPassRemote(),
+      /*expect_success=*/true);
+
+  EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
+  EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+
+  // When a response is received, it should not be forwarded to the client
+  // since the initiator document has been unloaded.
+  EXPECT_CALL(renderer_loader_client, OnReceiveResponse(_, _, _)).Times(0);
+  GetLastPendingRequest()->client->OnReceiveResponse(
+      CreateResponseHead({{kTestResponseHeaderName, kTestResponseHeaderValue}}),
+      /*body=*/{}, std::nullopt);
+  EXPECT_TRUE(base::test::RunUntil(
+      [&]() { return loader_service().NumLoadersForTesting() == 0u; }));
+  EXPECT_EQ(loader_service().NumLoadersForTesting(), 0u);
+}
+
+
 // This test initially provides an unbind factory to KeepAliveURLLoaderService.
 // After that, provides a bound factory via UpdateFactory.
 TEST_F(KeepAliveURLLoaderServiceTest, LoadRequestAfterUpdateFactory) {
@@ -618,6 +705,52 @@
   }
 }
 
+// Verifies that a cloned `URLLoaderFactory` shares its `FactoryContext` with
+// the original receiver, ensuring that updates via `UpdateFactory()` apply to
+// requests initiated from cloned factory handles.
+TEST_F(KeepAliveURLLoaderServiceTest,
+       LoadRequestWithClonedFactoryAfterUpdateFactory) {
+  FakeRemoteURLLoaderFactory renderer_loader_factory;
+
+  auto unbound_factory =
+      std::make_unique<network::WrapperPendingSharedURLLoaderFactory>();
+  scoped_refptr<PolicyContainerHost> policy_container_host =
+      static_cast<RenderFrameHostImpl*>(main_rfh())
+          ->policy_container_host()
+          ->Clone();
+  auto context = loader_service().BindFactory(
+      renderer_loader_factory.BindNewPipeAndPassReceiver(),
+      network::SharedURLLoaderFactory::Create(std::move(unbound_factory)),
+      policy_container_host);
+  GetNavigationRequest()->SetKeepAliveURLLoaderFactoryContextForTesting(
+      context);
+  pending_navigation_->Commit();
+
+  FakeRemoteURLLoaderFactory cloned_factory;
+  renderer_loader_factory.Clone(cloned_factory.BindNewPipeAndPassReceiver());
+  renderer_loader_factory.FlushForTesting();
+
+  renderer_loader_factory.reset_remote_url_loader();
+  mojo::Remote<network::mojom::URLLoaderFactory> factory;
+  network_url_loader_factory().Clone(factory.BindNewPipeAndPassReceiver());
+  auto pending_factory = std::make_unique<blink::PendingURLLoaderFactoryBundle>(
+      factory.Unbind(), blink::PendingURLLoaderFactoryBundle::SchemeMap(),
+      blink::PendingURLLoaderFactoryBundle::OriginMap(),
+      /*local_resource_loader_config=*/nullptr,
+      /*bypass_redirect_checks=*/false);
+  context->UpdateFactory(
+      network::SharedURLLoaderFactory::Create(std::move(pending_factory)));
+
+  {
+    MockReceiverURLLoaderClient renderer_loader_client;
+    cloned_factory.CreateLoaderAndStart(
+        CreateResourceRequest(GURL(kTestRequestUrl)),
+        renderer_loader_client.BindNewPipeAndPassRemote(),
+        /*expect_success=*/true);
+    EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
+  }
+}
+
 TEST_F(KeepAliveURLLoaderServiceTest, ForwardOnReceiveResponse) {
   FakeRemoteURLLoaderFactory renderer_loader_factory;
   MockReceiverURLLoaderClient renderer_loader_client;
@@ -1378,6 +1511,58 @@
   EXPECT_EQ(network_url_loader_factory().NumPending(), 1);
 }
 
+// Verifies that FetchLater requests initiated from a cloned
+// `FetchLaterLoaderFactory` after the initiating document has been unloaded
+// correctly inherit the document lifecycle state from the shared
+// `FactoryContext`.
+TEST_F(FetchLaterKeepAliveURLLoaderServiceTest,
+       LoadFetchLaterRequestFromClonedFactoryAfterPageIsUnloaded) {
+  FakeRemoteFetchLaterLoaderFactory renderer_loader_factory;
+  BindFetchLaterLoaderFactory(renderer_loader_factory);
+
+  FakeRemoteFetchLaterLoaderFactory cloned_factory;
+  renderer_loader_factory.Clone(
+      cloned_factory.BindNewEndpointAndPassReceiver());
+  renderer_loader_factory.FlushForTesting();
+
+  // Deletes the current RenderFrameHost and then loads a request via the cloned
+  // factory.
+  DeleteContents();
+  cloned_factory.CreateLoader(
+      CreateFetchLaterResourceRequest(GURL(kTestRequestUrl)),
+      /*expect_success=*/true);
+
+  EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+  EXPECT_EQ(network_url_loader_factory().NumPending(), 0);
+}
+
+// Verifies that cloning a `FetchLaterLoaderFactory` after the initiating
+// document has been unloaded correctly binds the new factory receiver to the
+// shared `FactoryContext`, allowing requests to be created with the unloaded
+// document state.
+TEST_F(FetchLaterKeepAliveURLLoaderServiceTest,
+       CloneFetchLaterFactoryAfterPageIsUnloaded) {
+  FakeRemoteFetchLaterLoaderFactory renderer_loader_factory;
+  BindFetchLaterLoaderFactory(renderer_loader_factory);
+
+  // Deletes the current RenderFrameHost first before cloning the factory.
+  DeleteContents();
+
+  FakeRemoteFetchLaterLoaderFactory cloned_factory;
+  renderer_loader_factory.Clone(
+      cloned_factory.BindNewEndpointAndPassReceiver());
+  renderer_loader_factory.FlushForTesting();
+
+  // Loads a request via the cloned factory.
+  cloned_factory.CreateLoader(
+      CreateFetchLaterResourceRequest(GURL(kTestRequestUrl)),
+      /*expect_success=*/true);
+
+  EXPECT_EQ(loader_service().NumLoadersForTesting(), 1u);
+  EXPECT_EQ(network_url_loader_factory().NumPending(), 0);
+}
+
+
 class KeepAliveURLLoaderServiceRetryTest
     : public KeepAliveURLLoaderServiceTestBase {
  protected: