Implement current/pending session description methods.
BUG=webrtc:6917
Review-Url: https://codereview.webrtc.org/2590753002
Cr-Commit-Position: refs/heads/master@{#15722}
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 6d7d07d..cdf5a97 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -1381,6 +1381,26 @@
return session_->remote_description();
}
+const SessionDescriptionInterface* PeerConnection::current_local_description()
+ const {
+ return session_->current_local_description();
+}
+
+const SessionDescriptionInterface* PeerConnection::current_remote_description()
+ const {
+ return session_->current_remote_description();
+}
+
+const SessionDescriptionInterface* PeerConnection::pending_local_description()
+ const {
+ return session_->pending_local_description();
+}
+
+const SessionDescriptionInterface* PeerConnection::pending_remote_description()
+ const {
+ return session_->pending_remote_description();
+}
+
void PeerConnection::Close() {
TRACE_EVENT0("webrtc", "PeerConnection::Close");
// Update stats here so that we have the most recent stats for tracks and
diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h
index 70fd867..5269e3a 100644
--- a/webrtc/api/peerconnection.h
+++ b/webrtc/api/peerconnection.h
@@ -113,6 +113,12 @@
const SessionDescriptionInterface* local_description() const override;
const SessionDescriptionInterface* remote_description() const override;
+ const SessionDescriptionInterface* current_local_description() const override;
+ const SessionDescriptionInterface* current_remote_description()
+ const override;
+ const SessionDescriptionInterface* pending_local_description() const override;
+ const SessionDescriptionInterface* pending_remote_description()
+ const override;
// JSEP01
// Deprecated, use version without constraints.
diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h
index 9c2301c..e81eee2 100644
--- a/webrtc/api/peerconnectioninterface.h
+++ b/webrtc/api/peerconnectioninterface.h
@@ -493,6 +493,25 @@
virtual const SessionDescriptionInterface* local_description() const = 0;
virtual const SessionDescriptionInterface* remote_description() const = 0;
+ // A "current" description the one currently negotiated from a complete
+ // offer/answer exchange.
+ virtual const SessionDescriptionInterface* current_local_description() const {
+ return nullptr;
+ }
+ virtual const SessionDescriptionInterface* current_remote_description()
+ const {
+ return nullptr;
+ }
+ // A "pending" description is one that's part of an incomplete offer/answer
+ // exchange (thus, either an offer or a pranswer). Once the offer/answer
+ // exchange is finished, the "pending" description will become "current".
+ virtual const SessionDescriptionInterface* pending_local_description() const {
+ return nullptr;
+ }
+ virtual const SessionDescriptionInterface* pending_remote_description()
+ const {
+ return nullptr;
+ }
// Create a new offer.
// The CreateSessionDescriptionObserver callback will be called when done.
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index 7b33c0c..c52df41 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -2893,6 +2893,76 @@
EXPECT_NE(initial_ufrags[1], subsequent_ufrags[1]);
}
+// Tests that the methods to return current/pending descriptions work as
+// expected at different points in the offer/answer exchange. This test does
+// one offer/answer exchange as the offerer, then another as the answerer.
+TEST_F(PeerConnectionInterfaceTest, CurrentAndPendingDescriptions) {
+ // This disables DTLS so we can apply an answer to ourselves.
+ CreatePeerConnection();
+
+ // Create initial local offer and get SDP (which will also be used as
+ // answer/pranswer);
+ std::unique_ptr<SessionDescriptionInterface> offer;
+ ASSERT_TRUE(DoCreateOffer(&offer, nullptr));
+ std::string sdp;
+ EXPECT_TRUE(offer->ToString(&sdp));
+
+ // Set local offer.
+ SessionDescriptionInterface* local_offer = offer.release();
+ EXPECT_TRUE(DoSetLocalDescription(local_offer));
+ EXPECT_EQ(local_offer, pc_->pending_local_description());
+ EXPECT_EQ(nullptr, pc_->pending_remote_description());
+ EXPECT_EQ(nullptr, pc_->current_local_description());
+ EXPECT_EQ(nullptr, pc_->current_remote_description());
+
+ // Set remote pranswer.
+ SessionDescriptionInterface* remote_pranswer =
+ webrtc::CreateSessionDescription(SessionDescriptionInterface::kPrAnswer,
+ sdp, nullptr);
+ EXPECT_TRUE(DoSetRemoteDescription(remote_pranswer));
+ EXPECT_EQ(local_offer, pc_->pending_local_description());
+ EXPECT_EQ(remote_pranswer, pc_->pending_remote_description());
+ EXPECT_EQ(nullptr, pc_->current_local_description());
+ EXPECT_EQ(nullptr, pc_->current_remote_description());
+
+ // Set remote answer.
+ SessionDescriptionInterface* remote_answer = webrtc::CreateSessionDescription(
+ SessionDescriptionInterface::kAnswer, sdp, nullptr);
+ EXPECT_TRUE(DoSetRemoteDescription(remote_answer));
+ EXPECT_EQ(nullptr, pc_->pending_local_description());
+ EXPECT_EQ(nullptr, pc_->pending_remote_description());
+ EXPECT_EQ(local_offer, pc_->current_local_description());
+ EXPECT_EQ(remote_answer, pc_->current_remote_description());
+
+ // Set remote offer.
+ SessionDescriptionInterface* remote_offer = webrtc::CreateSessionDescription(
+ SessionDescriptionInterface::kOffer, sdp, nullptr);
+ EXPECT_TRUE(DoSetRemoteDescription(remote_offer));
+ EXPECT_EQ(remote_offer, pc_->pending_remote_description());
+ EXPECT_EQ(nullptr, pc_->pending_local_description());
+ EXPECT_EQ(local_offer, pc_->current_local_description());
+ EXPECT_EQ(remote_answer, pc_->current_remote_description());
+
+ // Set local pranswer.
+ SessionDescriptionInterface* local_pranswer =
+ webrtc::CreateSessionDescription(SessionDescriptionInterface::kPrAnswer,
+ sdp, nullptr);
+ EXPECT_TRUE(DoSetLocalDescription(local_pranswer));
+ EXPECT_EQ(remote_offer, pc_->pending_remote_description());
+ EXPECT_EQ(local_pranswer, pc_->pending_local_description());
+ EXPECT_EQ(local_offer, pc_->current_local_description());
+ EXPECT_EQ(remote_answer, pc_->current_remote_description());
+
+ // Set local answer.
+ SessionDescriptionInterface* local_answer = webrtc::CreateSessionDescription(
+ SessionDescriptionInterface::kAnswer, sdp, nullptr);
+ EXPECT_TRUE(DoSetLocalDescription(local_answer));
+ EXPECT_EQ(nullptr, pc_->pending_remote_description());
+ EXPECT_EQ(nullptr, pc_->pending_local_description());
+ EXPECT_EQ(remote_offer, pc_->current_remote_description());
+ EXPECT_EQ(local_answer, pc_->current_local_description());
+}
+
class PeerConnectionMediaConfigTest : public testing::Test {
protected:
void SetUp() override {
diff --git a/webrtc/api/peerconnectionproxy.h b/webrtc/api/peerconnectionproxy.h
index f002014..2110ee2 100644
--- a/webrtc/api/peerconnectionproxy.h
+++ b/webrtc/api/peerconnectionproxy.h
@@ -47,6 +47,14 @@
CreateDataChannel, const std::string&, const DataChannelInit*)
PROXY_CONSTMETHOD0(const SessionDescriptionInterface*, local_description)
PROXY_CONSTMETHOD0(const SessionDescriptionInterface*, remote_description)
+ PROXY_CONSTMETHOD0(const SessionDescriptionInterface*,
+ pending_local_description)
+ PROXY_CONSTMETHOD0(const SessionDescriptionInterface*,
+ pending_remote_description)
+ PROXY_CONSTMETHOD0(const SessionDescriptionInterface*,
+ current_local_description)
+ PROXY_CONSTMETHOD0(const SessionDescriptionInterface*,
+ current_remote_description)
PROXY_METHOD2(void, CreateOffer, CreateSessionDescriptionObserver*,
const MediaConstraintsInterface*)
PROXY_METHOD2(void, CreateAnswer, CreateSessionDescriptionObserver*,
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index ea724a3..5d414ba 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -623,7 +623,7 @@
bool WebRtcSession::GetSslRole(const std::string& transport_name,
rtc::SSLRole* role) {
- if (!local_desc_ || !remote_desc_) {
+ if (!local_description() || !remote_description()) {
LOG(LS_INFO) << "Local and Remote descriptions must be applied to get "
<< "SSL Role of the session.";
return false;
@@ -669,25 +669,31 @@
transport_controller_->SetIceRole(cricket::ICEROLE_CONTROLLING);
}
- local_desc_.reset(desc_temp.release());
+ if (action == kAnswer) {
+ current_local_description_.reset(desc_temp.release());
+ pending_local_description_.reset(nullptr);
+ current_remote_description_.reset(pending_remote_description_.release());
+ } else {
+ pending_local_description_.reset(desc_temp.release());
+ }
// Transport and Media channels will be created only when offer is set.
- if (action == kOffer && !CreateChannels(local_desc_->description())) {
+ if (action == kOffer && !CreateChannels(local_description()->description())) {
// TODO(mallinath) - Handle CreateChannel failure, as new local description
// is applied. Restore back to old description.
return BadLocalSdp(desc->type(), kCreateChannelFailed, err_desc);
}
// Remove unused channels if MediaContentDescription is rejected.
- RemoveUnusedChannels(local_desc_->description());
+ RemoveUnusedChannels(local_description()->description());
if (!UpdateSessionState(action, cricket::CS_LOCAL, err_desc)) {
return false;
}
- if (remote_desc_) {
+ if (remote_description()) {
// Now that we have a local description, we can push down remote candidates.
- UseCandidatesInSessionDescription(remote_desc_.get());
+ UseCandidatesInSessionDescription(remote_description());
}
pending_ice_restarts_.clear();
@@ -710,12 +716,25 @@
return false;
}
- std::unique_ptr<SessionDescriptionInterface> old_remote_desc(
- remote_desc_.release());
- remote_desc_.reset(desc_temp.release());
+ const SessionDescriptionInterface* old_remote_description =
+ remote_description();
+ // Grab ownership of the description being replaced for the remainder of this
+ // method, since it's used below.
+ std::unique_ptr<SessionDescriptionInterface> replaced_remote_description;
+ Action action = GetAction(desc->type());
+ if (action == kAnswer) {
+ replaced_remote_description.reset(
+ pending_remote_description_ ? pending_remote_description_.release()
+ : current_remote_description_.release());
+ current_remote_description_.reset(desc_temp.release());
+ pending_remote_description_.reset(nullptr);
+ current_local_description_.reset(pending_local_description_.release());
+ } else {
+ replaced_remote_description.reset(pending_remote_description_.release());
+ pending_remote_description_.reset(desc_temp.release());
+ }
// Transport and Media channels will be created only when offer is set.
- Action action = GetAction(desc->type());
if (action == kOffer && !CreateChannels(desc->description())) {
// TODO(mallinath) - Handle CreateChannel failure, as new local description
// is applied. Restore back to old description.
@@ -731,19 +750,20 @@
return false;
}
- if (local_desc_ && !UseCandidatesInSessionDescription(desc)) {
+ if (local_description() && !UseCandidatesInSessionDescription(desc)) {
return BadRemoteSdp(desc->type(), kInvalidCandidates, err_desc);
}
- if (old_remote_desc) {
+ if (old_remote_description) {
for (const cricket::ContentInfo& content :
- old_remote_desc->description()->contents()) {
+ old_remote_description->description()->contents()) {
// Check if this new SessionDescription contains new ICE ufrag and
// password that indicates the remote peer requests an ICE restart.
// TODO(deadbeef): When we start storing both the current and pending
// remote description, this should reset pending_ice_restarts and compare
// against the current description.
- if (CheckForRemoteIceRestart(old_remote_desc.get(), desc, content.name)) {
+ if (CheckForRemoteIceRestart(old_remote_description, desc,
+ content.name)) {
if (action == kOffer) {
pending_ice_restarts_.insert(content.name);
}
@@ -756,7 +776,7 @@
// description plus any candidates added since then. We should remove
// this once we're sure it won't break anything.
WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
- old_remote_desc.get(), content.name, desc);
+ old_remote_description, content.name, desc);
}
}
}
@@ -839,9 +859,11 @@
}
} else if (action == kAnswer) {
const cricket::ContentGroup* local_bundle =
- local_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
+ local_description()->description()->GetGroupByName(
+ cricket::GROUP_TYPE_BUNDLE);
const cricket::ContentGroup* remote_bundle =
- remote_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
+ remote_description()->description()->GetGroupByName(
+ cricket::GROUP_TYPE_BUNDLE);
if (local_bundle && remote_bundle) {
// The answerer decides the transport to bundle on.
const cricket::ContentGroup* answer_bundle =
@@ -888,11 +910,11 @@
if (!ch) {
return true;
} else if (source == cricket::CS_LOCAL) {
- return ch->PushdownLocalDescription(local_desc_->description(), action,
- err);
+ return ch->PushdownLocalDescription(local_description()->description(),
+ action, err);
} else {
- return ch->PushdownRemoteDescription(remote_desc_->description(), action,
- err);
+ return ch->PushdownRemoteDescription(remote_description()->description(),
+ action, err);
}
};
@@ -907,11 +929,11 @@
RTC_DCHECK(signaling_thread()->IsCurrent());
if (source == cricket::CS_LOCAL) {
- return PushdownLocalTransportDescription(local_desc_->description(), action,
- error_desc);
+ return PushdownLocalTransportDescription(local_description()->description(),
+ action, error_desc);
}
- return PushdownRemoteTransportDescription(remote_desc_->description(), action,
- error_desc);
+ return PushdownRemoteTransportDescription(remote_description()->description(),
+ action, error_desc);
}
bool WebRtcSession::PushdownLocalTransportDescription(
@@ -1059,7 +1081,7 @@
}
bool WebRtcSession::ProcessIceMessage(const IceCandidateInterface* candidate) {
- if (!remote_desc_) {
+ if (!remote_description()) {
LOG(LS_ERROR) << "ProcessIceMessage: ICE candidates can't be added "
<< "without any remote session description.";
return false;
@@ -1077,7 +1099,7 @@
}
// Add this candidate to the remote session description.
- if (!remote_desc_->AddCandidate(candidate)) {
+ if (!mutable_remote_description()->AddCandidate(candidate)) {
LOG(LS_ERROR) << "ProcessIceMessage: Candidate cannot be used.";
return false;
}
@@ -1092,7 +1114,7 @@
bool WebRtcSession::RemoveRemoteIceCandidates(
const std::vector<cricket::Candidate>& candidates) {
- if (!remote_desc_) {
+ if (!remote_description()) {
LOG(LS_ERROR) << "RemoveRemoteIceCandidates: ICE candidates can't be "
<< "removed without any remote session description.";
return false;
@@ -1103,7 +1125,8 @@
return false;
}
- size_t number_removed = remote_desc_->RemoveCandidates(candidates);
+ size_t number_removed =
+ mutable_remote_description()->RemoveCandidates(candidates);
if (number_removed != candidates.size()) {
LOG(LS_ERROR) << "RemoveRemoteIceCandidates: Failed to remove candidates. "
<< "Requested " << candidates.size() << " but only "
@@ -1157,18 +1180,20 @@
bool WebRtcSession::GetLocalTrackIdBySsrc(uint32_t ssrc,
std::string* track_id) {
- if (!local_desc_) {
+ if (!local_description()) {
return false;
}
- return webrtc::GetTrackIdBySsrc(local_desc_->description(), ssrc, track_id);
+ return webrtc::GetTrackIdBySsrc(local_description()->description(), ssrc,
+ track_id);
}
bool WebRtcSession::GetRemoteTrackIdBySsrc(uint32_t ssrc,
std::string* track_id) {
- if (!remote_desc_) {
+ if (!remote_description()) {
return false;
}
- return webrtc::GetTrackIdBySsrc(remote_desc_->description(), ssrc, track_id);
+ return webrtc::GetTrackIdBySsrc(remote_description()->description(), ssrc,
+ track_id);
}
std::string WebRtcSession::BadStateErrMsg(State state) {
@@ -1186,8 +1211,8 @@
uint32_t send_ssrc = 0;
// The Dtmf is negotiated per channel not ssrc, so we only check if the ssrc
// exists.
- if (!local_desc_ ||
- !GetAudioSsrcByTrackId(local_desc_->description(), track_id,
+ if (!local_description() ||
+ !GetAudioSsrcByTrackId(local_description()->description(), track_id,
&send_ssrc)) {
LOG(LS_ERROR) << "CanInsertDtmf: Track does not exist: " << track_id;
return false;
@@ -1203,8 +1228,9 @@
return false;
}
uint32_t send_ssrc = 0;
- if (!VERIFY(local_desc_ && GetAudioSsrcByTrackId(local_desc_->description(),
- track_id, &send_ssrc))) {
+ if (!VERIFY(local_description() &&
+ GetAudioSsrcByTrackId(local_description()->description(),
+ track_id, &send_ssrc))) {
LOG(LS_ERROR) << "InsertDtmf: Track does not exist: " << track_id;
return false;
}
@@ -1402,8 +1428,8 @@
if (ice_observer_) {
ice_observer_->OnIceCandidate(&candidate);
}
- if (local_desc_) {
- local_desc_->AddCandidate(&candidate);
+ if (local_description()) {
+ mutable_local_description()->AddCandidate(&candidate);
}
}
}
@@ -1421,8 +1447,8 @@
}
}
- if (local_desc_) {
- local_desc_->RemoveCandidates(candidates);
+ if (local_description()) {
+ mutable_local_description()->RemoveCandidates(candidates);
}
if (ice_observer_) {
ice_observer_->OnIceCandidatesRemoved(candidates);
@@ -1444,12 +1470,12 @@
// Returns the media index for a local ice candidate given the content name.
bool WebRtcSession::GetLocalCandidateMediaIndex(const std::string& content_name,
int* sdp_mline_index) {
- if (!local_desc_ || !sdp_mline_index) {
+ if (!local_description() || !sdp_mline_index) {
return false;
}
bool content_found = false;
- const ContentInfos& contents = local_desc_->description()->contents();
+ const ContentInfos& contents = local_description()->description()->contents();
for (size_t index = 0; index < contents.size(); ++index) {
if (contents[index].name == content_name) {
*sdp_mline_index = static_cast<int>(index);
@@ -1490,14 +1516,15 @@
bool WebRtcSession::UseCandidate(const IceCandidateInterface* candidate) {
size_t mediacontent_index = static_cast<size_t>(candidate->sdp_mline_index());
- size_t remote_content_size = remote_desc_->description()->contents().size();
+ size_t remote_content_size =
+ remote_description()->description()->contents().size();
if (mediacontent_index >= remote_content_size) {
LOG(LS_ERROR) << "UseCandidate: Invalid candidate media index.";
return false;
}
cricket::ContentInfo content =
- remote_desc_->description()->contents()[mediacontent_index];
+ remote_description()->description()->contents()[mediacontent_index];
std::vector<cricket::Candidate> candidates;
candidates.push_back(candidate->candidate());
// Invoking BaseSession method to handle remote candidates.
@@ -1835,8 +1862,8 @@
// Verify m-lines in Answer when compared against Offer.
if (action == kAnswer) {
const cricket::SessionDescription* offer_desc =
- (source == cricket::CS_LOCAL) ? remote_desc_->description()
- : local_desc_->description();
+ (source == cricket::CS_LOCAL) ? remote_description()->description()
+ : local_description()->description();
if (!VerifyMediaDescriptions(sdesc->description(), offer_desc)) {
return BadAnswerSdp(source, kMlineMismatch, err_desc);
}
@@ -1890,7 +1917,7 @@
*valid = true;
const SessionDescriptionInterface* current_remote_desc =
- remote_desc ? remote_desc : remote_desc_.get();
+ remote_desc ? remote_desc : remote_description();
if (!current_remote_desc) {
return false;
diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h
index fa46ada..ef31560 100644
--- a/webrtc/api/webrtcsession.h
+++ b/webrtc/api/webrtcsession.h
@@ -247,10 +247,24 @@
void MaybeStartGathering();
const SessionDescriptionInterface* local_description() const {
- return local_desc_.get();
+ return pending_local_description_ ? pending_local_description_.get()
+ : current_local_description_.get();
}
const SessionDescriptionInterface* remote_description() const {
- return remote_desc_.get();
+ return pending_remote_description_ ? pending_remote_description_.get()
+ : current_remote_description_.get();
+ }
+ const SessionDescriptionInterface* current_local_description() const {
+ return current_local_description_.get();
+ }
+ const SessionDescriptionInterface* current_remote_description() const {
+ return current_remote_description_.get();
+ }
+ const SessionDescriptionInterface* pending_local_description() const {
+ return pending_local_description_.get();
+ }
+ const SessionDescriptionInterface* pending_remote_description() const {
+ return pending_remote_description_.get();
}
// Get the id used as a media stream track's "id" field from ssrc.
@@ -354,6 +368,17 @@
kAnswer,
};
+ // Non-const versions of local_description()/remote_description(), for use
+ // internally.
+ SessionDescriptionInterface* mutable_local_description() {
+ return pending_local_description_ ? pending_local_description_.get()
+ : current_local_description_.get();
+ }
+ SessionDescriptionInterface* mutable_remote_description() {
+ return pending_remote_description_ ? pending_remote_description_.get()
+ : current_remote_description_.get();
+ }
+
// Log session state.
void LogState(State old_state, State new_state);
@@ -519,8 +544,10 @@
IceObserver* ice_observer_;
PeerConnectionInterface::IceConnectionState ice_connection_state_;
bool ice_connection_receiving_;
- std::unique_ptr<SessionDescriptionInterface> local_desc_;
- std::unique_ptr<SessionDescriptionInterface> remote_desc_;
+ std::unique_ptr<SessionDescriptionInterface> current_local_description_;
+ std::unique_ptr<SessionDescriptionInterface> pending_local_description_;
+ std::unique_ptr<SessionDescriptionInterface> current_remote_description_;
+ std::unique_ptr<SessionDescriptionInterface> pending_remote_description_;
// If the remote peer is using a older version of implementation.
bool older_version_remote_peer_;
bool dtls_enabled_;