Set the IceConnectionReceivingTimeout as a RTCConfiguration parameter.

Set the IceConnectionReceivingTimeout as a RTCConfiguration parameter.

BUG=4901
Committed: https://crrev.com/4edc39c5692ab01286d0c4a4911bf6b705032b6f
Cr-Commit-Position: refs/heads/master@{#9832}

[honghaiz3, 2015-08-26 15:58:28]

Patchset #1 (id:1) has been deleted

[honghaiz3, 2015-08-26 17:41:16]

Patchset #1 (id:20001) has been deleted

[honghaiz3, 2015-08-26 17:51:51]

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

[honghaiz3, 2015-08-26 17:51:52]

Because the timeout value can only be set at the peer connection initialization
phase now, we can make it simpler in the
BaseSession::SetIceConnectionReceivingTimeout
method by setting the value only.
But keeping the current logic makes it safer in case its value is set after some
transports or transport channels have been created. 
Let me know if you prefer to making it a simple assignment.

[juberti1, 2015-08-26 18:25:46]

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnectioninterface.h (left):

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:258:
audio_jitter_buffer_max_packets(50),
let's put these as constants in the RTCConfiguration class, so that we can
document them.

[pthatcher2, 2015-08-26 18:33:17]

pthatcher@chromium.org changed reviewers:
+ pthatcher@chromium.org

[pthatcher2, 2015-08-26 18:33:18]

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnection.cc (right):

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:657: bool PeerConnection::UpdateIce(const
RTCConfiguration& config) {
I think we need to handle it being updated here as well.

[honghaiz3, 2015-08-26 19:15:18]

Patchset #2 (id:60001) has been deleted

[honghaiz3, 2015-08-26 19:20:26]

Thanks! PTAL.

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnection.cc (right):

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnection.cc:657: bool PeerConnection::UpdateIce(const
RTCConfiguration& config) {
On 2015/08/26 18:33:18, pthatcher2 wrote:
> I think we need to handle it being updated here as well.

Done. Thanks!

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnectioninterface.h (left):

https://codereview.webrtc.org/1315503003/diff/40001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:258:
audio_jitter_buffer_max_packets(50),
On 2015/08/26 18:25:46, juberti1 wrote:
> let's put these as constants in the RTCConfiguration class, so that we can
> document them.

Done (for integer parameters).

[honghaiz3, 2015-08-26 20:05:43]

Patchset #2 (id:80001) has been deleted

[pthatcher1, 2015-08-26 20:08:57]

lgtm

[honghaiz3, 2015-08-27 18:50:24]

Justin, 

Will you take another look at the CL?

[honghaiz3, 2015-08-27 18:50:38]

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

[commit-bot: I haz the power, 2015-08-27 18:50:52]

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

[juberti, 2015-08-27 18:51:30]

juberti@google.com changed reviewers:
+ juberti@google.com

[juberti, 2015-08-27 18:51:30]

lgtm with one comment

https://codereview.webrtc.org/1315503003/diff/120001/talk/app/webrtc/java/src...
File talk/app/webrtc/java/src/org/webrtc/PeerConnection.java (right):

https://codereview.webrtc.org/1315503003/diff/120001/talk/app/webrtc/java/src...
talk/app/webrtc/java/src/org/webrtc/PeerConnection.java:151:
iceConnectionReceivingTimeout = 2500;
Can we set these to -1 and have the default values filled in by the C++ layer? I
hate to have magic numbers duplicated in different places.

[commit-bot: I haz the power, 2015-08-27 18:52:33]

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

[commit-bot: I haz the power, 2015-08-27 18:52:34]

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

[honghaiz3, 2015-08-27 20:06:22]

Will wait until tomorrow in case you have more comments.

https://codereview.webrtc.org/1315503003/diff/120001/talk/app/webrtc/java/src...
File talk/app/webrtc/java/src/org/webrtc/PeerConnection.java (right):

https://codereview.webrtc.org/1315503003/diff/120001/talk/app/webrtc/java/src...
talk/app/webrtc/java/src/org/webrtc/PeerConnection.java:151:
iceConnectionReceivingTimeout = 2500;
On 2015/08/27 18:51:30, juberti wrote:
> Can we set these to -1 and have the default values filled in by the C++ layer?
I
> hate to have magic numbers duplicated in different places.
Good point! I also set the value in peerconnectioninterface.h to -1 for the same
reason. When both are set to -1, it will pick the default value in
p2ptransportchannel.cc.

[juberti, 2015-08-27 20:31:04]

https://codereview.webrtc.org/1315503003/diff/140001/talk/app/webrtc/peerconn...
File talk/app/webrtc/peerconnectioninterface.h (right):

https://codereview.webrtc.org/1315503003/diff/140001/talk/app/webrtc/peerconn...
talk/app/webrtc/peerconnectioninterface.h:240: static const int
kIceConnectionReceivingTimeout = -1;
If you are using -1, I don't think we need to have a specific constant for this.
We could just do something like kDefaultIntValue = -1 and use that everywhere we
want to use a default value (i.e. we could use it for audio jitter max packets
as well).

[honghaiz3, 2015-08-27 21:02:26]

https://codereview.webrtc.org/1315503003/diff/140001/talk/app/webrtc/peerconn...
File talk/app/webrtc/peerconnectioninterface.h (right):

https://codereview.webrtc.org/1315503003/diff/140001/talk/app/webrtc/peerconn...
talk/app/webrtc/peerconnectioninterface.h:240: static const int
kIceConnectionReceivingTimeout = -1;
On 2015/08/27 20:31:04, juberti wrote:
> If you are using -1, I don't think we need to have a specific constant for
this.
> We could just do something like kDefaultIntValue = -1 and use that everywhere
we
> want to use a default value (i.e. we could use it for audio jitter max packets
> as well).

I just used the kUndefined as it was done in the next struct. 
Wasn't so sure if we can set the audio jitter max packets to -1 here, so did not
do that (For Android, the value here will be overriden by that in the java
class, but for IOS I am not sure if setting it to -1 here may have unexpected
consequences).

[honghaiz3, 2015-08-28 21:17:36]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2015-08-28 21:17:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, juberti@google.com
Link to the patchset: https://codereview.webrtc.org/1315503003/#ps160001 (title:
" ")

[commit-bot: I haz the power, 2015-08-28 21:17:46]

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

[commit-bot: I haz the power, 2015-08-28 21:19:20]

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

[commit-bot: I haz the power, 2015-08-28 21:19:21]

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

[honghaiz3, 2015-08-28 23:01:00]

The CQ bit was checked by honghaiz@webrtc.org

[honghaiz3, 2015-08-28 23:01:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, juberti@google.com
Link to the patchset: https://codereview.webrtc.org/1315503003/#ps180001 (title:
" ")

[commit-bot: I haz the power, 2015-08-28 23:01:17]

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

[commit-bot: I haz the power, 2015-08-29 01:01:26]

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

[commit-bot: I haz the power, 2015-08-29 01:01:27]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[honghaiz3, 2015-08-31 18:07:04]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-08-31 18:07:17]

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

[commit-bot: I haz the power, 2015-08-31 20:07:29]

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

[commit-bot: I haz the power, 2015-08-31 20:07:30]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[honghaiz3, 2015-08-31 21:26:05]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-08-31 21:26:15]

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

[commit-bot: I haz the power, 2015-08-31 23:26:21]

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

[commit-bot: I haz the power, 2015-08-31 23:26:22]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[honghaiz3, 2015-08-31 23:31:50]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-08-31 23:32:04]

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

[commit-bot: I haz the power, 2015-09-01 01:32:22]

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

[commit-bot: I haz the power, 2015-09-01 01:32:23]

Try jobs failed on following builders:
  android on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[honghaiz3, 2015-09-01 16:52:43]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-09-01 16:52:54]

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

[commit-bot: I haz the power, 2015-09-01 16:54:01]

Committed patchset #6 (id:180001)

[commit-bot: I haz the power, 2015-09-01 16:54:12]

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