Add a continual_gathering_policy in PeerConnection RTCConfiguration

Passing the new policy from PeerConnection RTCConfiguration to
p2ptransportchannel.  This CL does not use the new policy yet.
BUG=

Committed: https://crrev.com/1f429e34180ca19a7fb98b89bacd34d42e9b01ec
Cr-Commit-Position: refs/heads/master@{#10092}

[honghaiz3, 2015-09-25 17:11:28]

Patchset #1 (id:1) has been deleted

[honghaiz3, 2015-09-25 17:19:01]

Patchset #1 (id:20001) has been deleted

[honghaiz3, 2015-09-25 17:20:38]

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

[honghaiz3, 2015-09-25 17:20:38]

Please take a look at this CL, especially on the naming.

[pthatcher1, 2015-09-25 17:37:47]

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

https://codereview.webrtc.org/1369773003/diff/40001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/PeerConnection.java:149: public boolean
iceContinueGettingPorts;
I think we should call this something like "iceGatherPolicy", with an enum of
something like "GatherOnce" and "GatherContinuously".  That way, we can new
behaviors in the future.

https://codereview.webrtc.org/1369773003/diff/40001/talk/app/webrtc/webrtcses...
File talk/app/webrtc/webrtcsession.cc (right):

https://codereview.webrtc.org/1369773003/diff/40001/talk/app/webrtc/webrtcses...
talk/app/webrtc/webrtcsession.cc:788: ice_config.continue_getting_ports =
config.ice_continue_getting_ports;
Instead of copying all the settings, can we just embed a whole IceConfig into
the RTCConfiguration?

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

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.h:253: bool continue_getting_ports_ = false;
Why not store the config instead of storing both receiving_timeout_ and
continue_getting_ports_?

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport.h
File webrtc/p2p/base/transport.h (right):

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport...
webrtc/p2p/base/transport.h:144: bool continue_getting_ports = false;
We need to comment what effect these have here.

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport...
File webrtc/p2p/base/transportcontroller.h (right):

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport...
webrtc/p2p/base/transportcontroller.h:49: void SetIceRole(IceRole ice_role);
Can we role the IceRole into the IceConfig and call it IceParameters?  That will
match ORTC a lot more.

In fact, I think it might make sense to make a CL that creates SetParameters
with role+receive_timeout and then this CL can just gather policy and will be a
much smaller CL.

[pthatcher1, 2015-09-25 17:58:36]

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

https://codereview.webrtc.org/1369773003/diff/40001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/PeerConnection.java:149: public boolean
iceContinueGettingPorts;
On 2015/09/25 17:37:46, pthatcher1 wrote:
> I think we should call this something like "iceGatherPolicy", with an enum of
> something like "GatherOnce" and "GatherContinuously".  That way, we can new
> behaviors in the future.  

Actually, ORTC already has a GatherPolicy, which is like iceTransportsType here
of all/nohost/relay.  This is orthogonal to that, so perhaps it should be
"ContinuousGatherPolicy" or "NetworkChangeGatherPolicy"

[honghaiz3, 2015-09-25 20:10:20]

PTAL.

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

https://codereview.webrtc.org/1369773003/diff/40001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/PeerConnection.java:149: public boolean
iceContinueGettingPorts;
On 2015/09/25 17:37:46, pthatcher1 wrote:
> I think we should call this something like "iceGatherPolicy", with an enum of
> something like "GatherOnce" and "GatherContinuously".  That way, we can new
> behaviors in the future.  

Done.

https://codereview.webrtc.org/1369773003/diff/40001/talk/app/webrtc/webrtcses...
File talk/app/webrtc/webrtcsession.cc (right):

https://codereview.webrtc.org/1369773003/diff/40001/talk/app/webrtc/webrtcses...
talk/app/webrtc/webrtcsession.cc:788: ice_config.continue_getting_ports =
config.ice_continue_getting_ports;
On 2015/09/25 17:37:47, pthatcher1 wrote:
> Instead of copying all the settings, can we just embed a whole IceConfig into
> the RTCConfiguration?
It is probably better to separate the PeerConnection interface from the internal
implementation. This is also typical for most other policies.

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

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.h:253: bool continue_getting_ports_ = false;
On 2015/09/25 17:37:47, pthatcher1 wrote:
> Why not store the config instead of storing both receiving_timeout_ and
> continue_getting_ports_?
The config is used to pass the infomation down. 
But this two values are used completely separate here. 
It is more convenient to have two variables when using them.
Plus, when setting we need to check the value of timeout but not the other var.
If we just blindly save the whole ice_config, then we need to check the
receiving time everytime when using it. 
Renamed it

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport.h
File webrtc/p2p/base/transport.h (right):

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport...
webrtc/p2p/base/transport.h:144: bool continue_getting_ports = false;
On 2015/09/25 17:37:47, pthatcher1 wrote:
> We need to comment what effect these have here.

Done.

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport...
File webrtc/p2p/base/transportcontroller.h (right):

https://codereview.webrtc.org/1369773003/diff/40001/webrtc/p2p/base/transport...
webrtc/p2p/base/transportcontroller.h:49: void SetIceRole(IceRole ice_role);
On 2015/09/25 17:37:47, pthatcher1 wrote:
> Can we role the IceRole into the IceConfig and call it IceParameters?  That
will
> match ORTC a lot more.
> 
> In fact, I think it might make sense to make a CL that creates SetParameters
> with role+receive_timeout and then this CL can just gather policy and will be
a
> much smaller CL.

No, based on offline discussion.

[pthatcher1, 2015-09-25 22:26:57]

Just name changes :).

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnectioninterface.h (right):

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:228: enum ContinuousGatheringPolicy {
Based on http://blog.dictionary.com/continually-vs-continuously/, this should
really be called ContinualGatheringPolicy

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:230: kGatheringContinuously
kGatherOnce
kGatherContinuosly

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:230: kGatheringContinuously
Cand kGatherContinually

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:230: kGatheringContinuously
The new enum style is GATHERING_CONTINUALLY.

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/webrtcses...
File talk/app/webrtc/webrtcsession.cc (right):

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/webrtcses...
talk/app/webrtc/webrtcsession.cc:790:
PeerConnectionInterface::kGatheringContinuously);
gather_continually.

