From e13a7ad5bf7400250dae3fbd081abfdd414321c4 Mon Sep 17 00:00:00 2001
From: Keita Suzuki <[email protected]>
Date: Tue, 30 Jun 2026 19:38:37 -0700
Subject: [PATCH] ServiceWorker: Handle loader detach during fallback callback

ServiceWorkerMainResourceLoader::Fallback() unbinds its Mojo receiver
before running |fallback_callback_|. The callback may synchronously tear
down the navigation loader stack, which calls DetachedFromRequest() and,
with the receiver already unbound, deletes |this|. Guard against this by
taking a WeakPtr before running the callback and returning early if the
loader is gone when it returns.

Add a unit test that detaches the loader from inside the fallback
callback.

TAG=agy
CONV=7fc40a18-110b-4e65-83f9-4a0b829923eb

Bug: 522418913
Change-Id: Ib9e38a6c36d671348ee3d697ffc33b02f52f021e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8017182
Commit-Queue: Keita Suzuki <[email protected]>
Reviewed-by: Shunya Shishido <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1655219}
---

diff --git a/content/browser/service_worker/service_worker_main_resource_loader.cc b/content/browser/service_worker/service_worker_main_resource_loader.cc
index cf6c9e44..09e5d55 100644
--- a/content/browser/service_worker/service_worker_main_resource_loader.cc
+++ b/content/browser/service_worker/service_worker_main_resource_loader.cc
@@ -1062,9 +1062,16 @@
       receiver_.Unbind();
 
   if (fallback_callback_) {
-    if (network::mojom::URLLoaderFactory* factory =
-            std::move(fallback_callback_)
-                .Run(std::move(response_header_params))) {
+    // Running the fallback callback may synchronously destroy the owner of
+    // `this`, which in turn calls `DetachedFromRequest()`. Since `receiver_`
+    // is no longer bound at this point, that triggers `delete this`.
+    base::WeakPtr<ServiceWorkerMainResourceLoader> weak_this = AsWeakPtr();
+    network::mojom::URLLoaderFactory* factory =
+        std::move(fallback_callback_).Run(std::move(response_header_params));
+    if (!weak_this) {
+      return;
+    }
+    if (factory) {
       // Fallback to the default factory, and pass the original parameters/mojo
       // pipes of the initial request received in `StartRequest()`.
       factory->CreateLoaderAndStart(std::move(receiver), request_id_, options_,
diff --git a/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc b/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc
index c24765f9..1f187ec 100644
--- a/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc
+++ b/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc
@@ -1447,6 +1447,43 @@
   EXPECT_FALSE(loader);
 }
 
+// Tests that the loader handles being detached while running the fallback
+// callback. This can happen if the navigation is cancelled while the fallback
+// callback is on the stack.
+TEST_F(ServiceWorkerMainResourceLoaderTest, DetachedDuringFallbackCallback) {
+  service_worker_->RespondWithFallback();
+  SetupNetworkResponse();
+
+  std::unique_ptr<network::ResourceRequest> request = CreateRequest();
+  service_worker_client_ = std::make_unique<ScopedServiceWorkerClient>(
+      CreateServiceWorkerClient(helper_->context(), request->url));
+  service_worker_client()->AddMatchingRegistration(registration_.get());
+  service_worker_client()->SetControllerRegistration(
+      registration_, /*notify_controllerchange=*/false);
+
+  base::RunLoop run_loop;
+  loader_ = std::make_unique<ServiceWorkerMainResourceLoader>(
+      base::BindLambdaForTesting(
+          [&](ResponseHeadUpdateParams) -> network::mojom::URLLoaderFactory* {
+            // Simulate the owning interceptor being torn down while the
+            // fallback callback is running.
+            loader_.release()->DetachedFromRequest();
+            run_loop.Quit();
+            return fake_url_loader_factory_.get();
+          }),
+      /*fetch_event_client_id=*/"", service_worker_client()->AsWeakPtr(),
+      /*find_registration_start_time=*/base::TimeTicks::Now());
+  base::WeakPtr<ServiceWorkerMainResourceLoader> loader = loader_->AsWeakPtr();
+  loader_->StartRequest(loader_remote_.BindNewPipeAndPassReceiver(),
+                        /*request_id=*/0, /*options=*/0, *request,
+                        client_.CreateRemote(),
+                        net::MutableNetworkTrafficAnnotationTag());
+  run_loop.Run();
+
+  // The loader should have been deleted.
+  EXPECT_FALSE(loader);
+}
+
 TEST_F(ServiceWorkerMainResourceLoaderTest, ConnectionErrorDuringFetchEvent) {
   service_worker_->DeferResponse();
   StartRequest(CreateRequest());

diff --git a/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc b/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc
index c24765f9..1f187ec 100644
--- a/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc
+++ b/content/browser/service_worker/service_worker_main_resource_loader_unittest.cc
@@ -1447,6 +1447,43 @@
   EXPECT_FALSE(loader);
 }
 
+// Tests that the loader handles being detached while running the fallback
+// callback. This can happen if the navigation is cancelled while the fallback
+// callback is on the stack.
+TEST_F(ServiceWorkerMainResourceLoaderTest, DetachedDuringFallbackCallback) {
+  service_worker_->RespondWithFallback();
+  SetupNetworkResponse();
+
+  std::unique_ptr<network::ResourceRequest> request = CreateRequest();
+  service_worker_client_ = std::make_unique<ScopedServiceWorkerClient>(
+      CreateServiceWorkerClient(helper_->context(), request->url));
+  service_worker_client()->AddMatchingRegistration(registration_.get());
+  service_worker_client()->SetControllerRegistration(
+      registration_, /*notify_controllerchange=*/false);
+
+  base::RunLoop run_loop;
+  loader_ = std::make_unique<ServiceWorkerMainResourceLoader>(
+      base::BindLambdaForTesting(
+          [&](ResponseHeadUpdateParams) -> network::mojom::URLLoaderFactory* {
+            // Simulate the owning interceptor being torn down while the
+            // fallback callback is running.
+            loader_.release()->DetachedFromRequest();
+            run_loop.Quit();
+            return fake_url_loader_factory_.get();
+          }),
+      /*fetch_event_client_id=*/"", service_worker_client()->AsWeakPtr(),
+      /*find_registration_start_time=*/base::TimeTicks::Now());
+  base::WeakPtr<ServiceWorkerMainResourceLoader> loader = loader_->AsWeakPtr();
+  loader_->StartRequest(loader_remote_.BindNewPipeAndPassReceiver(),
+                        /*request_id=*/0, /*options=*/0, *request,
+                        client_.CreateRemote(),
+                        net::MutableNetworkTrafficAnnotationTag());
+  run_loop.Run();
+
+  // The loader should have been deleted.
+  EXPECT_FALSE(loader);
+}
+
 TEST_F(ServiceWorkerMainResourceLoaderTest, ConnectionErrorDuringFetchEvent) {
   service_worker_->DeferResponse();
   StartRequest(CreateRequest());