Metric & meta-metric for CECPQ1 handshake latency.

net/socket/ssl_client_socket_pool.cc

Index: net/socket/ssl_client_socket_pool.cc
diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc
index e60c880e9f3f43b0a1a7af68c328daf52d5703a9..a6b24d1712172d3b4a2db3f0ee5bdc958570fde6 100644
--- a/net/socket/ssl_client_socket_pool.cc
+++ b/net/socket/ssl_client_socket_pool.cc
@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/feature_list.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
@@ -30,6 +31,13 @@
 
 namespace net {
 
+namespace {
+#if !defined(OS_NACL)

[davidben, 2016/07/29 12:26:58]

#include "build/build_config.h" (assuming the comment below doesn't just lose
the OS_NACL bits)

[mab, 2016/07/29 22:47:08]

done

+const base::Feature kPostQuantumExperiment{"SSLPostQuantumExperiment",
+                                           base::FEATURE_DISABLED_BY_DEFAULT};

[davidben, 2016/07/29 12:26:58]

Having two base::Features for the same thing seems a little wonky. Given it's
const and we don't allow static initializers, it clearly works, but I'd feel a
little better if we hid this behind, I dunno, a static
SSLClientSocket::IsPostQuantumExperimentEnabled().

Also means we probably don't need the OS_NACL ifdef everywhere. (I don't believe
NaCl can ever hit those hosts. Also it probably doesn't even send UMA to the
same bucket, if at all. It's for Chromoting.)

Tangentially, we should fix the part where OS_NACL doesn't initialize a
base::FeatureList...

[mab, 2016/07/29 22:47:08]

You're right, and it's even expressly prohibited to have multiple instances of
the same base::Feature.  Done.

+#endif
+}  // namespace
+
 SSLSocketParams::SSLSocketParams(
     const scoped_refptr<TransportSocketParams>& direct_params,
     const scoped_refptr<SOCKSSocketParams>& socks_proxy_params,
@@ -373,6 +381,7 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
 
     const char *str, *cipher_str, *mac_str;
     bool is_aead;
+    bool is_cecpq1 = false;
     SSLCipherSuiteToStrings(&str, &cipher_str, &mac_str, &is_aead,
                             cipher_suite);
     // UMA_HISTOGRAM_... macros cache the Histogram instance and thus only work
@@ -384,7 +393,7 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
       UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_KeyExchange.ECDHE",
                                   ssl_info.key_exchange_info);
     } else if (strncmp(str, "CECPQ1_", 7) == 0) {
-      // Nothing.
+      is_cecpq1 = true;
     } else {
       DCHECK_EQ(0, strcmp(str, "RSA"));
     }
@@ -427,6 +436,27 @@ int SSLConnectJob::DoSSLConnectComplete(int result) {
                                    base::TimeDelta::FromMilliseconds(1),
                                    base::TimeDelta::FromMinutes(1),
                                    100);
+#if !defined(OS_NACL)

[agl, 2016/07/29 00:00:29]

(nit: I would have a blank line above this one.)

+        // These are hosts that we expect to always offer CECPQ1.  Connections
+        // to them, whether or not this browser is in the experiment group, form
+        // the basis of our comparisons.
+        bool cecpq1_supported =
+            (host == "play.google.com" || host == "checkout.google.com" ||
+             host == "wallet.google.com");
+        if (cecpq1_supported) {
+          UMA_HISTOGRAM_CUSTOM_TIMES(
+              "Net.SSL_Connection_Latency_PostQuantumSupported_Full_Handshake",
+              connect_duration, base::TimeDelta::FromMilliseconds(1),
+              base::TimeDelta::FromMinutes(1), 100);
+          if (base::FeatureList::IsEnabled(kPostQuantumExperiment)) {
+            // But don't trust that these hosts offer CECPQ1: make sure.  If
+            // we're doing everything right on the server side, |is_cecpq1|
+            // should always be true if we get here.
+            UMA_HISTOGRAM_BOOLEAN("Net.SSL_Connection_PostQuantum_Negotiated",
+                                  is_cecpq1);
+          }
+        }
+#endif
       }
     }
   }