diff --git a/api/jsep.h b/api/jsep.h index 581f689d1c..8fd2dacf65 100644 --- a/api/jsep.h +++ b/api/jsep.h @@ -27,6 +27,7 @@ #include #include "api/optional.h" +#include "api/rtcerror.h" #include "rtc_base/refcount.h" namespace cricket { @@ -197,7 +198,19 @@ class CreateSessionDescriptionObserver : public rtc::RefCountInterface { // TODO(deadbeef): Make this take an std::unique_ptr<> to avoid confusion // around ownership. virtual void OnSuccess(SessionDescriptionInterface* desc) = 0; - virtual void OnFailure(const std::string& error) = 0; + // The OnFailure callback takes an RTCError, which consists of an + // error code and a string. + // RTCError is non-copyable, so it must be passed using std::move. + // Earlier versions of the API used a string argument. This version + // is deprecated; in order to let clients remove the old version, it has a + // default implementation. If both versions are unimplemented, the + // result will be a runtime error (stack overflow). This is intentional. + virtual void OnFailure(RTCError error) { + OnFailure(error.message()); + } + virtual void OnFailure(const std::string& error) { + OnFailure(RTCError(RTCErrorType::INTERNAL_ERROR, std::string(error))); + } protected: ~CreateSessionDescriptionObserver() override = default; @@ -207,7 +220,14 @@ class CreateSessionDescriptionObserver : public rtc::RefCountInterface { class SetSessionDescriptionObserver : public rtc::RefCountInterface { public: virtual void OnSuccess() = 0; - virtual void OnFailure(const std::string& error) = 0; + // See description in CreateSessionDescriptionObserver for OnFailure. + virtual void OnFailure(RTCError error) { + std::string message(error.message()); + OnFailure(message); + } + virtual void OnFailure(const std::string& error) { + OnFailure(RTCError(RTCErrorType::INTERNAL_ERROR, std::string(error))); + } protected: ~SetSessionDescriptionObserver() override = default; diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index e37d9d88d8..fadb47bcca 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -115,7 +115,7 @@ struct SetSessionDescriptionMsg : public rtc::MessageData { } rtc::scoped_refptr observer; - std::string error; + RTCError error; }; struct CreateSessionDescriptionMsg : public rtc::MessageData { @@ -124,7 +124,7 @@ struct CreateSessionDescriptionMsg : public rtc::MessageData { : observer(observer) {} rtc::scoped_refptr observer; - std::string error; + RTCError error; }; struct GetStatsMsg : public rtc::MessageData { @@ -623,7 +623,7 @@ class PeerConnection::SetRemoteDescriptionObserverAdapter if (error.ok()) pc_->PostSetSessionDescriptionSuccess(wrapper_); else - pc_->PostSetSessionDescriptionFailure(wrapper_, error.message()); + pc_->PostSetSessionDescriptionFailure(wrapper_, std::move(error)); } private: @@ -1654,14 +1654,16 @@ void PeerConnection::CreateOffer(CreateSessionDescriptionObserver* observer, if (IsClosed()) { std::string error = "CreateOffer called when PeerConnection is closed."; RTC_LOG(LS_ERROR) << error; - PostCreateSessionDescriptionFailure(observer, error); + PostCreateSessionDescriptionFailure( + observer, RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error))); return; } if (!ValidateOfferAnswerOptions(options)) { std::string error = "CreateOffer called with invalid options."; RTC_LOG(LS_ERROR) << error; - PostCreateSessionDescriptionFailure(observer, error); + PostCreateSessionDescriptionFailure( + observer, RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error))); return; } @@ -1670,7 +1672,7 @@ void PeerConnection::CreateOffer(CreateSessionDescriptionObserver* observer, if (IsUnifiedPlan()) { RTCError error = HandleLegacyOfferOptions(options); if (!error.ok()) { - PostCreateSessionDescriptionFailure(observer, error.message()); + PostCreateSessionDescriptionFailure(observer, std::move(error)); return; } } @@ -1753,7 +1755,8 @@ void PeerConnection::CreateAnswer( &offer_answer_options)) { std::string error = "CreateAnswer called with invalid constraints."; RTC_LOG(LS_ERROR) << error; - PostCreateSessionDescriptionFailure(observer, error); + PostCreateSessionDescriptionFailure( + observer, RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error))); return; } @@ -1774,7 +1777,8 @@ void PeerConnection::CreateAnswer(CreateSessionDescriptionObserver* observer, "PeerConnection cannot create an answer in a state other than " "have-remote-offer or have-local-pranswer."; RTC_LOG(LS_ERROR) << error; - PostCreateSessionDescriptionFailure(observer, error); + PostCreateSessionDescriptionFailure( + observer, RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error))); return; } @@ -1817,7 +1821,9 @@ void PeerConnection::SetLocalDescription( } if (!desc) { - PostSetSessionDescriptionFailure(observer, "SessionDescription is NULL."); + PostSetSessionDescriptionFailure( + observer, + RTCError(RTCErrorType::INTERNAL_ERROR, "SessionDescription is NULL.")); return; } @@ -1826,7 +1832,9 @@ void PeerConnection::SetLocalDescription( if (session_error() != SessionError::kNone) { std::string error_message = GetSessionErrorMsg(); RTC_LOG(LS_ERROR) << "SetLocalDescription: " << error_message; - PostSetSessionDescriptionFailure(observer, std::move(error_message)); + PostSetSessionDescriptionFailure( + observer, + RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error_message))); return; } @@ -1835,7 +1843,9 @@ void PeerConnection::SetLocalDescription( std::string error_message = GetSetDescriptionErrorMessage( cricket::CS_LOCAL, desc->GetType(), error); RTC_LOG(LS_ERROR) << error_message; - PostSetSessionDescriptionFailure(observer, std::move(error_message)); + PostSetSessionDescriptionFailure( + observer, + RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error_message))); return; } @@ -1854,7 +1864,9 @@ void PeerConnection::SetLocalDescription( std::string error_message = GetSetDescriptionErrorMessage(cricket::CS_LOCAL, type, error); RTC_LOG(LS_ERROR) << error_message; - PostSetSessionDescriptionFailure(observer, std::move(error_message)); + PostSetSessionDescriptionFailure( + observer, + RTCError(RTCErrorType::INTERNAL_ERROR, std::move(error_message))); return; } RTC_DCHECK(local_description()); @@ -3108,14 +3120,14 @@ void PeerConnection::OnMessage(rtc::Message* msg) { case MSG_SET_SESSIONDESCRIPTION_FAILED: { SetSessionDescriptionMsg* param = static_cast(msg->pdata); - param->observer->OnFailure(param->error); + param->observer->OnFailure(std::move(param->error)); delete param; break; } case MSG_CREATE_SESSIONDESCRIPTION_FAILED: { CreateSessionDescriptionMsg* param = static_cast(msg->pdata); - param->observer->OnFailure(param->error); + param->observer->OnFailure(std::move(param->error)); delete param; break; } @@ -3401,18 +3413,20 @@ void PeerConnection::PostSetSessionDescriptionSuccess( void PeerConnection::PostSetSessionDescriptionFailure( SetSessionDescriptionObserver* observer, - const std::string& error) { + RTCError&& error) { + RTC_DCHECK(!error.ok()); SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer); - msg->error = error; + msg->error = std::move(error); signaling_thread()->Post(RTC_FROM_HERE, this, MSG_SET_SESSIONDESCRIPTION_FAILED, msg); } void PeerConnection::PostCreateSessionDescriptionFailure( CreateSessionDescriptionObserver* observer, - const std::string& error) { + RTCError error) { + RTC_DCHECK(!error.ok()); CreateSessionDescriptionMsg* msg = new CreateSessionDescriptionMsg(observer); - msg->error = error; + msg->error = std::move(error); signaling_thread()->Post(RTC_FROM_HERE, this, MSG_CREATE_SESSIONDESCRIPTION_FAILED, msg); } @@ -5665,7 +5679,7 @@ RTCError PeerConnection::ValidateSessionDescription( if ((source == cricket::CS_LOCAL && !ExpectSetLocalDescription(type)) || (source == cricket::CS_REMOTE && !ExpectSetRemoteDescription(type))) { LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_PARAMETER, + RTCErrorType::INVALID_STATE, "Called in wrong state: " + GetSignalingStateString(signaling_state())); } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index f5b53ae382..9051910d15 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -382,10 +382,10 @@ class PeerConnection : public PeerConnectionInternal, void PostSetSessionDescriptionSuccess( SetSessionDescriptionObserver* observer); void PostSetSessionDescriptionFailure(SetSessionDescriptionObserver* observer, - const std::string& error); + RTCError&& error); void PostCreateSessionDescriptionFailure( CreateSessionDescriptionObserver* observer, - const std::string& error); + RTCError error); // Synchronous implementations of SetLocalDescription/SetRemoteDescription // that return an RTCError instead of invoking a callback. diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc index e19931f6f0..6d3cf76b38 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -1243,4 +1243,18 @@ TEST_F(PeerConnectionJsepTest, CurrentDirectionResetWhenRtpTransceiverStopped) { EXPECT_FALSE(transceiver->current_direction()); } +// Tests that you can't set an answer on a PeerConnection before setting +// the offer. +TEST_F(PeerConnectionJsepTest, AnswerBeforeOfferFails) { + auto caller = CreatePeerConnection(); + auto callee = CreatePeerConnection(); + RTCError error_out; + caller->AddAudioTrack("audio"); + auto offer = caller->CreateOffer(); + callee->SetRemoteDescription(std::move(offer), &error_out); + auto answer = callee->CreateAnswer(); + EXPECT_FALSE(caller->SetRemoteDescription(std::move(answer), &error_out)); + EXPECT_EQ(RTCErrorType::INVALID_STATE, error_out.type()); +} + } // namespace webrtc diff --git a/pc/test/mockpeerconnectionobservers.h b/pc/test/mockpeerconnectionobservers.h index 162bd89c8f..345a2b6613 100644 --- a/pc/test/mockpeerconnectionobservers.h +++ b/pc/test/mockpeerconnectionobservers.h @@ -230,14 +230,14 @@ class MockCreateSessionDescriptionObserver : called_(false), error_("MockCreateSessionDescriptionObserver not called") {} virtual ~MockCreateSessionDescriptionObserver() {} - virtual void OnSuccess(SessionDescriptionInterface* desc) { + void OnSuccess(SessionDescriptionInterface* desc) override { called_ = true; error_ = ""; desc_.reset(desc); } - virtual void OnFailure(const std::string& error) { + void OnFailure(webrtc::RTCError error) override { called_ = true; - error_ = error; + error_ = error.message(); } bool called() const { return called_; } bool result() const { return error_.empty(); } @@ -263,10 +263,11 @@ class MockSetSessionDescriptionObserver called_ = true; error_ = ""; } - void OnFailure(const std::string& error) override { + void OnFailure(webrtc::RTCError error) override { called_ = true; - error_ = error; + error_ = error.message(); } + bool called() const { return called_; } bool result() const { return error_.empty(); } const std::string& error() const { return error_; } diff --git a/pc/test/peerconnectiontestwrapper.h b/pc/test/peerconnectiontestwrapper.h index 338b31df57..4d15244eba 100644 --- a/pc/test/peerconnectiontestwrapper.h +++ b/pc/test/peerconnectiontestwrapper.h @@ -64,7 +64,7 @@ class PeerConnectionTestWrapper // Implements CreateSessionDescriptionObserver. void OnSuccess(webrtc::SessionDescriptionInterface* desc) override; - void OnFailure(const std::string& error) override {} + void OnFailure(webrtc::RTCError) override {} void CreateOffer(const webrtc::MediaConstraintsInterface* constraints); void CreateAnswer(const webrtc::MediaConstraintsInterface* constraints);