When using NSS, only schedule transport socket reads when the transport buffer is empty.

net/socket/ssl_client_socket_nss.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.
 
 // This file includes code SSLClientSocketNSS::DoVerifyCertComplete() derived
 // from AuthCertificateCallback() in
 // mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp.
 
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
@@ skipping 2083 matching lines @@
   }
   return network_moved;
 }
 
 int SSLClientSocketNSS::Core::BufferRecv() {
   DCHECK(OnNSSTaskRunner());
 
   if (transport_recv_busy_)
     return ERR_IO_PENDING;
 
+  // Determine how much was requested from |nss_bufs_| by NSS but that was
+  // unavailable.

[wtc, 2012/12/20 02:41:22]

This comment is also not very clear. It could be interpreted
to mean:
  if NSS requests 5 bytes but nss_bufs_ has only 2 bytes,
  then memio_GetReadRequest should return 3 because 3 bytes
  are unavailable.

[Ryan Sleevi, 2012/12/20 02:55:58]

That is correct, because that is what it does.

+  int requested = memio_GetReadRequest(nss_bufs_);
+  if (requested == 0) {
+    // This is not a perfect match of error codes, as no operation is
+    // actually pending. However, returning 0 would be interpreted as a
+    // possible sign of EOF, which is also an inappropriate match.
+    return ERR_IO_PENDING;
+  }
+
+  // Known Issue: While only reading |requested| bytes is the correct

[wtc, 2012/12/20 02:41:22]

Don't add this comment. This isn't an issue. The current code
is also correct because it buffers any data not consumed by
the higher layer.

+  // implementation, it has the downside of resulting in frequent reads:
+  // One read for the SSL record header (~5 bytes) and one read for the SSL
+  // record body. Rather than issuing these small reads to the underlying
+  // socket (and constantly allocating new IOBuffers), a single Read()
+  // request to fill |nss_bufs_| is issued. As long as an SSL client socket
+  // cannot be gracefully shutdown (eg: via SSL close alerts) and then
+  // re-used for non-SSL traffic, this over-subscribed Read()ing will not
+  // cause issues.

[wtc, 2012/12/20 02:41:22]

The SSLClientSocketNSS constructor is documented to take ownership of its
transport_socket argument, so the scenario of reusing the transport socket for
non-SSL traffic after SSL is gracefully shut down cannot happen. We can update
the comment in ssl_client_socket_nss.h to state that explicitly.

   char* buf;
   int nb = memio_GetReadParams(nss_bufs_, &buf);
   int rv;
   if (!nb) {
     // buffer too full to read into, so no I/O possible at moment
     rv = ERR_IO_PENDING;
   } else {
     scoped_refptr<IOBuffer> read_buffer(new IOBuffer(nb));
     if (OnNetworkTaskRunner()) {
       rv = DoBufferRecv(read_buffer, nb);
@@ skipping 1335 matching lines @@
   EnsureThreadIdAssigned();
   base::AutoLock auto_lock(lock_);
   return valid_thread_id_ == base::PlatformThread::CurrentId();
 }
 
 ServerBoundCertService* SSLClientSocketNSS::GetServerBoundCertService() const {
   return server_bound_cert_service_;
 }
 
 }  // namespace net