Change error handlers for Set*Description to use RTCError

Needed in order to return error codes to Chromium.

Bug: chromium:819629, chromium:589455
Change-Id: Iab22250db62a348eee21c6d8bfc44020a7380586
Reviewed-on: https://webrtc-review.googlesource.com/60522
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22367}
This commit is contained in:
Harald Alvestrand 2018-03-09 15:18:03 +01:00 committed by Commit Bot
parent a5aa68b73f
commit 5081c0cc6d
6 changed files with 78 additions and 29 deletions

View File

@ -27,6 +27,7 @@
#include <vector>
#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;

View File

@ -115,7 +115,7 @@ struct SetSessionDescriptionMsg : public rtc::MessageData {
}
rtc::scoped_refptr<webrtc::SetSessionDescriptionObserver> 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<webrtc::CreateSessionDescriptionObserver> 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<SetSessionDescriptionMsg*>(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<CreateSessionDescriptionMsg*>(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()));
}

View File

@ -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.

View File

@ -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

View File

@ -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_; }

View File

@ -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);