Use absl::string_view type as parameter for RTCError message

Bug: webrtc:13579
Change-Id: Ia9f90e6c3b008fc614d378cae4c407becfc597c9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298447
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39610}
This commit is contained in:
Danil Chapovalov 2023-03-20 17:37:22 +01:00 committed by WebRTC LUCI CQ
parent 5afb0146b5
commit a2d85e4565
6 changed files with 65 additions and 65 deletions

View file

@ -379,7 +379,10 @@ rtc_library("rtc_error") {
"../rtc_base:macromagic",
"../rtc_base/system:rtc_export",
]
absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
absl_deps = [
"//third_party/abseil-cpp/absl/strings",
"//third_party/abseil-cpp/absl/types:optional",
]
}
rtc_source_set("packet_socket_factory") {

View file

@ -10,11 +10,13 @@
#include "api/rtc_error.h"
#include "rtc_base/arraysize.h"
#include <iterator>
#include "absl/strings/string_view.h"
namespace {
const char* kRTCErrorTypeNames[] = {
absl::string_view kRTCErrorTypeNames[] = {
"NONE",
"UNSUPPORTED_OPERATION",
"UNSUPPORTED_PARAMETER",
@ -30,11 +32,11 @@ const char* kRTCErrorTypeNames[] = {
};
static_assert(
static_cast<int>(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA) ==
(arraysize(kRTCErrorTypeNames) - 1),
(std::size(kRTCErrorTypeNames) - 1),
"kRTCErrorTypeNames must have as many strings as RTCErrorType "
"has values.");
const char* kRTCErrorDetailTypeNames[] = {
absl::string_view kRTCErrorDetailTypeNames[] = {
"NONE",
"DATA_CHANNEL_FAILURE",
"DTLS_FAILURE",
@ -46,7 +48,7 @@ const char* kRTCErrorDetailTypeNames[] = {
};
static_assert(
static_cast<int>(webrtc::RTCErrorDetailType::HARDWARE_ENCODER_ERROR) ==
(arraysize(kRTCErrorDetailTypeNames) - 1),
(std::size(kRTCErrorDetailTypeNames) - 1),
"kRTCErrorDetailTypeNames must have as many strings as "
"RTCErrorDetailType has values.");
@ -63,16 +65,16 @@ const char* RTCError::message() const {
return message_.c_str();
}
void RTCError::set_message(std::string message) {
message_ = std::move(message);
void RTCError::set_message(absl::string_view message) {
message_ = std::string(message);
}
const char* ToString(RTCErrorType error) {
absl::string_view ToString(RTCErrorType error) {
int index = static_cast<int>(error);
return kRTCErrorTypeNames[index];
}
const char* ToString(RTCErrorDetailType error) {
absl::string_view ToString(RTCErrorDetailType error) {
int index = static_cast<int>(error);
return kRTCErrorDetailTypeNames[index];
}

View file

@ -17,6 +17,7 @@
#include <string>
#include <utility> // For std::move.
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
@ -108,8 +109,8 @@ class RTC_EXPORT RTCError {
RTCError() {}
explicit RTCError(RTCErrorType type) : type_(type) {}
RTCError(RTCErrorType type, std::string message)
: type_(type), message_(std::move(message)) {}
RTCError(RTCErrorType type, absl::string_view message)
: type_(type), message_(message) {}
// In many use cases, it is better to use move than copy,
// but copy and assignment are provided for those cases that need it.
@ -133,7 +134,7 @@ class RTC_EXPORT RTCError {
// stable.
const char* message() const;
void set_message(std::string message);
void set_message(absl::string_view message);
RTCErrorDetailType error_detail() const { return error_detail_; }
void set_error_detail(RTCErrorDetailType detail) { error_detail_ = detail; }
@ -158,8 +159,8 @@ class RTC_EXPORT RTCError {
//
// Only intended to be used for logging/diagnostics. The returned char* points
// to literal string that lives for the whole duration of the program.
RTC_EXPORT const char* ToString(RTCErrorType error);
RTC_EXPORT const char* ToString(RTCErrorDetailType error);
RTC_EXPORT absl::string_view ToString(RTCErrorType error);
RTC_EXPORT absl::string_view ToString(RTCErrorDetailType error);
#ifdef WEBRTC_UNIT_TEST
inline std::ostream& operator<<( // no-presubmit-check TODO(webrtc:8982)

View file

@ -14,9 +14,10 @@
#include "test/gtest.h"
namespace webrtc {
namespace {
const int kDefaultMoveOnlyIntValue = 0xbadf00d;
constexpr int kDefaultMoveOnlyIntValue = 0xbadf00d;
// Class that has no copy constructor, ensuring that RTCErrorOr can
struct MoveOnlyInt {
@ -55,46 +56,47 @@ struct MoveOnlyInt2 {
int value = kDefaultMoveOnlyIntValue;
};
} // namespace
namespace webrtc {
// Test that the default constructor creates a "no error" error.
TEST(RTCErrorTest, DefaultConstructor) {
RTCError e;
EXPECT_EQ(RTCErrorType::NONE, e.type());
EXPECT_EQ(std::string(), e.message());
EXPECT_EQ(e.type(), RTCErrorType::NONE);
EXPECT_STREQ(e.message(), "");
EXPECT_TRUE(e.ok());
}
TEST(RTCErrorTest, NormalConstructors) {
RTCError a(RTCErrorType::INVALID_PARAMETER);
EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, a.type());
EXPECT_EQ(std::string(), a.message());
EXPECT_EQ(a.type(), RTCErrorType::INVALID_PARAMETER);
EXPECT_STREQ(a.message(), "");
// Constructor that takes const char* message.
RTCError b(RTCErrorType::UNSUPPORTED_PARAMETER, "foobar");
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, b.type());
EXPECT_EQ(std::string("foobar"), b.message());
EXPECT_EQ(b.type(), RTCErrorType::UNSUPPORTED_PARAMETER);
EXPECT_STREQ(b.message(), "foobar");
// Constructor that takes absl::string_view message.
RTCError c(RTCErrorType::SYNTAX_ERROR, absl::string_view("baz"));
EXPECT_EQ(c.type(), RTCErrorType::SYNTAX_ERROR);
EXPECT_STREQ(c.message(), "baz");
// Constructor that takes std::string message.
RTCError c(RTCErrorType::INVALID_RANGE, std::string("new"));
EXPECT_EQ(RTCErrorType::INVALID_RANGE, c.type());
EXPECT_EQ(std::string("new"), c.message());
RTCError d(RTCErrorType::INVALID_RANGE, std::string("new"));
EXPECT_EQ(d.type(), RTCErrorType::INVALID_RANGE);
EXPECT_STREQ(d.message(), "new");
}
TEST(RTCErrorTest, MoveConstructor) {
// Static string.
RTCError a(RTCErrorType::INVALID_PARAMETER, "foo");
RTCError b(std::move(a));
EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, b.type());
EXPECT_EQ(std::string("foo"), b.message());
EXPECT_EQ(b.type(), RTCErrorType::INVALID_PARAMETER);
EXPECT_STREQ(b.message(), "foo");
// Non-static string.
RTCError c(RTCErrorType::UNSUPPORTED_PARAMETER, std::string("bar"));
RTCError d(std::move(c));
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, d.type());
EXPECT_EQ(std::string("bar"), d.message());
EXPECT_EQ(d.type(), RTCErrorType::UNSUPPORTED_PARAMETER);
EXPECT_STREQ(d.message(), "bar");
}
TEST(RTCErrorTest, MoveAssignment) {
@ -102,24 +104,21 @@ TEST(RTCErrorTest, MoveAssignment) {
RTCError e(RTCErrorType::INVALID_PARAMETER, "foo");
e = RTCError(RTCErrorType::UNSUPPORTED_PARAMETER, "bar");
EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, e.type());
EXPECT_EQ(std::string("bar"), e.message());
EXPECT_EQ(e.type(), RTCErrorType::UNSUPPORTED_PARAMETER);
EXPECT_STREQ(e.message(), "bar");
e = RTCError(RTCErrorType::SYNTAX_ERROR, std::string("baz"));
EXPECT_EQ(std::string("baz"), e.message());
e = RTCError(RTCErrorType::SYNTAX_ERROR, absl::string_view("baz"));
EXPECT_STREQ(e.message(), "baz");
e = RTCError(RTCErrorType::SYNTAX_ERROR, std::string("another"));
EXPECT_EQ(std::string("another"), e.message());
e = RTCError(RTCErrorType::SYNTAX_ERROR, "last");
EXPECT_EQ(std::string("last"), e.message());
EXPECT_STREQ(e.message(), "another");
}
// Test that the error returned by RTCError::OK() is a "no error" error.
TEST(RTCErrorTest, OKConstant) {
RTCError ok = RTCError::OK();
EXPECT_EQ(RTCErrorType::NONE, ok.type());
EXPECT_EQ(std::string(), ok.message());
EXPECT_EQ(ok.type(), RTCErrorType::NONE);
EXPECT_STREQ(ok.message(), "");
EXPECT_TRUE(ok.ok());
}
@ -135,33 +134,26 @@ TEST(RTCErrorTest, OkMethod) {
// std::strings.
TEST(RTCErrorTest, SetMessage) {
RTCError e;
// Try all combinations of "is static string"/"is non-static string" calls.
e.set_message("foo");
EXPECT_EQ(std::string("foo"), e.message());
EXPECT_STREQ(e.message(), "foo");
e.set_message("bar");
EXPECT_EQ(std::string("bar"), e.message());
e.set_message(absl::string_view("bar"));
EXPECT_STREQ(e.message(), "bar");
e.set_message(std::string("string"));
EXPECT_EQ(std::string("string"), e.message());
e.set_message(std::string("more"));
EXPECT_EQ(std::string("more"), e.message());
e.set_message("love to test");
EXPECT_EQ(std::string("love to test"), e.message());
EXPECT_STREQ(e.message(), "string");
}
// Test that the default constructor creates an "INTERNAL_ERROR".
TEST(RTCErrorOrTest, DefaultConstructor) {
RTCErrorOr<MoveOnlyInt> e;
EXPECT_EQ(RTCErrorType::INTERNAL_ERROR, e.error().type());
EXPECT_EQ(e.error().type(), RTCErrorType::INTERNAL_ERROR);
}
// Test that an RTCErrorOr can be implicitly constructed from a value.
TEST(RTCErrorOrTest, ImplicitValueConstructor) {
RTCErrorOr<MoveOnlyInt> e = [] { return MoveOnlyInt(100); }();
EXPECT_EQ(100, e.value().value);
EXPECT_EQ(e.value().value, 100);
}
// Test that an RTCErrorOr can be implicitly constructed from an RTCError.
@ -169,20 +161,20 @@ TEST(RTCErrorOrTest, ImplicitErrorConstructor) {
RTCErrorOr<MoveOnlyInt> e = [] {
return RTCError(RTCErrorType::SYNTAX_ERROR);
}();
EXPECT_EQ(RTCErrorType::SYNTAX_ERROR, e.error().type());
EXPECT_EQ(e.error().type(), RTCErrorType::SYNTAX_ERROR);
}
TEST(RTCErrorOrTest, MoveConstructor) {
RTCErrorOr<MoveOnlyInt> a(MoveOnlyInt(5));
RTCErrorOr<MoveOnlyInt> b(std::move(a));
EXPECT_EQ(5, b.value().value);
EXPECT_EQ(b.value().value, 5);
}
TEST(RTCErrorOrTest, MoveAssignment) {
RTCErrorOr<MoveOnlyInt> a(MoveOnlyInt(5));
RTCErrorOr<MoveOnlyInt> b(MoveOnlyInt(10));
a = std::move(b);
EXPECT_EQ(10, a.value().value);
EXPECT_EQ(a.value().value, 10);
}
TEST(RTCErrorOrTest, ConversionConstructor) {
@ -194,7 +186,7 @@ TEST(RTCErrorOrTest, ConversionAssignment) {
RTCErrorOr<MoveOnlyInt> a(MoveOnlyInt(5));
RTCErrorOr<MoveOnlyInt2> b(MoveOnlyInt2(10));
b = std::move(a);
EXPECT_EQ(5, b.value().value);
EXPECT_EQ(b.value().value, 5);
}
TEST(RTCErrorOrTest, OkMethod) {
@ -207,14 +199,14 @@ TEST(RTCErrorOrTest, OkMethod) {
TEST(RTCErrorOrTest, MoveError) {
RTCErrorOr<int> e({RTCErrorType::SYNTAX_ERROR, "message"});
RTCError err = e.MoveError();
EXPECT_EQ(RTCErrorType::SYNTAX_ERROR, err.type());
EXPECT_EQ(std::string("message"), err.message());
EXPECT_EQ(err.type(), RTCErrorType::SYNTAX_ERROR);
EXPECT_STREQ(err.message(), "message");
}
TEST(RTCErrorOrTest, MoveValue) {
RTCErrorOr<MoveOnlyInt> e(MoveOnlyInt(88));
MoveOnlyInt value = e.MoveValue();
EXPECT_EQ(88, value.value);
EXPECT_EQ(value.value, 88);
}
// Death tests.
@ -239,4 +231,5 @@ TEST(RTCErrorOrDeathTest, MoveErrorValue) {
#endif // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
} // namespace
} // namespace webrtc

View file

@ -856,7 +856,7 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation {
std::string error_message =
GetSetDescriptionErrorMessage(cricket::CS_REMOTE, type_, error_);
RTC_LOG(LS_ERROR) << error_message;
error_.set_message(std::move(error_message));
error_.set_message(error_message);
}
observer_->OnSetRemoteDescriptionComplete(error_);

View file

@ -17,6 +17,7 @@
#include <vector>
#include "absl/algorithm/container.h"
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "rtc_base/checks.h"
@ -52,7 +53,7 @@ const char kSendDirection[] = "send";
const char kReceiveDirection[] = "recv";
const char kPayloadType[] = "pt";
RTCError ParseError(const std::string& message) {
RTCError ParseError(absl::string_view message) {
return RTCError(RTCErrorType::SYNTAX_ERROR, message);
}