Increase STUN RTOs

Increase STUN RTOs to work better on poor networks, such as 2G networks.

BUG=b/34822484

Review-Url: https://codereview.webrtc.org/2677743002
Cr-Commit-Position: refs/heads/master@{#16503}
Committed: https://chromium.googlesource.com/external/webrtc/+/94a2f21c0573be1d909c9df157deb08bd9650262

[Taylor Brandstetter, 2017-02-03 23:28:17]

deadbeef@webrtc.org changed reviewers:
+ deadbeef@webrtc.org

[Taylor Brandstetter, 2017-02-03 23:28:17]

lgtm

May also be nice to have an end-to-end test that uses VirtualSocketServer +
FakeClock to simulate a large RTT

[skvlad, 2017-02-03 23:46:29]

skvlad@webrtc.org changed reviewers:
+ skvlad@webrtc.org

[skvlad, 2017-02-03 23:46:30]

lgtm

[pthatcher1, 2017-02-03 23:49:55]

The CQ bit was checked by pthatcher@webrtc.org

[commit-bot: I haz the power, 2017-02-03 23:50:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-03 23:55:35]

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

[commit-bot: I haz the power, 2017-02-03 23:55:36]

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

[pthatcher1, 2017-02-04 01:06:24]

Description was changed from

==========
Increase STUN RTOs
==========

to

==========
Increase STUN RTOs

BUG=b/34822484
==========

[pthatcher1, 2017-02-04 01:06:46]

Description was changed from

==========
Increase STUN RTOs

BUG=b/34822484
==========

to

==========
Increase STUN RTOs to work better on poor networks, such as 2G networks.

BUG=b/34822484
==========

[pthatcher1, 2017-02-04 01:06:51]

The CQ bit was checked by pthatcher@webrtc.org

[commit-bot: I haz the power, 2017-02-04 01:07:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-04 01:14:14]

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

[commit-bot: I haz the power, 2017-02-04 01:14:15]

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

[pthatcher1, 2017-02-04 08:31:51]

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

[commit-bot: I haz the power, 2017-02-04 08:32:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[pthatcher1, 2017-02-04 08:34:55]

Description was changed from

==========
Increase STUN RTOs to work better on poor networks, such as 2G networks.

BUG=b/34822484
==========

to

==========
Increase STUN RTOs to work better on poor networks, such as 2G networks.

Also make the simulated clock testing system work more robustly and quickly and
switch over tests to the simulated clock.

Finally, fix a bug found in the "candidates done" code while switching lots of
tests to the simulated clock.

BUG=b/34822484
==========

[commit-bot: I haz the power, 2017-02-04 08:41:35]

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

[commit-bot: I haz the power, 2017-02-04 08:41:36]

Dry run: Try jobs failed on following builders:
  mac_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/22400)
  win_x64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/22306)

[pthatcher1, 2017-02-04 09:53:26]

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

[commit-bot: I haz the power, 2017-02-04 09:53:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[pthatcher1, 2017-02-04 10:01:59]

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

[commit-bot: I haz the power, 2017-02-04 10:02:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-04 11:00:46]

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

[commit-bot: I haz the power, 2017-02-04 11:00:47]

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

[Taylor Brandstetter, 2017-02-06 19:23:43]

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/base/gunit.h
File webrtc/base/gunit.h (right):

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/base/gunit.h#newc...
webrtc/base/gunit.h:95: // TODO(pthatcher): Increase this to 10ms to speed up
some tests.
I'd recommend removing this TODO. Individual tests can use larger time intervals
if they want, or we can add another macro that takes a time interval argument,
but 1ms is the smallest time unit allowed by queued operations so I think it's
the only thing that makes sense to as a default.

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/base/messagequeue.cc
File webrtc/base/messagequeue.cc (right):

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/base/messagequeue...
webrtc/base/messagequeue.cc:162: // "queues_not_done" is 0.
How does that happen? That seems like a deeper problem.

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/base/messagequeue.h
File webrtc/base/messagequeue.h (right):

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/base/messagequeue...
webrtc/base/messagequeue.h:217: // false.
I'd suggest moving this comment to the CC file.

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/base/messagequeue...
webrtc/base/messagequeue.h:218: virtual bool WaitForProcess();
I'd suggest renaming this to "IsProcessingMessages".

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

https://codereview.webrtc.org/2677743002/diff/140001/webrtc/p2p/base/stunport...
webrtc/p2p/base/stunport_unittest.cc:228: // we can't use a simulated clock for
the lookup.
This is a sleep(1000) now... Is there a way to avoid this? Why doesn't
EXPECT_TRUE_WAIT still work?

[pthatcher1, 2017-02-07 03:00:27]

Please re-review with the use of real clocks in the unit tests.  They all pass,
but occasionally one will fail when using gtest-parallel.  So there might be
some increased flakiness, which should be fixed when we switch to the fake
clock.

[skvlad, 2017-02-07 03:05:46]

lgtm. Please remove the reference to the fake clock from the CL description.

https://codereview.webrtc.org/2677743002/diff/180001/webrtc/p2p/base/stunrequ...
File webrtc/p2p/base/stunrequest_unittest.cc (right):

https://codereview.webrtc.org/2677743002/diff/180001/webrtc/p2p/base/stunrequ...
webrtc/p2p/base/stunrequest_unittest.cc:61: total += 250 << i;
could it be more clear with a hardcoded array?

std::vector<int> retransmits = {250, 500, 1000, 2000, 4000, 8000, 8000, 8000,
8000, 8000 };
return retransmits[i];

[Taylor Brandstetter, 2017-02-07 16:27:44]

lgtm, though if possible it would be great to land the fake clock CL before this
one. I don't want to make tests more flaky and lengthy

[pthatcher1, 2017-02-08 00:22:16]

Description was changed from

==========
Increase STUN RTOs to work better on poor networks, such as 2G networks.

Also make the simulated clock testing system work more robustly and quickly and
switch over tests to the simulated clock.

Finally, fix a bug found in the "candidates done" code while switching lots of
tests to the simulated clock.

BUG=b/34822484
==========

to

==========
Increase STUN RTOs to work better on poor networks, such as 2G networks.

BUG=b/34822484
==========

[pthatcher1, 2017-02-08 21:43:41]

The CQ bit was checked by pthatcher@webrtc.org

[pthatcher1, 2017-02-08 21:43:41]

The patchset sent to the CQ was uploaded after l-g-t-m from deadbeef@webrtc.org,
skvlad@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2677743002/#ps220001 (title:
"merge")

[commit-bot: I haz the power, 2017-02-08 21:43:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-08 22:42:23]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1486590221310480,
"parent_rev": "1749bc372e93bf9f6362f8a739ec08daeefceef5", "commit_rev":
"94a2f21c0573be1d909c9df157deb08bd9650262"}

[commit-bot: I haz the power, 2017-02-08 22:42:27]

Description was changed from

==========
Increase STUN RTOs to work better on poor networks, such as 2G networks.

BUG=b/34822484
==========

to

==========
Increase STUN RTOs to work better on poor networks, such as 2G networks.

BUG=b/34822484

Review-Url: https://codereview.webrtc.org/2677743002
Cr-Commit-Position: refs/heads/master@{#16503}
Committed:
https://chromium.googlesource.com/external/webrtc/+/94a2f21c0573be1d909c9df15...
==========

[commit-bot: I haz the power, 2017-02-08 22:42:28]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/external/webrtc/+/94a2f21c0573be1d909c9df15...