Implement the "needs-ice-restart" logic for SetConfiguration.
Changing the configuration will cause subsequently generated offers to change the ufrag/pwd as necessary, so that a new round of gathering is started that uses the new configuration. This CL also makes some minor unrelated changes: changing the reference SDP in the PC tests to more match what we generate, and relaxing the network thread requirement for JsepTransport (since there's no reason the "needs-ice-restart" flag can't be accessed from the signaling thread). BUG=webrtc:6714 Review-Url: https://codereview.webrtc.org/2563153002 Cr-Commit-Position: refs/heads/master@{#15527}
This commit is contained in:
deadbeef
Commit bot
parent
3edec7cf1b
commit
d1a38b591d
@ -1298,9 +1298,16 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration) {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(deadbeef): Shouldn't have to hop to the worker thread twice...
|
||||
// TODO(deadbeef): Shouldn't have to hop to the network thread twice...
|
||||
session_->SetIceConfig(session_->ParseIceConfig(configuration));
|
||||
|
||||
// As described in JSEP, calling setConfiguration with new ICE servers or
|
||||
// candidate policy must set a "needs-ice-restart" bit so that the next offer
|
||||
// triggers an ICE restart which will pick up the changes.
|
||||
if (configuration.servers != configuration_.servers ||
|
||||
configuration.type != configuration_.type) {
|
||||
session_->SetNeedsIceRestartFlag();
|
||||
}
|
||||
configuration_ = configuration;
|
||||
return true;
|
||||
}
|
||||
|
||||
@ -214,6 +214,11 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
|
||||
std::vector<std::string> urls;
|
||||
std::string username;
|
||||
std::string password;
|
||||
bool operator==(const IceServer& o) const {
|
||||
return uri == o.uri && urls == o.urls && username == o.username &&
|
||||
password == o.password;
|
||||
}
|
||||
bool operator!=(const IceServer& o) const { return !(*this == o); }
|
||||
};
|
||||
typedef std::vector<IceServer> IceServers;
|
||||
|
||||
|
||||
@ -73,11 +73,11 @@ static const char kSdpStringWithStream1[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -86,6 +86,10 @@ static const char kSdpStringWithStream1[] =
|
||||
"a=ssrc:1 mslabel:stream1\r\n"
|
||||
"a=ssrc:1 label:audiotrack0\r\n"
|
||||
"m=video 1 RTP/AVPF 120\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=mid:video\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -101,11 +105,11 @@ static const char kSdpStringWithStream1AudioTrackOnly[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtpmap:103 ISAC/16000\r\n"
|
||||
@ -122,12 +126,12 @@ static const char kSdpStringWithStream1And2[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"a=msid-semantic: WMS stream1 stream2\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=msid-semantic: WMS stream1 stream2\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -137,6 +141,10 @@ static const char kSdpStringWithStream1And2[] =
|
||||
"a=ssrc:3 cname:stream2\r\n"
|
||||
"a=ssrc:3 msid:stream2 audiotrack1\r\n"
|
||||
"m=video 1 RTP/AVPF 120\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=mid:video\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -152,16 +160,20 @@ static const char kSdpStringWithoutStreams[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=rtpmap:103 ISAC/16000\r\n"
|
||||
"m=video 1 RTP/AVPF 120\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=mid:video\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -173,17 +185,21 @@ static const char kSdpStringWithMsidWithoutStreams[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"a=msid-semantic: WMS\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=msid-semantic: WMS\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=rtpmap:103 ISAC/16000\r\n"
|
||||
"m=video 1 RTP/AVPF 120\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=mid:video\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -195,11 +211,11 @@ static const char kSdpStringWithoutStreamsAudioOnly[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -211,17 +227,21 @@ static const char kSdpStringSendOnlyWithoutStreams[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=sendonly\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
"a=rtpmap:103 ISAC/16000\r\n"
|
||||
"m=video 1 RTP/AVPF 120\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=mid:video\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=sendonly\r\n"
|
||||
@ -233,14 +253,14 @@ static const char kSdpStringInit[] =
|
||||
"o=- 0 0 IN IP4 127.0.0.1\r\n"
|
||||
"s=-\r\n"
|
||||
"t=0 0\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=msid-semantic: WMS\r\n";
|
||||
|
||||
static const char kSdpStringAudio[] =
|
||||
"m=audio 1 RTP/AVPF 103\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=mid:audio\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -248,6 +268,10 @@ static const char kSdpStringAudio[] =
|
||||
|
||||
static const char kSdpStringVideo[] =
|
||||
"m=video 1 RTP/AVPF 120\r\n"
|
||||
"a=ice-ufrag:e5785931\r\n"
|
||||
"a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
|
||||
"a=fingerprint:sha-256 58:AB:6E:F5:F1:E4:57:B7:E9:46:F4:86:04:28:F9:A7:ED:"
|
||||
"BD:AB:AE:40:EF:CE:9A:51:2C:2A:B1:9B:8B:78:84\r\n"
|
||||
"a=mid:video\r\n"
|
||||
"a=sendrecv\r\n"
|
||||
"a=rtcp-mux\r\n"
|
||||
@ -326,6 +350,18 @@ bool GetFirstSsrc(const cricket::ContentInfo* content_info, int* ssrc) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Get the ufrags out of an SDP blob. Useful for testing ICE restart
|
||||
// behavior.
|
||||
std::vector<std::string> GetUfrags(
|
||||
const webrtc::SessionDescriptionInterface* desc) {
|
||||
std::vector<std::string> ufrags;
|
||||
for (const cricket::TransportInfo& info :
|
||||
desc->description()->transport_infos()) {
|
||||
ufrags.push_back(info.description.ice_ufrag);
|
||||
}
|
||||
return ufrags;
|
||||
}
|
||||
|
||||
void SetSsrcToZero(std::string* sdp) {
|
||||
const char kSdpSsrcAtribute[] = "a=ssrc:";
|
||||
const char kSdpSsrcAtributeZero[] = "a=ssrc:0";
|
||||
@ -2727,6 +2763,116 @@ TEST_F(PeerConnectionInterfaceTest, OnAddTrackCallback) {
|
||||
EXPECT_EQ(observer_.last_added_track_label_, kVideoTracks[0]);
|
||||
}
|
||||
|
||||
// Test that when SetConfiguration is called and the configuration is
|
||||
// changing, the next offer causes an ICE restart.
|
||||
TEST_F(PeerConnectionInterfaceTest, SetConfigurationCausingIceRetart) {
|
||||
PeerConnectionInterface::RTCConfiguration config;
|
||||
config.type = PeerConnectionInterface::kRelay;
|
||||
// Need to pass default constraints to prevent disabling of DTLS...
|
||||
FakeConstraints default_constraints;
|
||||
CreatePeerConnection(config, &default_constraints);
|
||||
AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label");
|
||||
|
||||
// Do initial offer/answer so there's something to restart.
|
||||
CreateOfferAsLocalDescription();
|
||||
CreateAnswerAsRemoteDescription(kSdpStringWithStream1);
|
||||
|
||||
// Grab the ufrags.
|
||||
std::vector<std::string> initial_ufrags = GetUfrags(pc_->local_description());
|
||||
|
||||
// Change ICE policy, which should trigger an ICE restart on the next offer.
|
||||
config.type = PeerConnectionInterface::kAll;
|
||||
EXPECT_TRUE(pc_->SetConfiguration(config));
|
||||
CreateOfferAsLocalDescription();
|
||||
|
||||
// Grab the new ufrags.
|
||||
std::vector<std::string> subsequent_ufrags =
|
||||
GetUfrags(pc_->local_description());
|
||||
|
||||
// Sanity check.
|
||||
EXPECT_EQ(initial_ufrags.size(), subsequent_ufrags.size());
|
||||
// Check that each ufrag is different.
|
||||
for (int i = 0; i < static_cast<int>(initial_ufrags.size()); ++i) {
|
||||
EXPECT_NE(initial_ufrags[i], subsequent_ufrags[i]);
|
||||
}
|
||||
}
|
||||
|
||||
// Test that when SetConfiguration is called and the configuration *isn't*
|
||||
// changing, the next offer does *not* cause an ICE restart.
|
||||
TEST_F(PeerConnectionInterfaceTest, SetConfigurationNotCausingIceRetart) {
|
||||
PeerConnectionInterface::RTCConfiguration config;
|
||||
config.type = PeerConnectionInterface::kRelay;
|
||||
// Need to pass default constraints to prevent disabling of DTLS...
|
||||
FakeConstraints default_constraints;
|
||||
CreatePeerConnection(config, &default_constraints);
|
||||
AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label");
|
||||
|
||||
// Do initial offer/answer so there's something to restart.
|
||||
CreateOfferAsLocalDescription();
|
||||
CreateAnswerAsRemoteDescription(kSdpStringWithStream1);
|
||||
|
||||
// Grab the ufrags.
|
||||
std::vector<std::string> initial_ufrags = GetUfrags(pc_->local_description());
|
||||
|
||||
// Call SetConfiguration with a config identical to what the PC was
|
||||
// constructed with.
|
||||
EXPECT_TRUE(pc_->SetConfiguration(config));
|
||||
CreateOfferAsLocalDescription();
|
||||
|
||||
// Grab the new ufrags.
|
||||
std::vector<std::string> subsequent_ufrags =
|
||||
GetUfrags(pc_->local_description());
|
||||
|
||||
EXPECT_EQ(initial_ufrags, subsequent_ufrags);
|
||||
}
|
||||
|
||||
// Test for a weird corner case scenario:
|
||||
// 1. Audio/video session established.
|
||||
// 2. SetConfiguration changes ICE config; ICE restart needed.
|
||||
// 3. ICE restart initiated by remote peer, but only for one m= section.
|
||||
// 4. Next createOffer should initiate an ICE restart, but only for the other
|
||||
// m= section; it would be pointless to do an ICE restart for the m= section
|
||||
// that was already restarted.
|
||||
TEST_F(PeerConnectionInterfaceTest, SetConfigurationCausingPartialIceRestart) {
|
||||
PeerConnectionInterface::RTCConfiguration config;
|
||||
config.type = PeerConnectionInterface::kRelay;
|
||||
// Need to pass default constraints to prevent disabling of DTLS...
|
||||
FakeConstraints default_constraints;
|
||||
CreatePeerConnection(config, &default_constraints);
|
||||
AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label");
|
||||
|
||||
// Do initial offer/answer so there's something to restart.
|
||||
CreateOfferAsLocalDescription();
|
||||
CreateAnswerAsRemoteDescription(kSdpStringWithStream1);
|
||||
|
||||
// Change ICE policy, which should set the "needs-ice-restart" flag.
|
||||
config.type = PeerConnectionInterface::kAll;
|
||||
EXPECT_TRUE(pc_->SetConfiguration(config));
|
||||
|
||||
// Do ICE restart for the first m= section, initiated by remote peer.
|
||||
webrtc::JsepSessionDescription* remote_offer =
|
||||
new webrtc::JsepSessionDescription(SessionDescriptionInterface::kOffer);
|
||||
EXPECT_TRUE(remote_offer->Initialize(kSdpStringWithStream1, nullptr));
|
||||
remote_offer->description()->transport_infos()[0].description.ice_ufrag =
|
||||
"modified";
|
||||
EXPECT_TRUE(DoSetRemoteDescription(remote_offer));
|
||||
CreateAnswerAsLocalDescription();
|
||||
|
||||
// Grab the ufrags.
|
||||
std::vector<std::string> initial_ufrags = GetUfrags(pc_->local_description());
|
||||
ASSERT_EQ(2, initial_ufrags.size());
|
||||
|
||||
// Create offer and grab the new ufrags.
|
||||
CreateOfferAsLocalDescription();
|
||||
std::vector<std::string> subsequent_ufrags =
|
||||
GetUfrags(pc_->local_description());
|
||||
ASSERT_EQ(2, subsequent_ufrags.size());
|
||||
|
||||
// Ensure that only the ufrag for the second m= section changed.
|
||||
EXPECT_EQ(initial_ufrags[0], subsequent_ufrags[0]);
|
||||
EXPECT_NE(initial_ufrags[1], subsequent_ufrags[1]);
|
||||
}
|
||||
|
||||
class PeerConnectionMediaConfigTest : public testing::Test {
|
||||
protected:
|
||||
void SetUp() override {
|
||||
|
||||
@ -1330,6 +1330,14 @@ bool WebRtcSession::IceRestartPending(const std::string& content_name) const {
|
||||
pending_ice_restarts_.end();
|
||||
}
|
||||
|
||||
void WebRtcSession::SetNeedsIceRestartFlag() {
|
||||
transport_controller_->SetNeedsIceRestartFlag();
|
||||
}
|
||||
|
||||
bool WebRtcSession::NeedsIceRestart(const std::string& content_name) const {
|
||||
return transport_controller_->NeedsIceRestart(content_name);
|
||||
}
|
||||
|
||||
void WebRtcSession::OnCertificateReady(
|
||||
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
|
||||
transport_controller_->SetLocalCertificate(certificate);
|
||||
|
||||
@ -278,8 +278,19 @@ class WebRtcSession :
|
||||
|
||||
cricket::DataChannelType data_channel_type() const;
|
||||
|
||||
// Returns true if there was an ICE restart initiated by the remote offer.
|
||||
bool IceRestartPending(const std::string& content_name) const;
|
||||
|
||||
// Set the "needs-ice-restart" flag as described in JSEP. After the flag is
|
||||
// set, offers should generate new ufrags/passwords until an ICE restart
|
||||
// occurs.
|
||||
void SetNeedsIceRestartFlag();
|
||||
// Returns true if the ICE restart flag above was set, and no ICE restart has
|
||||
// occurred yet for this transport (by applying a local description with
|
||||
// changed ufrag/password). If the transport has been deleted as a result of
|
||||
// bundling, returns false.
|
||||
bool NeedsIceRestart(const std::string& content_name) const;
|
||||
|
||||
// Called when an RTCCertificate is generated or retrieved by
|
||||
// WebRTCSessionDescriptionFactory. Should happen before setLocalDescription.
|
||||
void OnCertificateReady(
|
||||
|
||||
@ -338,6 +338,18 @@ void WebRtcSessionDescriptionFactory::OnMessage(rtc::Message* msg) {
|
||||
|
||||
void WebRtcSessionDescriptionFactory::InternalCreateOffer(
|
||||
CreateSessionDescriptionRequest request) {
|
||||
if (session_->local_description()) {
|
||||
for (const cricket::TransportInfo& transport :
|
||||
session_->local_description()->description()->transport_infos()) {
|
||||
// If the needs-ice-restart flag is set as described by JSEP, we should
|
||||
// generate an offer with a new ufrag/password to trigger an ICE restart.
|
||||
if (session_->NeedsIceRestart(transport.content_name)) {
|
||||
request.options.transport_options[transport.content_name].ice_restart =
|
||||
true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
cricket::SessionDescription* desc(session_desc_factory_.CreateOffer(
|
||||
request.options, session_->local_description()
|
||||
? session_->local_description()->description()
|
||||
|
||||
@ -176,6 +176,11 @@ bool JsepTransport::SetLocalTransportDescription(
|
||||
error_desc);
|
||||
}
|
||||
|
||||
bool ice_restarting =
|
||||
local_description_set_ &&
|
||||
IceCredentialsChanged(local_description_->ice_ufrag,
|
||||
local_description_->ice_pwd, description.ice_ufrag,
|
||||
description.ice_pwd);
|
||||
local_description_.reset(new TransportDescription(description));
|
||||
|
||||
rtc::SSLFingerprint* local_fp =
|
||||
@ -199,11 +204,17 @@ bool JsepTransport::SetLocalTransportDescription(
|
||||
if (action == CA_PRANSWER || action == CA_ANSWER) {
|
||||
ret &= NegotiateTransportDescription(action, error_desc);
|
||||
}
|
||||
if (ret) {
|
||||
local_description_set_ = true;
|
||||
if (!ret) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return ret;
|
||||
if (needs_ice_restart_ && ice_restarting) {
|
||||
needs_ice_restart_ = false;
|
||||
LOG(LS_VERBOSE) << "needs-ice-restart flag cleared for transport " << mid();
|
||||
}
|
||||
|
||||
local_description_set_ = true;
|
||||
return true;
|
||||
}
|
||||
|
||||
bool JsepTransport::SetRemoteTransportDescription(
|
||||
@ -233,6 +244,17 @@ bool JsepTransport::SetRemoteTransportDescription(
|
||||
return ret;
|
||||
}
|
||||
|
||||
void JsepTransport::SetNeedsIceRestartFlag() {
|
||||
if (!needs_ice_restart_) {
|
||||
needs_ice_restart_ = true;
|
||||
LOG(LS_VERBOSE) << "needs-ice-restart flag set for transport " << mid();
|
||||
}
|
||||
}
|
||||
|
||||
bool JsepTransport::NeedsIceRestart() const {
|
||||
return needs_ice_restart_;
|
||||
}
|
||||
|
||||
void JsepTransport::GetSslRole(rtc::SSLRole* ssl_role) const {
|
||||
RTC_DCHECK(ssl_role);
|
||||
*ssl_role = secure_role_;
|
||||
|
||||
@ -288,6 +288,18 @@ class JsepTransport : public sigslot::has_slots<> {
|
||||
ContentAction action,
|
||||
std::string* error_desc);
|
||||
|
||||
// Set the "needs-ice-restart" flag as described in JSEP. After the flag is
|
||||
// set, offers should generate new ufrags/passwords until an ICE restart
|
||||
// occurs.
|
||||
//
|
||||
// This and the below method can be called safely from any thread as long as
|
||||
// SetXTransportDescription is not in progress.
|
||||
void SetNeedsIceRestartFlag();
|
||||
// Returns true if the ICE restart flag above was set, and no ICE restart has
|
||||
// occurred yet for this transport (by applying a local description with
|
||||
// changed ufrag/password).
|
||||
bool NeedsIceRestart() const;
|
||||
|
||||
void GetSslRole(rtc::SSLRole* ssl_role) const;
|
||||
|
||||
// TODO(deadbeef): Make this const. See comment in transportcontroller.h.
|
||||
@ -348,6 +360,8 @@ class JsepTransport : public sigslot::has_slots<> {
|
||||
std::string* error_desc);
|
||||
|
||||
const std::string mid_;
|
||||
// needs-ice-restart bit as described in JSEP.
|
||||
bool needs_ice_restart_ = false;
|
||||
rtc::scoped_refptr<rtc::RTCCertificate> certificate_;
|
||||
rtc::SSLRole secure_role_ = rtc::SSL_CLIENT;
|
||||
std::unique_ptr<rtc::SSLFingerprint> remote_fingerprint_;
|
||||
|
||||
@ -73,6 +73,41 @@ TEST_F(JsepTransportTest, TestIceCredentialsChanged) {
|
||||
EXPECT_FALSE(cricket::IceCredentialsChanged("u1", "p1", "u1", "p1"));
|
||||
}
|
||||
|
||||
// Tests SetNeedsIceRestartFlag and NeedsIceRestart, ensuring NeedsIceRestart
|
||||
// only starts returning "false" once an ICE restart has been initiated.
|
||||
TEST_F(JsepTransportTest, NeedsIceRestart) {
|
||||
// Do initial offer/answer so there's something to restart.
|
||||
cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1);
|
||||
cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1);
|
||||
ASSERT_TRUE(transport_->SetLocalTransportDescription(
|
||||
local_desc, cricket::CA_OFFER, nullptr));
|
||||
ASSERT_TRUE(transport_->SetRemoteTransportDescription(
|
||||
remote_desc, cricket::CA_ANSWER, nullptr));
|
||||
|
||||
// Flag initially should be false.
|
||||
EXPECT_FALSE(transport_->NeedsIceRestart());
|
||||
|
||||
// After setting flag, it should be true.
|
||||
transport_->SetNeedsIceRestartFlag();
|
||||
EXPECT_TRUE(transport_->NeedsIceRestart());
|
||||
|
||||
// Doing an identical offer/answer shouldn't clear the flag.
|
||||
ASSERT_TRUE(transport_->SetLocalTransportDescription(
|
||||
local_desc, cricket::CA_OFFER, nullptr));
|
||||
ASSERT_TRUE(transport_->SetRemoteTransportDescription(
|
||||
remote_desc, cricket::CA_ANSWER, nullptr));
|
||||
EXPECT_TRUE(transport_->NeedsIceRestart());
|
||||
|
||||
// Doing an offer/answer that restarts ICE should clear the flag.
|
||||
cricket::TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2);
|
||||
cricket::TransportDescription ice_restart_remote_desc(kIceUfrag2, kIcePwd2);
|
||||
ASSERT_TRUE(transport_->SetLocalTransportDescription(
|
||||
ice_restart_local_desc, cricket::CA_OFFER, nullptr));
|
||||
ASSERT_TRUE(transport_->SetRemoteTransportDescription(
|
||||
ice_restart_remote_desc, cricket::CA_ANSWER, nullptr));
|
||||
EXPECT_FALSE(transport_->NeedsIceRestart());
|
||||
}
|
||||
|
||||
TEST_F(JsepTransportTest, TestGetStats) {
|
||||
EXPECT_TRUE(SetupChannel());
|
||||
cricket::TransportStats stats;
|
||||
|
||||
@ -80,6 +80,21 @@ void TransportController::SetIceRole(IceRole ice_role) {
|
||||
rtc::Bind(&TransportController::SetIceRole_n, this, ice_role));
|
||||
}
|
||||
|
||||
void TransportController::SetNeedsIceRestartFlag() {
|
||||
for (auto& kv : transports_) {
|
||||
kv.second->SetNeedsIceRestartFlag();
|
||||
}
|
||||
}
|
||||
|
||||
bool TransportController::NeedsIceRestart(
|
||||
const std::string& transport_name) const {
|
||||
const JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (!transport) {
|
||||
return false;
|
||||
}
|
||||
return transport->NeedsIceRestart();
|
||||
}
|
||||
|
||||
bool TransportController::GetSslRole(const std::string& transport_name,
|
||||
rtc::SSLRole* role) const {
|
||||
return network_thread_->Invoke<bool>(
|
||||
@ -187,7 +202,7 @@ TransportChannel* TransportController::CreateTransportChannel_n(
|
||||
}
|
||||
|
||||
// Need to create a new channel.
|
||||
JsepTransport* transport = GetOrCreateJsepTransport_n(transport_name);
|
||||
JsepTransport* transport = GetOrCreateJsepTransport(transport_name);
|
||||
|
||||
// Create DTLS channel wrapping ICE channel, and configure it.
|
||||
TransportChannelImpl* ice =
|
||||
@ -251,7 +266,7 @@ void TransportController::DestroyTransportChannel_n(
|
||||
}
|
||||
channels_.erase(it);
|
||||
|
||||
JsepTransport* t = GetJsepTransport_n(transport_name);
|
||||
JsepTransport* t = GetJsepTransport(transport_name);
|
||||
bool channel_removed = t->RemoveChannel(component);
|
||||
RTC_DCHECK(channel_removed);
|
||||
// Just as we create a Transport when its first channel is created,
|
||||
@ -362,16 +377,14 @@ TransportController::GetChannelIterator_n(const std::string& transport_name,
|
||||
});
|
||||
}
|
||||
|
||||
const JsepTransport* TransportController::GetJsepTransport_n(
|
||||
const JsepTransport* TransportController::GetJsepTransport(
|
||||
const std::string& transport_name) const {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
auto it = transports_.find(transport_name);
|
||||
return (it == transports_.end()) ? nullptr : it->second.get();
|
||||
}
|
||||
|
||||
JsepTransport* TransportController::GetJsepTransport_n(
|
||||
JsepTransport* TransportController::GetJsepTransport(
|
||||
const std::string& transport_name) {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
auto it = transports_.find(transport_name);
|
||||
return (it == transports_.end()) ? nullptr : it->second.get();
|
||||
}
|
||||
@ -392,11 +405,11 @@ TransportController::RefCountedChannel* TransportController::GetChannel_n(
|
||||
return (it == channels_.end()) ? nullptr : &(*it);
|
||||
}
|
||||
|
||||
JsepTransport* TransportController::GetOrCreateJsepTransport_n(
|
||||
JsepTransport* TransportController::GetOrCreateJsepTransport(
|
||||
const std::string& transport_name) {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
|
||||
JsepTransport* transport = GetJsepTransport_n(transport_name);
|
||||
JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (transport) {
|
||||
return transport;
|
||||
}
|
||||
@ -447,7 +460,7 @@ bool TransportController::GetSslRole_n(const std::string& transport_name,
|
||||
rtc::SSLRole* role) const {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
|
||||
const JsepTransport* t = GetJsepTransport_n(transport_name);
|
||||
const JsepTransport* t = GetJsepTransport(transport_name);
|
||||
if (!t) {
|
||||
return false;
|
||||
}
|
||||
@ -483,7 +496,7 @@ bool TransportController::GetLocalCertificate_n(
|
||||
rtc::scoped_refptr<rtc::RTCCertificate>* certificate) const {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
|
||||
const JsepTransport* t = GetJsepTransport_n(transport_name);
|
||||
const JsepTransport* t = GetJsepTransport(transport_name);
|
||||
if (!t) {
|
||||
return false;
|
||||
}
|
||||
@ -512,7 +525,7 @@ bool TransportController::SetLocalTransportDescription_n(
|
||||
std::string* err) {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
|
||||
JsepTransport* transport = GetJsepTransport_n(transport_name);
|
||||
JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (!transport) {
|
||||
// If we didn't find a transport, that's not an error;
|
||||
// it could have been deleted as a result of bundling.
|
||||
@ -556,7 +569,7 @@ bool TransportController::SetRemoteTransportDescription_n(
|
||||
SetIceRole_n(ICEROLE_CONTROLLING);
|
||||
}
|
||||
|
||||
JsepTransport* transport = GetJsepTransport_n(transport_name);
|
||||
JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (!transport) {
|
||||
// If we didn't find a transport, that's not an error;
|
||||
// it could have been deleted as a result of bundling.
|
||||
@ -586,7 +599,7 @@ bool TransportController::AddRemoteCandidates_n(
|
||||
return false;
|
||||
}
|
||||
|
||||
JsepTransport* transport = GetJsepTransport_n(transport_name);
|
||||
JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (!transport) {
|
||||
// If we didn't find a transport, that's not an error;
|
||||
// it could have been deleted as a result of bundling.
|
||||
@ -625,7 +638,7 @@ bool TransportController::RemoveRemoteCandidates_n(const Candidates& candidates,
|
||||
for (const auto& kv : candidates_by_transport_name) {
|
||||
const std::string& transport_name = kv.first;
|
||||
const Candidates& candidates = kv.second;
|
||||
JsepTransport* transport = GetJsepTransport_n(transport_name);
|
||||
JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (!transport) {
|
||||
// If we didn't find a transport, that's not an error;
|
||||
// it could have been deleted as a result of bundling.
|
||||
@ -646,7 +659,7 @@ bool TransportController::ReadyForRemoteCandidates_n(
|
||||
const std::string& transport_name) const {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
|
||||
const JsepTransport* transport = GetJsepTransport_n(transport_name);
|
||||
const JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (!transport) {
|
||||
return false;
|
||||
}
|
||||
@ -657,7 +670,7 @@ bool TransportController::GetStats_n(const std::string& transport_name,
|
||||
TransportStats* stats) {
|
||||
RTC_DCHECK(network_thread_->IsCurrent());
|
||||
|
||||
JsepTransport* transport = GetJsepTransport_n(transport_name);
|
||||
JsepTransport* transport = GetJsepTransport(transport_name);
|
||||
if (!transport) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@ -64,6 +64,16 @@ class TransportController : public sigslot::has_slots<>,
|
||||
void SetIceConfig(const IceConfig& config);
|
||||
void SetIceRole(IceRole ice_role);
|
||||
|
||||
// Set the "needs-ice-restart" flag as described in JSEP. After the flag is
|
||||
// set, offers should generate new ufrags/passwords until an ICE restart
|
||||
// occurs.
|
||||
void SetNeedsIceRestartFlag();
|
||||
// Returns true if the ICE restart flag above was set, and no ICE restart has
|
||||
// occurred yet for this transport (by applying a local description with
|
||||
// changed ufrag/password). If the transport has been deleted as a result of
|
||||
// bundling, returns false.
|
||||
bool NeedsIceRestart(const std::string& transport_name) const;
|
||||
|
||||
bool GetSslRole(const std::string& transport_name, rtc::SSLRole* role) const;
|
||||
|
||||
// Specifies the identity to use in this session.
|
||||
@ -203,15 +213,15 @@ class TransportController : public sigslot::has_slots<>,
|
||||
std::vector<RefCountedChannel>::const_iterator GetChannelIterator_n(
|
||||
const std::string& transport_name,
|
||||
int component) const;
|
||||
const JsepTransport* GetJsepTransport_n(
|
||||
const JsepTransport* GetJsepTransport(
|
||||
const std::string& transport_name) const;
|
||||
JsepTransport* GetJsepTransport_n(const std::string& transport_name);
|
||||
JsepTransport* GetJsepTransport(const std::string& transport_name);
|
||||
const RefCountedChannel* GetChannel_n(const std::string& transport_name,
|
||||
int component) const;
|
||||
RefCountedChannel* GetChannel_n(const std::string& transport_name,
|
||||
int component);
|
||||
|
||||
JsepTransport* GetOrCreateJsepTransport_n(const std::string& transport_name);
|
||||
JsepTransport* GetOrCreateJsepTransport(const std::string& transport_name);
|
||||
void DestroyAllChannels_n();
|
||||
|
||||
bool SetSslMaxProtocolVersion_n(rtc::SSLProtocolVersion version);
|
||||
|
||||
@ -793,4 +793,45 @@ TEST_F(TransportControllerTest, TestSetRemoteIceLiteInAnswer) {
|
||||
EXPECT_EQ(ICEMODE_LITE, channel->remote_ice_mode());
|
||||
}
|
||||
|
||||
// Tests SetNeedsIceRestartFlag and NeedsIceRestart, setting the flag and then
|
||||
// initiating an ICE restart for one of the transports.
|
||||
TEST_F(TransportControllerTest, NeedsIceRestart) {
|
||||
CreateChannel("audio", 1);
|
||||
CreateChannel("video", 1);
|
||||
|
||||
// Do initial offer/answer so there's something to restart.
|
||||
TransportDescription local_desc(kIceUfrag1, kIcePwd1);
|
||||
TransportDescription remote_desc(kIceUfrag1, kIcePwd1);
|
||||
ASSERT_TRUE(transport_controller_->SetLocalTransportDescription(
|
||||
"audio", local_desc, CA_OFFER, nullptr));
|
||||
ASSERT_TRUE(transport_controller_->SetLocalTransportDescription(
|
||||
"video", local_desc, CA_OFFER, nullptr));
|
||||
ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription(
|
||||
"audio", remote_desc, CA_ANSWER, nullptr));
|
||||
ASSERT_TRUE(transport_controller_->SetRemoteTransportDescription(
|
||||
"video", remote_desc, CA_ANSWER, nullptr));
|
||||
|
||||
// Initially NeedsIceRestart should return false.
|
||||
EXPECT_FALSE(transport_controller_->NeedsIceRestart("audio"));
|
||||
EXPECT_FALSE(transport_controller_->NeedsIceRestart("video"));
|
||||
|
||||
// Set the needs-ice-restart flag and verify NeedsIceRestart starts returning
|
||||
// true.
|
||||
transport_controller_->SetNeedsIceRestartFlag();
|
||||
EXPECT_TRUE(transport_controller_->NeedsIceRestart("audio"));
|
||||
EXPECT_TRUE(transport_controller_->NeedsIceRestart("video"));
|
||||
// For a nonexistent transport, false should be returned.
|
||||
EXPECT_FALSE(transport_controller_->NeedsIceRestart("deadbeef"));
|
||||
|
||||
// Do ICE restart but only for audio.
|
||||
TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2);
|
||||
ASSERT_TRUE(transport_controller_->SetLocalTransportDescription(
|
||||
"audio", ice_restart_local_desc, CA_OFFER, nullptr));
|
||||
ASSERT_TRUE(transport_controller_->SetLocalTransportDescription(
|
||||
"video", local_desc, CA_OFFER, nullptr));
|
||||
// NeedsIceRestart should still be true for video.
|
||||
EXPECT_FALSE(transport_controller_->NeedsIceRestart("audio"));
|
||||
EXPECT_TRUE(transport_controller_->NeedsIceRestart("video"));
|
||||
}
|
||||
|
||||
} // namespace cricket {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user