Add absl::string_view overload for RtcEventLogOutput::Write

Bug: webrtc:13579
Change-Id: I13f63fb6be6aa62c2e011c18327499fa16b5824e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267641
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Ali Tofigh <alito@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37440}
This commit is contained in:
Björn Terelius 2022-07-05 10:58:52 +02:00 committed by WebRTC LUCI CQ
parent 8feb6fd1e9
commit 63299a3124
14 changed files with 52 additions and 26 deletions

View file

@ -659,6 +659,7 @@ rtc_library("create_peer_connection_quality_test_frame_generator") {
rtc_source_set("libjingle_logging_api") { rtc_source_set("libjingle_logging_api") {
visibility = [ "*" ] visibility = [ "*" ]
sources = [ "rtc_event_log_output.h" ] sources = [ "rtc_event_log_output.h" ]
absl_deps = [ "//third_party/abseil-cpp/absl/strings:strings" ]
} }
rtc_library("rtc_event_log_output_file") { rtc_library("rtc_event_log_output_file") {

View file

@ -13,6 +13,8 @@
#include <string> #include <string>
#include "absl/strings/string_view.h"
namespace webrtc { namespace webrtc {
// NOTE: This class is still under development and may change without notice. // NOTE: This class is still under development and may change without notice.
@ -32,6 +34,11 @@ class RtcEventLogOutput {
// after the first time `false` is returned. Write() may not be called on // after the first time `false` is returned. Write() may not be called on
// an inactive output sink. // an inactive output sink.
virtual bool Write(const std::string& output) = 0; virtual bool Write(const std::string& output) = 0;
// TODO(bugs.webrtc.org/13579): Make pure virtual and remove the string ref
// once all classes implement the string_view version.
virtual bool Write(absl::string_view output) {
return Write(std::string(output));
}
// Indicates that buffers should be written to disk if applicable. // Indicates that buffers should be written to disk if applicable.
virtual void Flush() {} virtual void Flush() {}

View file

@ -55,14 +55,18 @@ bool RtcEventLogOutputFile::IsActive() const {
} }
bool RtcEventLogOutputFile::Write(const std::string& output) { bool RtcEventLogOutputFile::Write(const std::string& output) {
return Write(absl::string_view(output));
}
bool RtcEventLogOutputFile::Write(absl::string_view output) {
RTC_DCHECK(IsActiveInternal()); RTC_DCHECK(IsActiveInternal());
// No single write may be so big, that it would risk overflowing the // No single write may be so big, that it would risk overflowing the
// calculation of (written_bytes_ + output.length()). // calculation of (written_bytes_ + output.length()).
RTC_DCHECK_LT(output.length(), kMaxReasonableFileSize); RTC_DCHECK_LT(output.size(), kMaxReasonableFileSize);
if (max_size_bytes_ == RtcEventLog::kUnlimitedOutput || if (max_size_bytes_ == RtcEventLog::kUnlimitedOutput ||
written_bytes_ + output.length() <= max_size_bytes_) { written_bytes_ + output.size() <= max_size_bytes_) {
if (file_.Write(output.c_str(), output.size())) { if (file_.Write(output.data(), output.size())) {
written_bytes_ += output.size(); written_bytes_ += output.size();
return true; return true;
} else { } else {

View file

@ -38,6 +38,7 @@ class RtcEventLogOutputFile final : public RtcEventLogOutput {
bool IsActive() const override; bool IsActive() const override;
bool Write(const std::string& output) override; bool Write(const std::string& output) override;
bool Write(absl::string_view output) override;
private: private:
RtcEventLogOutputFile(FileWrapper file, size_t max_size_bytes); RtcEventLogOutputFile(FileWrapper file, size_t max_size_bytes);

View file

@ -72,15 +72,15 @@ TEST_F(RtcEventLogOutputFileTest, UnlimitedOutputFile) {
EXPECT_EQ(GetOutputFileContents(), output_str); EXPECT_EQ(GetOutputFileContents(), output_str);
} }
// Do not allow writing more bytes to the file than // Do not allow writing more bytes to the file than max file size.
TEST_F(RtcEventLogOutputFileTest, LimitedOutputFileCappedToCapacity) { TEST_F(RtcEventLogOutputFileTest, LimitedOutputFileCappedToCapacity) {
// Fit two bytes, then the third should be rejected. // Fit two bytes, then the third should be rejected.
auto output_file = auto output_file =
std::make_unique<RtcEventLogOutputFile>(output_file_name_, 2); std::make_unique<RtcEventLogOutputFile>(output_file_name_, 2);
output_file->Write("1"); output_file->Write(absl::string_view("1"));
output_file->Write("2"); output_file->Write(absl::string_view("2"));
output_file->Write("3"); output_file->Write(absl::string_view("3"));
// Unsuccessful writes close the file; no need to delete the output to flush. // Unsuccessful writes close the file; no need to delete the output to flush.
EXPECT_EQ(GetOutputFileContents(), "12"); EXPECT_EQ(GetOutputFileContents(), "12");
@ -108,20 +108,20 @@ TEST_F(RtcEventLogOutputFileTest, DoNotWritePartialLines) {
TEST_F(RtcEventLogOutputFileTest, UnsuccessfulWriteReturnsFalse) { TEST_F(RtcEventLogOutputFileTest, UnsuccessfulWriteReturnsFalse) {
auto output_file = auto output_file =
std::make_unique<RtcEventLogOutputFile>(output_file_name_, 2); std::make_unique<RtcEventLogOutputFile>(output_file_name_, 2);
EXPECT_FALSE(output_file->Write("abc")); EXPECT_FALSE(output_file->Write(absl::string_view("abc")));
} }
TEST_F(RtcEventLogOutputFileTest, SuccessfulWriteReturnsTrue) { TEST_F(RtcEventLogOutputFileTest, SuccessfulWriteReturnsTrue) {
auto output_file = auto output_file =
std::make_unique<RtcEventLogOutputFile>(output_file_name_, 3); std::make_unique<RtcEventLogOutputFile>(output_file_name_, 3);
EXPECT_TRUE(output_file->Write("abc")); EXPECT_TRUE(output_file->Write(absl::string_view("abc")));
} }
// Even if capacity is reached, a successful write leaves the output active. // Even if capacity is reached, a successful write leaves the output active.
TEST_F(RtcEventLogOutputFileTest, FileStillActiveAfterSuccessfulWrite) { TEST_F(RtcEventLogOutputFileTest, FileStillActiveAfterSuccessfulWrite) {
auto output_file = auto output_file =
std::make_unique<RtcEventLogOutputFile>(output_file_name_, 3); std::make_unique<RtcEventLogOutputFile>(output_file_name_, 3);
ASSERT_TRUE(output_file->Write("abc")); ASSERT_TRUE(output_file->Write(absl::string_view("abc")));
EXPECT_TRUE(output_file->IsActive()); EXPECT_TRUE(output_file->IsActive());
} }
@ -130,7 +130,7 @@ TEST_F(RtcEventLogOutputFileTest, FileStillActiveAfterSuccessfulWrite) {
TEST_F(RtcEventLogOutputFileTest, FileInactiveAfterUnsuccessfulWrite) { TEST_F(RtcEventLogOutputFileTest, FileInactiveAfterUnsuccessfulWrite) {
auto output_file = auto output_file =
std::make_unique<RtcEventLogOutputFile>(output_file_name_, 2); std::make_unique<RtcEventLogOutputFile>(output_file_name_, 2);
ASSERT_FALSE(output_file->Write("abc")); ASSERT_FALSE(output_file->Write(absl::string_view("abc")));
EXPECT_FALSE(output_file->IsActive()); EXPECT_FALSE(output_file->IsActive());
} }
@ -145,9 +145,9 @@ class RtcEventLogOutputFileDeathTest : public RtcEventLogOutputFileTest {};
TEST_F(RtcEventLogOutputFileDeathTest, WritingToInactiveFileForbidden) { TEST_F(RtcEventLogOutputFileDeathTest, WritingToInactiveFileForbidden) {
RtcEventLogOutputFile output_file(output_file_name_, 2); RtcEventLogOutputFile output_file(output_file_name_, 2);
ASSERT_FALSE(output_file.Write("abc")); ASSERT_FALSE(output_file.Write(absl::string_view("abc")));
ASSERT_FALSE(output_file.IsActive()); ASSERT_FALSE(output_file.IsActive());
EXPECT_DEATH(output_file.Write("abc"), ""); EXPECT_DEATH(output_file.Write(absl::string_view("abc")), "");
} }
TEST_F(RtcEventLogOutputFileDeathTest, DisallowUnreasonableFileSizeLimits) { TEST_F(RtcEventLogOutputFileDeathTest, DisallowUnreasonableFileSizeLimits) {

View file

@ -136,10 +136,10 @@ void GoogCcStatePrinter::PrintHeaders(RtcEventLogOutput* log) {
int ix = 0; int ix = 0;
for (const auto& logger : loggers_) { for (const auto& logger : loggers_) {
if (ix++) if (ix++)
log->Write(" "); log->Write(absl::string_view(" "));
log->Write(logger->name()); log->Write(logger->name());
} }
log->Write("\n"); log->Write(absl::string_view("\n"));
log->Flush(); log->Flush();
} }
@ -160,11 +160,11 @@ void GoogCcStatePrinter::PrintState(RtcEventLogOutput* log,
int ix = 0; int ix = 0;
for (const auto& logger : loggers_) { for (const auto& logger : loggers_) {
if (ix++) if (ix++)
log->Write(" "); log->Write(absl::string_view(" "));
logger->WriteValue(log); logger->WriteValue(log);
} }
log->Write("\n"); log->Write(absl::string_view("\n"));
log->Flush(); log->Flush();
} }

View file

@ -2810,8 +2810,10 @@ TEST_P(PeerConnectionIntegrationTest, RtcEventLogOutputWriteCalled) {
auto output = std::make_unique<testing::NiceMock<MockRtcEventLogOutput>>(); auto output = std::make_unique<testing::NiceMock<MockRtcEventLogOutput>>();
ON_CALL(*output, IsActive()).WillByDefault(::testing::Return(true)); ON_CALL(*output, IsActive()).WillByDefault(::testing::Return(true));
ON_CALL(*output, Write(::testing::_)).WillByDefault(::testing::Return(true)); ON_CALL(*output, Write(::testing::A<const std::string&>()))
EXPECT_CALL(*output, Write(::testing::_)).Times(::testing::AtLeast(1)); .WillByDefault(::testing::Return(true));
EXPECT_CALL(*output, Write(::testing::A<const std::string&>()))
.Times(::testing::AtLeast(1));
EXPECT_TRUE(caller()->pc()->StartRtcEventLog( EXPECT_TRUE(caller()->pc()->StartRtcEventLog(
std::move(output), webrtc::RtcEventLog::kImmediateOutput)); std::move(output), webrtc::RtcEventLog::kImmediateOutput));

View file

@ -437,7 +437,8 @@ static const char kDtlsSdesFallbackSdp[] =
class RtcEventLogOutputNull final : public RtcEventLogOutput { class RtcEventLogOutputNull final : public RtcEventLogOutput {
public: public:
bool IsActive() const override { return true; } bool IsActive() const override { return true; }
bool Write(const std::string& output) override { return true; } bool Write(const std::string& /*output*/) override { return true; }
bool Write(const absl::string_view /*output*/) override { return true; }
}; };
using ::cricket::StreamParams; using ::cricket::StreamParams;

View file

@ -1251,6 +1251,7 @@ class MockRtcEventLogOutput : public webrtc::RtcEventLogOutput {
virtual ~MockRtcEventLogOutput() = default; virtual ~MockRtcEventLogOutput() = default;
MOCK_METHOD(bool, IsActive, (), (const, override)); MOCK_METHOD(bool, IsActive, (), (const, override));
MOCK_METHOD(bool, Write, (const std::string&), (override)); MOCK_METHOD(bool, Write, (const std::string&), (override));
MOCK_METHOD(bool, Write, (absl::string_view), (override));
}; };
// This helper object is used for both specifying how many audio/video frames // This helper object is used for both specifying how many audio/video frames

View file

@ -33,6 +33,10 @@ bool FileLogWriter::IsActive() const {
} }
bool FileLogWriter::Write(const std::string& value) { bool FileLogWriter::Write(const std::string& value) {
return Write(absl::string_view(value));
}
bool FileLogWriter::Write(absl::string_view value) {
// We don't expect the write to fail. If it does, we don't want to risk // We don't expect the write to fail. If it does, we don't want to risk
// silently ignoring it. // silently ignoring it.
RTC_CHECK_EQ(std::fwrite(value.data(), 1, value.size(), out_), value.size()) RTC_CHECK_EQ(std::fwrite(value.data(), 1, value.size(), out_), value.size())

View file

@ -25,6 +25,7 @@ class FileLogWriter final : public RtcEventLogOutput {
~FileLogWriter() final; ~FileLogWriter() final;
bool IsActive() const override; bool IsActive() const override;
bool Write(const std::string& value) override; bool Write(const std::string& value) override;
bool Write(absl::string_view value) override;
void Flush() override; void Flush() override;
private: private:

View file

@ -27,6 +27,10 @@ class MemoryLogWriter final : public RtcEventLogOutput {
buffer_.append(value); buffer_.append(value);
return true; return true;
} }
bool Write(absl::string_view value) override {
buffer_.append(value.data(), value.size());
return true;
}
void Flush() override {} void Flush() override {}
private: private:

View file

@ -48,12 +48,12 @@ StatesPrinter::~StatesPrinter() = default;
void StatesPrinter::PrintHeaders() { void StatesPrinter::PrintHeaders() {
if (!writer_) if (!writer_)
return; return;
writer_->Write(printers_[0].headers_); writer_->Write(absl::string_view(printers_[0].headers_));
for (size_t i = 1; i < printers_.size(); ++i) { for (size_t i = 1; i < printers_.size(); ++i) {
writer_->Write(" "); writer_->Write(absl::string_view(" "));
writer_->Write(printers_[i].headers_); writer_->Write(absl::string_view(printers_[i].headers_));
} }
writer_->Write("\n"); writer_->Write(absl::string_view("\n"));
} }
void StatesPrinter::PrintRow() { void StatesPrinter::PrintRow() {

View file

@ -29,9 +29,9 @@ VideoQualityAnalyzer::VideoQualityAnalyzer(
VideoQualityAnalyzer::~VideoQualityAnalyzer() = default; VideoQualityAnalyzer::~VideoQualityAnalyzer() = default;
void VideoQualityAnalyzer::PrintHeaders() { void VideoQualityAnalyzer::PrintHeaders() {
writer_->Write( writer_->Write(absl::string_view(
"capture_time render_time capture_width capture_height render_width " "capture_time render_time capture_width capture_height render_width "
"render_height psnr\n"); "render_height psnr\n"));
} }
std::function<void(const VideoFramePair&)> VideoQualityAnalyzer::Handler() { std::function<void(const VideoFramePair&)> VideoQualityAnalyzer::Handler() {