admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[PeerConnection] Implement restartIce().

Browse Source 
This is part of "Perfect Negotiation" (https://crbug.com/980872).
Spec PR here (merged): https://github.com/w3c/webrtc-pc/pull/2169
Spec: https://w3c.github.io/webrtc-pc/#dfn-localufragstoreplace

The restartIce() makes the next createOffer() generate new ICE
credentials, as if {iceRestart:true} was passed in as options. It also
causes negotiationneeded. This is better than manually restarting ICE
because it survives rollbacks (when that is implemented) and
restartIce() can be called regardless of current signalingState.

Bug: chromium:980881
Change-Id: I8e70bec31ce9d4d6a303bd35e91b2dcc28fcad60
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144941
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28596}
This commit is contained in:
Henrik Boström 2019-07-18 11:16:56 +02:00 committed by Commit Bot
parent b41d5f1f40
commit 79b6980020
6 changed files with 333 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
api/peer_connection_interface.h
View File

@ -935,6 +935,14 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
virtual const SessionDescriptionInterface* pending_local_description() const;
virtual const SessionDescriptionInterface* pending_remote_description() const;
// Tells the PeerConnection that ICE should be restarted. This triggers a need
// for negotiation and subsequent CreateOffer() calls will act as if
// RTCOfferAnswerOptions::ice_restart is true.
// https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-restartice
// TODO(hbos): Remove default implementation when downstream projects
// implement this.
virtual void RestartIce() {}
// Create a new offer.
// The CreateSessionDescriptionObserver callback will be called when done.
virtual void CreateOffer(CreateSessionDescriptionObserver* observer,

1
api/peer_connection_proxy.h
View File

@ -86,6 +86,7 @@ PROXY_CONSTMETHOD0(const SessionDescriptionInterface*,
pending_local_description)
PROXY_CONSTMETHOD0(const SessionDescriptionInterface*,
pending_remote_description)
PROXY_METHOD0(void, RestartIce)
PROXY_METHOD2(void,
CreateOffer,
CreateSessionDescriptionObserver*,

98
pc/peer_connection.cc
View File

@ -690,6 +690,55 @@ const ContentInfo* FindTransceiverMSection(
} // namespace
class PeerConnection::LocalIceCredentialsToReplace {
public:
// Sets the ICE credentials that need restarting to the ICE credentials of
// the current and pending descriptions.
void SetIceCredentialsFromLocalDescriptions(
const SessionDescriptionInterface* current_local_description,
const SessionDescriptionInterface* pending_local_description) {
ice_credentials_.clear();
if (current_local_description) {
AppendIceCredentialsFromSessionDescription(*current_local_description);
}
if (pending_local_description) {
AppendIceCredentialsFromSessionDescription(*pending_local_description);
}
}
void ClearIceCredentials() { ice_credentials_.clear(); }
// Returns true if we have ICE credentials that need restarting.
bool HasIceCredentials() const { return !ice_credentials_.empty(); }
// Returns true if |local_description| shares no ICE credentials with the
// ICE credentials that need restarting.
bool SatisfiesIceRestart(
const SessionDescriptionInterface& local_description) const {
for (const auto& transport_info :
local_description.description()->transport_infos()) {
if (ice_credentials_.find(std::make_pair(
transport_info.description.ice_ufrag,
transport_info.description.ice_pwd)) != ice_credentials_.end()) {
return false;
}
}
return true;
}
private:
void AppendIceCredentialsFromSessionDescription(
const SessionDescriptionInterface& desc) {
for (const auto& transport_info : desc.description()->transport_infos()) {
ice_credentials_.insert(
std::make_pair(transport_info.description.ice_ufrag,
transport_info.description.ice_pwd));
}
}
std::set<std::pair<std::string, std::string>> ice_credentials_;
};
// Upon completion, posts a task to execute the callback of the
// SetSessionDescriptionObserver asynchronously on the same thread. At this
// point, the state of the peer connection might no longer reflect the effects
@ -878,7 +927,8 @@ PeerConnection::PeerConnection(PeerConnectionFactory* factory,
local_streams_(StreamCollection::Create()),
remote_streams_(StreamCollection::Create()),
call_(std::move(call)),
call_ptr_(call_.get()) {}
call_ptr_(call_.get()),
local_ice_credentials_to_replace_(new LocalIceCredentialsToReplace()) {}
PeerConnection::~PeerConnection() {
TRACE_EVENT0("webrtc", "PeerConnection::~PeerConnection");
@ -1963,6 +2013,13 @@ rtc::scoped_refptr<DataChannelInterface> PeerConnection::CreateDataChannel(
return DataChannelProxy::Create(signaling_thread(), channel.get());
}
void PeerConnection::RestartIce() {
RTC_DCHECK_RUN_ON(signaling_thread());
local_ice_credentials_to_replace_->SetIceCredentialsFromLocalDescriptions(
current_local_description_.get(), pending_local_description_.get());
UpdateNegotiationNeeded();
}
void PeerConnection::CreateOffer(CreateSessionDescriptionObserver* observer,
const RTCOfferAnswerOptions& options) {
RTC_DCHECK_RUN_ON(signaling_thread());
@ -2448,6 +2505,12 @@ RTCError PeerConnection::ApplyLocalDescription(
}
}
if (type == SdpType::kAnswer &&
local_ice_credentials_to_replace_->SatisfiesIceRestart(
*current_local_description_)) {
local_ice_credentials_to_replace_->ClearIceCredentials();
}
return RTCError::OK();
}
@ -2923,6 +2986,12 @@ RTCError PeerConnection::ApplyRemoteDescription(
UpdateEndedRemoteMediaStreams();
}
if (type == SdpType::kAnswer &&
local_ice_credentials_to_replace_->SatisfiesIceRestart(
*current_local_description_)) {
local_ice_credentials_to_replace_->ClearIceCredentials();
}
return RTCError::OK();
}
@ -4313,8 +4382,10 @@ void PeerConnection::GetOptionsForOffer(
}
// Apply ICE restart flag and renomination flag.
bool ice_restart = offer_answer_options.ice_restart ||
local_ice_credentials_to_replace_->HasIceCredentials();
for (auto& options : session_options->media_description_options) {
options.transport_options.ice_restart = offer_answer_options.ice_restart;
options.transport_options.ice_restart = ice_restart;
options.transport_options.enable_ice_renomination =
configuration_.enable_ice_renomination;
}
@ -7311,19 +7382,24 @@ bool PeerConnection::CheckIfNegotiationIsNeeded() {
// 1. If any implementation-specific negotiation is required, as described at
// the start of this section, return true.
// 2. Let description be connection.[[CurrentLocalDescription]].
// 2. If connection's [[RestartIce]] internal slot is true, return true.
if (local_ice_credentials_to_replace_->HasIceCredentials()) {
return true;
}
// 3. Let description be connection.[[CurrentLocalDescription]].
const SessionDescriptionInterface* description = current_local_description();
if (!description)
return true;
// 3. If connection has created any RTCDataChannels, and no m= section in
// 4. If connection has created any RTCDataChannels, and no m= section in
// description has been negotiated yet for data, return true.
if (!sctp_data_channels_.empty()) {
if (!cricket::GetFirstDataContent(description->description()->contents()))
return true;
}
// 4. For each transceiver in connection's set of transceivers, perform the
// 5. For each transceiver in connection's set of transceivers, perform the
// following checks:
for (const auto& transceiver : transceivers_) {
const ContentInfo* current_local_msection =
@ -7332,7 +7408,7 @@ bool PeerConnection::CheckIfNegotiationIsNeeded() {
const ContentInfo* current_remote_msection = FindTransceiverMSection(
transceiver.get(), current_remote_description());
// 4.3 If transceiver is stopped and is associated with an m= section,
// 5.3 If transceiver is stopped and is associated with an m= section,
// but the associated m= section is not yet rejected in
// connection.[[CurrentLocalDescription]] or
// connection.[[CurrentRemoteDescription]], return true.
@ -7345,17 +7421,17 @@ bool PeerConnection::CheckIfNegotiationIsNeeded() {
continue;
}
// 4.1 If transceiver isn't stopped and isn't yet associated with an m=
// 5.1 If transceiver isn't stopped and isn't yet associated with an m=
// section in description, return true.
if (!current_local_msection)
return true;
const MediaContentDescription* current_local_media_description =
current_local_msection->media_description();
// 4.2 If transceiver isn't stopped and is associated with an m= section
// 5.2 If transceiver isn't stopped and is associated with an m= section
// in description then perform the following checks:
// 4.2.1 If transceiver.[[Direction]] is "sendrecv" or "sendonly", and the
// 5.2.1 If transceiver.[[Direction]] is "sendrecv" or "sendonly", and the
// associated m= section in description either doesn't contain a single
// "a=msid" line, or the number of MSIDs from the "a=msid" lines in this
// m= section, or the MSID values themselves, differ from what is in
@ -7381,7 +7457,7 @@ bool PeerConnection::CheckIfNegotiationIsNeeded() {
return true;
}
// 4.2.2 If description is of type "offer", and the direction of the
// 5.2.2 If description is of type "offer", and the direction of the
// associated m= section in neither connection.[[CurrentLocalDescription]]
// nor connection.[[CurrentRemoteDescription]] matches
// transceiver.[[Direction]], return true.
@ -7403,7 +7479,7 @@ bool PeerConnection::CheckIfNegotiationIsNeeded() {
}
}
// 4.2.3 If description is of type "answer", and the direction of the
// 5.2.3 If description is of type "answer", and the direction of the
// associated m= section in the description does not match
// transceiver.[[Direction]] intersected with the offered direction (as
// described in [JSEP] (section 5.3.1.)), return true.

12
pc/peer_connection.h
View File

@ -15,6 +15,7 @@
#include <memory>
#include <set>
#include <string>
#include <utility>
#include <vector>
#include "api/media_transport_interface.h"
@ -169,6 +170,8 @@ class PeerConnection : public PeerConnectionInternal,
const SessionDescriptionInterface* pending_remote_description()
const override;
void RestartIce() override;
// JSEP01
void CreateOffer(CreateSessionDescriptionObserver* observer,
const RTCOfferAnswerOptions& options) override;
@ -287,6 +290,13 @@ class PeerConnection : public PeerConnectionInternal,
private:
class SetRemoteDescriptionObserverAdapter;
friend class SetRemoteDescriptionObserverAdapter;
// Represents the [[LocalIceCredentialsToReplace]] internal slot in the spec.
// It makes the next CreateOffer() produce new ICE credentials even if
// RTCOfferAnswerOptions::ice_restart is false.
// https://w3c.github.io/webrtc-pc/#dfn-localufragstoreplace
// TODO(hbos): When JsepTransportController/JsepTransport supports rollback,
// move this type of logic to JsepTransportController/JsepTransport.
class LocalIceCredentialsToReplace;
struct RtpSenderInfo {
RtpSenderInfo() : first_ssrc(0) {}
@ -1366,6 +1376,8 @@ class PeerConnection : public PeerConnectionInternal,
std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
video_bitrate_allocator_factory_;
std::unique_ptr<LocalIceCredentialsToReplace>
local_ice_credentials_to_replace_ RTC_GUARDED_BY(signaling_thread());
bool is_negotiation_needed_ RTC_GUARDED_BY(signaling_thread()) = false;
};

223
pc/peer_connection_ice_unittest.cc
View File

@ -225,6 +225,26 @@ class PeerConnectionIceBaseTest : public ::testing::Test {
return cricket::ICEROLE_UNKNOWN;
}
// Returns a list of (ufrag, pwd) pairs in the order that they appear in
// |description|, or the empty list if |description| is null.
std::vector<std::pair<std::string, std::string>> GetIceCredentials(
const SessionDescriptionInterface* description) {
std::vector<std::pair<std::string, std::string>> ice_credentials;
if (!description)
return ice_credentials;
const auto* desc = description->description();
for (const auto& content_info : desc->contents()) {
const auto* transport_info =
desc->GetTransportInfoByName(content_info.name);
if (transport_info) {
ice_credentials.push_back(
std::make_pair(transport_info->description.ice_ufrag,
transport_info->description.ice_pwd));
}
}
return ice_credentials;
}
bool AddCandidateToFirstTransport(cricket::Candidate* candidate,
SessionDescriptionInterface* sdesc) {
auto* desc = sdesc->description();
@ -813,6 +833,209 @@ TEST_P(PeerConnectionIceTest, LaterAnswerHasSameIceCredentialsIfNoIceRestart) {
EXPECT_EQ(answer_transport_desc->ice_pwd, local_transport_desc->ice_pwd);
}
TEST_P(PeerConnectionIceTest, RestartIceGeneratesNewCredentials) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
auto initial_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
caller->pc()->RestartIce();
ASSERT_TRUE(caller->CreateOfferAndSetAsLocal());
auto restarted_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
EXPECT_NE(initial_ice_credentials, restarted_ice_credentials);
}
TEST_P(PeerConnectionIceTest,
RestartIceWhileLocalOfferIsPendingGeneratesNewCredentialsInNextOffer) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
auto initial_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
// ICE restart becomes needed while an O/A is pending and |caller| is the
// offerer.
caller->pc()->RestartIce();
ASSERT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
ASSERT_TRUE(caller->CreateOfferAndSetAsLocal());
auto restarted_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
EXPECT_NE(initial_ice_credentials, restarted_ice_credentials);
}
TEST_P(PeerConnectionIceTest,
RestartIceWhileRemoteOfferIsPendingGeneratesNewCredentialsInNextOffer) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
auto initial_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
ASSERT_TRUE(caller->SetRemoteDescription(callee->CreateOfferAndSetAsLocal()));
// ICE restart becomes needed while an O/A is pending and |caller| is the
// answerer.
caller->pc()->RestartIce();
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateAnswerAndSetAsLocal()));
ASSERT_TRUE(caller->CreateOfferAndSetAsLocal());
auto restarted_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
EXPECT_NE(initial_ice_credentials, restarted_ice_credentials);
}
TEST_P(PeerConnectionIceTest, RestartIceTriggeredByRemoteSide) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
auto initial_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
// Remote restart and O/A exchange with |caller| as the answerer should
// restart ICE locally as well.
callee->pc()->RestartIce();
ASSERT_TRUE(callee->ExchangeOfferAnswerWith(caller.get()));
auto restarted_ice_credentials =
GetIceCredentials(caller->pc()->local_description());
EXPECT_NE(initial_ice_credentials, restarted_ice_credentials);
}
TEST_P(PeerConnectionIceTest, RestartIceCausesNegotiationNeeded) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
caller->observer()->clear_negotiation_needed();
caller->pc()->RestartIce();
EXPECT_TRUE(caller->observer()->negotiation_needed());
}
// In Unified Plan, "onnegotiationneeded" is spec-compliant, including not
// firing multipe times in a row, or firing when returning to the stable
// signaling state if negotiation is still needed. In Plan B it fires any time
// something changes. As such, some tests are SdpSemantics-specific.
class PeerConnectionIceTestUnifiedPlan : public PeerConnectionIceBaseTest {
protected:
PeerConnectionIceTestUnifiedPlan()
: PeerConnectionIceBaseTest(SdpSemantics::kUnifiedPlan) {}
};
TEST_F(PeerConnectionIceTestUnifiedPlan,
RestartIceWhileLocalOfferIsPendingCausesNegotiationNeededWhenStable) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
// ICE restart becomes needed while an O/A is pending and |caller| is the
// offerer.
caller->observer()->clear_negotiation_needed();
caller->pc()->RestartIce();
// In Unified Plan, the event should not fire until we are back in the stable
// signaling state.
EXPECT_FALSE(caller->observer()->negotiation_needed());
ASSERT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
EXPECT_TRUE(caller->observer()->negotiation_needed());
}
TEST_F(PeerConnectionIceTestUnifiedPlan,
RestartIceWhileRemoteOfferIsPendingCausesNegotiationNeededWhenStable) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
// Establish initial credentials as the caller.
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
ASSERT_TRUE(caller->SetRemoteDescription(callee->CreateOfferAndSetAsLocal()));
// ICE restart becomes needed while an O/A is pending and |caller| is the
// answerer.
caller->observer()->clear_negotiation_needed();
caller->pc()->RestartIce();
// In Unified Plan, the event should not fire until we are back in the stable
// signaling state.
EXPECT_FALSE(caller->observer()->negotiation_needed());
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateAnswerAndSetAsLocal()));
EXPECT_TRUE(caller->observer()->negotiation_needed());
}
TEST_F(PeerConnectionIceTestUnifiedPlan,
RestartIceTriggeredByRemoteSideCauseNegotiationNotNeeded) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
// Local restart.
caller->pc()->RestartIce();
caller->observer()->clear_negotiation_needed();
// Remote restart and O/A exchange with |caller| as the answerer should
// restart ICE locally as well.
callee->pc()->RestartIce();
ASSERT_TRUE(callee->ExchangeOfferAnswerWith(caller.get()));
// Having restarted ICE by the remote offer, we do not need to renegotiate ICE
// credentials when back in the stable signaling state.
EXPECT_FALSE(caller->observer()->negotiation_needed());
}
TEST_F(PeerConnectionIceTestUnifiedPlan,
RestartIceTwiceDoesNotFireNegotiationNeededTwice) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
caller->pc()->RestartIce();
EXPECT_TRUE(caller->observer()->negotiation_needed());
caller->observer()->clear_negotiation_needed();
caller->pc()->RestartIce();
EXPECT_FALSE(caller->observer()->negotiation_needed());
}
// In Plan B, "onnegotiationneeded" is not spec-compliant, firing based on if
// something changed rather than if negotiation is needed. In Unified Plan it
// fires according to spec. As such, some tests are SdpSemantics-specific.
class PeerConnectionIceTestPlanB : public PeerConnectionIceBaseTest {
protected:
PeerConnectionIceTestPlanB()
: PeerConnectionIceBaseTest(SdpSemantics::kPlanB) {}
};
TEST_F(PeerConnectionIceTestPlanB,
RestartIceWhileOfferIsPendingCausesNegotiationNeededImmediately) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
caller->observer()->clear_negotiation_needed();
caller->pc()->RestartIce();
EXPECT_TRUE(caller->observer()->negotiation_needed());
caller->observer()->clear_negotiation_needed();
ASSERT_TRUE(
caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal()));
// In Plan B, the event fired early so we don't expect it to fire now. This is
// not spec-compliant but follows the pattern of existing Plan B behavior.
EXPECT_FALSE(caller->observer()->negotiation_needed());
}
TEST_F(PeerConnectionIceTestPlanB,
RestartIceTwiceDoesFireNegotiationNeededTwice) {
auto caller = CreatePeerConnectionWithAudioVideo();
auto callee = CreatePeerConnectionWithAudioVideo();
ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
caller->observer()->clear_negotiation_needed();
caller->pc()->RestartIce();
EXPECT_TRUE(caller->observer()->negotiation_needed());
caller->observer()->clear_negotiation_needed();
caller->pc()->RestartIce();
// In Plan B, the event fires every time something changed, even if we have
// already fired the event. This is not spec-compliant but follows the same
// pattern of existing Plan B behavior.
EXPECT_TRUE(caller->observer()->negotiation_needed());
}
// The following parameterized test verifies that if an offer is sent with a
// modified ICE ufrag and/or ICE pwd, then the answer should identify that the
// other side has initiated an ICE restart and generate a new ufrag and pwd.

2
pc/test/fake_peer_connection_base.h
View File

@ -144,6 +144,8 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
return nullptr;
}
void RestartIce() override {}
void CreateOffer(CreateSessionDescriptionObserver* observer,
const RTCOfferAnswerOptions& options) override {}