Reset accept_callback_ before calling it from TCPServerSocket.

Reset accept_callback_ before calling it from TCPServerSocket.

TCPServerSocket worked properly before, but then it was broken by r112863.

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

[James Hawkins, 2012-03-28 23:25:22]

http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_li...
File net/socket/tcp_server_socket_libevent.cc (right):

http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_li...
net/socket/tcp_server_socket_libevent.cc:190: CompletionCallback callback =
accept_callback_;
accept_callback_.Reset().Run(result);

[Sergey Ulanov, 2012-03-29 00:05:38]

http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_li...
File net/socket/tcp_server_socket_libevent.cc (right):

http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_li...
net/socket/tcp_server_socket_libevent.cc:190: CompletionCallback callback =
accept_callback_;
On 2012/03/28 23:25:22, James Hawkins wrote:
> accept_callback_.Reset().Run(result);

I don't think Callback<>.Reset() returns anything, or does it?
CallbackBase.Reset() is defined as returning NULL.

[Sergey Ulanov, 2012-03-30 17:23:36]

+wtc
Wan-Teh, Can you please review this CL? 
It's rather small, shouldn't take more than 15 seconds to review :)

[wtc, 2012-03-30 20:22:00]

LGTM.  Thanks.

[James Hawkins, 2012-03-30 20:25:01]

LGTM. I'll look for the Reset().Run() thing.

http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_li...
File net/socket/tcp_server_socket_libevent.cc (right):

http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_li...
net/socket/tcp_server_socket_libevent.cc:190: CompletionCallback callback =
accept_callback_;
On 2012/03/29 00:05:38, sergeyu wrote:
> On 2012/03/28 23:25:22, James Hawkins wrote:
> > accept_callback_.Reset().Run(result);
> 
> I don't think Callback<>.Reset() returns anything, or does it?
> CallbackBase.Reset() is defined as returning NULL.

Huh.  I could swear someone just added this recently; I'll look for it.

[wtc, 2012-03-30 20:32:17]

I also checked http://codereview.chromium.org/8824006
to ensure that the other callbacks were converted correctly.

[commit-bot: I haz the power, 2012-03-30 20:35:09]

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

[cbentzel, 2012-03-30 21:11:43]

On Fri, Mar 30, 2012 at 4:25 PM, <jhawkins@chromium.org> wrote:

> LGTM. I'll look for the Reset().Run() thing.


Comment from the peanut gallery: I guess I find the existing code more
straightforward from Reset().Run() - I don't have to think as much about
lifetime issues. But if this becomes a common idiom I can live with it.


>
>
>
> http://codereview.chromium.**org/9837122/diff/1/net/socket/**
>
tcp_server_socket_libevent.cc<http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_libevent.cc>
> File net/socket/tcp_server_socket_**libevent.cc (right):
>
> http://codereview.chromium.**org/9837122/diff/1/net/socket/**
>
tcp_server_socket_libevent.cc#**newcode190<http://codereview.chromium.org/9837122/diff/1/net/socket/tcp_server_socket_libevent.cc#newcode190>
> net/socket/tcp_server_socket_**libevent.cc:190: CompletionCallback
> callback = accept_callback_;
> On 2012/03/29 00:05:38, sergeyu wrote:
>
>> On 2012/03/28 23:25:22, James Hawkins wrote:
>> > accept_callback_.Reset().Run(**result);
>>
>
>  I don't think Callback<>.Reset() returns anything, or does it?
>> CallbackBase.Reset() is defined as returning NULL.
>>
>
> Huh.  I could swear someone just added this recently; I'll look for it.
>
>
http://codereview.chromium.**org/9837122/<http://codereview.chromium.org/9837...
>

[commit-bot: I haz the power, 2012-03-31 00:33:31]

List of reviewers changed. cbentzel@chromium.org did a drive-by without
LGTM'ing!

[commit-bot: I haz the power, 2012-03-31 00:34:57]

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

[cbentzel, 2012-03-31 01:31:22]

LGTM

On Fri, Mar 30, 2012 at 8:34 PM, <commit-bot@chromium.org> wrote:

> CQ is trying da patch. Follow status at
>
https://chromium-status.**appspot.com/cq/sergeyu@**chromium.org/9837122/1<htt...
>
>
>
http://codereview.chromium.**org/9837122/<http://codereview.chromium.org/9837...
>