Add UMA histogram for Expect-CT header processing

net/http/transport_security_state.cc

Index: net/http/transport_security_state.cc
diff --git a/net/http/transport_security_state.cc b/net/http/transport_security_state.cc
index 4ce0823e3653a58bf1bf5d58672d2646996a55c1..17241f1d05150a8bfda4f191a5cb303719a53459 100644
--- a/net/http/transport_security_state.cc
+++ b/net/http/transport_security_state.cc
@@ -1294,24 +1294,55 @@ void TransportSecurityState::ProcessExpectCTHeader(
     const SSLInfo& ssl_info) {
   DCHECK(CalledOnValidThread());
 
-  if (!expect_ct_reporter_)
-    return;
+  // Records the result of processing an Expect-CT header. This enum is
+  // histogrammed, so do not reorder or remove values.

[eroman, 2016/08/25 23:39:31]

Given this, can you assign values to *each* of the enums not just the first?

[estark, 2016/08/27 15:20:05]

Done.

+  enum ExpectCTHeaderResult {
+    // An Expect-CT header was received, but it had the wrong value.
+    EXPECT_CT_HEADER_BAD_VALUE = 0,
+    // The Expect-CT header was ignored because the build was old.
+    EXPECT_CT_HEADER_BUILD_NOT_TIMELY,
+    // The Expect-CT header was ignored because the certificate did not chain to
+    // a public root.
+    EXPECT_CT_HEADER_PRIVATE_ROOT,
+    // The Expect-CT header was ignored because CT compliance details were
+    // unavailable.
+    EXPECT_CT_HEADER_COMPLIANCE_DETAILS_UNAVAILABLE,
+    // The request satisified the Expect-CT compliance policy, so no action was
+    // taken.
+    EXPECT_CT_HEADER_COMPLIED,
+    // The Expect-CT header was ignored because there was no corresponding
+    // preload list entry.
+    EXPECT_CT_HEADER_NOT_PRELOADED,
+    // The Expect-CT header was processed successfully and passed on to the
+    // delegate to send a report.
+    EXPECT_CT_HEADER_PROCESSED,
+    EXPECT_CT_HEADER_MAX

[eroman, 2016/08/25 23:39:31]

I think it is more idiomatic to use a _LAST value and set it to the exact value
of last element (to prevent needing to handle this sentinel value inside of
switch statements).

Given this is only used locally in the function though, I guess it doesn't
really matter.

[estark, 2016/08/27 15:20:05]

Done.

+  };
+
+  ExpectCTHeaderResult result = EXPECT_CT_HEADER_PROCESSED;
 
-  if (value != "preload")
-    return;
-
-  if (!IsBuildTimely())
+  if (!expect_ct_reporter_)
     return;
 
-  if (!ssl_info.is_issued_by_known_root ||
-      !ssl_info.ct_compliance_details_available ||
-      ssl_info.ct_cert_policy_compliance ==
-          ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS) {
-    return;
+  ExpectCTState state;
+  if (value != "preload") {
+    result = EXPECT_CT_HEADER_BAD_VALUE;
+  } else if (!IsBuildTimely()) {
+    result = EXPECT_CT_HEADER_BUILD_NOT_TIMELY;
+  } else if (!ssl_info.is_issued_by_known_root) {
+    result = EXPECT_CT_HEADER_PRIVATE_ROOT;
+  } else if (!ssl_info.ct_compliance_details_available) {
+    result = EXPECT_CT_HEADER_COMPLIANCE_DETAILS_UNAVAILABLE;
+  } else if (ssl_info.ct_cert_policy_compliance ==
+             ct::CertPolicyCompliance::CERT_POLICY_COMPLIES_VIA_SCTS) {
+    result = EXPECT_CT_HEADER_COMPLIED;
+  } else if (!GetStaticExpectCTState(host_port_pair.host(), &state)) {
+    result = EXPECT_CT_HEADER_NOT_PRELOADED;
   }
 
-  ExpectCTState state;
-  if (!GetStaticExpectCTState(host_port_pair.host(), &state))
+  UMA_HISTOGRAM_ENUMERATION("Net.ExpectCTHeaderResult", result,
+                            EXPECT_CT_HEADER_MAX);
+  if (result != EXPECT_CT_HEADER_PROCESSED)
     return;
 
   expect_ct_reporter_->OnExpectCTFailed(host_port_pair, state.report_uri,