Drop buffers in idle SSLClientSockets (and SSLServerSockets).

net/socket/socket_bio_adapter.h

Index: net/socket/socket_bio_adapter.h
diff --git a/net/socket/socket_bio_adapter.h b/net/socket/socket_bio_adapter.h
new file mode 100644
index 0000000000000000000000000000000000000000..4b21eded93e738668b4fc56ac8f394d2c1094b90
--- /dev/null
+++ b/net/socket/socket_bio_adapter.h
@@ -0,0 +1,112 @@
+// Copyright 2016 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.
+
+#ifndef NET_SOCKET_SOCKET_BIO_ADAPTER_H_
+#define NET_SOCKET_SOCKET_BIO_ADAPTER_H_
+
+#include <openssl/base.h>
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "net/base/completion_callback.h"
+#include "net/base/net_export.h"
+
+namespace net {
+
+class GrowableIOBuffer;
+class IOBuffer;
+class StreamSocket;
+
+// An adapter to convert between StreamSocket and OpenSSL BIO I/O models.

[Ryan Sleevi, 2016/10/12 23:30:27]

Could you expand a little more in this class description (which I think you did
fine in the CL description)

[davidben, 2016/10/13 23:40:13]

Done.

+class NET_EXPORT_PRIVATE SocketBIOAdapter {
+ public:
+  class Delegate {

[Ryan Sleevi, 2016/10/12 23:30:27]

Similarly, a little expansion on the Delegate (and perhaps suggested user
pattern)

[davidben, 2016/10/13 23:40:13]

Done. I kept the pattern a bit vague because it really depends on what you're
doing with the SSL*. SSL{Client,Server}Socket's pattern is more a consequence of
it mapping StreamSocket model -> OpenSSL model -> StreamSocket model, and this
class is more concerned with the first arrow.

+   public:
+    // Called after a BIO_read call failed due to an asynchronous operation. The
+    // delegate should retry any high-level operations which were blocked on a
+    // read.
+    virtual void OnReadReady() = 0;
+
+    // Called after a BIO_write call failed due to an asynchronous operation.
+    // The delegate should retry any high-level operations which were blocked on
+    // a write.
+    virtual void OnWriteReady() = 0;
+  };
+
+  // Creates a new SocketBIOAdapter for the specified socket. |socket| and
+  // |delegate| must remain valid for the lifetime of the SocketBIOAdapter.
+  SocketBIOAdapter(StreamSocket* socket,
+                   int read_buffer_size,
+                   int write_buffer_size,

[Ryan Sleevi, 2016/10/12 23:30:27]

style: Why int, instead of size_t? -
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

[davidben, 2016/10/13 23:40:13]

The entire net stack (and BIO for that matter...) uses int everywhere. I tried
to have it use size_t but it ended up casting everywhere and not actually being
safer. I think size_t-ing this ought to wait until we have a size_t story for
Read and Write. Those two basically block everything else.

+                   Delegate* delegate);
+  ~SocketBIOAdapter();
+
+  BIO* bio() { return bio_.get(); }
+
+  // Returns true if any data has been read from the underlying StreamSocket but

[Ryan Sleevi, 2016/10/12 23:30:27]

nit: StreamSocket, but

[davidben, 2016/10/13 23:40:13]

Done.

+  // not yet consumed by the BIO.
+  bool HasPendingReadData();
+
+ private:
+  int BIORead(char* out, int len);
+  void HandleSocketReadResult(int result);
+  void OnSocketReadComplete(int result);
+
+  int BIOWrite(const char* in, int len);
+  void SocketWrite();
+  void HandleSocketWriteResult(int result);
+  void OnSocketWriteComplete(int result);
+  void CallOnReadReady();
+
+  static SocketBIOAdapter* GetAdapter(BIO* bio);
+  static int BIOReadWrapper(BIO* bio, char* out, int len);
+  static int BIOWriteWrapper(BIO* bio, const char* in, int len);
+  static long BIOCtrlWrapper(BIO* bio, int cmd, long larg, void* parg);
+
+  static const BIO_METHOD kBIOMethod;

[Ryan Sleevi, 2016/10/12 23:30:27]

This can be moved fully in the .cc, can it not?

[davidben, 2016/10/13 23:40:13]

It needs to have access to the statics which, in turn, needs to have access to
BIORead and BIOWrite which, in turn, ought to be private to the
SocketBIOAdapter. We'd need the dummy friend class trick to avoid putting them
in the header, and that seemed pointless. This header:

a) Already must include BoringSSL's forward decl header for the public
SocketBIOAdapter::bio. These bits don't force any additional headers be
included.
b) Is only used in three compilation units.
c) Even if it did force an include of bio.h, each compilation unit which
includes this header would include bio.h anyway.

[davidben, 2016/10/13 23:44:08]

Oh, I guess (b) and (c) aren't true. It's in two additional compilation units
(until the SSLClientContext refactor). But regardless it doesn't induce other
headers. I don't think it makes sense to add a:

  friend class SocketBIOAdapterPrivateMethodHolder;

and to move all the private methods (but not the fields) over here.

+
+  bssl::UniquePtr<BIO> bio_;
+
+  // The pointer is non-owning so this class may be used with both
+  // ClientSocketHandles and raw StreamSockets.
+  StreamSocket* socket_;
+
+  CompletionCallback read_callback_;
+  CompletionCallback write_callback_;
+
+  // The capacity of the read buffer.
+  int read_buffer_size_;
+  // A buffer containing data from the most recent socket Read(). The buffer is
+  // deallocated when unused.
+  scoped_refptr<IOBuffer> read_buffer_;
+  // The number of bytes of read_buffer_ consumed.
+  int read_offset_;
+  // The result of the most recent socket Read(). If ERR_IO_PENDING, there is a
+  // socket Read() in progress. If another error, Read() has failed. Otherwise,
+  // it is the number of bytes in the buffer (zero if empty).
+  int read_result_;
+
+  // The capacity of the write buffer.
+  int write_buffer_size_;
+  // A ring buffer of data to be written to the transport. The offset of the
+  // buffer is the start of the ring buffer and is advanced on successful
+  // Write(). The buffer is deallocated when unused.
+  scoped_refptr<GrowableIOBuffer> write_buffer_;
+  // The number of bytes of data in write_buffer_.
+  int write_buffer_data_size_;
+  // The most recent socket Write() error. If ERR_IO_PENDING, there is a socket
+  // Write() in progress. If OK, there is no socket Write() in progress and none
+  // have failed.
+  int write_error_;
+
+  Delegate* delegate_;
+
+  base::WeakPtrFactory<SocketBIOAdapter> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(SocketBIOAdapter);
+};
+
+}  // namespace net
+
+#endif  // NET_SOCKET_SOCKET_BIO_ADAPTER_H_