Stop sending stun binding requests after certain amount of time.

Stop sending stun binding requests after certain amount of time.

Also stop it if the request timed out.

It is going to be complicated to keep this and make it sync with the connection bind request as they may be on two different ports.

BUG=

Committed: https://crrev.com/45b0efd378abef87676e6ec55516b10ce583ddac
Cr-Commit-Position: refs/heads/master@{#10899}

[honghaiz3, 2015-11-21 00:26:52]

Description was changed from

==========
Disable the stun binding request for keeping stun alive.

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

to

==========
Disable the stun binding request for keeping stun alive.

It is going to be complicated to keep this and make it sync with the connection
bind request as they may be on two different ports. 

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

[honghaiz3, 2015-11-21 00:27:10]

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

[pthatcher1, 2015-12-02 22:38:08]

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

https://codereview.webrtc.org/1465843004/diff/1/webrtc/p2p/base/stunport.cc#n...
webrtc/p2p/base/stunport.cc:400: requests_.Send(new StunBindingRequest(this,
false, stun_addr));
Can we change StunBindingRequest::keep_alive_ to
StunBindingRequest::keep_alive_deadline_?  It looks like it would be pretty
easy, and then we could pass in something like 2 minutes here.

[honghaiz3, 2015-12-03 00:47:15]

Patchset #2 (id:20001) has been deleted

[honghaiz3, 2015-12-03 00:48:01]

Patchset #2 (id:40001) has been deleted

[honghaiz3, 2015-12-03 00:58:23]

Patchset #2 (id:60001) has been deleted

[honghaiz3, 2015-12-03 01:00:01]

Also removed re-starting the request if the stun-binding requests timed out as
it has been retried for more than 50 seconds.

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

https://codereview.webrtc.org/1465843004/diff/1/webrtc/p2p/base/stunport.cc#n...
webrtc/p2p/base/stunport.cc:400: requests_.Send(new StunBindingRequest(this,
false, stun_addr));
On 2015/12/02 22:38:08, pthatcher1 wrote:
> Can we change StunBindingRequest::keep_alive_ to
> StunBindingRequest::keep_alive_deadline_?  It looks like it would be pretty
> easy, and then we could pass in something like 2 minutes here.  

Done.

[honghaiz3, 2015-12-03 01:01:44]

Description was changed from

==========
Disable the stun binding request for keeping stun alive.

It is going to be complicated to keep this and make it sync with the connection
bind request as they may be on two different ports. 

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

to

==========
Stop sending stun binding requests after certain amount of time.

Also stop it if the request timed out. 

It is going to be complicated to keep this and make it sync with the connection
bind request as they may be on two different ports. 


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

[pthatcher1, 2015-12-03 21:00:19]

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport.cc
File webrtc/p2p/base/stunport.cc (right):

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:28: // Stop sending STUN binding request after this
amount of time (in milliseconds)
request => requests

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:30: const int KEEP_ALIVE_DEADLINE = 120 * 1000;
2 * 60 * 1000;  // 2 minutes

Might be more clear

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:68: if (rtc::TimeSince(port_->start_time()) <=
keep_alive_deadline_) {
I was thinking that "deadline" would be more an absolute time, not a relative
time.  In otherwords, "December 7th at 7pm PST", not "in 2 weeks".  The latter
is more like a "timeout" than a deadline".  I was thinking more like this:

if (now < keep_alive_deadline_) {
  // ...
}


That way the caller could specify deadline = now + timeout.

[pthatcher1, 2015-12-03 21:00:57]

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport.cc
File webrtc/p2p/base/stunport.cc (right):

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:249: // prepare STUN candidate.
This comment is no longer true.

[honghaiz3, 2015-12-03 22:18:38]

Patchset #3 (id:100001) has been deleted

[honghaiz3, 2015-12-03 22:18:43]

Patchset #3 (id:120001) has been deleted

[honghaiz3, 2015-12-03 22:21:22]

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport.cc
File webrtc/p2p/base/stunport.cc (right):

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:28: // Stop sending STUN binding request after this
amount of time (in milliseconds)
On 2015/12/03 21:00:19, pthatcher1 wrote:
> request => requests

Done.

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:30: const int KEEP_ALIVE_DEADLINE = 120 * 1000;
On 2015/12/03 21:00:19, pthatcher1 wrote:
> 2 * 60 * 1000;  // 2 minutes
> 
> Might be more clear

Done.

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:68: if (rtc::TimeSince(port_->start_time()) <=
keep_alive_deadline_) {
On 2015/12/03 21:00:19, pthatcher1 wrote:
> I was thinking that "deadline" would be more an absolute time, not a relative
> time.  In otherwords, "December 7th at 7pm PST", not "in 2 weeks".  The latter
> is more like a "timeout" than a deadline".  I was thinking more like this:
> 
> if (now < keep_alive_deadline_) {
>   // ...
> }
> 
> 
> That way the caller could specify deadline = now + timeout.

Done.

https://codereview.webrtc.org/1465843004/diff/80001/webrtc/p2p/base/stunport....
webrtc/p2p/base/stunport.cc:249: // prepare STUN candidate.
On 2015/12/03 21:00:57, pthatcher1 wrote:
> This comment is no longer true.
I don't understand. why?

[pthatcher1, 2015-12-03 22:45:54]

lgtm

https://codereview.webrtc.org/1465843004/diff/140001/webrtc/p2p/base/stunport.cc
File webrtc/p2p/base/stunport.cc (right):

https://codereview.webrtc.org/1465843004/diff/140001/webrtc/p2p/base/stunport...
webrtc/p2p/base/stunport.cc:351: // open during the call.
Sorry, I meant this comment.

[honghaiz3, 2015-12-03 22:53:02]

Thanks!

https://codereview.webrtc.org/1465843004/diff/140001/webrtc/p2p/base/stunport.cc
File webrtc/p2p/base/stunport.cc (right):

https://codereview.webrtc.org/1465843004/diff/140001/webrtc/p2p/base/stunport...
webrtc/p2p/base/stunport.cc:351: // open during the call.
On 2015/12/03 22:45:53, pthatcher1 wrote:
> Sorry, I meant this comment.

Done.

[honghaiz3, 2015-12-03 22:53:10]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2015-12-03 22:53:10]

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/1465843004/#ps160001 (title:
" ")

[commit-bot: I haz the power, 2015-12-03 22:53:14]

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

[commit-bot: I haz the power, 2015-12-04 00:53:35]

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

[commit-bot: I haz the power, 2015-12-04 00:53:36]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_clang_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_compile_x64_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_compile_x86_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios32_sim_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios64_sim_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_arm64_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_asan on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_compile_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_msan on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_tsan2 on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_asan on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_compile_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_compile_x64_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_x64_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_x64_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_x64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  presubmit on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_compile_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_drmemory_light on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_x64_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_x64_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_x64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[honghaiz3, 2015-12-04 01:32:23]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-12-04 01:33:04]

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

