Simplify thread yielding by removing support for high-priority tasks

Remove the `PostHighPriorityTask` method and the internal
`high_priority_messages_` queue from the `Thread` class.

Instead, change `IsYieldRequested()` to return true whenever there are
any pending tasks in the standard message queue, rather than only
checking for high-priority tasks.

This means that the sequence of batched operations is always executed
when the thread is idle, rather than blocking the thread, ensuring that
media-related calls to the same thread get processed as fast as
possible.

Bug: webrtc:42222804
Change-Id: I758c33dc2c33d79fc5e6beea1926529f78e49255
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/456760
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#47249}
diff --git a/pc/scoped_operations_batcher.cc b/pc/scoped_operations_batcher.cc
index 00c19cf..b5b277f 100644
--- a/pc/scoped_operations_batcher.cc
+++ b/pc/scoped_operations_batcher.cc
@@ -55,7 +55,7 @@
           }
         }
         ++task_idx;
-        if (!target_thread_is_current && target_thread_->IsYieldRequested()) {
+        if (!target_thread_is_current && target_thread_->HasPendingTasks()) {
           return;
         }
       }
diff --git a/pc/scoped_operations_batcher_unittest.cc b/pc/scoped_operations_batcher_unittest.cc
index 0f188f7..03aa42d 100644
--- a/pc/scoped_operations_batcher_unittest.cc
+++ b/pc/scoped_operations_batcher_unittest.cc
@@ -75,7 +75,7 @@
   EXPECT_EQ(return_task_thread, signaling_thread);
 }
 
-TEST(ScopedOperationsBatcherTest, YieldsToHighPriorityTasks) {
+TEST(ScopedOperationsBatcherTest, YieldsToOtherTasks) {
   auto target_thread = Thread::Create();
   target_thread->Start();
 
@@ -86,16 +86,17 @@
     batcher.push_back([&] { execution_order.push_back(1); });
     batcher.push_back([&] {
       execution_order.push_back(2);
-      // Post a high-priority task that should interrupt the batch.
-      target_thread->PostHighPriorityTask(
-          [&] { execution_order.push_back(3); });
+      // Post a task that should interrupt the batch since we now yield to any
+      // pending task.
+      target_thread->PostTask([&] { execution_order.push_back(3); });
     });
     batcher.push_back([&] { execution_order.push_back(4); });
     batcher.push_back([&] { execution_order.push_back(5); });
   }
 
-  // Expect the high priority task (3) to execute immediately after the task
-  // that posted it (2). The regular tasks (4, 5) should yield.
+  // Expect the task (3) to execute immediately after the task
+  // that posted it (2). The operations remaining in the batch (4, 5)
+  // should resume after the thread has processed the new task.
   EXPECT_EQ(execution_order, std::vector<int>({1, 2, 3, 4, 5}));
 }
 
diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc
index 4bea295..81a7b6b 100644
--- a/rtc_base/thread.cc
+++ b/rtc_base/thread.cc
@@ -391,7 +391,6 @@
   ThreadManager::Remove(this);
   // Clear.
   CurrentTaskQueueSetter set_current(this);
-  high_priority_messages_.clear();
   messages_.clear();
   delayed_messages_ = {};
 }
@@ -442,13 +441,6 @@
         messages_.push_back(std::move(delayed_messages_.top().functor));
         delayed_messages_.pop();
       }
-      // Pull a high priority message off the queue, if available.
-      if (!high_priority_messages_.empty()) {
-        absl::AnyInvocable<void() &&> task =
-            std::move(high_priority_messages_.front());
-        high_priority_messages_.pop_front();
-        return task;
-      }
       // Pull a message off the message queue, if available.
       if (!messages_.empty()) {
         absl::AnyInvocable<void() &&> task = std::move(messages_.front());
@@ -494,15 +486,6 @@
 void Thread::PostTaskImpl(absl::AnyInvocable<void() &&> task,
                           const PostTaskTraits& /* traits */,
                           const Location& /* location */) {
-  PostTaskInternal(std::move(task), /*high_priority=*/false);
-}
-
-void Thread::PostHighPriorityTask(absl::AnyInvocable<void() &&> task) {
-  PostTaskInternal(std::move(task), /*high_priority=*/true);
-}
-
-void Thread::PostTaskInternal(absl::AnyInvocable<void() &&> task,
-                              bool high_priority) {
   if (IsQuitting()) {
     return;
   }
@@ -513,11 +496,7 @@
 
   {
     MutexLock lock(&mutex_);
-    if (high_priority) {
-      high_priority_messages_.push_back(std::move(task));
-    } else {
-      messages_.push_back(std::move(task));
-    }
+    messages_.push_back(std::move(task));
     WakeUpSocketServer();
   }
 }
@@ -554,7 +533,7 @@
 int Thread::GetDelay() {
   MutexLock lock(&mutex_);
 
-  if (!high_priority_messages_.empty() || !messages_.empty())
+  if (!messages_.empty())
     return 0;
 
   if (!delayed_messages_.empty()) {
@@ -990,7 +969,7 @@
   }
 }
 
-bool Thread::IsYieldRequested() const {
+bool Thread::HasPendingTasks() const {
   RTC_DCHECK_RUN_ON(this);
 #if RTC_DCHECK_IS_ON
   // If you've hit this, then there's a cooperative task running from inside a
@@ -998,7 +977,7 @@
   RTC_DCHECK_EQ(running_synchronous_blocking_call_count_, 0);
 #endif
   MutexLock lock(&mutex_);
-  return !high_priority_messages_.empty();
+  return !messages_.empty();
 }
 
 }  // namespace webrtc
diff --git a/rtc_base/thread.h b/rtc_base/thread.h
index 0629461..2437272 100644
--- a/rtc_base/thread.h
+++ b/rtc_base/thread.h
@@ -317,11 +317,12 @@
   // ProcessMessages occasionally.
   virtual void Run();
 
-  // Called from cooperative tasks to know if they should yield.
+  // Returns true if there are pending tasks in the message queue.
+  // Cooperative tasks can use this to know if they should yield.
   // If a task yields, it is up to the task itself how or if to
   // continue the ongoing operation. Typically this can be handled
   // by using PostTask() to queue up a continuation task.
-  bool IsYieldRequested() const;
+  bool HasPendingTasks() const;
 
   // Convenience method to invoke a functor on another thread.
   // Blocks the current thread until execution is complete.
@@ -345,12 +346,6 @@
     return result;
   }
 
