From 1985cb7b30ce45730451a3f23fb23cb5e4b6ceb1 Mon Sep 17 00:00:00 2001
From: Adam Rice <[email protected]>
Date: Thu, 17 Sep 2026 00:42:45 -0700
Subject: [PATCH] Call URLRequestHttpJob::NotifyConnectedCallback via WeakPtr

net::URLRequestHttpJob::NotifyConnectedCallback was called via an
unretained `this` pointer. However, in some cases it could be called
after the job had been destroyed. Call it via a WeakPtr instead.

BUG=560939312

Change-Id: I1c289ec5797d97892ff51bde4866755a709cb458
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8412191
Commit-Queue: Adam Rice <[email protected]>
Reviewed-by: Nidhi Jaju <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1699580}
---
diff --git a/net/http/http_transaction_test_util.h b/net/http/http_transaction_test_util.h
index 24c6b81..28b1a54c 100644
--- a/net/http/http_transaction_test_util.h
+++ b/net/http/http_transaction_test_util.h
@@ -264,6 +264,10 @@
 
   RequestPriority priority() const { return priority_; }
 
+  const ConnectedCallback& connected_callback() const {
+    return connected_callback_;
+  }
+
   base::WeakPtr<MockNetworkTransaction> AsWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index f93d02b8..fc2c9a8 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -558,9 +558,13 @@
 }
 
 int URLRequestHttpJob::NotifyConnectedCallback(
+    base::WeakPtr<URLRequestHttpJob> job,
     const TransportInfo& info,
     CompletionOnceCallback callback) {
-  return URLRequestJob::NotifyConnected(info, std::move(callback));
+  if (!job) {
+    return ERR_ABORTED;
+  }
+  return job->NotifyConnected(info, std::move(callback));
 }
 
 PrivacyMode URLRequestHttpJob::DeterminePrivacyMode() const {
@@ -747,8 +751,9 @@
     }
 
     if (rv == OK) {
-      transaction_->SetConnectedCallback(base::BindRepeating(
-          &URLRequestHttpJob::NotifyConnectedCallback, base::Unretained(this)));
+      transaction_->SetConnectedCallback(
+          base::BindRepeating(&URLRequestHttpJob::NotifyConnectedCallback,
+                              weak_factory_.GetWeakPtr()));
       transaction_->SetRequestHeadersCallback(request_headers_callback_);
       transaction_->SetEarlyResponseHeadersCallback(
           early_response_headers_callback_);
diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h
index 566ff74..8bbc3e8 100644
--- a/net/url_request/url_request_http_job.h
+++ b/net/url_request/url_request_http_job.h
@@ -158,9 +158,11 @@
       const std::optional<HttpRequestHeaders>& headers);
   // This just forwards the call to URLRequestJob::NotifyConnected().
   // We need it because that method is protected and cannot be bound in a
-  // callback in this class.
-  int NotifyConnectedCallback(const TransportInfo& info,
-                              CompletionOnceCallback callback);
+  // callback in this class. It is static because base::BindRepeating only
+  // supports binding WeakPtr directly to methods with a void return type.
+  static int NotifyConnectedCallback(base::WeakPtr<URLRequestHttpJob> job,
+                                     const TransportInfo& info,
+                                     CompletionOnceCallback callback);
 
   void RestartTransaction();
 #if BUILDFLAG(ENABLE_DEVICE_BOUND_SESSIONS)
diff --git a/net/url_request/url_request_http_job_unittest.cc b/net/url_request/url_request_http_job_unittest.cc
index 1c61450..f6afb1d1 100644
--- a/net/url_request/url_request_http_job_unittest.cc
+++ b/net/url_request/url_request_http_job_unittest.cc
@@ -1194,6 +1194,18 @@
   EXPECT_EQ(HIGHEST, network_layer().last_transaction()->priority());
 }
 
+TEST_F(URLRequestHttpJobTest, ConnectedCallbackAfterDestruction) {
+  req_->Start();
+  ASSERT_TRUE(network_layer().last_transaction());
+  HttpTransaction::ConnectedCallback callback =
+      network_layer().last_transaction()->connected_callback();
+  ASSERT_TRUE(callback);
+
+  req_.reset();
+
+  EXPECT_EQ(ERR_ABORTED, callback.Run(TransportInfo(), base::DoNothing()));
+}
+
 TEST_F(URLRequestHttpJobTest, HSTSInternalRedirectTest) {
   // Setup HSTS state.
   context_->transport_security_state()->AddHSTS(

diff --git a/net/url_request/url_request_http_job_unittest.cc b/net/url_request/url_request_http_job_unittest.cc
index 1c61450..f6afb1d1 100644
--- a/net/url_request/url_request_http_job_unittest.cc
+++ b/net/url_request/url_request_http_job_unittest.cc
@@ -1194,6 +1194,18 @@
   EXPECT_EQ(HIGHEST, network_layer().last_transaction()->priority());
 }
 
+TEST_F(URLRequestHttpJobTest, ConnectedCallbackAfterDestruction) {
+  req_->Start();
+  ASSERT_TRUE(network_layer().last_transaction());
+  HttpTransaction::ConnectedCallback callback =
+      network_layer().last_transaction()->connected_callback();
+  ASSERT_TRUE(callback);
+
+  req_.reset();
+
+  EXPECT_EQ(ERR_ABORTED, callback.Run(TransportInfo(), base::DoNothing()));
+}
+
 TEST_F(URLRequestHttpJobTest, HSTSInternalRedirectTest) {
   // Setup HSTS state.
   context_->transport_security_state()->AddHSTS(