Only log ALTERNATE_PROTOCOL_USAGE_BROKEN once per request

Change-Id: I10d3f8ff2bde72db39dd7e26b73c82a7f9155e4a
Bug: 1024613
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913968
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Auto-Submit: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715146}

net/http/http_stream_factory_job_controller.cc

@@ -998,6 +998,7 @@ HttpStreamFactory::JobController::GetAlternativeServiceInfoInternal(
   // First alternative service that is not marked as broken.
   AlternativeServiceInfo first_alternative_service_info;
 
+  bool is_any_broken = false;
   for (const AlternativeServiceInfo& alternative_service_info :
        alternative_service_info_vector) {
     DCHECK(IsAlternateProtocolValid(alternative_service_info.protocol()));
@@ -1011,7 +1012,11 @@ HttpStreamFactory::JobController::GetAlternativeServiceInfoInternal(
           return NetLogAltSvcParams(&alternative_service_info, is_broken);
         });
     if (is_broken) {
-      HistogramAlternateProtocolUsage(ALTERNATE_PROTOCOL_USAGE_BROKEN, false);
+      if (!is_any_broken) {
+        // Only log the broken alternative service once per request.
+        is_any_broken = true;
+        HistogramAlternateProtocolUsage(ALTERNATE_PROTOCOL_USAGE_BROKEN, false);
+      }
       continue;
     }
 

net/quic/quic_network_transaction_unittest.cc

@@ -4123,6 +4123,61 @@ TEST_P(QuicNetworkTransactionTest, RemoteAltSvcWorkingWhileLocalAltSvcBroken) {
   SendRequestAndExpectQuicResponse("hello!");
 }
 
+// Verify that when multiple alternatives are broken,
+// ALTERNATE_PROTOCOL_USAGE_BROKEN is only logged once.
+// This is a regression test for crbug/1024613.
+TEST_P(QuicNetworkTransactionTest, BrokenAlternativeOnlyRecordedOnce) {
+  base::HistogramTester histogram_tester;
+
+  MockRead http_reads[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"), MockRead(kQuicAlternativeServiceHeader),
+      MockRead("hello world"),
+      MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
+      MockRead(ASYNC, OK)};
+
+  StaticSocketDataProvider http_data(http_reads, base::span<MockWrite>());
+  socket_factory_.AddSocketDataProvider(&http_data);
+  AddCertificate(&ssl_data_);
+  socket_factory_.AddSSLSocketDataProvider(&ssl_data_);
+
+  GURL origin1 = request_.url;  // mail.example.org
+
+  scoped_refptr<X509Certificate> cert(
+      ImportCertFromFile(GetTestCertsDirectory(), "wildcard.pem"));
+  ASSERT_TRUE(cert->VerifyNameMatch("mail.example.org"));
+
+  ProofVerifyDetailsChromium verify_details;
+  verify_details.cert_verify_result.verified_cert = cert;
+  verify_details.cert_verify_result.is_issued_by_known_root = true;
+  crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
+
+  CreateSession();
+
+  // Set up alternative service for |origin1|.
+  AlternativeService local_alternative(kProtoQUIC, "mail.example.org", 443);
+  base::Time expiration = base::Time::Now() + base::TimeDelta::FromDays(1);
+  AlternativeServiceInfoVector alternative_services;
+  alternative_services.push_back(
+      AlternativeServiceInfo::CreateQuicAlternativeServiceInfo(
+          local_alternative, expiration,
+          session_->params().quic_params.supported_versions));
+  alternative_services.push_back(
+      AlternativeServiceInfo::CreateQuicAlternativeServiceInfo(
+          local_alternative, expiration,
+          session_->params().quic_params.supported_versions));
+  http_server_properties_->SetAlternativeServices(url::SchemeHostPort(origin1),
+                                                  NetworkIsolationKey(),
+                                                  alternative_services);
+
+  http_server_properties_->MarkAlternativeServiceBroken(local_alternative,
+                                                        NetworkIsolationKey());
+
+  SendRequestAndExpectHttpResponse("hello world");
+
+  histogram_tester.ExpectBucketCount("Net.AlternateProtocolUsage",
+                                     ALTERNATE_PROTOCOL_USAGE_BROKEN, 1);
+}
+
 // Verify that with retry_without_alt_svc_on_quic_errors enabled, if a QUIC
 // request is reset from, then QUIC will be marked as broken and the request
 // retried over TCP. Then, subsequent requests will go over a new TCP