-  // Posts a task that jumps the queue (gets added as the next task to execute,
-  // ahead of other pending tasks). An internal yielding request flag is also
-  // raised that cooperative aware tasks can check by calling
-  // `IsYieldRequested()` and immediately exit.
-  void PostHighPriorityTask(absl::AnyInvocable<void() &&> task);
-
   // Allows BlockingCall to specified `thread`. Thread never will be
   // dereferenced and will be used only for reference-based comparison, so
   // instance can be safely deleted. If NDEBUG is defined and RTC_DCHECK_IS_ON
@@ -476,8 +471,6 @@
  private:
   static const int kSlowDispatchLoggingThreshold = 50;  // 50 ms
 
-  void PostTaskInternal(absl::AnyInvocable<void() &&> task, bool high_priority);
-
   // Get() will process I/O until:
   //  1) A task is available (returns it)
   //  2) cmsWait seconds have elapsed (returns empty task)
@@ -513,8 +506,6 @@
   void ClearCurrentTaskQueue();
 
   std::deque<absl::AnyInvocable<void() &&>> messages_ RTC_GUARDED_BY(mutex_);
-  std::deque<absl::AnyInvocable<void() &&>> high_priority_messages_
-      RTC_GUARDED_BY(mutex_);
   std::priority_queue<DelayedMessage> delayed_messages_ RTC_GUARDED_BY(mutex_);
   uint32_t delayed_next_num_ RTC_GUARDED_BY(mutex_);
 #if RTC_DCHECK_IS_ON
diff --git a/rtc_base/thread_unittest.cc b/rtc_base/thread_unittest.cc
index 97ed808..ce011e7 100644
--- a/rtc_base/thread_unittest.cc
+++ b/rtc_base/thread_unittest.cc
@@ -873,155 +873,54 @@
   EXPECT_EQ(TaskQueueBase::Current(), current_tq);
 }
 
