Fix intermittent connection failure in CRD.

Fix intermittent connection failure in CRD.

rtc::P2PTransportChannel expects SetRemoteIceCredentials() to be called
to set remove ICE candidate, but LibjingleTransport was never calling
it, and instead it was passing ICE credentials as part of remote
candidates. Fixed LibjingleTransport to call SetRemoteIceCredentials()
explicitly whenever it receives a new candidate.

BUG=476775
Committed: https://crrev.com/60afdae4bee5696a700e0468598740454c0a0c75
Cr-Commit-Position: refs/heads/master@{#325320}

[Sergey Ulanov, 2015-04-15 20:08:23]

sergeyu@chromium.org changed reviewers:
+ jamiewalch@chromium.org, jiayl@chromium.org

[Sergey Ulanov, 2015-04-15 20:08:23]

JiaYang - please take a look at
remoting/protocol/libjingle_transport_factory.cc. Do you see any problems with
this solution?
Jamie - please review and approve.

I plan to implement a cleaner solution when I fix crbug.com/473758 - ICE
credentials will be passed separately from candidates, which matches how it
works in PeerConnection.

[jiayl, 2015-04-15 20:13:00]

https://codereview.chromium.org/1070233004/diff/1/remoting/protocol/libjingle...
File remoting/protocol/libjingle_transport_factory.cc (right):

https://codereview.chromium.org/1070233004/diff/1/remoting/protocol/libjingle...
remoting/protocol/libjingle_transport_factory.cc:154:
pending_candidates_.front().password());
Do all candidates share the same username/password? Add a check for that, and
call SetRemoteIceCredentials just once?

[Jamie, 2015-04-15 20:14:49]

LGTM for OWNERs, but I'll defer to JiaYang for the review details, since I'm not
familiar with this code.

[Sergey Ulanov, 2015-04-15 20:49:34]

https://codereview.chromium.org/1070233004/diff/1/remoting/protocol/libjingle...
File remoting/protocol/libjingle_transport_factory.cc (right):

https://codereview.chromium.org/1070233004/diff/1/remoting/protocol/libjingle...
remoting/protocol/libjingle_transport_factory.cc:154:
pending_candidates_.front().password());
On 2015/04/15 20:13:00, jiayl wrote:
> Do all candidates share the same username/password? Add a check for that, and
> call SetRemoteIceCredentials just once?

Credentials may change when the remote peer restarts ICE. We could call
SetRemoteIceCredentials() only when they change, but it looks like
SetRemoteIceCredentials() verifies it anyway, so I'm not sure we would get
anything from duplicating the same check here.

[jiayl, 2015-04-15 20:51:08]

lgtm

[Sergey Ulanov, 2015-04-15 20:52:27]

On 2015/04/15 20:49:34, Sergey Ulanov wrote:
>
https://codereview.chromium.org/1070233004/diff/1/remoting/protocol/libjingle...
> File remoting/protocol/libjingle_transport_factory.cc (right):
> 
>
https://codereview.chromium.org/1070233004/diff/1/remoting/protocol/libjingle...
> remoting/protocol/libjingle_transport_factory.cc:154:
> pending_candidates_.front().password());
> On 2015/04/15 20:13:00, jiayl wrote:
> > Do all candidates share the same username/password? Add a check for that,
and
> > call SetRemoteIceCredentials just once?
> 
> Credentials may change when the remote peer restarts ICE. We could call
> SetRemoteIceCredentials() only when they change, but it looks like
> SetRemoteIceCredentials() verifies it anyway, so I'm not sure we would get
> anything from duplicating the same check here.

Also I agree that it doesn't look pretty. This is a short-term fix. I plan to
improve it when fixing crbug.com/473758.

[Sergey Ulanov, 2015-04-15 20:53:28]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2015-04-15 20:54:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070233004/1

[commit-bot: I haz the power, 2015-04-15 22:05:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-04-15 22:06:53]

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