Commit 6b22c715 authored by Tommi's avatar Tommi Committed by Commit Bot

Migrate WebRtcRtpDumpHandler and WebRtcRtpDumpWriter over to sequenced task runner.

BUG=689520

Change-Id: Ib34ee776f40c25b4d4306d54a7bd50756107e13c
Reviewed-on: https://chromium-review.googlesource.com/570998
Commit-Queue: Tommi <tommi@chromium.org>
Reviewed-by: default avatarMax Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487098}
parent b7b6562e
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/task_scheduler/post_task.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/media/webrtc/webrtc_rtp_dump_writer.h" #include "chrome/browser/media/webrtc/webrtc_rtp_dump_writer.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
...@@ -52,11 +53,10 @@ WebRtcRtpDumpHandler::WebRtcRtpDumpHandler(const base::FilePath& dump_dir) ...@@ -52,11 +53,10 @@ WebRtcRtpDumpHandler::WebRtcRtpDumpHandler(const base::FilePath& dump_dir)
incoming_state_(STATE_NONE), incoming_state_(STATE_NONE),
outgoing_state_(STATE_NONE), outgoing_state_(STATE_NONE),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
} }
WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() { WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
// Reset dump writer first to stop writing. // Reset dump writer first to stop writing.
if (dump_writer_) { if (dump_writer_) {
...@@ -65,15 +65,15 @@ WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() { ...@@ -65,15 +65,15 @@ WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() {
} }
if (incoming_state_ != STATE_NONE && !incoming_dump_path_.empty()) { if (incoming_state_ != STATE_NONE && !incoming_dump_path_.empty()) {
BrowserThread::PostTask( base::PostTaskWithTraits(
BrowserThread::FILE, FROM_HERE, FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), base::BindOnce(base::IgnoreResult(&base::DeleteFile),
incoming_dump_path_, false)); incoming_dump_path_, false));
} }
if (outgoing_state_ != STATE_NONE && !outgoing_dump_path_.empty()) { if (outgoing_state_ != STATE_NONE && !outgoing_dump_path_.empty()) {
BrowserThread::PostTask( base::PostTaskWithTraits(
BrowserThread::FILE, FROM_HERE, FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), base::BindOnce(base::IgnoreResult(&base::DeleteFile),
outgoing_dump_path_, false)); outgoing_dump_path_, false));
} }
...@@ -81,7 +81,7 @@ WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() { ...@@ -81,7 +81,7 @@ WebRtcRtpDumpHandler::~WebRtcRtpDumpHandler() {
bool WebRtcRtpDumpHandler::StartDump(RtpDumpType type, bool WebRtcRtpDumpHandler::StartDump(RtpDumpType type,
std::string* error_message) { std::string* error_message) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
if (!dump_writer_ && g_ongoing_rtp_dumps >= kMaxOngoingRtpDumpsAllowed) { if (!dump_writer_ && g_ongoing_rtp_dumps >= kMaxOngoingRtpDumpsAllowed) {
*error_message = "Max RTP dump limit reached."; *error_message = "Max RTP dump limit reached.";
...@@ -139,7 +139,7 @@ bool WebRtcRtpDumpHandler::StartDump(RtpDumpType type, ...@@ -139,7 +139,7 @@ bool WebRtcRtpDumpHandler::StartDump(RtpDumpType type,
void WebRtcRtpDumpHandler::StopDump(RtpDumpType type, void WebRtcRtpDumpHandler::StopDump(RtpDumpType type,
const GenericDoneCallback& callback) { const GenericDoneCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
// Returns an error if any type of dump specified by the caller cannot be // Returns an error if any type of dump specified by the caller cannot be
// stopped. // stopped.
...@@ -177,7 +177,7 @@ void WebRtcRtpDumpHandler::StopDump(RtpDumpType type, ...@@ -177,7 +177,7 @@ void WebRtcRtpDumpHandler::StopDump(RtpDumpType type,
} }
bool WebRtcRtpDumpHandler::ReadyToRelease() const { bool WebRtcRtpDumpHandler::ReadyToRelease() const {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
return incoming_state_ != STATE_STARTED && return incoming_state_ != STATE_STARTED &&
incoming_state_ != STATE_STOPPING && incoming_state_ != STATE_STOPPING &&
...@@ -185,7 +185,7 @@ bool WebRtcRtpDumpHandler::ReadyToRelease() const { ...@@ -185,7 +185,7 @@ bool WebRtcRtpDumpHandler::ReadyToRelease() const {
} }
WebRtcRtpDumpHandler::ReleasedDumps WebRtcRtpDumpHandler::ReleaseDumps() { WebRtcRtpDumpHandler::ReleasedDumps WebRtcRtpDumpHandler::ReleaseDumps() {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
DCHECK(ReadyToRelease()); DCHECK(ReadyToRelease());
base::FilePath incoming_dump, outgoing_dump; base::FilePath incoming_dump, outgoing_dump;
...@@ -210,7 +210,7 @@ void WebRtcRtpDumpHandler::OnRtpPacket(const uint8_t* packet_header, ...@@ -210,7 +210,7 @@ void WebRtcRtpDumpHandler::OnRtpPacket(const uint8_t* packet_header,
size_t header_length, size_t header_length,
size_t packet_length, size_t packet_length,
bool incoming) { bool incoming) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
if ((incoming && incoming_state_ != STATE_STARTED) || if ((incoming && incoming_state_ != STATE_STARTED) ||
(!incoming && outgoing_state_ != STATE_STARTED)) { (!incoming && outgoing_state_ != STATE_STARTED)) {
...@@ -222,7 +222,7 @@ void WebRtcRtpDumpHandler::OnRtpPacket(const uint8_t* packet_header, ...@@ -222,7 +222,7 @@ void WebRtcRtpDumpHandler::OnRtpPacket(const uint8_t* packet_header,
} }
void WebRtcRtpDumpHandler::StopOngoingDumps(const base::Closure& callback) { void WebRtcRtpDumpHandler::StopOngoingDumps(const base::Closure& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
DCHECK(!callback.is_null()); DCHECK(!callback.is_null());
// No ongoing dumps, return directly. // No ongoing dumps, return directly.
...@@ -232,11 +232,11 @@ void WebRtcRtpDumpHandler::StopOngoingDumps(const base::Closure& callback) { ...@@ -232,11 +232,11 @@ void WebRtcRtpDumpHandler::StopOngoingDumps(const base::Closure& callback) {
return; return;
} }
// If the FILE thread is working on stopping the dumps, wait for the FILE // If the background task runner is working on stopping the dumps, wait for it
// thread to return and check the states again. // to complete and then check the states again.
if (incoming_state_ == STATE_STOPPING || outgoing_state_ == STATE_STOPPING) { if (incoming_state_ == STATE_STOPPING || outgoing_state_ == STATE_STOPPING) {
BrowserThread::PostTaskAndReply( dump_writer_->background_task_runner()->PostTaskAndReply(
BrowserThread::FILE, FROM_HERE, base::BindOnce(&base::DoNothing), FROM_HERE, base::BindOnce(&base::DoNothing),
base::BindOnce(&WebRtcRtpDumpHandler::StopOngoingDumps, base::BindOnce(&WebRtcRtpDumpHandler::StopOngoingDumps,
weak_ptr_factory_.GetWeakPtr(), callback)); weak_ptr_factory_.GetWeakPtr(), callback));
return; return;
...@@ -266,7 +266,7 @@ void WebRtcRtpDumpHandler::StopOngoingDumps(const base::Closure& callback) { ...@@ -266,7 +266,7 @@ void WebRtcRtpDumpHandler::StopOngoingDumps(const base::Closure& callback) {
void WebRtcRtpDumpHandler::SetDumpWriterForTesting( void WebRtcRtpDumpHandler::SetDumpWriterForTesting(
std::unique_ptr<WebRtcRtpDumpWriter> writer) { std::unique_ptr<WebRtcRtpDumpWriter> writer) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
dump_writer_ = std::move(writer); dump_writer_ = std::move(writer);
++g_ongoing_rtp_dumps; ++g_ongoing_rtp_dumps;
...@@ -276,7 +276,7 @@ void WebRtcRtpDumpHandler::SetDumpWriterForTesting( ...@@ -276,7 +276,7 @@ void WebRtcRtpDumpHandler::SetDumpWriterForTesting(
} }
void WebRtcRtpDumpHandler::OnMaxDumpSizeReached() { void WebRtcRtpDumpHandler::OnMaxDumpSizeReached() {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
RtpDumpType type = RtpDumpType type =
(incoming_state_ == STATE_STARTED) (incoming_state_ == STATE_STARTED)
...@@ -290,15 +290,15 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback, ...@@ -290,15 +290,15 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback,
RtpDumpType ended_type, RtpDumpType ended_type,
bool incoming_success, bool incoming_success,
bool outgoing_success) { bool outgoing_success) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_);
if (DumpTypeContainsIncoming(ended_type)) { if (DumpTypeContainsIncoming(ended_type)) {
DCHECK_EQ(STATE_STOPPING, incoming_state_); DCHECK_EQ(STATE_STOPPING, incoming_state_);
incoming_state_ = STATE_STOPPED; incoming_state_ = STATE_STOPPED;
if (!incoming_success) { if (!incoming_success) {
BrowserThread::PostTask( base::PostTaskWithTraits(
BrowserThread::FILE, FROM_HERE, FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), base::BindOnce(base::IgnoreResult(&base::DeleteFile),
incoming_dump_path_, false)); incoming_dump_path_, false));
...@@ -313,8 +313,8 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback, ...@@ -313,8 +313,8 @@ void WebRtcRtpDumpHandler::OnDumpEnded(const base::Closure& callback,
outgoing_state_ = STATE_STOPPED; outgoing_state_ = STATE_STOPPED;
if (!outgoing_success) { if (!outgoing_success) {
BrowserThread::PostTask( base::PostTaskWithTraits(
BrowserThread::FILE, FROM_HERE, FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
base::BindOnce(base::IgnoreResult(&base::DeleteFile), base::BindOnce(base::IgnoreResult(&base::DeleteFile),
outgoing_dump_path_, false)); outgoing_dump_path_, false));
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/sequence_checker.h"
#include "chrome/browser/media/webrtc/rtp_dump_type.h" #include "chrome/browser/media/webrtc/rtp_dump_type.h"
class WebRtcRtpDumpWriter; class WebRtcRtpDumpWriter;
...@@ -114,6 +115,8 @@ class WebRtcRtpDumpHandler { ...@@ -114,6 +115,8 @@ class WebRtcRtpDumpHandler {
bool incoming_succeeded, bool incoming_succeeded,
bool outgoing_succeeded); bool outgoing_succeeded);
SEQUENCE_CHECKER(main_sequence_);
// The absolute path to the directory containing the incoming/outgoing dumps. // The absolute path to the directory containing the incoming/outgoing dumps.
const base::FilePath dump_dir_; const base::FilePath dump_dir_;
......
...@@ -16,7 +16,9 @@ ...@@ -16,7 +16,9 @@
#include "base/location.h" #include "base/location.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/sequenced_task_runner.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/task_scheduler/task_scheduler.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/media/webrtc/webrtc_rtp_dump_writer.h" #include "chrome/browser/media/webrtc/webrtc_rtp_dump_writer.h"
#include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_browser_thread_bundle.h"
...@@ -99,6 +101,11 @@ class WebRtcRtpDumpHandlerTest : public testing::Test { ...@@ -99,6 +101,11 @@ class WebRtcRtpDumpHandlerTest : public testing::Test {
EXPECT_GT(base::WriteFile(*outgoing_dump, dummy, arraysize(dummy)), 0); EXPECT_GT(base::WriteFile(*outgoing_dump, dummy, arraysize(dummy)), 0);
} }
void FlushTaskRunners() {
base::TaskScheduler::GetInstance()->FlushForTesting();
base::RunLoop().RunUntilIdle();
}
MOCK_METHOD2(OnStopDumpFinished, MOCK_METHOD2(OnStopDumpFinished,
void(bool success, const std::string& error)); void(bool success, const std::string& error));
...@@ -283,9 +290,10 @@ TEST_F(WebRtcRtpDumpHandlerTest, DumpsCleanedUpIfNotReleased) { ...@@ -283,9 +290,10 @@ TEST_F(WebRtcRtpDumpHandlerTest, DumpsCleanedUpIfNotReleased) {
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
FlushTaskRunners();
handler_.reset(); handler_.reset();
base::RunLoop().RunUntilIdle(); FlushTaskRunners();
EXPECT_FALSE(base::PathExists(incoming_dump)); EXPECT_FALSE(base::PathExists(incoming_dump));
EXPECT_FALSE(base::PathExists(outgoing_dump)); EXPECT_FALSE(base::PathExists(outgoing_dump));
...@@ -309,6 +317,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, DumpDeletedIfEndDumpFailed) { ...@@ -309,6 +317,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, DumpDeletedIfEndDumpFailed) {
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
FlushTaskRunners();
EXPECT_FALSE(base::PathExists(incoming_dump)); EXPECT_FALSE(base::PathExists(incoming_dump));
EXPECT_TRUE(base::PathExists(outgoing_dump)); EXPECT_TRUE(base::PathExists(outgoing_dump));
...@@ -317,6 +326,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, DumpDeletedIfEndDumpFailed) { ...@@ -317,6 +326,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, DumpDeletedIfEndDumpFailed) {
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
FlushTaskRunners();
EXPECT_FALSE(base::PathExists(outgoing_dump)); EXPECT_FALSE(base::PathExists(outgoing_dump));
} }
...@@ -331,12 +341,13 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhileStoppingDumps) { ...@@ -331,12 +341,13 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhileStoppingDumps) {
handler_->StopDump(RTP_DUMP_BOTH, handler_->StopDump(RTP_DUMP_BOTH,
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle();
handler_->StopOngoingDumps( handler_->StopOngoingDumps(
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopOngoingDumpsFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopOngoingDumpsFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle(); FlushTaskRunners();
WebRtcRtpDumpHandler::ReleasedDumps dumps(handler_->ReleaseDumps()); WebRtcRtpDumpHandler::ReleasedDumps dumps(handler_->ReleaseDumps());
EXPECT_FALSE(dumps.incoming_dump_path.empty()); EXPECT_FALSE(dumps.incoming_dump_path.empty());
...@@ -353,7 +364,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhileDumping) { ...@@ -353,7 +364,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhileDumping) {
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopOngoingDumpsFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopOngoingDumpsFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle(); FlushTaskRunners();
WebRtcRtpDumpHandler::ReleasedDumps dumps(handler_->ReleaseDumps()); WebRtcRtpDumpHandler::ReleasedDumps dumps(handler_->ReleaseDumps());
EXPECT_FALSE(dumps.incoming_dump_path.empty()); EXPECT_FALSE(dumps.incoming_dump_path.empty());
...@@ -371,6 +382,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhenAlreadyStopped) { ...@@ -371,6 +382,7 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhenAlreadyStopped) {
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
FlushTaskRunners();
} }
EXPECT_CALL(*this, OnStopOngoingDumpsFinished()); EXPECT_CALL(*this, OnStopOngoingDumpsFinished());
...@@ -390,12 +402,13 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhileStoppingOneDump) { ...@@ -390,12 +402,13 @@ TEST_F(WebRtcRtpDumpHandlerTest, StopOngoingDumpsWhileStoppingOneDump) {
handler_->StopDump(RTP_DUMP_INCOMING, handler_->StopDump(RTP_DUMP_INCOMING,
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopDumpFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle();
handler_->StopOngoingDumps( handler_->StopOngoingDumps(
base::Bind(&WebRtcRtpDumpHandlerTest::OnStopOngoingDumpsFinished, base::Bind(&WebRtcRtpDumpHandlerTest::OnStopOngoingDumpsFinished,
base::Unretained(this))); base::Unretained(this)));
base::RunLoop().RunUntilIdle(); FlushTaskRunners();
WebRtcRtpDumpHandler::ReleasedDumps dumps(handler_->ReleaseDumps()); WebRtcRtpDumpHandler::ReleasedDumps dumps(handler_->ReleaseDumps());
EXPECT_FALSE(dumps.incoming_dump_path.empty()); EXPECT_FALSE(dumps.incoming_dump_path.empty());
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/task_scheduler/post_task.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "third_party/zlib/zlib.h" #include "third_party/zlib/zlib.h"
...@@ -90,13 +91,12 @@ void AppendToBuffer(const uint8_t* src, ...@@ -90,13 +91,12 @@ void AppendToBuffer(const uint8_t* src,
} // namespace } // namespace
// This class is running on the FILE thread for compressing and writing the // This class runs on the backround task runner, compresses and writes the
// dump buffer to disk. // dump buffer to disk.
class WebRtcRtpDumpWriter::FileThreadWorker { class WebRtcRtpDumpWriter::FileWorker {
public: public:
explicit FileThreadWorker(const base::FilePath& dump_path) explicit FileWorker(const base::FilePath& dump_path) : dump_path_(dump_path) {
: dump_path_(dump_path) { DETACH_FROM_SEQUENCE(sequence_checker_);
thread_checker_.DetachFromThread();
memset(&stream_, 0, sizeof(stream_)); memset(&stream_, 0, sizeof(stream_));
int result = deflateInit2(&stream_, int result = deflateInit2(&stream_,
...@@ -110,8 +110,8 @@ class WebRtcRtpDumpWriter::FileThreadWorker { ...@@ -110,8 +110,8 @@ class WebRtcRtpDumpWriter::FileThreadWorker {
DCHECK_EQ(Z_OK, result); DCHECK_EQ(Z_OK, result);
} }
~FileThreadWorker() { ~FileWorker() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Makes sure all allocations are freed. // Makes sure all allocations are freed.
deflateEnd(&stream_); deflateEnd(&stream_);
...@@ -125,7 +125,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker { ...@@ -125,7 +125,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker {
bool end_stream, bool end_stream,
FlushResult* result, FlushResult* result,
size_t* bytes_written) { size_t* bytes_written) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// This is called either when the in-memory buffer is full or the dump // This is called either when the in-memory buffer is full or the dump
// should be ended. // should be ended.
...@@ -153,7 +153,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker { ...@@ -153,7 +153,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker {
// dump. // dump.
size_t CompressAndWriteBufferToFile(std::vector<uint8_t>* buffer, size_t CompressAndWriteBufferToFile(std::vector<uint8_t>* buffer,
FlushResult* result) { FlushResult* result) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(buffer->size()); DCHECK(buffer->size());
*result = FLUSH_RESULT_SUCCESS; *result = FLUSH_RESULT_SUCCESS;
...@@ -193,7 +193,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker { ...@@ -193,7 +193,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker {
// Compresses |input| into |output|. // Compresses |input| into |output|.
bool Compress(std::vector<uint8_t>* input, std::vector<uint8_t>* output) { bool Compress(std::vector<uint8_t>* input, std::vector<uint8_t>* output) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
int result = Z_OK; int result = Z_OK;
output->resize(std::max(kMinimumGzipOutputBufferSize, input->size())); output->resize(std::max(kMinimumGzipOutputBufferSize, input->size()));
...@@ -217,7 +217,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker { ...@@ -217,7 +217,7 @@ class WebRtcRtpDumpWriter::FileThreadWorker {
// Ends the compression stream and completes the dump file. // Ends the compression stream and completes the dump file.
bool EndDumpFile() { bool EndDumpFile() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::vector<uint8_t> output_buffer; std::vector<uint8_t> output_buffer;
output_buffer.resize(kMinimumGzipOutputBufferSize); output_buffer.resize(kMinimumGzipOutputBufferSize);
...@@ -247,9 +247,9 @@ class WebRtcRtpDumpWriter::FileThreadWorker { ...@@ -247,9 +247,9 @@ class WebRtcRtpDumpWriter::FileThreadWorker {
z_stream stream_; z_stream stream_;
base::ThreadChecker thread_checker_; SEQUENCE_CHECKER(sequence_checker_);
DISALLOW_COPY_AND_ASSIGN(FileThreadWorker); DISALLOW_COPY_AND_ASSIGN(FileWorker);
}; };
WebRtcRtpDumpWriter::WebRtcRtpDumpWriter( WebRtcRtpDumpWriter::WebRtcRtpDumpWriter(
...@@ -260,20 +260,21 @@ WebRtcRtpDumpWriter::WebRtcRtpDumpWriter( ...@@ -260,20 +260,21 @@ WebRtcRtpDumpWriter::WebRtcRtpDumpWriter(
: max_dump_size_(max_dump_size), : max_dump_size_(max_dump_size),
max_dump_size_reached_callback_(max_dump_size_reached_callback), max_dump_size_reached_callback_(max_dump_size_reached_callback),
total_dump_size_on_disk_(0), total_dump_size_on_disk_(0),
incoming_file_thread_worker_(new FileThreadWorker(incoming_dump_path)), background_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
outgoing_file_thread_worker_(new FileThreadWorker(outgoing_dump_path)), {base::MayBlock(), base::TaskPriority::BACKGROUND})),
weak_ptr_factory_(this) { incoming_file_thread_worker_(new FileWorker(incoming_dump_path)),
} outgoing_file_thread_worker_(new FileWorker(outgoing_dump_path)),
weak_ptr_factory_(this) {}
WebRtcRtpDumpWriter::~WebRtcRtpDumpWriter() { WebRtcRtpDumpWriter::~WebRtcRtpDumpWriter() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
bool success = BrowserThread::DeleteSoon( bool success = background_task_runner_->DeleteSoon(
BrowserThread::FILE, FROM_HERE, incoming_file_thread_worker_.release()); FROM_HERE, incoming_file_thread_worker_.release());
DCHECK(success); DCHECK(success);
success = BrowserThread::DeleteSoon( success = background_task_runner_->DeleteSoon(
BrowserThread::FILE, FROM_HERE, outgoing_file_thread_worker_.release()); FROM_HERE, outgoing_file_thread_worker_.release());
DCHECK(success); DCHECK(success);
} }
...@@ -281,7 +282,7 @@ void WebRtcRtpDumpWriter::WriteRtpPacket(const uint8_t* packet_header, ...@@ -281,7 +282,7 @@ void WebRtcRtpDumpWriter::WriteRtpPacket(const uint8_t* packet_header,
size_t header_length, size_t header_length,
size_t packet_length, size_t packet_length,
bool incoming) { bool incoming) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
static const size_t kMaxInMemoryBufferSize = 65536; static const size_t kMaxInMemoryBufferSize = 65536;
...@@ -317,7 +318,7 @@ void WebRtcRtpDumpWriter::WriteRtpPacket(const uint8_t* packet_header, ...@@ -317,7 +318,7 @@ void WebRtcRtpDumpWriter::WriteRtpPacket(const uint8_t* packet_header,
void WebRtcRtpDumpWriter::EndDump(RtpDumpType type, void WebRtcRtpDumpWriter::EndDump(RtpDumpType type,
const EndDumpCallback& finished_callback) { const EndDumpCallback& finished_callback) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(type == RTP_DUMP_OUTGOING || incoming_file_thread_worker_ != NULL); DCHECK(type == RTP_DUMP_OUTGOING || incoming_file_thread_worker_ != NULL);
DCHECK(type == RTP_DUMP_INCOMING || outgoing_file_thread_worker_ != NULL); DCHECK(type == RTP_DUMP_INCOMING || outgoing_file_thread_worker_ != NULL);
...@@ -335,7 +336,7 @@ void WebRtcRtpDumpWriter::EndDump(RtpDumpType type, ...@@ -335,7 +336,7 @@ void WebRtcRtpDumpWriter::EndDump(RtpDumpType type,
} }
size_t WebRtcRtpDumpWriter::max_dump_size() const { size_t WebRtcRtpDumpWriter::max_dump_size() const {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return max_dump_size_; return max_dump_size_;
} }
...@@ -357,7 +358,7 @@ WebRtcRtpDumpWriter::EndDumpContext::~EndDumpContext() { ...@@ -357,7 +358,7 @@ WebRtcRtpDumpWriter::EndDumpContext::~EndDumpContext() {
void WebRtcRtpDumpWriter::FlushBuffer(bool incoming, void WebRtcRtpDumpWriter::FlushBuffer(bool incoming,
bool end_stream, bool end_stream,
const FlushDoneCallback& callback) { const FlushDoneCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::unique_ptr<std::vector<uint8_t>> new_buffer(new std::vector<uint8_t>()); std::unique_ptr<std::vector<uint8_t>> new_buffer(new std::vector<uint8_t>());
...@@ -373,32 +374,31 @@ void WebRtcRtpDumpWriter::FlushBuffer(bool incoming, ...@@ -373,32 +374,31 @@ void WebRtcRtpDumpWriter::FlushBuffer(bool incoming,
std::unique_ptr<size_t> bytes_written(new size_t(0)); std::unique_ptr<size_t> bytes_written(new size_t(0));
FileThreadWorker* worker = incoming ? incoming_file_thread_worker_.get() FileWorker* worker = incoming ? incoming_file_thread_worker_.get()
: outgoing_file_thread_worker_.get(); : outgoing_file_thread_worker_.get();
// Using "Unretained(worker)" because |worker| is owner by this object and it // Using "Unretained(worker)" because |worker| is owner by this object and it
// guaranteed to be deleted on the FILE thread before this object goes away. // guaranteed to be deleted on the backround task runner before this object
base::Closure task = // goes away.
base::Bind(&FileThreadWorker::CompressAndWriteToFileOnFileThread, base::OnceClosure task = base::BindOnce(
base::Unretained(worker), base::Passed(&new_buffer), &FileWorker::CompressAndWriteToFileOnFileThread, base::Unretained(worker),
end_stream, result.get(), bytes_written.get()); std::move(new_buffer), end_stream, result.get(), bytes_written.get());
// OnFlushDone is necessary to avoid running the callback after this // OnFlushDone is necessary to avoid running the callback after this
// object is gone. // object is gone.
base::Closure reply = base::Bind( base::OnceClosure reply = base::BindOnce(
&WebRtcRtpDumpWriter::OnFlushDone, weak_ptr_factory_.GetWeakPtr(), &WebRtcRtpDumpWriter::OnFlushDone, weak_ptr_factory_.GetWeakPtr(),
callback, base::Passed(&result), base::Passed(&bytes_written)); callback, std::move(result), std::move(bytes_written));
// Define the task and reply outside the method call so that getting and // Define the task and reply outside the method call so that getting and
// passing the scoped_ptr does not depend on the argument evaluation order. // passing the scoped_ptr does not depend on the argument evaluation order.
BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE, task, reply); background_task_runner_->PostTaskAndReply(FROM_HERE, std::move(task),
std::move(reply));
if (end_stream) { if (end_stream) {
bool success = BrowserThread::DeleteSoon( bool success = background_task_runner_->DeleteSoon(
BrowserThread::FILE, FROM_HERE, incoming ? incoming_file_thread_worker_.release()
FROM_HERE, : outgoing_file_thread_worker_.release());
incoming ? incoming_file_thread_worker_.release()
: outgoing_file_thread_worker_.release());
DCHECK(success); DCHECK(success);
} }
} }
...@@ -407,7 +407,7 @@ void WebRtcRtpDumpWriter::OnFlushDone( ...@@ -407,7 +407,7 @@ void WebRtcRtpDumpWriter::OnFlushDone(
const FlushDoneCallback& callback, const FlushDoneCallback& callback,
const std::unique_ptr<FlushResult>& result, const std::unique_ptr<FlushResult>& result,
const std::unique_ptr<size_t>& bytes_written) { const std::unique_ptr<size_t>& bytes_written) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
total_dump_size_on_disk_ += *bytes_written; total_dump_size_on_disk_ += *bytes_written;
...@@ -427,7 +427,7 @@ void WebRtcRtpDumpWriter::OnFlushDone( ...@@ -427,7 +427,7 @@ void WebRtcRtpDumpWriter::OnFlushDone(
void WebRtcRtpDumpWriter::OnDumpEnded(EndDumpContext context, void WebRtcRtpDumpWriter::OnDumpEnded(EndDumpContext context,
bool incoming, bool incoming,
bool success) { bool success) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DVLOG(2) << "Dump ended, incoming = " << incoming DVLOG(2) << "Dump ended, incoming = " << incoming
<< ", succeeded = " << success; << ", succeeded = " << success;
......
...@@ -14,7 +14,8 @@ ...@@ -14,7 +14,8 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/threading/thread_checker.h" #include "base/sequence_checker.h"
#include "base/sequenced_task_runner.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "chrome/browser/media/webrtc/rtp_dump_type.h" #include "chrome/browser/media/webrtc/rtp_dump_type.h"
...@@ -65,6 +66,11 @@ class WebRtcRtpDumpWriter { ...@@ -65,6 +66,11 @@ class WebRtcRtpDumpWriter {
size_t max_dump_size() const; size_t max_dump_size() const;
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner()
const {
return background_task_runner_;
}
private: private:
enum FlushResult { enum FlushResult {
// Flushing has succeeded and the dump size is under the max limit. // Flushing has succeeded and the dump size is under the max limit.
...@@ -75,7 +81,7 @@ class WebRtcRtpDumpWriter { ...@@ -75,7 +81,7 @@ class WebRtcRtpDumpWriter {
FLUSH_RESULT_FAILURE FLUSH_RESULT_FAILURE
}; };
class FileThreadWorker; class FileWorker;
typedef base::Callback<void(bool)> FlushDoneCallback; typedef base::Callback<void(bool)> FlushDoneCallback;
...@@ -127,11 +133,12 @@ class WebRtcRtpDumpWriter { ...@@ -127,11 +133,12 @@ class WebRtcRtpDumpWriter {
// The total on-disk size of the compressed incoming and outgoing dumps. // The total on-disk size of the compressed incoming and outgoing dumps.
size_t total_dump_size_on_disk_; size_t total_dump_size_on_disk_;
// File thread workers must be called and deleted on the FILE thread. // File workers must be called and deleted on the backround task runner.
std::unique_ptr<FileThreadWorker> incoming_file_thread_worker_; scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
std::unique_ptr<FileThreadWorker> outgoing_file_thread_worker_; std::unique_ptr<FileWorker> incoming_file_thread_worker_;
std::unique_ptr<FileWorker> outgoing_file_thread_worker_;
base::ThreadChecker thread_checker_; SEQUENCE_CHECKER(sequence_checker_);
base::WeakPtrFactory<WebRtcRtpDumpWriter> weak_ptr_factory_; base::WeakPtrFactory<WebRtcRtpDumpWriter> weak_ptr_factory_;
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/sequenced_task_runner.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_browser_thread_bundle.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
...@@ -49,6 +50,12 @@ static void CreateFakeRtpPacketHeader(size_t csrc_count, ...@@ -49,6 +50,12 @@ static void CreateFakeRtpPacketHeader(size_t csrc_count,
static_cast<uint16_t>(extension_header_count)); static_cast<uint16_t>(extension_header_count));
} }
static void FlushTaskRunner(base::SequencedTaskRunner* task_runner) {
base::RunLoop run_loop;
task_runner->PostTask(FROM_HERE, run_loop.QuitClosure());
run_loop.Run();
}
class WebRtcRtpDumpWriterTest : public testing::Test { class WebRtcRtpDumpWriterTest : public testing::Test {
public: public:
WebRtcRtpDumpWriterTest() WebRtcRtpDumpWriterTest()
...@@ -242,9 +249,9 @@ TEST_F(WebRtcRtpDumpWriterTest, NoDumpFileIfNoPacketDumped) { ...@@ -242,9 +249,9 @@ TEST_F(WebRtcRtpDumpWriterTest, NoDumpFileIfNoPacketDumped) {
base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone, base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone,
base::Unretained(this))); base::Unretained(this)));
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
EXPECT_FALSE(base::PathExists(incoming_dump_path_)); EXPECT_FALSE(base::PathExists(incoming_dump_path_));
...@@ -269,9 +276,9 @@ TEST_F(WebRtcRtpDumpWriterTest, WriteAndFlushSmallSizeDump) { ...@@ -269,9 +276,9 @@ TEST_F(WebRtcRtpDumpWriterTest, WriteAndFlushSmallSizeDump) {
base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone, base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone,
base::Unretained(this))); base::Unretained(this)));
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
...@@ -311,9 +318,9 @@ TEST_F(WebRtcRtpDumpWriterTest, WriteOverMaxLimit) { ...@@ -311,9 +318,9 @@ TEST_F(WebRtcRtpDumpWriterTest, WriteOverMaxLimit) {
base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone, base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone,
base::Unretained(this))); base::Unretained(this)));
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
VerifyDumps(kPacketCount, kPacketCount); VerifyDumps(kPacketCount, kPacketCount);
...@@ -328,9 +335,9 @@ TEST_F(WebRtcRtpDumpWriterTest, DestroyWriterBeforeEndDumpCallback) { ...@@ -328,9 +335,9 @@ TEST_F(WebRtcRtpDumpWriterTest, DestroyWriterBeforeEndDumpCallback) {
writer_.reset(); writer_.reset();
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); // Two |RunUntilIdle()| calls are needed as the first run posts a task that
// we need to give a chance to run with the second call.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
...@@ -359,9 +366,9 @@ TEST_F(WebRtcRtpDumpWriterTest, EndDumpsSeparately) { ...@@ -359,9 +366,9 @@ TEST_F(WebRtcRtpDumpWriterTest, EndDumpsSeparately) {
base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone, base::Bind(&WebRtcRtpDumpWriterTest::OnEndDumpDone,
base::Unretained(this))); base::Unretained(this)));
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
content::RunAllPendingInMessageLoop(content::BrowserThread::FILE); FlushTaskRunner(writer_->background_task_runner().get());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment