Blimp: relocate thread-unsafe member getter in DropConnection code.

Blimp: relocate thread-unsafe member getter in DropConnection code.

There is logic in the ClientSession/ClientContext which attempts
to drop a connection on the IO thread. Unfortunately, in doing so,
it called a thread-unsafe member getter within the call to PostTask.

This CL moves the getter into a helper method which runs entirely
on the IO thread.

R=nyquist@chromium.org,wez@chromium.org
CC=steimel@chromium.org
BUG=647814
Committed: https://crrev.com/6ecbaf9d15c6bff239e1a38502de55146d3e56e6
Cr-Commit-Position: refs/heads/master@{#419491}

[Kevin M, 2016-09-16 21:20:27]

Looks like the regression was here: https://codereview.chromium.org/2187893002

[Wez, 2016-09-17 01:56:31]

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
File blimp/client/session/blimp_client_session.cc (right):

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
blimp/client/session/blimp_client_session.cc:162: FROM_HERE,
base::Bind(&DropConnectionOnIOThread, net_components_.get()));
How come we are able to Bind(..., net_components_.get()) here but we have to use
base::Unretained above? Just because it's not a member function?

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
File blimp/client/session/blimp_client_session.h (right):

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
blimp/client/session/blimp_client_session.h:90: // May be called on any thread.
Eek, let's not leave inviting comments like that in the codebase :P

I think what we really mean here is just that this should be called on the main
thread, and will trigger the connection-drop on the IO thread, right?

[Kevin M, 2016-09-19 17:00:24]

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
File blimp/client/session/blimp_client_session.cc (right):

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
blimp/client/session/blimp_client_session.cc:162: FROM_HERE,
base::Bind(&DropConnectionOnIOThread, net_components_.get()));
On 2016/09/17 01:56:31, Wez wrote:
> How come we are able to Bind(..., net_components_.get()) here but we have to
use
> base::Unretained above? Just because it's not a member function?

It's a function pointer, not a bound method pointer.

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
File blimp/client/session/blimp_client_session.h (right):

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
blimp/client/session/blimp_client_session.h:90: // May be called on any thread.
On 2016/09/17 01:56:31, Wez wrote:
> Eek, let's not leave inviting comments like that in the codebase :P
> 
> I think what we really mean here is just that this should be called on the
main
> thread, and will trigger the connection-drop on the IO thread, right?

Done.

[Wez, 2016-09-19 17:04:28]

lgtm

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
File blimp/client/session/blimp_client_session.cc (right):

https://codereview.chromium.org/2344893009/diff/1/blimp/client/session/blimp_...
blimp/client/session/blimp_client_session.cc:162: FROM_HERE,
base::Bind(&DropConnectionOnIOThread, net_components_.get()));
On 2016/09/19 17:00:23, Kevin M wrote:
> On 2016/09/17 01:56:31, Wez wrote:
> > How come we are able to Bind(..., net_components_.get()) here but we have to
> use
> > base::Unretained above? Just because it's not a member function?
> 
> It's a function pointer, not a bound method pointer.

Interesting; I thought Bind would still balk at a bare pointer in that case.

[Kevin M, 2016-09-19 17:05:27]

The CQ bit was checked by kmarshall@chromium.org

[commit-bot: I haz the power, 2016-09-19 17:05:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 17:18:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-19 17:21:19]

Description was changed from

==========
Blimp: relocate thread-unsafe member getter in DropConnection code.

There is logic in the ClientSession/ClientContext which attempts
to drop a connection on the IO thread. Unfortunately, in doing so,
it called a thread-unsafe member getter within the call to PostTask.

This CL moves the getter into a helper method which runs entirely
on the IO thread.


R=nyquist@chromium.org,wez@chromium.org
CC=steimel@chromium.org
BUG=647814
==========

to

==========
Blimp: relocate thread-unsafe member getter in DropConnection code.

There is logic in the ClientSession/ClientContext which attempts
to drop a connection on the IO thread. Unfortunately, in doing so,
it called a thread-unsafe member getter within the call to PostTask.

This CL moves the getter into a helper method which runs entirely
on the IO thread.

R=nyquist@chromium.org,wez@chromium.org
CC=steimel@chromium.org
BUG=647814

Committed: https://crrev.com/6ecbaf9d15c6bff239e1a38502de55146d3e56e6
Cr-Commit-Position: refs/heads/master@{#419491}
==========

[commit-bot: I haz the power, 2016-09-19 17:21:21]

Patchset 2 (id:??) landed as
https://crrev.com/6ecbaf9d15c6bff239e1a38502de55146d3e56e6
Cr-Commit-Position: refs/heads/master@{#419491}