[honghaiz3, 2015-09-25 23:03:55]

Patchset #3 (id:80001) has been deleted

[honghaiz3, 2015-09-25 23:05:00]

Thanks! PTAL.

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
File talk/app/webrtc/peerconnectioninterface.h (right):

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:228: enum ContinuousGatheringPolicy {
On 2015/09/25 22:26:57, pthatcher1 wrote:
> Based on http://blog.dictionary.com/continually-vs-continuously/, this should
> really be called ContinualGatheringPolicy

Done.

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:230: kGatheringContinuously
On 2015/09/25 22:26:57, pthatcher1 wrote:
> Cand kGatherContinually

Done.

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:230: kGatheringContinuously
On 2015/09/25 22:26:57, pthatcher1 wrote:
> The new enum style is GATHERING_CONTINUALLY.

Done.

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/peerconne...
talk/app/webrtc/peerconnectioninterface.h:230: kGatheringContinuously
On 2015/09/25 22:26:57, pthatcher1 wrote:
> Cand kGatherContinually

Done.

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/webrtcses...
File talk/app/webrtc/webrtcsession.cc (right):

https://codereview.webrtc.org/1369773003/diff/60001/talk/app/webrtc/webrtcses...
talk/app/webrtc/webrtcsession.cc:790:
PeerConnectionInterface::kGatheringContinuously);
On 2015/09/25 22:26:57, pthatcher1 wrote:
> gather_continually.

Done.

[pthatcher1, 2015-09-25 23:45:56]

lgtm

[honghaiz3, 2015-09-26 00:04:13]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-09-26 00:04:27]

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

[commit-bot: I haz the power, 2015-09-26 01:10:42]

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

[commit-bot: I haz the power, 2015-09-26 01:10:43]

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

[honghaiz3, 2015-09-28 02:49:45]

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

[commit-bot: I haz the power, 2015-09-28 02:49:54]

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

[commit-bot: I haz the power, 2015-09-28 03:33:06]

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

[commit-bot: I haz the power, 2015-09-28 03:33:06]

Dry run: This issue passed the CQ dry run.

[honghaiz3, 2015-09-28 14:56:08]

The CQ bit was checked by honghaiz@webrtc.org

[commit-bot: I haz the power, 2015-09-28 14:56:16]

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

[commit-bot: I haz the power, 2015-09-28 14:57:40]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2015-09-28 14:57:50]

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