https://codereview.chromium.org/2411033003

I tested it on a simple test (open TheVerge.com, wait 30s), and wow, I can only see 15 buffers created from SocketBIOAdapter::BIORead (255 KiB total) where before we had 104 send /receive buffers (1768 KiB total). And I don't see a single byte coming from SocketBIOAdapter::BIOWrite - i.e. all write buffers were freed.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3418e81ff9636db1d9e0de7c60e002788b51f5ea

commit 3418e81ff9636db1d9e0de7c60e002788b51f5ea
Author: davidben <davidben@chromium.org>
Date: Wed Oct 19 00:09:45 2016

Drop buffers in idle SSLClientSockets (and SSLServerSockets).

As part of this, significantly refactor how both classes handle the
transport.  BoringSSL's SSL stack expects the transport as a BIO. BIOs
follow the UNIX-style non-blocking I/O. Our net stack, however, is based
on asynchronous callbacks. Both socket had some complex code to bridge
the two I/O models.

Factor all this code into a SocketBIOAdapter. This takes a StreamSocket
and returns a BIO which acts on the socket. The UNIX model assumes
external knowledge of when to retry operations (usually a select loop),
so the SocketBIOAdapter has a delegate interface which signals
OnReadReady and OnWriteReady. By being factored out, it can also be
independently unit-tested, which is handy.

It also implements the weird hack we have where write errors route into
read errors. In doing so, it fixes a case where whether that error was
routed correctly depended on whether transport Writes failed
synchronously (if BufferSend synchronously succeeded in DoWriteLoop,
DoPayloadRead wasn't run, but it was run if asynchronous). This requires
tweaking a test expectation.

This removes uses of the bizarre "zero-copy" BIO pair stuff which can
now be removed from BoringSSL. It also unifies client and server I/O
handling which fixes transport error mapping on the
server. (Accordingly, some server test expectations also needed fixes.)

Oh, and on top of all this, actually drop the buffers when not needed as this
was sort of the point of this exercise. Having the code factored out makes this
a lot simpler to reason about and avoids adding even more complexity to BIO
pairs.

BUG= 652456 , 399455 

Review-Url: https://codereview.chromium.org/2411033003
Cr-Commit-Position: refs/heads/master@{#426097}

[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/net.gypi
[add] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/socket_bio_adapter.cc
[add] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/socket_bio_adapter.h
[add] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/socket_bio_adapter_unittest.cc
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/socket_test_util.cc
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/socket_test_util.h
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/ssl_server_socket_impl.cc
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/socket/ssl_server_socket_unittest.cc
[modify] https://crrev.com/3418e81ff9636db1d9e0de7c60e002788b51f5ea/net/ssl/openssl_ssl_util.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ea6b17294f2ce1ea405e020ca07d2e048a6a083

commit 8ea6b17294f2ce1ea405e020ca07d2e048a6a083
Author: davidben <davidben@chromium.org>
Date: Tue Mar 07 23:53:50 2017

Only pump SSL_read extra times when there is data available synchronously.

If SSL_read is run an extra time, it will implicitly trigger a read from
the transport. However, if we've already returned data, it's possible
the consumer had no need for this extra data to begin with. The end
result is that, in this situation, we would leave buffers allocated and
sockets in the select loop when we shouldn't.

Prior to the SocketBIOAdapter change, we wouldn't read from the
transport as that would be nothing driving the DoTransportIO loop
(though if writes were driving the transport we would read, which was
weird). This restores the first behavior.

BUG= 652456 

Review-Url: https://codereview.chromium.org/2736103002
Cr-Commit-Position: refs/heads/master@{#455297}

[modify] https://crrev.com/8ea6b17294f2ce1ea405e020ca07d2e048a6a083/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/8ea6b17294f2ce1ea405e020ca07d2e048a6a083/net/socket/ssl_client_socket_unittest.cc

+ mariakhomenko@ and dskiba@: David's followup CL which landed today (#5) solved the majority case of BIO read buffers. 

We'd missed that some cases idle sockets still had the buffers around in the original CL. With this, the only retained SSL read buffers that should be retained are the ones that are actually reading (of course, I said that last time too...), like long-polling HTTP/1.1 requests, WebSockets, or HTTP/2 sessions. Those would need ReadIfReady which is a different bug.