HttpStreamPool: Destroy Group when closing the last idle stream

Group::CloseOneIdleStream() could close a stream that is the last
active stream in the group. We should destroy the group when it's the
case. Otherwise, the group will be alive forever.

Bug: 346835898
Change-Id: I79656f5f078fac93223f433e2f51310a1b6674dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6056788
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Nidhi Jaju <nidhijaju@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1389672}
diff --git a/net/http/http_stream_pool_group.cc b/net/http/http_stream_pool_group.cc
index e50057dc..ab18b75 100644
--- a/net/http/http_stream_pool_group.cc
+++ b/net/http/http_stream_pool_group.cc
@@ -251,8 +251,17 @@
     return false;
   }
 
+  RecordNetLogClosingSocket(*idle_stream_sockets_.front().stream_socket,
+                            kExceededSocketLimits);
   idle_stream_sockets_.pop_front();
   pool_->DecrementTotalIdleStreamCount();
+  if (CanComplete()) {
+    // Use PostTask since MaybeComplete() may delete `this`, and this method
+    // could be called while iterating all groups.
+    base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&Group::MaybeComplete, weak_ptr_factory_.GetWeakPtr()));
+  }
   return true;
 }
 
@@ -304,11 +313,13 @@
 void HttpStreamPool::Group::CloseIdleStreams(
     std::string_view net_log_close_reason_utf8) {
   CleanupIdleStreamSockets(CleanupMode::kForce, net_log_close_reason_utf8);
-  // Use PostTask since MaybeComplete() may delete `this`, and this method could
-  // be called while iterating all groups.
-  base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
-      FROM_HERE,
-      base::BindOnce(&Group::MaybeComplete, weak_ptr_factory_.GetWeakPtr()));
+  if (CanComplete()) {
+    // Use PostTask since MaybeComplete() may delete `this`, and this method
+    // could be called while iterating all groups.
+    base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&Group::MaybeComplete, weak_ptr_factory_.GetWeakPtr()));
+  }
 }
 
 void HttpStreamPool::Group::CancelJobs(int error) {
@@ -374,8 +385,12 @@
       std::make_unique<AttemptManager>(this, http_network_session()->net_log());
 }
 
+bool HttpStreamPool::Group::CanComplete() const {
+  return ActiveStreamSocketCount() == 0 && !attempt_manager_;
+}
+
 void HttpStreamPool::Group::MaybeComplete() {
-  if (ActiveStreamSocketCount() > 0 || attempt_manager_) {
+  if (!CanComplete()) {
     return;
   }
 
diff --git a/net/http/http_stream_pool_group.h b/net/http/http_stream_pool_group.h
index 02371b0..db90cd7 100644
--- a/net/http/http_stream_pool_group.h
+++ b/net/http/http_stream_pool_group.h
@@ -126,7 +126,8 @@
   // Tries to process a pending request.
   void ProcessPendingRequest();
 
-  // Closes one idle stream socket. Returns true if it closed a stream.
+  // Closes one idle stream socket. Returns true if it closed a stream. Called
+  // when the pool reached the stream count limit.
   bool CloseOneIdleStreamSocket();
 
   // Returns the number of handed out streams.
@@ -204,6 +205,9 @@
 
   void EnsureAttemptManager();
 
+  // Returns true when `this` can be deleted.
+  bool CanComplete() const;
+
   void MaybeComplete();
 
   const raw_ptr<HttpStreamPool> pool_;
diff --git a/net/http/http_stream_pool_group_unittest.cc b/net/http/http_stream_pool_group_unittest.cc
index 6dca845..622d1d9 100644
--- a/net/http/http_stream_pool_group_unittest.cc
+++ b/net/http/http_stream_pool_group_unittest.cc
@@ -298,6 +298,20 @@
   ASSERT_EQ(group.IdleStreamSocketCount(), 0u);
 }
 
+// Test that a group is destroyed when closing an idle stream that is the last
+// stream in the group.
+TEST_F(HttpStreamPoolGroupTest, DestroyGroupAfterCloseOneIdleStream) {
+  Group& group = GetOrCreateTestGroup();
+
+  auto stream_socket = std::make_unique<FakeStreamSocket>();
+  group.AddIdleStreamSocket(std::move(stream_socket));
+  ASSERT_EQ(group.IdleStreamSocketCount(), 1u);
+
+  ASSERT_TRUE(group.CloseOneIdleStreamSocket());
+  FastForwardUntilNoTasksRemain();
+  ASSERT_FALSE(GetTestGroup());
+}
+
 TEST_F(HttpStreamPoolGroupTest, IPAddressChangeCleanupIdleSocket) {
   auto stream_socket = std::make_unique<FakeStreamSocket>();