Update prflx candidates' generation when setting ICE credentials.

Update prflx candidates' generation when setting ICE credentials.

If a STUN ping arrives before the remote description does, a prflx
candidate will be created with an unknown generation.

Once the remote description does arrive, the candidate's generation
should be set so it can be sorted properly, and replaced by a non-prflx
candidate once the candidate is signaled.

BUG=webrtc:5752
R=honghaiz@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/0a1bc53758862014343c8186ea7c270f0cdd57c6
Cr-Commit-Position: refs/heads/master@{#12433}

[Taylor Brandstetter, 2016-04-09 01:12:56]

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

[Taylor Brandstetter, 2016-04-09 01:12:57]

https://codereview.webrtc.org/1868353004/diff/1/webrtc/p2p/base/p2ptransportc...
File webrtc/p2p/base/p2ptransportchannel_unittest.cc (right):

https://codereview.webrtc.org/1868353004/diff/1/webrtc/p2p/base/p2ptransportc...
webrtc/p2p/base/p2ptransportchannel_unittest.cc:1196:
ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]);
I think "3" was just a typo here. This test doesn't end up using kIceUfrag[3] at
all.

[pthatcher1, 2016-04-12 18:47:39]

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

https://codereview.webrtc.org/1868353004/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1307: remote_candidate_.set_password(params.pwd);
Should we also set_generation() here?

[Taylor Brandstetter, 2016-04-12 21:33:47]

Should the generation be changed to an rtc::Optional? Right now there's no way
to differentiate between "generation 0" and "unknown generation". Could come
back to bite us later.

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

https://codereview.webrtc.org/1868353004/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1307: remote_candidate_.set_password(params.pwd);
On 2016/04/12 18:47:39, pthatcher1 wrote:
> Should we also set_generation() here?

I guess it would be a small optimization. Or do we not need the below block of
code at all? Is it assured that if you don't have a generation, you also don't
have a password?

[pthatcher1, 2016-04-12 22:39:53]

Making candidate.generation an rtc::optional sounds like a good idea.  I think
Honghai and I considered it at one point.  I can't remember why we didn't choose
to do it.

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

https://codereview.webrtc.org/1868353004/diff/20001/webrtc/p2p/base/port.cc#n...
webrtc/p2p/base/port.cc:1307: remote_candidate_.set_password(params.pwd);
On 2016/04/12 21:33:46, Taylor Brandstetter wrote:
> On 2016/04/12 18:47:39, pthatcher1 wrote:
> > Should we also set_generation() here?
> 
> I guess it would be a small optimization. Or do we not need the below block of
> code at all? Is it assured that if you don't have a generation, you also don't
> have a password?

Oh, I see.  In the first case, you end up doing the second case also.  That
wasn't obvious to me.    Maybe calling set_generation() here and changing it to
an else if in the second case would be more readable.  But it's not worth doing
for the sake of an optimization.

You might have a generation but not a password.  But I can't think of how you'd
have a password but not a generation, unless we started trickling the password
also.  Actually, that might happen in the future, so maybe just keep this robust
like you have it.

[pthatcher1, 2016-04-12 22:40:05]

lgtm

[honghaiz3, 2016-04-18 16:12:37]

https://codereview.webrtc.org/1868353004/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/1868353004/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.cc:396:
conn->MaybeSetRemoteIceCredentialsAndGeneration(remote_ice_parameters_);
Why does this need to pass all ice parameters and iterate over all of them?
Isn't it sufficient to just pass the newly added one?

[Taylor Brandstetter, 2016-04-18 23:30:01]

https://codereview.webrtc.org/1868353004/diff/20001/webrtc/p2p/base/p2ptransp...
File webrtc/p2p/base/p2ptransportchannel.cc (right):

https://codereview.webrtc.org/1868353004/diff/20001/webrtc/p2p/base/p2ptransp...
webrtc/p2p/base/p2ptransportchannel.cc:396:
conn->MaybeSetRemoteIceCredentialsAndGeneration(remote_ice_parameters_);
On 2016/04/18 16:12:37, honghaiz3 wrote:
> Why does this need to pass all ice parameters and iterate over all of them?
> Isn't it sufficient to just pass the newly added one?

You're right. I don't remember what I was thinking. I updated the CL.

[honghaiz3, 2016-04-18 23:45:42]

lgtm

[Taylor Brandstetter, 2016-04-20 01:03:40]

Description was changed from

==========
Update prflx candidates' generation when setting ICE credentials.

If a STUN ping arrives before the remote description does, a prflx
candidate will be created with an unknown generation.

Once the remote description does arrive, the candidate's generation
should be set so it can be sorted properly, and replaced by a non-prflx
candidate once the candidate is signaled.

BUG=webrtc:5752
==========

to

==========
Update prflx candidates' generation when setting ICE credentials.

If a STUN ping arrives before the remote description does, a prflx
candidate will be created with an unknown generation.

Once the remote description does arrive, the candidate's generation
should be set so it can be sorted properly, and replaced by a non-prflx
candidate once the candidate is signaled.

BUG=webrtc:5752
R=honghaiz@webrtc.org, pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/0a1bc53758862014343c8186e...
==========

[Taylor Brandstetter, 2016-04-20 01:03:42]

Committed patchset #4 (id:60001) manually as
0a1bc53758862014343c8186ea7c270f0cdd57c6 (presubmit successful).

[commit-bot: I haz the power, 2016-04-20 01:03:42]

Description was changed from

==========
Update prflx candidates' generation when setting ICE credentials.

If a STUN ping arrives before the remote description does, a prflx
candidate will be created with an unknown generation.

Once the remote description does arrive, the candidate's generation
should be set so it can be sorted properly, and replaced by a non-prflx
candidate once the candidate is signaled.

BUG=webrtc:5752
R=honghaiz@webrtc.org, pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/0a1bc53758862014343c8186e...
==========

to

==========
Update prflx candidates' generation when setting ICE credentials.

If a STUN ping arrives before the remote description does, a prflx
candidate will be created with an unknown generation.

Once the remote description does arrive, the candidate's generation
should be set so it can be sorted properly, and replaced by a non-prflx
candidate once the candidate is signaled.

BUG=webrtc:5752
R=honghaiz@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/0a1bc53758862014343c8186ea7c270f0cdd57c6
Cr-Commit-Position: refs/heads/master@{#12433}
==========

[commit-bot: I haz the power, 2016-04-20 01:03:43]

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