[commit-bot: I haz the power, 2015-12-04 03:33:40]

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

[commit-bot: I haz the power, 2015-12-04 03:33:41]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_clang_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_compile_x64_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_compile_x86_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios32_sim_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios64_sim_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_arm64_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  ios_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_asan on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_compile_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_msan on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_tsan2 on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_asan on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_compile_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_compile_x64_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_x64_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_x64_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_x64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  presubmit on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_compile_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_drmemory_light on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_x64_gn_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_x64_gn_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_x64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[honghaiz3, 2015-12-04 16:35:41]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-12-04 16:35:47]

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

[commit-bot: I haz the power, 2015-12-04 16:57:28]

Description was changed from

==========
Stop sending stun binding requests after certain amount of time.

Also stop it if the request timed out. 

It is going to be complicated to keep this and make it sync with the connection
bind request as they may be on two different ports. 


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

to

==========
Stop sending stun binding requests after certain amount of time.

Also stop it if the request timed out. 

It is going to be complicated to keep this and make it sync with the connection
bind request as they may be on two different ports. 


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

[commit-bot: I haz the power, 2015-12-04 16:57:28]

Committed patchset #4 (id:160001)

[commit-bot: I haz the power, 2015-12-04 16:57:35]

Description was changed from

==========
Stop sending stun binding requests after certain amount of time.

Also stop it if the request timed out. 

It is going to be complicated to keep this and make it sync with the connection
bind request as they may be on two different ports. 


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

to

==========
Stop sending stun binding requests after certain amount of time.

Also stop it if the request timed out.

It is going to be complicated to keep this and make it sync with the connection
bind request as they may be on two different ports.

BUG=

Committed: https://crrev.com/45b0efd378abef87676e6ec55516b10ce583ddac
Cr-Commit-Position: refs/heads/master@{#10899}
==========

[commit-bot: I haz the power, 2015-12-04 16:57:36]

Patchset 4 (id:??) landed as
https://crrev.com/45b0efd378abef87676e6ec55516b10ce583ddac
Cr-Commit-Position: refs/heads/master@{#10899}