Implement SSL server socket over OpenSSL.

Implement SSL server socket over OpenSSL.

1) Mixed ssl_server_socket_nss.cc and ssl_client_socket_openssl.cc.
2) Moved common functions into openssl_util.cc.
3) Enabled SslServerSocketTest when USE_OPENSSL is defined.

BUG=

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

[byungchul, 2014-05-09 16:35:02]

Please review this.

[Ryan Sleevi, 2014-05-12 01:17:47]

Going to ask wtc to take primary review on this.

Adding haavard as FYI.

[Ryan Sleevi, 2014-05-12 01:35:25]

https://codereview.chromium.org/274783002/diff/1/net/net.gyp
File net/net.gyp (right):

https://codereview.chromium.org/274783002/diff/1/net/net.gyp#newcode245
net/net.gyp:245: 'socket/openssl_util.h',
You also need to update net/BUILD.gn

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.cc
File net/socket/openssl_util.cc (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.cc#n...
net/socket/openssl_util.cc:129: // any residual codes left on the error stack.
This documentation belongs in the header.

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.cc#n...
net/socket/openssl_util.cc:130: int MapOpenSSLError(int err, const
crypto::OpenSSLErrStackTracer& tracer) {
Definition order should match declaration order.

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.h
File net/socket/openssl_util.h (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.h#ne...
net/socket/openssl_util.h:6: // ssl_server_socket_openssl.cc to share common
functions of OpenSSL.
This documentation is inappropriate, as it's a layering violation. You should
describe how it can/should be used, not how it should be used.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
File net/socket/ssl_server_socket_openssl.cc (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:25: #endif
Remove this

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:31: bool g_openssl_server_sockets_init =
false;
This is not thread-safe. Use LazyInstance for thread-safe initialization.

Regardless, it's safe to call crypto::EnsureOpenSSLInit() multiple times, which
is already using a thread-safe LazyInstance, so just do that here.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:49: << "EnableSSLServerSockets() has not
been called yet!";
Remove this

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:273: }
This strikes me as a bad state machine (perhaps in the NSS socket as well). The
client sockets handle this better.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:295: if (rv != ERR_IO_PENDING) {
better to be

if (rv == ERR_IO_PENDING)
  return
net_log_....

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:433: CHECK_EQ(result, ret);
crbug.com/335557 was resolved. This code should be removed in the client socket
(and shouldn't exist here)

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:540: DCHECK(next_handshake_state_ ==
STATE_NONE);
DCHECK_EQ

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:584: c.Run(rv > OK ? OK : rv);
Use base::ResetAndReturn() here (base/callback_helpers.h)

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:597: c.Run(rv);
base::ResetAndReturn here

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:610: c.Run(rv);
base::ResetAndReturn here

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
File net/socket/ssl_server_socket_unittest.cc (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_unittest.cc:364: #if defined(USE_NSS) ||
defined(USE_OPENSSL) || defined(OS_WIN) || defined(OS_MACOSX)
This whole #ifdef should be removed if it's now supported for OpenSSL.

[wtc, 2014-05-12 18:08:55]

Byungchul: I will wait for your new patch set that addresses Ryan's review
comments.

The CL's description seems to have a typo:

  1) Mixed ssl_server_socket_nss.cc and ssl_client_socket_openssl.cc.

I think "ssl_server_socket_nss.cc" should be "ssl_server_socket_openssl.cc".

[byungchul, 2014-05-12 18:25:22]

PTAL

https://codereview.chromium.org/274783002/diff/1/net/net.gyp
File net/net.gyp (right):

https://codereview.chromium.org/274783002/diff/1/net/net.gyp#newcode245
net/net.gyp:245: 'socket/openssl_util.h',
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> You also need to update net/BUILD.gn

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.cc
File net/socket/openssl_util.cc (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.cc#n...
net/socket/openssl_util.cc:129: // any residual codes left on the error stack.
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> This documentation belongs in the header.

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.cc#n...
net/socket/openssl_util.cc:130: int MapOpenSSLError(int err, const
crypto::OpenSSLErrStackTracer& tracer) {
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> Definition order should match declaration order.

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.h
File net/socket/openssl_util.h (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/openssl_util.h#ne...
net/socket/openssl_util.h:6: // ssl_server_socket_openssl.cc to share common
functions of OpenSSL.
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> This documentation is inappropriate, as it's a layering violation. You should
> describe how it can/should be used, not how it should be used.

Removed though nss_ssl_util.h also has this comment.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
File net/socket/ssl_server_socket_openssl.cc (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:25: #endif
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> Remove this

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:31: bool g_openssl_server_sockets_init =
false;
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> This is not thread-safe. Use LazyInstance for thread-safe initialization.
> 
> Regardless, it's safe to call crypto::EnsureOpenSSLInit() multiple times,
which
> is already using a thread-safe LazyInstance, so just do that here.

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:49: << "EnableSSLServerSockets() has not
been called yet!";
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> Remove this

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:273: }
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> This strikes me as a bad state machine (perhaps in the NSS socket as well).
The
> client sockets handle this better.

These code here and below are imported from ssl_server_socket_nss.cc, and tested
for a while in the field. May I add a TODO to address your comment here and nss
in following CL?

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:295: if (rv != ERR_IO_PENDING) {
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> better to be
> 
> if (rv == ERR_IO_PENDING)
>   return
> net_log_....

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:433: CHECK_EQ(result, ret);
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> crbug.com/335557 was resolved. This code should be removed in the client
socket
> (and shouldn't exist here)

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:540: DCHECK(next_handshake_state_ ==
STATE_NONE);
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> DCHECK_EQ

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:584: c.Run(rv > OK ? OK : rv);
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> Use base::ResetAndReturn() here (base/callback_helpers.h)

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:597: c.Run(rv);
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> base::ResetAndReturn here

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_openssl.cc:610: c.Run(rv);
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> base::ResetAndReturn here

Done.

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
File net/socket/ssl_server_socket_unittest.cc (right):

https://codereview.chromium.org/274783002/diff/1/net/socket/ssl_server_socket...
net/socket/ssl_server_socket_unittest.cc:364: #if defined(USE_NSS) ||
defined(USE_OPENSSL) || defined(OS_WIN) || defined(OS_MACOSX)
On 2014/05/12 01:35:25, Ryan Sleevi wrote:
> This whole #ifdef should be removed if it's now supported for OpenSSL.

Done.

[byungchul, 2014-05-12 18:51:12]

On 2014/05/12 18:08:55, wtc wrote:
> Byungchul: I will wait for your new patch set that addresses Ryan's review
> comments.
> 
> The CL's description seems to have a typo:
> 
>   1) Mixed ssl_server_socket_nss.cc and ssl_client_socket_openssl.cc.
> 
> I think "ssl_server_socket_nss.cc" should be "ssl_server_socket_openssl.cc".

It's correct. I mean "ssl_server_socket_openssl is implemented by mixing
ssl_server_socket_nss.cc" should be "ssl_server_socket_openssl.cc".

[byungchul, 2014-05-14 20:35:31]

Wan-Teh, PTAL, uploaded new patch set addressing Ryan's comments.

[wtc, 2014-05-16 03:57:17]

Patch set 2 LGTM. Please commit the CL after making the suggested changes.

1. Error reporting could be improved. You can do that in a follow-up CL.

2. I suggest writing the 1) and 2) list items in the CL's description as
follows:


1) Implemented SSLServerSocketOpenSSL by mixing ssl_server_socket_nss.cc and
ssl_client_socket_openssl.cc.

2) Moved common functions between ssl_server_socket_openssl.cc and
ssl_client_socket_openssl.cc into openssl_util.cc.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
File net/socket/ssl_server_socket_nss.cc (right):

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_nss.cc:502: // SSLClientSocketNSS::OnSendComplete()
which handles it better.

Nit: it's not clear what "Refactor this with
SSLClientSocketNSS::OnSendComplete()". Do you mean "Copy the state machine of
SSLClientSocketNSS::OnSendComplete()"?

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_nss.cc:789: // up front.

Nit: We may want to delete this comment and the comment on lines 799-800. They
are clearer with the original code.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
File net/socket/ssl_server_socket_openssl.cc (right):

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:15: #include "net/socket/openssl_util.h"

Nit: There are two headers named openssl_util.h. It may be good to rename this
one "openssl_ssl_util.h".

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:18: #define GotoState(s)
next_handshake_state_ = s

Nit: I seem to remember the GotoState macro was added so that it could be
conditionally defined to emit a log message. If the macro is not conditionally
defined, we can do without this macro.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:23: // No-op because
CreateSSLServerSocket() calls crypto::EnsureOpenSSLInit().

Nit: add a period (.) at the end of the sentence.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:31: // It is safe and cheap to call
crypto::EnsureOpenSSLInit() multiple times.

Please move this comment to the header file that declares
crypto::EnsureOpenSSLInit().

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:82: int rv = ERR_UNEXPECTED;

Nit: ideally Init() should return the int error code that we need to return
here.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:114: return ERR_FAILED;

Please try to get the error code of SSL_export_keying_material and map it to an
int net::ERR_ error code. If that's too hard, you can add a TODO comment. Also,
please search in net/base/net_error_list.h and see if there is an existing error
code that we can use. Almost any error code is more informative than ERR_FAILED.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:120: return ERR_NOT_IMPLEMENTED;

Should we also call NOTIMPLEMENTED()?

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:185: return completed_handshake_;

Should we also test && transport_socket_->IsConnected()? I seem to remember we
removed that test to fix a bug.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:225: return false;

This probably should call NOTIMPLEMENTED().

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
File net/socket/ssl_server_socket_openssl.h (right):

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.h:98: virtual bool Init();

Do you know why Init() is a virtual method? Are there subclasses of
SSLClientSocketOpenSSL?

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.h:123: // error writing to the transport
socket. A value of OK indicates no error.

Nit: a error => an error

[byungchul, 2014-05-16 17:49:59]

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
File net/socket/ssl_server_socket_nss.cc (right):

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_nss.cc:502: // SSLClientSocketNSS::OnSendComplete()
which handles it better.
On 2014/05/16 03:57:17, wtc wrote:
> 
> Nit: it's not clear what "Refactor this with
> SSLClientSocketNSS::OnSendComplete()". Do you mean "Copy the state machine of
> SSLClientSocketNSS::OnSendComplete()"?

Done.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_nss.cc:789: // up front.
On 2014/05/16 03:57:17, wtc wrote:
> 
> Nit: We may want to delete this comment and the comment on lines 799-800. They
> are clearer with the original code.

Done.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
File net/socket/ssl_server_socket_openssl.cc (right):

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:15: #include "net/socket/openssl_util.h"
On 2014/05/16 03:57:17, wtc wrote:
> 
> Nit: There are two headers named openssl_util.h. It may be good to rename this
> one "openssl_ssl_util.h".

Done.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:18: #define GotoState(s)
next_handshake_state_ = s
On 2014/05/16 03:57:17, wtc wrote:
> 
> Nit: I seem to remember the GotoState macro was added so that it could be
> conditionally defined to emit a log message. If the macro is not conditionally
> defined, we can do without this macro.

Many codes are shared with client socket and nss socket. To keep same pattern
would make things more readable. client socket still have GotoState() with
logging.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:23: // No-op because
CreateSSLServerSocket() calls crypto::EnsureOpenSSLInit().
On 2014/05/16 03:57:17, wtc wrote:
> 
> Nit: add a period (.) at the end of the sentence.

Already there :)

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:31: // It is safe and cheap to call
crypto::EnsureOpenSSLInit() multiple times.
On 2014/05/16 03:57:17, wtc wrote:
> 
> Please move this comment to the header file that declares
> crypto::EnsureOpenSSLInit().

Done.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:82: int rv = ERR_UNEXPECTED;
On 2014/05/16 03:57:17, wtc wrote:
> 
> Nit: ideally Init() should return the int error code that we need to return
> here.

Done.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:114: return ERR_FAILED;
On 2014/05/16 03:57:17, wtc wrote:
> 
> Please try to get the error code of SSL_export_keying_material and map it to
an
> int net::ERR_ error code. If that's too hard, you can add a TODO comment.
Also,
> please search in net/base/net_error_list.h and see if there is an existing
error
> code that we can use. Almost any error code is more informative than
ERR_FAILED.

Done. Copied it from ssl_client_socket_openssl.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:120: return ERR_NOT_IMPLEMENTED;
On 2014/05/16 03:57:17, wtc wrote:
> 
> Should we also call NOTIMPLEMENTED()?

Done though nobody calls this.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:185: return completed_handshake_;
On 2014/05/16 03:57:17, wtc wrote:
> 
> Should we also test && transport_socket_->IsConnected()? I seem to remember we
> removed that test to fix a bug.

Not sure. Left a TODO to you.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.cc:225: return false;
On 2014/05/16 03:57:17, wtc wrote:
> 
> This probably should call NOTIMPLEMENTED().

Done.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
File net/socket/ssl_server_socket_openssl.h (right):

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.h:98: virtual bool Init();
On 2014/05/16 03:57:17, wtc wrote:
> 
> Do you know why Init() is a virtual method? Are there subclasses of
> SSLClientSocketOpenSSL?

Done.

https://codereview.chromium.org/274783002/diff/20001/net/socket/ssl_server_so...
net/socket/ssl_server_socket_openssl.h:123: // error writing to the transport
socket. A value of OK indicates no error.
On 2014/05/16 03:57:17, wtc wrote:
> 
> Nit: a error => an error

Already "an error".

[byungchul, 2014-05-16 17:56:15]

PTAL. Addressing comments needed a bit changes, even in ssl client sockets.

[wtc, 2014-05-16 19:36:14]

Patch set 6 LGTM.

https://codereview.chromium.org/274783002/diff/100001/crypto/openssl_util.h
File crypto/openssl_util.h (right):

https://codereview.chromium.org/274783002/diff/100001/crypto/openssl_util.h#n...
crypto/openssl_util.h:85: // before any other OpenSSL functions though it is
safe and cheap to called this

Nit: called => call

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
File net/socket/ssl_server_socket_nss.cc (right):

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
net/socket/ssl_server_socket_nss.cc:251: // TODO(wtc): Check
transport_socket_->IsConnected() as well.

This comment should say: find out if we should check
transport_socket_->IsConnected() as well.

I found that this code was originally added in

    http://codereview.chromium.org/7054010

and it hasn't changed since then.

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
File net/socket/ssl_server_socket_openssl.cc (right):

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
net/socket/ssl_server_socket_openssl.cc:193: // TODO(wtc): Check
transport_socket_->IsConnected() as well.

This comment should say "Find out if we should check transport_socket..."

[byungchul, 2014-05-16 19:50:32]

https://codereview.chromium.org/274783002/diff/100001/crypto/openssl_util.h
File crypto/openssl_util.h (right):

https://codereview.chromium.org/274783002/diff/100001/crypto/openssl_util.h#n...
crypto/openssl_util.h:85: // before any other OpenSSL functions though it is
safe and cheap to called this
On 2014/05/16 19:36:14, wtc wrote:
> 
> Nit: called => call

Done.

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
File net/socket/ssl_server_socket_nss.cc (right):

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
net/socket/ssl_server_socket_nss.cc:251: // TODO(wtc): Check
transport_socket_->IsConnected() as well.
On 2014/05/16 19:36:14, wtc wrote:
> 
> This comment should say: find out if we should check
> transport_socket_->IsConnected() as well.
> 
> I found that this code was originally added in
> 
>     http://codereview.chromium.org/7054010
> 
> and it hasn't changed since then.

Done.

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
File net/socket/ssl_server_socket_openssl.cc (right):

https://codereview.chromium.org/274783002/diff/100001/net/socket/ssl_server_s...
net/socket/ssl_server_socket_openssl.cc:193: // TODO(wtc): Check
transport_socket_->IsConnected() as well.
On 2014/05/16 19:36:14, wtc wrote:
> 
> This comment should say "Find out if we should check transport_socket..."

Done.

[byungchul, 2014-05-16 19:51:11]

The CQ bit was checked by byungchul@chromium.org

[commit-bot: I haz the power, 2014-05-16 19:52:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/byungchul@chromium.org/274783002/120001

[byungchul, 2014-05-16 23:20:02]

The CQ bit was checked by byungchul@chromium.org

[commit-bot: I haz the power, 2014-05-16 23:32:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/byungchul@chromium.org/274783002/120001

[commit-bot: I haz the power, 2014-05-17 16:02:10]

Change committed as 271218