Revert "Implement rollback for setRemoteDescription"
This reverts commit 16d4c4d4fbb8644033def1091d2d5c941c1b01fa.
Reason for revert: breaks downstream dependency. (The new enum value kRollback is not handled correctly downstream).
Original change's description:
> Implement rollback for setRemoteDescription
>
> Bug: chromium:980875
> Change-Id: I4575e9ad1902a20937f9812f49edee2a2441f76d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/153525
> Commit-Queue: Eldar Rello <elrello@microsoft.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Steve Anton <steveanton@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29422}
TBR=steveanton@webrtc.org,mbonadei@webrtc.org,aleloi@webrtc.org,hbos@webrtc.org,aleloi@google.com,hta@webrtc.org,shampson@webrtc.org,elrello@microsoft.com
Change-Id: If76f6b672fdc59b7f00dfc7c150abda16614cd04
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:980875
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156304
Reviewed-by: Alex Loiko <aleloi@webrtc.org>
Commit-Queue: Alex Loiko <aleloi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29427}
diff --git a/api/jsep.cc b/api/jsep.cc
index ddb39b6..01f5720 100644
--- a/api/jsep.cc
+++ b/api/jsep.cc
@@ -41,7 +41,6 @@
const char SessionDescriptionInterface::kOffer[] = "offer";
const char SessionDescriptionInterface::kPrAnswer[] = "pranswer";
const char SessionDescriptionInterface::kAnswer[] = "answer";
-const char SessionDescriptionInterface::kRollback[] = "rollback";
const char* SdpTypeToString(SdpType type) {
switch (type) {
@@ -51,8 +50,6 @@
return SessionDescriptionInterface::kPrAnswer;
case SdpType::kAnswer:
return SessionDescriptionInterface::kAnswer;
- case SdpType::kRollback:
- return SessionDescriptionInterface::kRollback;
}
return "";
}
@@ -64,8 +61,6 @@
return SdpType::kPrAnswer;
} else if (type_str == SessionDescriptionInterface::kAnswer) {
return SdpType::kAnswer;
- } else if (type_str == SessionDescriptionInterface::kRollback) {
- return SdpType::kRollback;
} else {
return absl::nullopt;
}
diff --git a/api/jsep.h b/api/jsep.h
index 3f7f12a..6da7827 100644
--- a/api/jsep.h
+++ b/api/jsep.h
@@ -103,11 +103,8 @@
kOffer, // Description must be treated as an SDP offer.
kPrAnswer, // Description must be treated as an SDP answer, but not a final
// answer.
- kAnswer, // Description must be treated as an SDP final answer, and the
- // offer-answer exchange must be considered complete after
- // receiving this.
- kRollback // Resets any pending offers and sets signaling state back to
- // stable.
+ kAnswer // Description must be treated as an SDP final answer, and the offer-
+ // answer exchange must be considered complete after receiving this.
};
// Returns the string form of the given SDP type. String forms are defined in
@@ -131,7 +128,6 @@
static const char kOffer[];
static const char kPrAnswer[];
static const char kAnswer[];
- static const char kRollback[];
virtual ~SessionDescriptionInterface() {}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index f526c37..a417641 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -659,9 +659,6 @@
// logs with TURN server logs. It will not be added if it's an empty string.
std::string turn_logging_id;
- // Added to be able to control rollout of this feature.
- bool enable_implicit_rollback = false;
-
//
// Don't forget to update operator== if adding something.
//
diff --git a/pc/jsep_session_description.cc b/pc/jsep_session_description.cc
index 7f30b50..cc544dc 100644
--- a/pc/jsep_session_description.cc
+++ b/pc/jsep_session_description.cc
@@ -152,10 +152,8 @@
const std::string& sdp,
SdpParseError* error_out) {
auto jsep_desc = std::make_unique<JsepSessionDescription>(type);
- if (type != SdpType::kRollback) {
- if (!SdpDeserialize(sdp, jsep_desc.get(), error_out)) {
- return nullptr;
- }
+ if (!SdpDeserialize(sdp, jsep_desc.get(), error_out)) {
+ return nullptr;
}
return std::move(jsep_desc);
}
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index d83b16e..52ae53c 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -471,16 +471,6 @@
use_datagram_transport_for_data_channels_receive_only;
}
-void JsepTransportController::RollbackTransportForMid(const std::string& mid) {
- if (!network_thread_->IsCurrent()) {
- network_thread_->Invoke<void>(RTC_FROM_HERE,
- [=] { RollbackTransportForMid(mid); });
- return;
- }
- RemoveTransportForMid(mid);
- MaybeDestroyJsepTransport(mid);
-}
-
std::unique_ptr<cricket::IceTransportInternal>
JsepTransportController::CreateIceTransport(const std::string transport_name,
bool rtcp) {
diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h
index bcaeed5..a46a71e 100644
--- a/pc/jsep_transport_controller.h
+++ b/pc/jsep_transport_controller.h
@@ -239,10 +239,6 @@
bool use_datagram_transport_for_data_channels,
bool use_datagram_transport_for_data_channels_receive_only);
- // TODO(elrello): For now the rollback only removes mid to transport mapping
- // and deletes unused transport, but doesn't consider anything more complex.
- void RollbackTransportForMid(const std::string& mid);
-
// If media transport is present enabled and supported,
// when this method is called, it creates a media transport and generates its
// offer. The new offer is then returned, and the created media transport will
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index c0e1831..c2723e7f 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -782,7 +782,6 @@
absl::optional<CryptoOptions> crypto_options;
bool offer_extmap_allow_mixed;
std::string turn_logging_id;
- bool enable_implicit_rollback;
};
static_assert(sizeof(stuff_being_tested_for_equality) == sizeof(*this),
"Did you add something to RTCConfiguration and forget to "
@@ -848,8 +847,7 @@
o.use_datagram_transport_for_data_channels_receive_only &&
crypto_options == o.crypto_options &&
offer_extmap_allow_mixed == o.offer_extmap_allow_mixed &&
- turn_logging_id == o.turn_logging_id &&
- enable_implicit_rollback == o.enable_implicit_rollback;
+ turn_logging_id == o.turn_logging_id;
}
bool PeerConnectionInterface::RTCConfiguration::operator!=(
@@ -2259,23 +2257,6 @@
return;
}
- // For SLD we support only explicit rollback.
- if (desc->GetType() == SdpType::kRollback) {
- if (IsUnifiedPlan()) {
- RTCError error = Rollback();
- if (error.ok()) {
- PostSetSessionDescriptionSuccess(observer);
- } else {
- PostSetSessionDescriptionFailure(observer, std::move(error));
- }
- } else {
- PostSetSessionDescriptionFailure(
- observer, RTCError(RTCErrorType::UNSUPPORTED_OPERATION,
- "Rollback not supported in Plan B"));
- }
- return;
- }
-
RTCError error = ValidateSessionDescription(desc.get(), cricket::CS_LOCAL);
if (!error.ok()) {
std::string error_message = GetSetDescriptionErrorMessage(
@@ -2648,24 +2629,7 @@
RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error_message)));
return;
}
- if (IsUnifiedPlan()) {
- if (configuration_.enable_implicit_rollback) {
- if (desc->GetType() == SdpType::kOffer &&
- signaling_state() == kHaveLocalOffer) {
- Rollback();
- }
- }
- // Explicit rollback.
- if (desc->GetType() == SdpType::kRollback) {
- observer->OnSetRemoteDescriptionComplete(Rollback());
- return;
- }
- } else if (desc->GetType() == SdpType::kRollback) {
- observer->OnSetRemoteDescriptionComplete(
- RTCError(RTCErrorType::UNSUPPORTED_OPERATION,
- "Rollback not supported in Plan B"));
- return;
- }
+
if (desc->GetType() == SdpType::kOffer) {
// Report to UMA the format of the received offer.
ReportSdpFormatReceived(*desc);
@@ -3418,12 +3382,8 @@
transceiver = CreateAndAddTransceiver(sender, receiver);
transceiver->internal()->set_direction(
RtpTransceiverDirection::kRecvOnly);
- if (type == SdpType::kOffer) {
- transceiver_stable_states_by_transceivers_[transceiver] =
- TransceiverStableState(RtpTransceiverDirection::kRecvOnly,
- absl::nullopt, absl::nullopt, true);
- }
}
+
// Check if the offer indicated simulcast but the answer rejected it.
// This can happen when simulcast is not supported on the remote party.
if (SimulcastIsRejected(old_local_content, *media_desc)) {
@@ -3456,20 +3416,6 @@
return std::move(error);
}
}
- if (type == SdpType::kOffer) {
- // Make sure we don't overwrite existing stable states and that the
- // state is really going to change when adding new record to the map.
- auto it = transceiver_stable_states_by_transceivers_.find(transceiver);
- if (it == transceiver_stable_states_by_transceivers_.end() &&
- (transceiver->internal()->mid() != content.name ||
- transceiver->internal()->mline_index() != mline_index)) {
- transceiver_stable_states_by_transceivers_[transceiver] =
- TransceiverStableState(transceiver->internal()->direction(),
- transceiver->internal()->mid(),
- transceiver->internal()->mline_index(), false);
- }
- }
-
// Associate the found or created RtpTransceiver with the m= section by
// setting the value of the RtpTransceiver's mid property to the MID of the m=
// section, and establish a mapping between the transceiver and the index of
@@ -5891,7 +5837,6 @@
} else {
RTC_DCHECK(type == SdpType::kAnswer);
ChangeSignalingState(PeerConnectionInterface::kStable);
- transceiver_stable_states_by_transceivers_.clear();
}
// Update internal objects according to the session description's media
@@ -7605,51 +7550,4 @@
return false;
}
-RTCError PeerConnection::Rollback() {
- auto state = signaling_state();
- if (state != PeerConnectionInterface::kHaveLocalOffer &&
- state != PeerConnectionInterface::kHaveRemoteOffer) {
- return RTCError(RTCErrorType::INVALID_STATE,
- "Called in wrong signalingState: " +
- GetSignalingStateString(signaling_state()));
- }
- RTC_DCHECK_RUN_ON(signaling_thread());
- RTC_DCHECK(IsUnifiedPlan());
-
- for (auto&& transceivers_stable_state_pair :
- transceiver_stable_states_by_transceivers_) {
- auto transceiver = transceivers_stable_state_pair.first;
- auto state = transceivers_stable_state_pair.second;
- RTC_DCHECK(transceiver->internal()->mid().has_value());
- std::string mid = transceiver->internal()->mid().value();
- transport_controller_->RollbackTransportForMid(mid);
- DestroyTransceiverChannel(transceiver);
-
- if (state.newly_created()) {
- // Remove added transceivers with no added track.
- if (transceiver->internal()->sender()->track()) {
- transceiver->internal()->set_created_by_addtrack(true);
- } else {
- int remaining_transceiver_count = 0;
- for (auto&& t : transceivers_) {
- if (t != transceiver) {
- transceivers_[remaining_transceiver_count++] = t;
- }
- }
- transceivers_.resize(remaining_transceiver_count);
- }
- }
- transceiver->internal()->sender_internal()->set_transport(nullptr);
- transceiver->internal()->receiver_internal()->set_transport(nullptr);
- transceiver->internal()->set_direction(state.direction());
- transceiver->internal()->set_mid(state.mid());
- transceiver->internal()->set_mline_index(state.mline_index());
- }
- transceiver_stable_states_by_transceivers_.clear();
- pending_local_description_.reset();
- pending_remote_description_.reset();
- ChangeSignalingState(PeerConnectionInterface::kStable);
- return RTCError::OK();
-}
-
} // namespace webrtc
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 393a1dd..c783ae9 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -391,34 +391,6 @@
FieldTrialFlag receive_only;
};
- // Captures partial state to be used for rollback. Applicable only in
- // Unified Plan.
- class TransceiverStableState {
- public:
- TransceiverStableState() {}
- TransceiverStableState(RtpTransceiverDirection direction,
- absl::optional<std::string> mid,
- absl::optional<size_t> mline_index,
- bool newly_created)
- : direction_(direction),
- mid_(mid),
- mline_index_(mline_index),
- newly_created_(newly_created) {}
- RtpTransceiverDirection direction() const { return direction_; }
- absl::optional<std::string> mid() const { return mid_; }
- absl::optional<size_t> mline_index() const { return mline_index_; }
- bool newly_created() const { return newly_created_; }
-
- private:
- RtpTransceiverDirection direction_ = RtpTransceiverDirection::kRecvOnly;
- absl::optional<std::string> mid_;
- absl::optional<size_t> mline_index_;
- // Indicates that the transceiver was created as part of applying a
- // description to track potential need for removing transceiver during
- // rollback.
- bool newly_created_ = false;
- };
-
// Implements MessageHandler.
void OnMessage(rtc::Message* msg) override;
@@ -1193,7 +1165,6 @@
void UpdateNegotiationNeeded();
bool CheckIfNegotiationIsNeeded();
- RTCError Rollback();
sigslot::signal1<DataChannel*> SignalDataChannelCreated_
RTC_GUARDED_BY(signaling_thread());
@@ -1315,11 +1286,7 @@
RTC_GUARDED_BY(signaling_thread()); // A pointer is passed to senders_
rtc::scoped_refptr<RTCStatsCollector> stats_collector_
RTC_GUARDED_BY(signaling_thread());
- // Holds changes made to transceivers during applying descriptors for
- // potential rollback. Gets cleared once signaling state goes to stable.
- std::map<rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>,
- TransceiverStableState>
- transceiver_stable_states_by_transceivers_;
+
std::vector<
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>>
transceivers_; // TODO(bugs.webrtc.org/9987): Accessed on both signaling
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index b06091b..3a0ef0f 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -230,7 +230,7 @@
// will set the whole offer/answer exchange in motion. Just need to wait for
// the signaling state to reach "stable".
void CreateAndSetAndSignalOffer() {
- auto offer = CreateOfferAndWait();
+ auto offer = CreateOffer();
ASSERT_NE(nullptr, offer);
EXPECT_TRUE(SetLocalDescriptionAndSendSdpMessage(std::move(offer)));
}
@@ -302,13 +302,6 @@
return ice_candidate_pair_change_history_;
}
- // Every PeerConnection signaling state in order that has been seen by the
- // observer.
- std::vector<PeerConnectionInterface::SignalingState>
- peer_connection_signaling_state_history() const {
- return peer_connection_signaling_state_history_;
- }
-
void AddAudioVideoTracks() {
AddAudioTrack();
AddVideoTrack();
@@ -584,14 +577,6 @@
network_manager()->set_mdns_responder(std::move(mdns_responder));
}
- // Returns null on failure.
- std::unique_ptr<SessionDescriptionInterface> CreateOfferAndWait() {
- rtc::scoped_refptr<MockCreateSessionDescriptionObserver> observer(
- new rtc::RefCountedObject<MockCreateSessionDescriptionObserver>());
- pc()->CreateOffer(observer, offer_answer_options_);
- return WaitForDescriptionFromObserver(observer);
- }
-
private:
explicit PeerConnectionWrapper(const std::string& debug_name)
: debug_name_(debug_name) {}
@@ -747,6 +732,14 @@
}
// Returns null on failure.
+ std::unique_ptr<SessionDescriptionInterface> CreateOffer() {
+ rtc::scoped_refptr<MockCreateSessionDescriptionObserver> observer(
+ new rtc::RefCountedObject<MockCreateSessionDescriptionObserver>());
+ pc()->CreateOffer(observer, offer_answer_options_);
+ return WaitForDescriptionFromObserver(observer);
+ }
+
+ // Returns null on failure.
std::unique_ptr<SessionDescriptionInterface> CreateAnswer() {
rtc::scoped_refptr<MockCreateSessionDescriptionObserver> observer(
new rtc::RefCountedObject<MockCreateSessionDescriptionObserver>());
@@ -901,7 +894,6 @@
void OnSignalingChange(
webrtc::PeerConnectionInterface::SignalingState new_state) override {
EXPECT_EQ(pc()->signaling_state(), new_state);
- peer_connection_signaling_state_history_.push_back(new_state);
}
void OnAddTrack(rtc::scoped_refptr<RtpReceiverInterface> receiver,
const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
@@ -1045,8 +1037,7 @@
ice_gathering_state_history_;
std::vector<cricket::CandidatePairChangeEvent>
ice_candidate_pair_change_history_;
- std::vector<PeerConnectionInterface::SignalingState>
- peer_connection_signaling_state_history_;
+
webrtc::FakeRtcEventLogFactory* event_log_factory_;
rtc::AsyncInvoker invoker_;
@@ -6000,61 +5991,6 @@
caller()->error_event().host_candidate.find(":"));
}
-TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
- AudioKeepsFlowingAfterImplicitRollback) {
- PeerConnectionInterface::RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kUnifiedPlan;
- config.enable_implicit_rollback = true;
- ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(config, config));
- ConnectFakeSignaling();
- caller()->AddAudioTrack();
- callee()->AddAudioTrack();
- caller()->CreateAndSetAndSignalOffer();
- ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
- MediaExpectations media_expectations;
- media_expectations.ExpectBidirectionalAudio();
- ASSERT_TRUE(ExpectNewFrames(media_expectations));
- SetSignalIceCandidates(false); // Workaround candidate outrace sdp.
- caller()->AddVideoTrack();
- callee()->AddVideoTrack();
- rtc::scoped_refptr<MockSetSessionDescriptionObserver> observer(
- new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
- callee()->pc()->SetLocalDescription(observer,
- callee()->CreateOfferAndWait().release());
- EXPECT_TRUE_WAIT(observer->called(), kDefaultTimeout);
- caller()->CreateAndSetAndSignalOffer(); // Implicit rollback.
- ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
- ASSERT_TRUE(ExpectNewFrames(media_expectations));
-}
-
-TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
- ImplicitRollbackVisitsStableState) {
- RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kUnifiedPlan;
- config.enable_implicit_rollback = true;
-
- ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(config, config));
-
- rtc::scoped_refptr<MockSetSessionDescriptionObserver> sld_observer(
- new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
- callee()->pc()->SetLocalDescription(sld_observer,
- callee()->CreateOfferAndWait().release());
- EXPECT_TRUE_WAIT(sld_observer->called(), kDefaultTimeout);
- EXPECT_EQ(sld_observer->error(), "");
-
- rtc::scoped_refptr<MockSetSessionDescriptionObserver> srd_observer(
- new rtc::RefCountedObject<MockSetSessionDescriptionObserver>());
- callee()->pc()->SetRemoteDescription(
- srd_observer, caller()->CreateOfferAndWait().release());
- EXPECT_TRUE_WAIT(srd_observer->called(), kDefaultTimeout);
- EXPECT_EQ(srd_observer->error(), "");
-
- EXPECT_THAT(callee()->peer_connection_signaling_state_history(),
- ElementsAre(PeerConnectionInterface::kHaveLocalOffer,
- PeerConnectionInterface::kStable,
- PeerConnectionInterface::kHaveRemoteOffer));
-}
-
INSTANTIATE_TEST_SUITE_P(PeerConnectionIntegrationTest,
PeerConnectionIntegrationTest,
Values(SdpSemantics::kPlanB,
diff --git a/pc/peer_connection_jsep_unittest.cc b/pc/peer_connection_jsep_unittest.cc
index 514374b..1fe8d07 100644
--- a/pc/peer_connection_jsep_unittest.cc
+++ b/pc/peer_connection_jsep_unittest.cc
@@ -1727,220 +1727,4 @@
error);
}
-TEST_F(PeerConnectionJsepTest, RollbackSupportedInUnifiedPlan) {
- RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kUnifiedPlan;
- config.enable_implicit_rollback = true;
- auto caller = CreatePeerConnection(config);
- auto callee = CreatePeerConnection(config);
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(caller->SetLocalDescription(caller->CreateRollback()));
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(caller->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(caller->SetRemoteDescription(callee->CreateOffer()));
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackNotSupportedInPlanB) {
- RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kPlanB;
- config.enable_implicit_rollback = true;
- auto caller = CreatePeerConnection(config);
- auto callee = CreatePeerConnection(config);
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_FALSE(caller->SetLocalDescription(caller->CreateRollback()));
- EXPECT_FALSE(caller->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_FALSE(caller->SetRemoteDescription(callee->CreateOffer()));
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackFailsInStableState) {
- auto caller = CreatePeerConnection();
- EXPECT_FALSE(caller->SetLocalDescription(caller->CreateRollback()));
- EXPECT_FALSE(caller->SetRemoteDescription(caller->CreateRollback()));
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackToStableStateAndClearLocalOffer) {
- auto caller = CreatePeerConnection();
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(caller->SetLocalDescription(caller->CreateRollback()));
- EXPECT_EQ(caller->signaling_state(), PeerConnectionInterface::kStable);
- EXPECT_EQ(caller->pc()->pending_local_description(), nullptr);
-
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(caller->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_EQ(caller->signaling_state(), PeerConnectionInterface::kStable);
- EXPECT_EQ(caller->pc()->pending_local_description(), nullptr);
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackToStableStateAndClearRemoteOffer) {
- auto caller = CreatePeerConnection();
- auto callee = CreatePeerConnection();
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_EQ(callee->signaling_state(), PeerConnectionInterface::kStable);
- EXPECT_EQ(callee->pc()->pending_remote_description(), nullptr);
-
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_TRUE(callee->SetLocalDescription(caller->CreateRollback()));
- EXPECT_EQ(callee->signaling_state(), PeerConnectionInterface::kStable);
- EXPECT_EQ(callee->pc()->pending_remote_description(), nullptr);
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackLocalOfferImplicitly) {
- RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kUnifiedPlan;
- config.enable_implicit_rollback = true;
- auto caller = CreatePeerConnection(config);
- auto callee = CreatePeerConnection(config);
- EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_EQ(callee->signaling_state(),
- PeerConnectionInterface::kHaveRemoteOffer);
-}
-
-TEST_F(PeerConnectionJsepTest, AttemptToRollbackLocalOfferImplicitly) {
- RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kUnifiedPlan;
- config.enable_implicit_rollback = true;
- auto caller = CreatePeerConnection(config);
- auto callee = CreatePeerConnection(config);
- EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
- EXPECT_FALSE(callee->SetRemoteDescription(
- CreateSessionDescription(SdpType::kOffer, "invalid sdp")));
- EXPECT_EQ(callee->signaling_state(),
- PeerConnectionInterface::kHaveLocalOffer);
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackRemovesTransceiver) {
- auto caller = CreatePeerConnection();
- caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- auto callee = CreatePeerConnection();
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{0});
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackKeepsTransceiverAndClearsMid) {
- auto caller = CreatePeerConnection();
- caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- auto callee = CreatePeerConnection();
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- callee->AddAudioTrack("a");
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
- // Transceiver can't be removed as track was added to it.
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
- // Mid got cleared to make it reusable.
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
- // Transceiver should be counted as addTrack-created after rollback.
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackRestoresMid) {
- auto caller = CreatePeerConnection();
- caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- auto callee = CreatePeerConnection();
- callee->AddAudioTrack("a");
- auto offer = callee->CreateOffer();
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
- EXPECT_NE(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
- EXPECT_TRUE(callee->SetLocalDescription(std::move(offer)));
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackRestoresMidAndRemovesTransceiver) {
- auto callee = CreatePeerConnection();
- callee->AddVideoTrack("a");
- auto offer = callee->CreateOffer();
- auto caller = CreatePeerConnection();
- caller->AddAudioTrack("b");
- caller->AddVideoTrack("c");
- auto mid = callee->pc()->GetTransceivers()[0]->mid();
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2});
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{1});
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), mid);
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->media_type(),
- cricket::MEDIA_TYPE_VIDEO);
- EXPECT_TRUE(callee->SetLocalDescription(std::move(offer)));
-}
-
-TEST_F(PeerConnectionJsepTest, ImplicitlyRollbackTransceiversWithSameMids) {
- RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kUnifiedPlan;
- config.enable_implicit_rollback = true;
- auto caller = CreatePeerConnection(config);
- caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
- auto callee = CreatePeerConnection(config);
- callee->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
- EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
- auto initial_mid = callee->pc()->GetTransceivers()[0]->mid();
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2});
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
- EXPECT_EQ(callee->pc()->GetTransceivers()[1]->mid(),
- caller->pc()->GetTransceivers()[0]->mid());
- EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal()); // Go to stable.
- EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
- EXPECT_NE(callee->pc()->GetTransceivers()[0]->mid(), initial_mid);
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackToNegotiatedStableState) {
- RTCConfiguration config;
- config.sdp_semantics = SdpSemantics::kUnifiedPlan;
- config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle;
- auto caller = CreatePeerConnection(config);
- caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- auto callee = CreatePeerConnection(config);
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
- EXPECT_TRUE(callee->CreateAnswerAndSetAsLocal());
- caller->AddVideoTrack("a");
- callee->AddVideoTrack("b");
- EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2});
- auto audio_transport =
- callee->pc()->GetTransceivers()[0]->sender()->dtls_transport();
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
- callee->pc()->GetTransceivers()[1]->sender()->dtls_transport());
- EXPECT_NE(callee->pc()->GetTransceivers()[1]->sender()->dtls_transport(),
- nullptr);
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateRollback()));
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
- audio_transport); // Audio must remain working after rollback.
- EXPECT_EQ(callee->pc()->GetTransceivers()[1]->sender()->dtls_transport(),
- nullptr);
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
-
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->sender()->dtls_transport(),
- audio_transport); // Audio transport is still the same.
-}
-
-TEST_F(PeerConnectionJsepTest, RollbackAfterMultipleSLD) {
- auto callee = CreatePeerConnection();
- callee->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
- callee->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
- EXPECT_TRUE(callee->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(callee->SetRemoteDescription(callee->CreateRollback()));
- EXPECT_EQ(callee->pc()->GetTransceivers().size(), size_t{2});
- EXPECT_EQ(callee->pc()->GetTransceivers()[0]->mid(), absl::nullopt);
- EXPECT_EQ(callee->pc()->GetTransceivers()[1]->mid(), absl::nullopt);
-}
-
-TEST_F(PeerConnectionJsepTest, NoRollbackNeeded) {
- auto caller = CreatePeerConnection();
- caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- auto callee = CreatePeerConnection();
- callee->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- EXPECT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
-}
-
} // namespace webrtc
diff --git a/pc/peer_connection_wrapper.cc b/pc/peer_connection_wrapper.cc
index 7c0b339..b4b0782 100644
--- a/pc/peer_connection_wrapper.cc
+++ b/pc/peer_connection_wrapper.cc
@@ -125,11 +125,6 @@
return answer;
}
-std::unique_ptr<SessionDescriptionInterface>
-PeerConnectionWrapper::CreateRollback() {
- return CreateSessionDescription(SdpType::kRollback, "");
-}
-
std::unique_ptr<SessionDescriptionInterface> PeerConnectionWrapper::CreateSdp(
rtc::FunctionView<void(CreateSessionDescriptionObserver*)> fn,
std::string* error_out) {
diff --git a/pc/peer_connection_wrapper.h b/pc/peer_connection_wrapper.h
index 4d2bc28..fafee24 100644
--- a/pc/peer_connection_wrapper.h
+++ b/pc/peer_connection_wrapper.h
@@ -87,7 +87,6 @@
const PeerConnectionInterface::RTCOfferAnswerOptions& options);
// Calls CreateAnswerAndSetAsLocal with default options.
std::unique_ptr<SessionDescriptionInterface> CreateAnswerAndSetAsLocal();
- std::unique_ptr<SessionDescriptionInterface> CreateRollback();
// Calls the underlying PeerConnection's SetLocalDescription method with the
// given session description and waits for the success/failure response.
diff --git a/pc/sdp_utils.cc b/pc/sdp_utils.cc
index f5385a6..5bfdaa4 100644
--- a/pc/sdp_utils.cc
+++ b/pc/sdp_utils.cc
@@ -29,10 +29,8 @@
SdpType type) {
RTC_DCHECK(sdesc);
auto clone = std::make_unique<JsepSessionDescription>(type);
- if (sdesc->description()) {
- clone->Initialize(sdesc->description()->Clone(), sdesc->session_id(),
- sdesc->session_version());
- }
+ clone->Initialize(sdesc->description()->Clone(), sdesc->session_id(),
+ sdesc->session_version());
// As of writing, our version of GCC does not allow returning a unique_ptr of
// a subclass as a unique_ptr of a base class. To get around this, we need to
// std::move the return value.
