Ping connections with the fast rate until each connection is pinged at least three times.

Ping connections with the fast rate until each connection is pinged at least three times,
or the connection times out or is pruned.

BUG=webrtc:5927
Committed: https://crrev.com/524ecc29ddb6054be000380c9e8582d0286cea6d
Cr-Commit-Position: refs/heads/master@{#12899}

[honghaiz3, 2016-05-25 04:16:03]

Patchset #1 (id:1) has been deleted

[honghaiz3, 2016-05-25 04:16:49]

Description was changed from

==========
Ping connections with the fast rate until each connection is pinged at least
three times.

BUG=
==========

to

==========
Ping connections with the fast rate until each connection is pinged at least
three times.

BUG=webrtc:5927
==========

[honghaiz3, 2016-05-25 04:17:23]

honghaiz@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[honghaiz3, 2016-05-25 04:18:14]

Description was changed from

==========
Ping connections with the fast rate until each connection is pinged at least
three times.

BUG=webrtc:5927
==========

to

==========
Ping connections with the fast rate until each connection is pinged at least
three times,
or the connection times out or is pruned.

BUG=webrtc:5927
==========

[pthatcher1, 2016-05-25 17:29:13]

lgtm, with some improvements readability and style.

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.cc:1312: ? STRONG_PING_INTERVAL
This might be more clear as :

bool need_more_pings_at_weak_interval = std::any_of(...,
  return conn->active() && conn->num_pings_sent() <
MIN_PINGS_AT_WEAK_INTERVAL));

int ping_interval = (weak() || need_more_pings_at_weak_interval) ?
weak_ping_interval_ : STRONG_PING_INTERVAL;

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.h (right):

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.h:42: static const int kMinNumPingsSent = 3;
Can we keep the same style?  And a slightly better name might be
MIN_PINGS_AT_WEAK_PING_INTERVAL

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:2837: * Temporarily disable this
until we use fake clock for the test.
We should have a TODO to re-enable it.  

And could you use the comment style consistent with the rest of the file?

[honghaiz3, 2016-05-25 18:39:22]

Patchset #2 (id:40001) has been deleted

[honghaiz3, 2016-05-25 18:39:29]

Patchset #2 (id:60001) has been deleted

[honghaiz3, 2016-05-25 18:40:02]

Thanks!

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.cc:1312: ? STRONG_PING_INTERVAL
On 2016/05/25 17:29:13, pthatcher1 wrote:
> This might be more clear as :
> 
> bool need_more_pings_at_weak_interval = std::any_of(...,
>   return conn->active() && conn->num_pings_sent() <
> MIN_PINGS_AT_WEAK_INTERVAL));
> 
> int ping_interval = (weak() || need_more_pings_at_weak_interval) ?
> weak_ping_interval_ : STRONG_PING_INTERVAL;
> 

Done.

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.h (right):

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.h:42: static const int kMinNumPingsSent = 3;
On 2016/05/25 17:29:13, pthatcher1 wrote:
> Can we keep the same style?  And a slightly better name might be
> MIN_PINGS_AT_WEAK_PING_INTERVAL

Done.

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):

https://codereview.webrtc.org/2009763002/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:2837: * Temporarily disable this
until we use fake clock for the test.
On 2016/05/25 17:29:13, pthatcher1 wrote:
> We should have a TODO to re-enable it.  
> 
> And could you use the comment style consistent with the rest of the file?

Done for both.

[honghaiz3, 2016-05-25 18:41:15]

Patchset #2 (id:80001) has been deleted

[honghaiz3, 2016-05-25 18:41:20]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2016-05-25 18:41:21]

The patchset sent to the CQ was uploaded after l-g-t-m from pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2009763002/#ps100001 (title:
"Address comments")

[commit-bot: I haz the power, 2016-05-25 18:41:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2009763002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2009763002/100001

[commit-bot: I haz the power, 2016-05-25 19:48:34]

Description was changed from

==========
Ping connections with the fast rate until each connection is pinged at least
three times,
or the connection times out or is pruned.

BUG=webrtc:5927
==========

to

==========
Ping connections with the fast rate until each connection is pinged at least
three times,
or the connection times out or is pruned.

BUG=webrtc:5927
==========

[commit-bot: I haz the power, 2016-05-25 19:48:35]

Committed patchset #2 (id:100001)

[commit-bot: I haz the power, 2016-05-25 19:48:39]

Description was changed from

==========
Ping connections with the fast rate until each connection is pinged at least
three times,
or the connection times out or is pruned.

BUG=webrtc:5927
==========

to

==========
Ping connections with the fast rate until each connection is pinged at least
three times,
or the connection times out or is pruned.

BUG=webrtc:5927

Committed: https://crrev.com/524ecc29ddb6054be000380c9e8582d0286cea6d
Cr-Commit-Position: refs/heads/master@{#12899}
==========

[commit-bot: I haz the power, 2016-05-25 19:48:40]

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