Gracefully handle an asynchronous write disconnect when an SSL read is pending

Gracefully handle an asynchronous write disconnect when an SSL read is pending

BUG=249848
R=wtc

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

[Ryan Sleevi, 2013-06-14 20:33:43]

wtc: PTAL

[wtc, 2013-06-14 21:45:07]

Patch set 1 LGTM.

https://codereview.chromium.org/17105003/diff/1/net/base/nss_memio.c
File net/base/nss_memio.c (right):

https://codereview.chromium.org/17105003/diff/1/net/base/nss_memio.c#newcode264
net/base/nss_memio.c:264: }

Should we also check secret->readbuf.last_err here?

If not, this asymmetry between memio_Recv and memio_Send is worth a comment.

I guess memio_Send probably doesn't need to report read error, especially
because read error cannot be reported until all the buffered data have been
consumed. On the other hand, memio_Recv needs to help report write error because
the upper layer may not have more data to write. I think we can put this comment
in memio_Recv.

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

https://codereview.chromium.org/17105003/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_unittest.cc:260: // Sets the the next Read() call
to return |error|.

With the removal of the "have_read_error_ = false;" in Read(), this comment
needs to say something like:

  // Sets the next Read() and all future calls to return |error|.

(By the way, there are two "the"s.

Similarly for the comment for SetNextWriteError.

https://codereview.chromium.org/17105003/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_unittest.cc:988: // Note: This will hang if this
bug has regressed. Simply checking that

"Hang" usually implies an idle wait. This bug is an infinite loop, right?

https://codereview.chromium.org/17105003/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_unittest.cc:997: // OpenSSL treats any errors as a
simple EOF.

These comments are actually referring to SSLClientSocketNSS and
SSLClientSocketOpenSSL, right?

[commit-bot: I haz the power, 2013-06-14 22:09:25]

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

[commit-bot: I haz the power, 2013-06-15 04:14:24]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-06-17 16:58:20]

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

[commit-bot: I haz the power, 2013-06-17 18:01:45]

Change committed as 206751