Perform ClientHello padding if the field trial is enabled

net/socket/ssl_client_socket_openssl.cc

Index: net/socket/ssl_client_socket_openssl.cc
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index c4af9575f1e447939f3e76fbce7f43fd4896cf92..24e3ad687e5b9524a4f1dafde6715aa674aa7e7c 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -474,12 +474,20 @@ int SSLClientSocketOpenSSL::Connect(const CompletionCallback& callback) {
   if (rv != OK) {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_SSL_CONNECT, rv);
     UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error", std::abs(rv));
+    if (ssl_config_.fastradio_padding_eligible)
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error_ClientPadding",
+                                  std::abs(rv));

[davidben, 2015/02/04 01:04:47]

It looks like SSL_Connection_Error metrics were added specifically for this
experiment. We already have a pile of connection metrics here:

https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/ssl_cli...

The ones here apply only to OpenSSL, but some of our ports still ship NSS.
They're also duplicated a lot. If I recall, the histogram macros expand out to a
decent amount of code with a static thing to cache them and everything. We don't
actually want to be duplicating them in lots of places like that.

This also is more directly comparable with the _Latency metrics. Not every
SSLClientSocket goes through SSLConnectJob.

We also already have error metrics at a higher level here:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/lo...

Both sets are also already keyed on Google, so that might avoid your extra
boolean. Though it would also somewhat assume that the higher-level check is
compatible with the lower-level check. I'll defer to Ryan there.

[jeremyim, 2015/02/04 17:26:32]

We were planning on using the latency metrics from ssl_client_socket_pool.cc and
we were only adding these metrics to measure error rates. We didn't have
confidence that the higher level main/sub frame error rates would give us
sufficient visibility into whether the padding introduced higher error rates.

I think we are ok with the metrics only being in OpenSSL since NSS doesn't
support fastradio padding.

[davidben, 2015/02/04 17:44:36]

SSL_Connection_Error is extremely generic and isn't specific to
OpenSSL/BoringSSL. It's very confusing to have metrics (unless they're so
specific so as to be measuring implementation quirks like
Net.SSLSessionVersionMatch) only be available on one side and not the other.

In fact, the immediate cause for my bringing this up is I noticed that metric
and immediately went to compare it before and after the NSS -> BoringSSL switch
across platforms. (It turns out the metric was newly added, so it wouldn't have
been helpful anyway. But there are still NSS -> BoringSSL switches to come.)

There's also still the other concerns like binary size from expanding the
histogram macro in so many places and being able to directly compare
SSL_Connection_Error and SSL_Connection_Latency.

     return rv;
   }
 
   // Set SSL to client mode. Handshake happens in the loop below.
   SSL_set_connect_state(ssl_);
 
+  // Enable fastradio padding.
+  SSL_enable_fastradio_padding(ssl_,
+                               ssl_config_.enable_fastradio_padding &&
+                                   ssl_config_.fastradio_padding_eligible);
+
   GotoState(STATE_HANDSHAKE);
   rv = DoHandshakeLoop(OK);
   if (rv == ERR_IO_PENDING) {
@@ -487,6 +495,9 @@ int SSLClientSocketOpenSSL::Connect(const CompletionCallback& callback) {
   } else {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_SSL_CONNECT, rv);
     UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error", std::abs(rv));
+    if (ssl_config_.fastradio_padding_eligible)
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error_ClientPadding",
+                                  std::abs(rv));
     if (rv < OK)
       OnHandshakeCompletion();
   }
@@ -1367,6 +1378,9 @@ void SSLClientSocketOpenSSL::OnHandshakeIOComplete(int result) {
   if (rv != ERR_IO_PENDING) {
     net_log_.EndEventWithNetErrorCode(NetLog::TYPE_SSL_CONNECT, rv);
     UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error", std::abs(rv));
+    if (ssl_config_.fastradio_padding_eligible)
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Net.SSL_Connection_Error_ClientPadding",
+                                  std::abs(rv));
     DoConnectCallback(rv);
   }
 }