From f05eec2b5d3c55a1a53fc026aeba7fdeeb3e1792 Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Wed, 30 Jan 2019 20:58:30 +0000 Subject: [PATCH] Handle role conflict in the standalone RTCIceTransport Bug: 926134 Change-Id: I202c76275a97d295866ea70079f95f814e6de5b6 Reviewed-on: https://chromium-review.googlesource.com/c/1443544 Commit-Queue: Steve Anton Commit-Queue: Seth Hampson Auto-Submit: Steve Anton Reviewed-by: Seth Hampson Cr-Original-Commit-Position: refs/heads/master@{#627108}(cherry picked from commit f1fbab6b3a447646ac2d4d76adb6185d9fe30524) Reviewed-on: https://chromium-review.googlesource.com/c/1446839 Reviewed-by: Steve Anton Cr-Commit-Position: refs/branch-heads/3683@{#74} Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896} --- .../adapters/ice_transport_adapter_impl.cc | 35 +++++++++++++++++++ .../adapters/ice_transport_adapter_impl.h | 1 + .../RTCIceTransport-extension.https.html | 20 +++++++++++ 3 files changed, 56 insertions(+) diff --git a/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.cc b/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.cc index f3017c8c3862..f14f11e822aa 100644 --- a/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.cc +++ b/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.cc @@ -37,6 +37,8 @@ IceTransportAdapterImpl::IceTransportAdapterImpl( this, &IceTransportAdapterImpl::OnStateChanged); p2p_transport_channel_->SignalNetworkRouteChanged.connect( this, &IceTransportAdapterImpl::OnNetworkRouteChanged); + p2p_transport_channel_->SignalRoleConflict.connect( + this, &IceTransportAdapterImpl::OnRoleConflict); // We need to set the ICE role even before Start is called since the Port // assumes that the role has been set before receiving incoming connectivity // checks. These checks can race with the information signaled for Start. @@ -141,4 +143,37 @@ void IceTransportAdapterImpl::OnNetworkRouteChanged( selected_connection->remote_candidate())); } +static const char* IceRoleToString(cricket::IceRole role) { + switch (role) { + case cricket::ICEROLE_CONTROLLING: + return "controlling"; + case cricket::ICEROLE_CONTROLLED: + return "controlled"; + default: + return "unknown"; + } +} + +static cricket::IceRole IceRoleReversed(cricket::IceRole role) { + switch (role) { + case cricket::ICEROLE_CONTROLLING: + return cricket::ICEROLE_CONTROLLED; + case cricket::ICEROLE_CONTROLLED: + return cricket::ICEROLE_CONTROLLING; + default: + return cricket::ICEROLE_UNKNOWN; + } +} + +void IceTransportAdapterImpl::OnRoleConflict( + cricket::IceTransportInternal* transport) { + DCHECK_EQ(transport, p2p_transport_channel_.get()); + // This logic is copied from JsepTransportController. + cricket::IceRole reversed_role = + IceRoleReversed(p2p_transport_channel_->GetIceRole()); + LOG(INFO) << "Got role conflict; switching to " + << IceRoleToString(reversed_role) << " role."; + p2p_transport_channel_->SetIceRole(reversed_role); +} + } // namespace blink diff --git a/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.h b/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.h index e6748a99f7dc..fd620d916c43 100644 --- a/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.h +++ b/third_party/blink/renderer/modules/peerconnection/adapters/ice_transport_adapter_impl.h @@ -47,6 +47,7 @@ class IceTransportAdapterImpl final : public IceTransportAdapter, void OnStateChanged(cricket::IceTransportInternal* transport); void OnNetworkRouteChanged( absl::optional new_network_route); + void OnRoleConflict(cricket::IceTransportInternal* transport); Delegate* const delegate_; std::unique_ptr port_allocator_; diff --git a/third_party/blink/web_tests/external/wpt/webrtc/RTCIceTransport-extension.https.html b/third_party/blink/web_tests/external/wpt/webrtc/RTCIceTransport-extension.https.html index 7803bde9b3aa..bf95ba301c58 100644 --- a/third_party/blink/web_tests/external/wpt/webrtc/RTCIceTransport-extension.https.html +++ b/third_party/blink/web_tests/external/wpt/webrtc/RTCIceTransport-extension.https.html @@ -254,6 +254,26 @@ ]); }, 'Two RTCIceTransports connect to each other'); +['controlling', 'controlled'].forEach(role => { + promise_test(async t => { + const [ localTransport, remoteTransport ] = + makeAndGatherTwoIceTransports(t); + localTransport.start(remoteTransport.getLocalParameters(), role); + localTransport.start(remoteTransport.getLocalParameters(), role); + const localWatcher = new EventWatcher(t, localTransport, 'statechange'); + const remoteWatcher = new EventWatcher(t, remoteTransport, 'statechange'); + await Promise.all([ + localWatcher.wait_for('statechange').then(() => { + assert_equals(localTransport.state, 'connected'); + }), + remoteWatcher.wait_for('statechange').then(() => { + assert_equals(remoteTransport.state, 'connected'); + }), + ]); + }, `Two RTCIceTransports configured with the ${role} role resolve the ` + + 'conflict in band and still connect.'); +}); + promise_test(async t => { async function waitForConnectedThenSelectedCandidatePairChange(t, transport, transportName) {