Add Net.CertVerifier.PathBuilderIterationCount histogram

Bug: 634470
Change-Id: I1a7d17d51b481ad3aabd220f3b88286c0d9ca405
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693387
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Auto-Submit: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676516}

net/cert/internal/path_builder.cc

@@ -9,6 +9,7 @@
 #include <unordered_set>
 
 #include "base/logging.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/strings/string_number_conversions.h"
 #include "crypto/sha2.h"
 #include "net/base/net_errors.h"
@@ -55,6 +56,11 @@ std::string PathDebugString(const ParsedCertificateList& certs) {
   return s;
 }
 
+void RecordIterationCountHistogram(uint32_t iteration_count) {
+  base::UmaHistogramCounts10000("Net.CertVerifier.PathBuilderIterationCount",
+                                iteration_count);
+}
+
 // This structure describes a certificate and its trust level. Note that |cert|
 // may be null to indicate an "empty" entry.
 struct IssuerEntry {
@@ -591,6 +597,7 @@ void CertPathBuilder::Run() {
       if (!deadline_.is_null() && base::TimeTicks::Now() > deadline_) {
         out_result_->exceeded_deadline = true;
       }
+      RecordIterationCountHistogram(iteration_count);
       return;
     }
 
@@ -612,6 +619,7 @@ void CertPathBuilder::Run() {
     AddResultPath(std::move(result_path));
 
     if (path_is_good) {
+      RecordIterationCountHistogram(iteration_count);
       // Found a valid path, return immediately.
       // TODO(mattm): add debug/test mode that tries all possible paths.
       return;

net/cert/internal/path_builder_unittest.cc

@@ -7,6 +7,7 @@
 #include "base/base_paths.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "net/cert/internal/cert_error_params.h"
 #include "net/cert/internal/cert_issuer_source_static.h"
 #include "net/cert/internal/common_cert_errors.h"
@@ -30,6 +31,7 @@ namespace net {
 namespace {
 
 using ::testing::_;
+using ::testing::ElementsAre;
 using ::testing::Invoke;
 using ::testing::NiceMock;
 using ::testing::Return;
@@ -450,12 +452,11 @@ TEST_F(PathBuilderMultiRootTest, TestCertIssuerOrdering) {
 }
 
 TEST_F(PathBuilderMultiRootTest, TestIterationLimit) {
-  // Both D(D) and C(D) are trusted roots.
+  // D(D) is the trust root.
   TrustStoreInMemory trust_store;
   trust_store.AddTrustAnchor(d_by_d_);
-  trust_store.AddTrustAnchor(c_by_d_);
 
-  // Certs B(C), and C(D) are all supplied.
+  // Certs B(C) and C(D) are supplied.
   CertIssuerSourceStatic sync_certs;
   sync_certs.AddCert(b_by_c_);
   sync_certs.AddCert(c_by_d_);
@@ -481,10 +482,21 @@ TEST_F(PathBuilderMultiRootTest, TestIterationLimit) {
       path_builder.SetIterationLimit(5);
     }
 
+    base::HistogramTester histogram_tester;
     path_builder.Run();
 
     EXPECT_EQ(!insufficient_limit, result.HasValidPath());
     EXPECT_EQ(insufficient_limit, result.exceeded_iteration_limit);
+
+    if (insufficient_limit) {
+      EXPECT_THAT(histogram_tester.GetAllSamples(
+                      "Net.CertVerifier.PathBuilderIterationCount"),
+                  ElementsAre(base::Bucket(/*sample=*/2, /*count=*/1)));
+    } else {
+      EXPECT_THAT(histogram_tester.GetAllSamples(
+                      "Net.CertVerifier.PathBuilderIterationCount"),
+                  ElementsAre(base::Bucket(/*sample=*/3, /*count=*/1)));
+    }
   }
 }
 

tools/metrics/histograms/histograms.xml

@@ -68535,6 +68535,16 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="Net.CertVerifier.PathBuilderIterationCount"
+    expires_after="2020-07-01">
+  <owner>mattm@chromium.org</owner>
+  <owner>net-dev@chromium.org</owner>
+  <summary>
+    When using the builtin cert verifier, records the number of iterations taken
+    during path building for each attempted verification.
+  </summary>
+</histogram>
+
 <histogram name="Net.CertVerifier_First_Job_Latency" units="ms"
     expires_after="M83">
   <owner>mattm@chromium.org</owner>