0

HttpStreamPool: Don't start QuicTask when failing

AttemptManager::MaybeAttemptQuic() should not start QuicTask when
`is_failing_` is true. Otherwise, we end up hitting CHECK(!is_failing_)
in AttemptManager::OnQuicTaskComplete() when a QUIC attempt completes
successfully.

Bug: 403341337
Change-Id: I06b50d92e16eff6c37061ad3af77331c8734b001
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6355632
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1433916}
This commit is contained in:
Kenichi Ishibashi 2025-03-17 18:12:01 -07:00 committed by Chromium LUCI CQ
parent 9f10272d21
commit 26845ffc10
2 changed files with 40 additions and 1 deletions

@ -1277,7 +1277,7 @@ void HttpStreamPool::AttemptManager::MaybeNotifySSLConfigReady() {
void HttpStreamPool::AttemptManager::MaybeAttemptQuic() {
CHECK(service_endpoint_request_);
if (!CanUseQuic() || quic_task_result_.has_value() ||
if (is_failing_ || !CanUseQuic() || quic_task_result_.has_value() ||
!service_endpoint_request_->EndpointsCryptoReady()) {
return;
}

@ -4778,6 +4778,45 @@ TEST_F(HttpStreamPoolAttemptManagerTest, QuicOkDnsAlpn) {
Optional(IsOk()));
}
// Regression test for crbug.com/403341337. QuicTask should not be started when
// the corresponding AttemptManager is failing.
TEST_F(HttpStreamPoolAttemptManagerTest, DontStartQuicAfterFailure) {
AddQuicData();
FakeServiceEndpointRequest* endpoint_request = resolver()->AddFakeRequest();
// Request a stream to create an AttemptManager.
StreamRequester requester;
requester.set_destination(kDefaultDestination)
.set_quic_version(quic_version())
.RequestStream(pool());
ASSERT_FALSE(requester.result().has_value());
// Simulate a network change event to fail the AttemptManager.
NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests();
FastForwardUntilNoTasksRemain();
// Complete the service endpoint resolution. QuicTask should not start.
endpoint_request
->add_endpoint(ServiceEndpointBuilder().add_v4("192.0.2.1").endpoint())
.CallOnServiceEndpointRequestFinished(OK);
requester.WaitForResult();
EXPECT_THAT(requester.result(), Optional(IsError(ERR_NETWORK_CHANGED)));
ASSERT_TRUE(pool()
.GetGroupForTesting(requester.GetStreamKey())
->GetAttemptManagerForTesting()
->is_failing());
ASSERT_FALSE(pool()
.GetGroupForTesting(requester.GetStreamKey())
->GetAttemptManagerForTesting()
->GetQuicTaskResultForTesting());
// Ensure that the attempt manager completes after the request is destroyed.
requester.ResetRequest();
WaitForAttemptManagerComplete(
*pool().GetGroupForTesting(requester.GetStreamKey()));
}
// Tests that QUIC is not attempted when marked broken.
TEST_F(HttpStreamPoolAttemptManagerTest, QuicBroken) {
AlternativeService alternative_service(NextProto::kProtoQUIC,