On OpenSSL/Android, only schedule transport socket reads when the transport socket buffer is empty.

On OpenSSL/Android, only schedule transport socket reads when the transport socket buffer is empty.

Rather then attempting to constantly keep the transport buffer
(transport_bio_) fully saturated by frequently issuing socket reads, let the
OS manage the socket buffer and only read data from the underlying transport
when the transport_bio_ is empty and needs more data. This significantly
reduces the number of Read()s issued against the underlying socket in optimal
conditions.

R=wtc@chromium.org
BUG=166741
TEST=none


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173880

[Ryan Sleevi, 2012-12-19 00:22:17]

wtc: PTAL.

piatek@ is testing this now.

I describe some of the motivations in this CL and in the comments, and broader
still in the actual bug. This is not strictly correct if we care about
close_notify & graceful SSL shutdown, but we don't, and this allows us to reduce
our socket calls to the OS.

[Ryan Sleevi, 2012-12-19 00:23:45]

https://codereview.chromium.org/11647005/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_openssl.cc (right):

https://codereview.chromium.org/11647005/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_openssl.cc:1055: }
Note that the only time this is called is line 985 (in DoTransportIO). I could
move this check into there, if you think it will be clearer, but I put it here
so that it can be constrasted with the description on line 1057

[wtc, 2012-12-19 00:51:21]

Patch set 1 LGTM.

https://codereview.chromium.org/11647005/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_openssl.cc (right):

https://codereview.chromium.org/11647005/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_openssl.cc:1058: // implementation, it has the
downside of resulting in frequent reads:

As long as the extra received data is not discarded, this
implementation is also correct and has better performance.
I would definitely do this to minimize the number of system
calls. So doing it this way isn't an issue.

https://codereview.chromium.org/11647005/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_openssl.cc:1064: // traffic, this over-subscribed
Read()ing will not cause issues.

SSLClientSocket does not support this usage. The Disconnect
method calls the Disconnect method of the transport socket.

[Ryan Sleevi, 2012-12-19 00:53:39]

https://codereview.chromium.org/11647005/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_openssl.cc (right):

https://codereview.chromium.org/11647005/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_openssl.cc:1064: // traffic, this over-subscribed
Read()ing will not cause issues.
On 2012/12/19 00:51:21, wtc wrote:
> 
> SSLClientSocket does not support this usage. The Disconnect
> method calls the Disconnect method of the transport socket.

Correct. That's why I went this route, but added it as a "Known Issue" in the
event our socket implementation ever needed to change.

[commit-bot: I haz the power, 2012-12-19 01:22:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/11647005/1

[commit-bot: I haz the power, 2012-12-19 02:54:51]

Retried try job too often on win_rel for step(s) sync_unit_tests

[commit-bot: I haz the power, 2012-12-19 05:48:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/11647005/1

[commit-bot: I haz the power, 2012-12-19 09:16:36]

Change committed as 173880