Do not delete a connection until it has not received anything for 30 seconds.

Do not delete a connection until it has not received anything for 30 seconds.

BUG=
R=pthatcher@webrtc.org

Committed: https://crrev.com/2cd7afe7e2dc011ab00bdbc131039b16aa8fbdeb
Cr-Commit-Position: refs/heads/master@{#10626}

[honghaiz3, 2015-11-09 17:51:58]

Patchset #1 (id:1) has been deleted

[honghaiz3, 2015-11-09 17:54:16]

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

[honghaiz3, 2015-11-09 18:14:47]

The CQ bit was checked by honghaiz@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-09 18:14:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1422623015/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1422623015/20001

[commit-bot: I haz the power, 2015-11-09 18:17:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-09 18:17:50]

Dry run: Try jobs failed on following builders:
  ios_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/10630)

[pthatcher1, 2015-11-11 21:04:54]

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port.cc
File webrtc/p2p/base/port.cc (right):

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1131: }
Can we just remove this check because it's covered by the last check?  Clearly
if we're receiving_ we have also received something in the last 30s.

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1136: }
Can we just remove this part because it's covered by the part below?  Clearly if
we have not received anything for 30s, we'll have hit write timeout.

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port_unit...
File webrtc/p2p/base/port_unittest.cc (right):

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port_unit...
webrtc/p2p/base/port_unittest.cc:1242: //     DEAD_CONNECTION_RECEIVE_TIMEOUT
milliseconds since last receiving.
Both of these say it has to be pruned first, but I don't think that's true. 
Even if it's not pruned, if doesn't receive a ping response for 30s, it will be
deleted because it goes to write timeout, right?

[honghaiz3, 2015-11-12 00:26:35]

Patchset #2 (id:40001) has been deleted

[honghaiz3, 2015-11-12 00:27:11]

PTAL.

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port.cc
File webrtc/p2p/base/port.cc (right):

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1131: }
On 2015/11/11 21:04:54, pthatcher1 wrote:
> Can we just remove this check because it's covered by the last check?  Clearly
> if we're receiving_ we have also received something in the last 30s.

Done. I thought this could be a shortcut for early return.

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1136: }
On 2015/11/11 21:04:54, pthatcher1 wrote:
> Can we just remove this part because it's covered by the part below?  Clearly
if
> we have not received anything for 30s, we'll have hit write timeout.

Done. I guess OK to remove it. write-timeout depends on the time to send the
ping request. If It has not been scheduled to send ping since last ping success,
it will not hit write-timeout, although the chance that happens is pretty small.

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port_unit...
File webrtc/p2p/base/port_unittest.cc (right):

https://codereview.webrtc.org/1422623015/diff/20001/webrtc/p2p/base/port_unit...
webrtc/p2p/base/port_unittest.cc:1242: //     DEAD_CONNECTION_RECEIVE_TIMEOUT
milliseconds since last receiving.
On 2015/11/11 21:04:54, pthatcher1 wrote:
> Both of these say it has to be pruned first, but I don't think that's true. 
> Even if it's not pruned, if doesn't receive a ping response for 30s, it will
be
> deleted because it goes to write timeout, right?
That condition was related to my original code.

[pthatcher1, 2015-11-12 00:42:13]

lgtm

[honghaiz3, 2015-11-12 17:17:54]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-11-12 17:18:04]

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

[commit-bot: I haz the power, 2015-11-12 17:34:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-12 17:34:22]

Try jobs failed on following builders:
  mac_asan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/10563)

[honghaiz3, 2015-11-12 17:43:46]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-11-12 17:44:01]

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

[honghaiz3, 2015-11-12 19:14:43]

Committed patchset #3 (id:80001) manually as
2cd7afe7e2dc011ab00bdbc131039b16aa8fbdeb (presubmit successful).

[commit-bot: I haz the power, 2015-11-12 19:14:48]

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