Metric & meta-metric for CECPQ1 handshake latency.

net/socket/ssl_client_socket_pool.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/ssl_client_socket_pool.h"
 
 #include <utility>
 
 #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"
 #include "base/profiler/scoped_tracker.h"
 #include "base/trace_event/trace_event.h"
 #include "base/values.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/socks_client_socket_pool.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/transport_client_socket_pool.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_info.h"
 
 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,
     const scoped_refptr<HttpProxySocketParams>& http_proxy_params,
     const HostPortPair& host_and_port,
     const SSLConfig& ssl_config,
     PrivacyMode privacy_mode,
     int load_flags,
     bool expect_spdy)
     : direct_params_(direct_params),
@@ skipping 323 matching lines @@
     UMA_HISTOGRAM_ENUMERATION("Net.SSLVersion", SSLConnectionStatusToVersion(
                                                     ssl_info.connection_status),
                               SSL_CONNECTION_VERSION_MAX);
 
     uint16_t cipher_suite =
         SSLConnectionStatusToCipherSuite(ssl_info.connection_status);
     UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_CipherSuite", cipher_suite);
 
     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
     // if the histogram name is constant, so don't generate it dynamically.
     if (strncmp(str, "DHE_", 4) == 0) {
       UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_KeyExchange.DHE",
                                   ssl_info.key_exchange_info);
     } else if (strncmp(str, "ECDHE_", 6) == 0) {
       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"));
     }
 
     if (ssl_info.handshake_type == SSLInfo::HANDSHAKE_RESUME) {
       UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Resume_Handshake",
                                  connect_duration,
                                  base::TimeDelta::FromMilliseconds(1),
                                  base::TimeDelta::FromMinutes(1),
                                  100);
@@ skipping 22 matching lines @@
                                    base::TimeDelta::FromMilliseconds(1),
                                    base::TimeDelta::FromMinutes(1),
                                    100);
       } else if (ssl_info.handshake_type == SSLInfo::HANDSHAKE_FULL) {
         UMA_HISTOGRAM_CUSTOM_TIMES("Net.SSL_Connection_Latency_Google_"
                                        "Full_Handshake",
                                    connect_duration,
                                    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
       }
     }
   }
 
   UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error", std::abs(result));
 
   if (result == OK || IsCertificateError(result)) {
     SetSocket(std::move(ssl_socket_));
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     error_response_info_.cert_request_info = new SSLCertRequestInfo;
@@ skipping 230 matching lines @@
   if (base_.CloseOneIdleSocket())
     return true;
   return base_.CloseOneIdleConnectionInHigherLayeredPool();
 }
 
 void SSLClientSocketPool::OnSSLConfigChanged() {
   FlushWithError(ERR_NETWORK_CHANGED);
 }
 
 }  // namespace net