Deprecate AutoThread and AutoSocketServerThread Bug: webrtc:469327588 No-Iwyu: Iwyu and thread_unittest don't get along Change-Id: I8f21f6afd27bc2359db4a8a374ad68666a6a6964 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/464506 Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> Commit-Queue: Evan Shrubsole <eshr@webrtc.org> Auto-Submit: Evan Shrubsole <eshr@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47490}
diff --git a/agents/prompts/knowledge_base.md b/agents/prompts/knowledge_base.md index d0545e6..beee14c 100644 --- a/agents/prompts/knowledge_base.md +++ b/agents/prompts/knowledge_base.md
@@ -8,8 +8,8 @@ You MUST NOT answer from your general knowledge alone. The WebRTC codebase is vast and specific. Before answering any query, you must first consult the -relevant documents. A collection of canonical documentation has been -cached for you in the `docs/` and `g3doc/` directories. +relevant documents. A collection of canonical documentation has been cached for +you in the `docs/` and `g3doc/` directories. ## Task-Oriented Guidance @@ -25,35 +25,32 @@ #### **Modernization** -* **Use Strong Time Types:** Prefer `webrtc::Timestamp` and `webrtc::TimeDelta` - over raw arithmetic types for time values. - See [issue 42223979](https://issues.webrtc.org/42223979). -* **Avoid AutoThread:** Do not use `AutoThread`. In tests, use `webrtc::test::RunLoop`. - See [issue 469327588](https://issues.webrtc.org/469327588). -* **Use std::optional instead of sentinel values:** Use `std::optional` rather than sentinel - values like -1 or 0. +- **Use Strong Time Types:** Prefer `Timestamp` and `TimeDelta` over raw + arithmetic types for time values. See + [issue 42223979](https://issues.webrtc.org/42223979). +- **Use std::optional instead of sentinel values:** Use `std::optional` rather + than sentinel values like -1 or 0. ### **Topic: Modifying BUILD.gn files** -* **For best practices and style in `BUILD.gn` files:** - * Run `gn format` to ensure consistent style. +- **For best practices and style in `BUILD.gn` files:** + - Run `gn format` to ensure consistent style. ### **Topic: Debugging** -* **For a "header file not found" error:** - * **Consult the "Debugging Workflow for 'Header Not Found'":** - 1. **Verify `deps`:** Check the `BUILD.gn` file of the failing - target. Is the dependency providing the header listed in `deps`? - 2. **Verify `#include`:** Is the path in the `#include` statement - correct? - 3. **Regenerate build files:** Suggest running `gn gen <out_dir>`. - 4. **Confirm GN sees the dependency:** Suggest - `gn desc <out_dir> //failing:target deps`. - 5. **Check for issues:** Suggest running - `gn check <out_dir> //failing:target`. -* **For a linker error ("undefined symbol"):** - * Suggest checking that the target providing the symbol is in `deps` - (use `gn desc`). -* **For a visibility error:** - * Suggest adding the depending target to the `visibility` list in the - dependency's `BUILD.gn` file. +- **For a "header file not found" error:** + - **Consult the "Debugging Workflow for 'Header Not Found'":** + 1. **Verify `deps`:** Check the `BUILD.gn` file of the failing target. Is + the dependency providing the header listed in `deps`? + 2. **Verify `#include`:** Is the path in the `#include` statement correct? + 3. **Regenerate build files:** Suggest running `gn gen <out_dir>`. + 4. **Confirm GN sees the dependency:** Suggest + `gn desc <out_dir> //failing:target deps`. + 5. **Check for issues:** Suggest running + `gn check <out_dir> //failing:target`. +- **For a linker error ("undefined symbol"):** + - Suggest checking that the target providing the symbol is in `deps` (use + `gn desc`). +- **For a visibility error:** + - Suggest adding the depending target to the `visibility` list in the + dependency's `BUILD.gn` file.
diff --git a/rtc_base/thread.h b/rtc_base/thread.h index 558255e..151cb45 100644 --- a/rtc_base/thread.h +++ b/rtc_base/thread.h
@@ -581,7 +581,9 @@ // thread, and provides utilities for testing with threads. It also does not // expose a direct dependency on webrtc::Thread. // -class AutoThread : public Thread { +class [[deprecated( + "Use test::RunLoop or GlobalSimulatedTimeController")]] AutoThread + : public Thread { public: AutoThread(); ~AutoThread() override; @@ -598,7 +600,9 @@ // NOTE: Use test::RunLoop instead of AutoSocketServerThread as it also adopts // the current thread, and provides utilities for testing with threads. It also // does not expose a direct dependency on webrtc::Thread. -class AutoSocketServerThread : public Thread { +class [[deprecated( + "Use test::RunLoop or " + "GlobalSimulatedTimeController")]] AutoSocketServerThread : public Thread { public: explicit AutoSocketServerThread(SocketServer* ss); ~AutoSocketServerThread() override; @@ -611,5 +615,4 @@ }; } // namespace webrtc - #endif // RTC_BASE_THREAD_H_
diff --git a/rtc_base/thread_unittest.cc b/rtc_base/thread_unittest.cc index ce011e7..ca3777b 100644 --- a/rtc_base/thread_unittest.cc +++ b/rtc_base/thread_unittest.cc
@@ -33,6 +33,7 @@ #include "rtc_base/event.h" #include "rtc_base/fake_clock.h" #include "rtc_base/internal/default_socket_server.h" +#include "rtc_base/logging.h" #include "rtc_base/network/received_packet.h" #include "rtc_base/null_socket_server.h" #include "rtc_base/socket.h" @@ -61,6 +62,26 @@ using ::testing::IsNull; using ::testing::NotNull; +class ScopedThread : public Thread { + public: + ScopedThread() + : Thread(CreateDefaultSocketServer(), /*do_init=*/false), + previous_thread_(ThreadManager::Instance()->CurrentThread()) { + DoInit(); + ThreadManager::Instance()->SetCurrentThread(this); + } + + ~ScopedThread() override { + Stop(); + DoDestroy(); + RTC_DCHECK_EQ(ThreadManager::Instance()->CurrentThread(), this); + ThreadManager::Instance()->SetCurrentThread(previous_thread_); + } + + private: + Thread* const previous_thread_; +}; + // Generates a sequence of numbers (collaboratively). class TestGenerator { public: @@ -166,7 +187,7 @@ // See: https://code.google.com/p/webrtc/issues/detail?id=2409 TEST(ThreadTest, DISABLED_Main) { - AutoThread main_thread; + ScopedThread main_thread; const SocketAddress addr("127.0.0.1", 0); // Create the messaging client on its own thread. @@ -215,12 +236,12 @@ } TEST(ThreadTest, DisallowBlockingCallsWithThread) { - AutoThread current; + ScopedThread current; RTC_DCHECK_DISALLOW_THREAD_BLOCKING_CALLS(); } TEST(ThreadTest, CountBlockingCalls) { - AutoThread current; + ScopedThread current; // When the test runs, this will print out: // (thread_unittest.cc:262): Blocking TestBody: total=2 (actual=1, could=1) @@ -262,7 +283,7 @@ #if RTC_DCHECK_IS_ON TEST(ThreadTest, CountBlockingCallsOneCallback) { - AutoThread current; + ScopedThread current; bool was_called_back = false; { Thread::ScopedCountBlockingCalls blocked_calls( @@ -275,7 +296,7 @@ } TEST(ThreadTest, CountBlockingCallsSkipCallback) { - AutoThread current; + ScopedThread current; bool was_called_back = false; { Thread::ScopedCountBlockingCalls blocked_calls( @@ -329,7 +350,7 @@ #if (!defined(NDEBUG) || RTC_DCHECK_IS_ON) TEST(ThreadTest, InvokeToThreadAllowedReturnsTrueWithoutPolicies) { - AutoThread main_thread; + ScopedThread main_thread; // Create and start the thread. auto thread1 = Thread::CreateWithSocketServer(); auto thread2 = Thread::CreateWithSocketServer(); @@ -340,7 +361,7 @@ } TEST(ThreadTest, InvokeAllowedWhenThreadsAdded) { - AutoThread main_thread; + ScopedThread main_thread; // Create and start the thread. auto thread1 = Thread::CreateWithSocketServer(); auto thread2 = Thread::CreateWithSocketServer(); @@ -359,7 +380,7 @@ } TEST(ThreadTest, InvokesDisallowedWhenDisallowAllInvokes) { - AutoThread main_thread; + ScopedThread main_thread; // Create and start the thread. auto thread1 = Thread::CreateWithSocketServer(); auto thread2 = Thread::CreateWithSocketServer(); @@ -373,7 +394,7 @@ #endif // (!defined(NDEBUG) || RTC_DCHECK_IS_ON) TEST(ThreadTest, InvokesAllowedByDefault) { - AutoThread main_thread; + ScopedThread main_thread; // Create and start the thread. auto thread1 = Thread::CreateWithSocketServer(); auto thread2 = Thread::CreateWithSocketServer(); @@ -407,7 +428,7 @@ #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) TEST(ThreadTest, TwoThreadsInvokeDeathTest) { GTEST_FLAG_SET(death_test_style, "threadsafe"); - AutoThread thread; + ScopedThread thread; Thread* main_thread = Thread::Current(); auto other_thread = Thread::CreateWithSocketServer(); other_thread->Start(); @@ -418,7 +439,7 @@ TEST(ThreadTest, ThreeThreadsInvokeDeathTest) { GTEST_FLAG_SET(death_test_style, "threadsafe"); - AutoThread thread; + ScopedThread thread; Thread* first = Thread::Current(); auto second = Thread::Create(); @@ -434,7 +455,7 @@ TEST(ThreadTest, DisallowBlockingCallDeathTest) { GTEST_FLAG_SET(death_test_style, "threadsafe"); - AutoThread thread; + ScopedThread thread; ASSERT_THAT(Thread::Current(), NotNull()); auto other_thread = Thread::CreateWithSocketServer(); other_thread->Start(); @@ -450,7 +471,7 @@ // to invoke A at the same time, thread A does not handle C's invoke while // invoking B. TEST(ThreadTest, ThreeThreadsBlockingCall) { - AutoThread thread; + ScopedThread thread; Thread* thread_a = Thread::Current(); auto thread_b = Thread::CreateWithSocketServer(); auto thread_c = Thread::CreateWithSocketServer(); @@ -561,7 +582,7 @@ // all messages (both delayed and non delayed) up until the current time, on // all registered message queues. TEST(ThreadManager, ProcessAllMessageQueues) { - AutoThread main_thread; + ScopedThread main_thread; Event entered_process_all_message_queues(true, false); auto a = Thread::CreateWithSocketServer(); auto b = Thread::CreateWithSocketServer(); @@ -937,7 +958,6 @@ return std::make_unique<ThreadFactory>(); } - INSTANTIATE_TEST_SUITE_P(RtcThread, TaskQueueTest, ::testing::Values(CreateDefaultThreadFactory));