From 1e6965a8572e338b3f2229ce7c957676e2b8508b Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 5 Sep 2022 11:27:57 +0200 Subject: [PATCH] Remove usage of MessageHandlerAutoCleanup in rtc_base unittests Bug: webrtc:11988 Change-Id: I95017df0cd188897a0507bb07e7e571343f80262 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274161 Reviewed-by: Mirko Bonadei Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#38008} --- .../recursive_critical_section_unittest.cc | 13 ++--- rtc_base/null_socket_server_unittest.cc | 24 +++----- rtc_base/socket_unittest.cc | 10 +--- rtc_base/synchronization/mutex_unittest.cc | 13 ++--- rtc_base/thread_unittest.cc | 58 +++++++++---------- 5 files changed, 46 insertions(+), 72 deletions(-) diff --git a/rtc_base/deprecated/recursive_critical_section_unittest.cc b/rtc_base/deprecated/recursive_critical_section_unittest.cc index 4e1c18d8bf..38f003d555 100644 --- a/rtc_base/deprecated/recursive_critical_section_unittest.cc +++ b/rtc_base/deprecated/recursive_critical_section_unittest.cc @@ -23,8 +23,6 @@ #include "rtc_base/arraysize.h" #include "rtc_base/checks.h" #include "rtc_base/event.h" -#include "rtc_base/location.h" -#include "rtc_base/message_handler.h" #include "rtc_base/platform_thread.h" #include "rtc_base/thread.h" #include "test/gtest.h" @@ -77,7 +75,7 @@ class CompareAndSwapVerifier { int zero_count_; }; -class RunnerBase : public MessageHandlerAutoCleanup { +class RunnerBase { public: explicit RunnerBase(int value) : threads_active_(0), @@ -98,8 +96,6 @@ class RunnerBase : public MessageHandlerAutoCleanup { int shared_value() const { return shared_value_; } protected: - // Derived classes must override OnMessage, and call BeforeStart and AfterEnd - // at the beginning and the end of OnMessage respectively. void BeforeStart() { ASSERT_TRUE(start_event_.Wait(kLongTime)); } // Returns true if all threads have finished. @@ -131,7 +127,7 @@ class LockRunner : public RunnerBase { public: LockRunner() : RunnerBase(0) {} - void OnMessage(Message* msg) override { + void Loop() { BeforeStart(); lock_.Lock(); @@ -155,12 +151,13 @@ class LockRunner : public RunnerBase { Lock lock_; }; +template void StartThreads(std::vector>* threads, - MessageHandler* handler) { + Runner* handler) { for (int i = 0; i < kNumThreads; ++i) { std::unique_ptr thread(Thread::Create()); thread->Start(); - thread->Post(RTC_FROM_HERE, handler); + thread->PostTask([handler] { handler->Loop(); }); threads->push_back(std::move(thread)); } } diff --git a/rtc_base/null_socket_server_unittest.cc b/rtc_base/null_socket_server_unittest.cc index 70f7cf8ca3..58a6211aba 100644 --- a/rtc_base/null_socket_server_unittest.cc +++ b/rtc_base/null_socket_server_unittest.cc @@ -16,36 +16,26 @@ #include "api/units/time_delta.h" #include "rtc_base/gunit.h" -#include "rtc_base/location.h" -#include "rtc_base/message_handler.h" #include "rtc_base/thread.h" #include "rtc_base/time_utils.h" #include "test/gtest.h" namespace rtc { -static const uint32_t kTimeout = 5000U; - -class NullSocketServerTest : public ::testing::Test, - public MessageHandlerAutoCleanup { - protected: - void OnMessage(Message* message) override { ss_.WakeUp(); } - - NullSocketServer ss_; -}; - -TEST_F(NullSocketServerTest, WaitAndSet) { +TEST(NullSocketServerTest, WaitAndSet) { + NullSocketServer ss; auto thread = Thread::Create(); EXPECT_TRUE(thread->Start()); - thread->Post(RTC_FROM_HERE, this, 0); + thread->PostTask([&ss] { ss.WakeUp(); }); // The process_io will be ignored. const bool process_io = true; - EXPECT_TRUE_WAIT(ss_.Wait(SocketServer::kForever, process_io), kTimeout); + EXPECT_TRUE_WAIT(ss.Wait(SocketServer::kForever, process_io), 5'000); } -TEST_F(NullSocketServerTest, TestWait) { +TEST(NullSocketServerTest, TestWait) { + NullSocketServer ss; int64_t start = TimeMillis(); - ss_.Wait(webrtc::TimeDelta::Millis(200), true); + ss.Wait(webrtc::TimeDelta::Millis(200), true); // The actual wait time is dependent on the resolution of the timer used by // the Event class. Allow for the event to signal ~20ms early. EXPECT_GE(TimeSince(start), 180); diff --git a/rtc_base/socket_unittest.cc b/rtc_base/socket_unittest.cc index 0b9c2f2c58..2ac563456d 100644 --- a/rtc_base/socket_unittest.cc +++ b/rtc_base/socket_unittest.cc @@ -25,7 +25,6 @@ #include "rtc_base/gunit.h" #include "rtc_base/location.h" #include "rtc_base/logging.h" -#include "rtc_base/message_handler.h" #include "rtc_base/net_helpers.h" #include "rtc_base/socket_address.h" #include "rtc_base/socket_server.h" @@ -691,11 +690,6 @@ void SocketTest::DeleteInReadCallbackInternal(const IPAddress& loopback) { EXPECT_TRUE_WAIT(deleter.deleted(), kTimeout); } -class Sleeper : public MessageHandlerAutoCleanup { - public: - void OnMessage(Message* msg) override { Thread::Current()->SleepMs(500); } -}; - void SocketTest::SocketServerWaitInternal(const IPAddress& loopback) { StreamSink sink; SocketAddress accept_addr; @@ -736,9 +730,7 @@ void SocketTest::SocketServerWaitInternal(const IPAddress& loopback) { // Shouldn't signal when blocked in a thread Send, where process_io is false. std::unique_ptr thread(Thread::CreateWithSocketServer()); thread->Start(); - Sleeper sleeper; - TypedMessageData data(client.get()); - thread->Send(RTC_FROM_HERE, &sleeper, 0, &data); + thread->Invoke(RTC_FROM_HERE, [] { Thread::SleepMs(500); }); EXPECT_FALSE(sink.Check(accepted.get(), SSE_READ)); // But should signal when process_io is true. diff --git a/rtc_base/synchronization/mutex_unittest.cc b/rtc_base/synchronization/mutex_unittest.cc index 5209d63329..a5ebc5f7d4 100644 --- a/rtc_base/synchronization/mutex_unittest.cc +++ b/rtc_base/synchronization/mutex_unittest.cc @@ -22,8 +22,6 @@ #include "benchmark/benchmark.h" #include "rtc_base/checks.h" #include "rtc_base/event.h" -#include "rtc_base/location.h" -#include "rtc_base/message_handler.h" #include "rtc_base/platform_thread.h" #include "rtc_base/synchronization/yield.h" #include "rtc_base/thread.h" @@ -33,8 +31,6 @@ namespace webrtc { namespace { using ::rtc::Event; -using ::rtc::Message; -using ::rtc::MessageHandler; using ::rtc::Thread; constexpr int kNumThreads = 16; @@ -77,7 +73,7 @@ class MutexLockLocker { }; template -class LockRunner : public rtc::MessageHandlerAutoCleanup { +class LockRunner { public: template explicit LockRunner(Args... args) @@ -106,7 +102,7 @@ class LockRunner : public rtc::MessageHandlerAutoCleanup { return shared_value; } - void OnMessage(Message* msg) override { + void Loop() { ASSERT_TRUE(start_event_.Wait(kLongTime)); locker_.Lock(); @@ -140,12 +136,13 @@ class LockRunner : public rtc::MessageHandlerAutoCleanup { MutexLocker locker_; }; +template void StartThreads(std::vector>& threads, - MessageHandler* handler) { + Runner* handler) { for (int i = 0; i < kNumThreads; ++i) { std::unique_ptr thread(Thread::Create()); thread->Start(); - thread->Post(RTC_FROM_HERE, handler); + thread->PostTask([handler] { handler->Loop(); }); threads.push_back(std::move(thread)); } } diff --git a/rtc_base/thread_unittest.cc b/rtc_base/thread_unittest.cc index e1237be578..a888534a19 100644 --- a/rtc_base/thread_unittest.cc +++ b/rtc_base/thread_unittest.cc @@ -58,10 +58,20 @@ class TestGenerator { int count; }; -struct TestMessage : public MessageData { - explicit TestMessage(int v) : value(v) {} +// Receives messages and sends on a socket. +class MessageClient : public TestGenerator { + public: + MessageClient(Thread* pth, Socket* socket) : socket_(socket) {} - int value; + ~MessageClient() { delete socket_; } + + void OnValue(int value) { + int result = Next(value); + EXPECT_GE(socket_->Send(&result, sizeof(result)), 0); + } + + private: + Socket* socket_; }; // Receives on a socket and sends by posting messages. @@ -70,7 +80,7 @@ class SocketClient : public TestGenerator, public sigslot::has_slots<> { SocketClient(Socket* socket, const SocketAddress& addr, Thread* post_thread, - MessageHandler* phandler) + MessageClient* phandler) : socket_(AsyncUDPSocket::Create(socket, addr)), post_thread_(post_thread), post_handler_(phandler) { @@ -90,32 +100,15 @@ class SocketClient : public TestGenerator, public sigslot::has_slots<> { uint32_t prev = reinterpret_cast(buf)[0]; uint32_t result = Next(prev); - post_thread_->PostDelayed(RTC_FROM_HERE, 200, post_handler_, 0, - new TestMessage(result)); + post_thread_->PostDelayedTask([post_handler_ = post_handler_, + result] { post_handler_->OnValue(result); }, + TimeDelta::Millis(200)); } private: AsyncUDPSocket* socket_; Thread* post_thread_; - MessageHandler* post_handler_; -}; - -// Receives messages and sends on a socket. -class MessageClient : public MessageHandlerAutoCleanup, public TestGenerator { - public: - MessageClient(Thread* pth, Socket* socket) : socket_(socket) {} - - ~MessageClient() override { delete socket_; } - - void OnMessage(Message* pmsg) override { - TestMessage* msg = static_cast(pmsg->pdata); - int result = Next(msg->value); - EXPECT_GE(socket_->Send(&result, sizeof(result)), 0); - delete msg; - } - - private: - Socket* socket_; + MessageClient* post_handler_; }; class CustomThread : public rtc::Thread { @@ -223,6 +216,7 @@ struct FunctorD { // See: https://code.google.com/p/webrtc/issues/detail?id=2409 TEST(ThreadTest, DISABLED_Main) { + rtc::AutoThread main_thread; const SocketAddress addr("127.0.0.1", 0); // Create the messaging client on its own thread. @@ -242,7 +236,8 @@ TEST(ThreadTest, DISABLED_Main) { th2->Start(); // Get the messages started. - th1->PostDelayed(RTC_FROM_HERE, 100, &msg_client, 0, new TestMessage(1)); + th1->PostDelayedTask([&msg_client] { msg_client.OnValue(1); }, + TimeDelta::Millis(100)); // Give the clients a little while to run. // Messages will be processed at 100, 300, 500, 700, 900. @@ -657,14 +652,17 @@ TEST(ThreadManager, ProcessAllMessageQueuesWithClearedQueue) { ThreadManager::ProcessAllMessageQueuesForTesting(); } -class RefCountedHandler : public MessageHandlerAutoCleanup, - public rtc::RefCountInterface { +class RefCountedHandler : public MessageHandler, public rtc::RefCountInterface { public: + ~RefCountedHandler() override { ThreadManager::Clear(this); } + void OnMessage(Message* msg) override {} }; -class EmptyHandler : public MessageHandlerAutoCleanup { +class EmptyHandler : public MessageHandler { public: + ~EmptyHandler() override { ThreadManager::Clear(this); } + void OnMessage(Message* msg) override {} }; @@ -679,7 +677,7 @@ TEST(ThreadManager, ClearReentrant) { // again in a re-entrant fashion, which previously triggered a DCHECK. // The inner handler will be removed in a re-entrant fashion from the // message queue of the thread while the outer handler is removed, verifying - // that the iterator is not invalidated in "MessageQueue::Clear". + // that the iterator is not invalidated in "Thread::Clear". t->Post(RTC_FROM_HERE, inner_handler, 0); t->Post(RTC_FROM_HERE, &handler, 0, new ScopedRefMessageData(inner_handler));