In Dev build, we would like to see if SSLClientSocketNSS object...

In Dev build, we would like to see if SSLClientSocketNSS object
is Connected and Disconnected on the same thread. Wan-Teh and I had
thought it is more logical to disconnect the transport socket before
closing the NSS SSL socket (and the underlying memory buffers).

Test to see if this stops memory corruption crashes, will revert
after a Dev release.

BUG=65948
TEST=accessing google.com and gmail via https and sync'ing of 
     personal stuff.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=74717

[ramant (doing other things), 2011-02-11 03:45:18]

Hi Wan-Teh,
  Added the checks (for release build to Dev channel) that we Connect and
Disconnect on the same thread. Will remove the changes after we release it Dev
Channel.

thanks much,
raman

[wtc, 2011-02-11 22:43:50]

LGTM.  I suggest some minor changes and have some questions
for you below.

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_ns...
File net/socket/ssl_client_socket_nss.cc (right):

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_ns...
net/socket/ssl_client_socket_nss.cc:632: EnsureThreadIdAssigned();
Can you explain why you call DetachFromThread and
EnsureThreadIdAssigned here?

Why don't you call them at the beginning of Connect()?

Why do we need to call DetachFromThread()?

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_nss.h
File net/socket/ssl_client_socket_nss.h (right):

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_ns...
net/socket/ssl_client_socket_nss.h:275: // code after fixing 65948.
Nit: please say "bug 65948" or http://crbug.com/65948 instead
of just "65948".  Make the same changes to lines 170-171 above.

I think the next line (line 276) can be removed.

[ramant (doing other things), 2011-02-11 23:42:41]

Hi Wan-Teh,
  Made all the changes you had suggested.

thanks,
raman

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_ns...
File net/socket/ssl_client_socket_nss.cc (right):

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_ns...
net/socket/ssl_client_socket_nss.cc:632: EnsureThreadIdAssigned();
On 2011/02/11 22:43:50, wtc wrote:
> Can you explain why you call DetachFromThread and
> EnsureThreadIdAssigned here?
> 
> Why don't you call them at the beginning of Connect()?
> 
> Why do we need to call DetachFromThread()?

Done.

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_nss.h
File net/socket/ssl_client_socket_nss.h (right):

http://codereview.chromium.org/6474027/diff/1/net/socket/ssl_client_socket_ns...
net/socket/ssl_client_socket_nss.h:275: // code after fixing 65948.
On 2011/02/11 22:43:50, wtc wrote:
> Nit: please say "bug 65948" or http://crbug.com/65948 instead
> of just "65948".  Make the same changes to lines 170-171 above.
> 
> I think the next line (line 276) can be removed.

Done.

[wtc, 2011-02-12 00:00:41]

LGTM.  Two more changes.

http://codereview.chromium.org/6474027/diff/5001/net/socket/ssl_client_socket...
File net/socket/ssl_client_socket_nss.h (right):

http://codereview.chromium.org/6474027/diff/5001/net/socket/ssl_client_socket...
net/socket/ssl_client_socket_nss.h:171: // code after fixing "bug 65948".
Please remove the quotes ("").  Sorry to have confused you.

http://codereview.chromium.org/6474027/diff/5001/net/socket/ssl_client_socket...
net/socket/ssl_client_socket_nss.h:174: void DetachFromThread();
Remove DetachFromThread() in this file and in the .cc file.

[ramant (doing other things), 2011-02-12 00:08:21]

Hi Wan-Teh,
  Made the changes. Thanks very much for your comments,
raman

http://codereview.chromium.org/6474027/diff/5001/net/socket/ssl_client_socket...
File net/socket/ssl_client_socket_nss.h (right):

http://codereview.chromium.org/6474027/diff/5001/net/socket/ssl_client_socket...
net/socket/ssl_client_socket_nss.h:171: // code after fixing "bug 65948".
On 2011/02/12 00:00:41, wtc wrote:
> Please remove the quotes ("").  Sorry to have confused you.

Done.

http://codereview.chromium.org/6474027/diff/5001/net/socket/ssl_client_socket...
net/socket/ssl_client_socket_nss.h:174: void DetachFromThread();
On 2011/02/12 00:00:41, wtc wrote:
> Remove DetachFromThread() in this file and in the .cc file.

Done.

[wtc, 2011-02-12 00:46:17]

LGTM!