-// Verifies that posting a high-priority task sets the "yield requested" flag
-// on the target thread, allowing a cooperative task to observe it.
-TEST(ThreadCooperativeTest, HighPriorityTaskTriggersYieldRequest) {
+// Uses `HasPendingTasks()` to detect when to yield to another posted task.
+TEST(ThreadCooperativeTest, TaskTriggersHasPendingTasks) {
   test::RunLoop loop;
   auto thread = Thread::Create();
   thread->Start();
 
-  bool yield_requested = false;
+  bool was_interrupted = false;
   Event task_started;
 
-  // Post a long running task that checks for yield
+  // Post a long running task that checks for pending tasks.
   thread->PostTask(
-      [&yield_requested, &loop, &task_started, thread = thread.get()] {
+      [&was_interrupted, &loop, &task_started, thread = thread.get()] {
         task_started.Set();
-        while (!thread->IsYieldRequested()) {
+        while (!thread->HasPendingTasks()) {
           // Busy loop/simulated work
         }
-        loop.PostTask([&yield_requested, &loop] {
-          yield_requested = true;
+        loop.PostTask([&was_interrupted, &loop] {
+          was_interrupted = true;
           loop.Quit();
         });
       });
 
-  // Wait for the task to start to ensure that the high priority task doesn't
+  // Wait for the task to start to ensure that the task doesn't
   // run first.
   task_started.Wait(Event::kForever);
 
-  // Post a high priority task that interrupts the busy loop.
-  thread->PostHighPriorityTask([] {});
+  // Post a task that interrupts the busy loop.
+  thread->PostTask([] {});
 
   loop.Run();
-  EXPECT_TRUE(yield_requested);
+  EXPECT_TRUE(was_interrupted);
 }
 
-// Verifies that a high-priority task is executed before a regular priority task
-// that was posted earlier, if the thread was busy.
-TEST(ThreadCooperativeTest, HighPriorityTaskRunsFirst) {
-  test::RunLoop loop;
-  auto thread = Thread::Create();
-  thread->Start();
-
-  std::vector<std::string> execution_order;
-  Event task1_started;
-  Event continue_execution;
-
-  // Task 1: Blocks the thread until signaled.
-  thread->PostTask([&] {
-    task1_started.Set();
-    continue_execution.Wait(Event::kForever);
-    loop.PostTask([&] { execution_order.push_back("Task1"); });
-  });
-
-  task1_started.Wait(Event::kForever);
-
-  // Task 2: Regular priority.
-  thread->PostTask(
-      [&] { loop.PostTask([&] { execution_order.push_back("Task2"); }); });
-
-  // Task 3: High priority. High priority tasks should run before "Task2".
-  thread->PostHighPriorityTask(
-      [&] { loop.PostTask([&] { execution_order.push_back("Task3"); }); });
-
-  // Signal Task 1 to continue.
-  continue_execution.Set();
-
-  // Now post a task to quit the loop.
-  thread->PostTask([&] { loop.PostTask([&] { loop.Quit(); }); });
-
-  loop.Run();
-
-  // Expect Task1 (resuming), Task3 (High Prio), Task2 (Low Prio - pushed to
-  // back).
-  EXPECT_THAT(execution_order, ElementsAre("Task1", "Task3", "Task2"));
-}
-
-// Verifies that multiple high-priority tasks execute in the sequence they were
-// posted (FIFO order).
-TEST(ThreadCooperativeTest, HighPriorityTasksExecuteInSequence) {
-  test::RunLoop loop;
-  auto thread = Thread::Create();
-  thread->Start();
-
-  std::vector<std::string> execution_order;
-  Event task1_started;
-  Event continue_execution;
-
-  // Block the thread.
-  thread->PostTask([&] {
-    task1_started.Set();
-    continue_execution.Wait(Event::kForever);
-  });
-
-  task1_started.Wait(Event::kForever);
-
-  // Post a normal priority task first to ensure high priority tasks run before
-  // it.
-  thread->PostTask(
-      [&] { loop.PostTask([&] { execution_order.push_back("Normal1"); }); });
-
-  // Post multiple high priority tasks while the thread is blocked.
-  thread->PostHighPriorityTask(
-      [&] { loop.PostTask([&] { execution_order.push_back("HP1"); }); });
-  thread->PostHighPriorityTask(
-      [&] { loop.PostTask([&] { execution_order.push_back("HP2"); }); });
-  thread->PostHighPriorityTask(
-      [&] { loop.PostTask([&] { execution_order.push_back("HP3"); }); });
-
-  // Unblock the thread.
-  continue_execution.Set();
-
-  // Quit after all tasks.
-  thread->PostTask([&] { loop.PostTask([&] { loop.Quit(); }); });
-
-  loop.Run();
-
-  EXPECT_THAT(execution_order, ElementsAre("HP1", "HP2", "HP3", "Normal1"));
-}
-
-TEST(ThreadCooperativeTest, YieldRequestedClearedAfterHighPriorityTask) {
+TEST(ThreadCooperativeTest, HasPendingTasksClearedAfterTask) {
   std::unique_ptr<Thread> thread(Thread::Create());
   thread->Start();
 
   // Initially false.
-  thread->BlockingCall([&] { EXPECT_FALSE(thread->IsYieldRequested()); });
+  thread->BlockingCall([&] { EXPECT_FALSE(thread->HasPendingTasks()); });
 
-  // Post high priority task.
-  thread->PostHighPriorityTask([&] {});
+  // Post task.
+  thread->PostTask([&] {});
 
-  // Post normal task.
-  thread->BlockingCall([&] { EXPECT_FALSE(thread->IsYieldRequested()); });
+  // Use `BlockingCall` to post normal task which implicitly blocks and waits
+  // for its functor to run, at which point the queue will be empty again.
+  thread->BlockingCall([&] { EXPECT_FALSE(thread->HasPendingTasks()); });
 }
 
-#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) && RTC_DCHECK_IS_ON
-TEST(ThreadCooperativeDeathTest, YieldInSynchronousBlockingCallTriggersDeath) {
-  std::unique_ptr<Thread> background_thread = Thread::Create();
-  background_thread->Start();
-  background_thread->BlockingCall([&] {
-    // Yielding from a top-level synchronous blocking call is allowed.
-    background_thread->IsYieldRequested();
-
-    // Call a nested BlockingCall.
-    background_thread->BlockingCall([&] {
-      // This violates the "no yielding in blocking call" rule.
-      EXPECT_DEATH(background_thread->IsYieldRequested(), "");
-    });
-  });
-}
-#endif
-
 class ThreadFactory : public TaskQueueFactory {
  public:
   std::unique_ptr<TaskQueueBase, TaskQueueDeleter> CreateTaskQueue(