TcpPort Reconnect should inform upper layer to start sending again

TcpPort Reconnect should inform upper layer to start sending again.

During the reconnection phase, EWOULDBLOCK has been returned to upper layer which stops the sending of video stream.

BUG=webrtc:4930
R=pthatcher@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/b594041ec8a3ae9f501260e2456d9d5ce6482819

[guoweis_webrtc, 2015-08-19 23:45:02]

guoweis@webrtc.org changed reviewers:
+ juberti@webrtc.org, pthatcher@webrtc.org, webrtc-network@google.com

[guoweis_webrtc, 2015-08-19 23:45:02]

PTAL.

[pthatcher1, 2015-08-20 03:47:28]

I'd like to talk about this logic change in person to make sure I fully
understand it.

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/port_unittest.cc
File webrtc/p2p/base/port_unittest.cc (right):

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/port_unittest...
webrtc/p2p/base/port_unittest.cc:237: is_recovered_from_ewouldblock_ = false;
Would it make sense to call this connection_ready_to_send_ ?

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc
File webrtc/p2p/base/tcpport.cc (right):

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc#ne...
webrtc/p2p/base/tcpport.cc:337: // TODO: Should STATE_WRITE_TIMEOUT return a
non-blocking error?
Would this be a better way to address it?  Not use EWOULDBLOCK?

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc#ne...
webrtc/p2p/base/tcpport.cc:362: Connection::OnReadyToSend();
If we fire OnReadyToSend() when we go from pretending to not pretending, should
we have a "not ready to send" signal that fires when we do the opposite (inside
of OnClose)?

[guoweis_webrtc, 2015-08-20 04:40:22]

On 2015/08/20 03:47:28, pthatcher1 wrote:
> I'd like to talk about this logic change in person to make sure I fully
> understand it.
> 
>
https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/port_unittest.cc
> File webrtc/p2p/base/port_unittest.cc (right):
> 
>
https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/port_unittest...
> webrtc/p2p/base/port_unittest.cc:237: is_recovered_from_ewouldblock_ = false;
> Would it make sense to call this connection_ready_to_send_ ?

I was calling it ready_to_send but it's not a good name. The reason is that when
the connection first connects (OnConnect signal), it's also ready to send.
OnReadyToSend is the signal really when it's recovered from EWOULDBLOCK (i.e.
write buffer is available again).

> 
> https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc
> File webrtc/p2p/base/tcpport.cc (right):
> 
>
https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc#ne...
> webrtc/p2p/base/tcpport.cc:337: // TODO: Should STATE_WRITE_TIMEOUT return a
> non-blocking error?
> Would this be a better way to address it?  Not use EWOULDBLOCK?
EWOULDBLOCK is wired as the signal that the upper layer shouldn't push more
traffic down and it is waiting lower layer to signal it's OK to send more. This
is wired in both standalone app and hangout. It won't be trivial to change this
notion.
> 
>
https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc#ne...
> webrtc/p2p/base/tcpport.cc:362: Connection::OnReadyToSend();
> If we fire OnReadyToSend() when we go from pretending to not pretending,
should
> we have a "not ready to send" signal that fires when we do the opposite
(inside
> of OnClose)?

[juberti1, 2015-08-20 05:33:01]

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/port_unittest.cc
File webrtc/p2p/base/port_unittest.cc (right):

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/port_unittest...
webrtc/p2p/base/port_unittest.cc:237: is_recovered_from_ewouldblock_ = false;
On 2015/08/20 03:47:27, pthatcher1 wrote:
> Would it make sense to call this connection_ready_to_send_ ?

+1

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc
File webrtc/p2p/base/tcpport.cc (right):

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc#ne...
webrtc/p2p/base/tcpport.cc:362: Connection::OnReadyToSend();
On 2015/08/20 03:47:27, pthatcher1 wrote:
> If we fire OnReadyToSend() when we go from pretending to not pretending,
should
> we have a "not ready to send" signal that fires when we do the opposite
(inside
> of OnClose)?

That is driven by EWOULDBLOCK being returned from Send().

But do we want to do this before letting Connection process the STUN response?
We might get called back with packets immediately.

[guoweis_webrtc, 2015-08-21 05:05:50]

PTAL.

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc
File webrtc/p2p/base/tcpport.cc (right):

https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc#ne...
webrtc/p2p/base/tcpport.cc:362: Connection::OnReadyToSend();
On 2015/08/20 05:33:00, juberti1 wrote:
> On 2015/08/20 03:47:27, pthatcher1 wrote:
> > If we fire OnReadyToSend() when we go from pretending to not pretending,
> should
> > we have a "not ready to send" signal that fires when we do the opposite
> (inside
> > of OnClose)?
> 
> That is driven by EWOULDBLOCK being returned from Send().
> 
> But do we want to do this before letting Connection process the STUN response?
> We might get called back with packets immediately.

Yes, this is possible. Moved this down

[guoweis_webrtc, 2015-08-24 14:10:15]

On 2015/08/21 05:05:50, guoweis wrote:
> PTAL.
> 
> https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc
> File webrtc/p2p/base/tcpport.cc (right):
> 
>
https://codereview.webrtc.org/1288553010/diff/1/webrtc/p2p/base/tcpport.cc#ne...
> webrtc/p2p/base/tcpport.cc:362: Connection::OnReadyToSend();
> On 2015/08/20 05:33:00, juberti1 wrote:
> > On 2015/08/20 03:47:27, pthatcher1 wrote:
> > > If we fire OnReadyToSend() when we go from pretending to not pretending,
> > should
> > > we have a "not ready to send" signal that fires when we do the opposite
> > (inside
> > > of OnClose)?
> > 
> > That is driven by EWOULDBLOCK being returned from Send().
> > 
> > But do we want to do this before letting Connection process the STUN
response?
> > We might get called back with packets immediately.
> 
> Yes, this is possible. Moved this down

PIng.

[pthatcher1, 2015-08-24 18:57:11]

lgtm

[guoweis_webrtc, 2015-08-24 18:58:12]

Committed patchset #2 (id:20001) manually as
b594041ec8a3ae9f501260e2456d9d5ce6482819 (presubmit successful).