Introduce a new constructor to PlatformThread.

The new constructor introduces two new changes:

* Support specifying thread priority at construction time.
  - Moving forward, the SetPriority() method will be removed.
* New thread function type.
  - The new type has 'void' as a return type and a polling loop
    inside PlatformThread, is not used.

The old function type is still supported until all places have been moved over.

In this CL, the first steps towards deprecating the old mechanism are taken
by moving parts of the code that were simple to move, over to the new callback
type.

BUG=webrtc:7187

Review-Url: https://codereview.webrtc.org/2708723003
Cr-Commit-Position: refs/heads/master@{#16779}

webrtc/base/event_tracer.cc

@@ -80,7 +80,7 @@ namespace rtc {
 namespace tracing {
 namespace {
 
-static bool EventTracingThreadFunc(void* params);
+static void EventTracingThreadFunc(void* params);
 
 // Atomic-int fast path for avoiding logging when disabled.
 static volatile int g_event_logging_active = 0;
@@ -89,7 +89,10 @@ static volatile int g_event_logging_active = 0;
 class EventLogger final {
  public:
   EventLogger()
-      : logging_thread_(EventTracingThreadFunc, this, "EventTracingThread"),
+      : logging_thread_(EventTracingThreadFunc,
+                        this,
+                        "EventTracingThread",
+                        kLowPriority),
         shutdown_event_(false, false) {}
   ~EventLogger() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); }
 
@@ -210,7 +213,6 @@ class EventLogger final {
     // Finally start, everything should be set up now.
     logging_thread_.Start();
     TRACE_EVENT_INSTANT0("webrtc", "EventLogger::Start");
-    logging_thread_.SetPriority(kLowPriority);
   }
 
   void Stop() {
@@ -326,11 +328,8 @@ class EventLogger final {
   bool output_file_owned_ = false;
 };
 
-static bool EventTracingThreadFunc(void* params) {
+static void EventTracingThreadFunc(void* params) {
   static_cast<EventLogger*>(params)->Log();
-  // False indicates that the thread function has done its job and doesn't need
-  // to be restarted again. Log() runs its own internal loop.
-  return false;
 }
 
 static EventLogger* volatile g_event_logger = nullptr;

webrtc/base/platform_thread.cc

@@ -92,14 +92,27 @@ struct ThreadAttributes {
 #endif  // defined(WEBRTC_WIN)
 }
 
-PlatformThread::PlatformThread(ThreadRunFunction func,
+PlatformThread::PlatformThread(ThreadRunFunctionDeprecated func,
                                void* obj,
                                const char* thread_name)
-    : run_function_(func),
+    : run_function_deprecated_(func),
       obj_(obj),
       name_(thread_name ? thread_name : "webrtc") {
   RTC_DCHECK(func);
   RTC_DCHECK(name_.length() < 64);
+  spawned_thread_checker_.DetachFromThread();
+}
+
+PlatformThread::PlatformThread(ThreadRunFunction func,
+                               void* obj,
+                               const char* thread_name,
+                               ThreadPriority priority /*= kNormalPriority*/)
+    : run_function_(func), priority_(priority), obj_(obj), name_(thread_name) {
+  RTC_DCHECK(func);
+  RTC_DCHECK(!name_.empty());
+  // TODO(tommi): Consider lowering the limit to 15 (limit on Linux).
+  RTC_DCHECK(name_.length() < 64);
+  spawned_thread_checker_.DetachFromThread();
 }
 
 PlatformThread::~PlatformThread() {
@@ -180,11 +193,14 @@ void PlatformThread::Stop() {
   thread_ = nullptr;
   thread_id_ = 0;
 #else
-  RTC_CHECK_EQ(1, AtomicOps::Increment(&stop_flag_));
+  if (!run_function_)
+    RTC_CHECK_EQ(1, AtomicOps::Increment(&stop_flag_));
   RTC_CHECK_EQ(0, pthread_join(thread_, nullptr));
-  AtomicOps::ReleaseStore(&stop_flag_, 0);
+  if (!run_function_)
+    AtomicOps::ReleaseStore(&stop_flag_, 0);
   thread_ = 0;
 #endif  // defined(WEBRTC_WIN)
+  spawned_thread_checker_.DetachFromThread();
 }
 
 // TODO(tommi): Deprecate the loop behavior in PlatformThread.
@@ -195,8 +211,16 @@ void PlatformThread::Stop() {
 // All implementations will need to be aware of how the thread should be stopped
 // and encouraging a busy polling loop, can be costly in terms of power and cpu.
 void PlatformThread::Run() {
-  if (!name_.empty())
-    rtc::SetCurrentThreadName(name_.c_str());
+  // Attach the worker thread checker to this thread.
+  RTC_DCHECK(spawned_thread_checker_.CalledOnValidThread());
+  rtc::SetCurrentThreadName(name_.c_str());
+
+  if (run_function_) {
+    SetPriority(priority_);
+    run_function_(obj_);
+    return;
+  }
+// TODO(tommi): Delete the below.
 #if !defined(WEBRTC_MAC) && !defined(WEBRTC_WIN)
   const struct timespec ts_null = {0};
 #endif
@@ -204,7 +228,7 @@ void PlatformThread::Run() {
     // The interface contract of Start/Stop is that for a successful call to
     // Start, there should be at least one call to the run function.  So we
     // call the function before checking |stop_|.
-    if (!run_function_(obj_))
+    if (!run_function_deprecated_(obj_))
       break;
 #if defined(WEBRTC_WIN)
     // Alertable sleep to permit RaiseFlag to run and update |stop_|.
@@ -221,8 +245,19 @@ void PlatformThread::Run() {
 }
 
 bool PlatformThread::SetPriority(ThreadPriority priority) {
-  RTC_DCHECK(thread_checker_.CalledOnValidThread());
-  RTC_DCHECK(IsRunning());
+#if RTC_DCHECK_IS_ON
+  if (run_function_) {
+    // The non-deprecated way of how this function gets called, is that it must
+    // be called on the worker thread itself.
+    RTC_DCHECK(spawned_thread_checker_.CalledOnValidThread());
+  } else {
+    // In the case of deprecated use of this method, it must be called on the
+    // same thread as the PlatformThread object is constructed on.
+    RTC_DCHECK(thread_checker_.CalledOnValidThread());
+    RTC_DCHECK(IsRunning());
+  }
+#endif
+
 #if defined(WEBRTC_WIN)
   return SetThreadPriority(thread_, priority) != FALSE;
 #elif defined(__native_client__)

webrtc/base/platform_thread.h

@@ -32,7 +32,8 @@ void SetCurrentThreadName(const char* name);
 // Callback function that the spawned thread will enter once spawned.
 // A return value of false is interpreted as that the function has no
 // more work to do and that the thread can be released.
-typedef bool (*ThreadRunFunction)(void*);
+typedef bool (*ThreadRunFunctionDeprecated)(void*);
+typedef void (*ThreadRunFunction)(void*);
 
 enum ThreadPriority {
 #ifdef WEBRTC_WIN
@@ -55,7 +56,13 @@ enum ThreadPriority {
 // called from the same thread, including instantiation.
 class PlatformThread {
  public:
-  PlatformThread(ThreadRunFunction func, void* obj, const char* thread_name);
+  PlatformThread(ThreadRunFunctionDeprecated func,
+                 void* obj,
+                 const char* thread_name);
+  PlatformThread(ThreadRunFunction func,
+                 void* obj,
+                 const char* thread_name,
+                 ThreadPriority priority = kNormalPriority);
   virtual ~PlatformThread();
 
   const std::string& name() const { return name_; }
@@ -74,6 +81,7 @@ class PlatformThread {
   void Stop();
 
   // Set the priority of the thread. Must be called when thread is running.
+  // TODO(tommi): Make private and only allow public support via ctor.
   bool SetPriority(ThreadPriority priority);
 
  protected:
@@ -85,12 +93,15 @@ class PlatformThread {
  private:
   void Run();
 
-  ThreadRunFunction const run_function_;
+  ThreadRunFunctionDeprecated const run_function_deprecated_ = nullptr;
+  ThreadRunFunction const run_function_ = nullptr;
+  const ThreadPriority priority_ = kNormalPriority;
   void* const obj_;
   // TODO(pbos): Make sure call sites use string literals and update to a const
   // char* instead of a std::string.
   const std::string name_;
   rtc::ThreadChecker thread_checker_;
+  rtc::ThreadChecker spawned_thread_checker_;
 #if defined(WEBRTC_WIN)
   static DWORD WINAPI StartThread(void* param);
 

webrtc/base/platform_thread_unittest.cc

@@ -16,20 +16,65 @@
 namespace rtc {
 namespace {
 // Function that does nothing, and reports success.
-bool NullRunFunction(void* obj) {
+bool NullRunFunctionDeprecated(void* obj) {
   webrtc::SleepMs(0);  // Hand over timeslice, prevents busy looping.
   return true;
 }
 
+void NullRunFunction(void* obj) {}
+
 // Function that sets a boolean.
-bool SetFlagRunFunction(void* obj) {
+bool SetFlagRunFunctionDeprecated(void* obj) {
   bool* obj_as_bool = static_cast<bool*>(obj);
   *obj_as_bool = true;
   webrtc::SleepMs(0);  // Hand over timeslice, prevents busy looping.
   return true;
 }
+
+void SetFlagRunFunction(void* obj) {
+  bool* obj_as_bool = static_cast<bool*>(obj);
+  *obj_as_bool = true;
+}
+
 }  // namespace
 
+TEST(PlatformThreadTest, StartStopDeprecated) {
+  PlatformThread thread(&NullRunFunctionDeprecated, nullptr,
+                        "PlatformThreadTest");
+  EXPECT_TRUE(thread.name() == "PlatformThreadTest");
+  EXPECT_TRUE(thread.GetThreadRef() == 0);
+  thread.Start();
+  EXPECT_TRUE(thread.GetThreadRef() != 0);
+  thread.Stop();
+  EXPECT_TRUE(thread.GetThreadRef() == 0);
+}
+
+TEST(PlatformThreadTest, StartStop2Deprecated) {
+  PlatformThread thread1(&NullRunFunctionDeprecated, nullptr,
+                         "PlatformThreadTest1");
+  PlatformThread thread2(&NullRunFunctionDeprecated, nullptr,
+                         "PlatformThreadTest2");
+  EXPECT_TRUE(thread1.GetThreadRef() == thread2.GetThreadRef());
+  thread1.Start();
+  thread2.Start();
+  EXPECT_TRUE(thread1.GetThreadRef() != thread2.GetThreadRef());
+  thread2.Stop();
+  thread1.Stop();
+}
+
+TEST(PlatformThreadTest, RunFunctionIsCalledDeprecated) {
+  bool flag = false;
+  PlatformThread thread(&SetFlagRunFunctionDeprecated, &flag,
+                        "RunFunctionIsCalled");
+  thread.Start();
+
+  // At this point, the flag may be either true or false.
+  thread.Stop();
+
+  // We expect the thread to have run at least once.
+  EXPECT_TRUE(flag);
+}
+
 TEST(PlatformThreadTest, StartStop) {
   PlatformThread thread(&NullRunFunction, nullptr, "PlatformThreadTest");
   EXPECT_TRUE(thread.name() == "PlatformThreadTest");
@@ -62,4 +107,5 @@ TEST(PlatformThreadTest, RunFunctionIsCalled) {
   // We expect the thread to have run at least once.
   EXPECT_TRUE(flag);
 }
+
 }  // rtc

webrtc/base/rate_limiter_unittest.cc

@@ -130,9 +130,8 @@ class ThreadTask {
   rtc::Event end_signal_;
 };
 
-bool RunTask(void* thread_task) {
+void RunTask(void* thread_task) {
   reinterpret_cast<ThreadTask*>(thread_task)->Run();
-  return false;
 }
 
 TEST_F(RateLimitTest, MultiThreadedUsage) {

webrtc/base/sequenced_task_checker_unittest.cc

@@ -35,14 +35,13 @@ class CallCalledSequentiallyOnThread {
   }
 
  private:
-  static bool Run(void* obj) {
+  static void Run(void* obj) {
     CallCalledSequentiallyOnThread* call_stuff_on_thread =
         static_cast<CallCalledSequentiallyOnThread*>(obj);
     EXPECT_EQ(
         call_stuff_on_thread->expect_true_,
         call_stuff_on_thread->sequenced_task_checker_->CalledSequentially());
     call_stuff_on_thread->thread_has_run_event_.Set();
-    return false;
   }
 
   const bool expect_true_;

webrtc/base/task_queue.h

@@ -232,7 +232,7 @@ class LOCKABLE TaskQueue {
 
  private:
 #if defined(WEBRTC_BUILD_LIBEVENT)
-  static bool ThreadMain(void* context);
+  static void ThreadMain(void* context);
   static void OnWakeup(int socket, short flags, void* context);  // NOLINT
   static void RunTask(int fd, short flags, void* context);       // NOLINT
   static void RunTimer(int fd, short flags, void* context);      // NOLINT
@@ -263,7 +263,7 @@ class LOCKABLE TaskQueue {
   class MultimediaTimer;
   typedef std::unordered_map<UINT_PTR, std::unique_ptr<QueuedTask>>
       DelayedTasks;
-  static bool ThreadMain(void* context);
+  static void ThreadMain(void* context);
   static bool ProcessQueuedMessages(DelayedTasks* delayed_tasks,
                                     std::vector<MultimediaTimer>* timers);
 

webrtc/base/task_queue_libevent.cc

@@ -256,7 +256,7 @@ void TaskQueue::PostTaskAndReply(std::unique_ptr<QueuedTask> task,
 }
 
 // static
-bool TaskQueue::ThreadMain(void* context) {
+void TaskQueue::ThreadMain(void* context) {
   TaskQueue* me = static_cast<TaskQueue*>(context);
 
   QueueContext queue_context(me);
@@ -269,8 +269,6 @@ bool TaskQueue::ThreadMain(void* context) {
 
   for (TimerEvent* timer : queue_context.pending_timers_)
     delete timer;
-
-  return false;
 }
 
 // static

webrtc/base/task_queue_win.cc

@@ -228,7 +228,7 @@ void TaskQueue::PostTaskAndReply(std::unique_ptr<QueuedTask> task,
 }
 
 // static
-bool TaskQueue::ThreadMain(void* context) {
+void TaskQueue::ThreadMain(void* context) {
   HANDLE timer_handles[MultimediaTimer::kMaxTimers];
   // Active multimedia timers.
   std::vector<MultimediaTimer> mm_timers;
@@ -283,8 +283,6 @@ bool TaskQueue::ThreadMain(void* context) {
       RTC_DCHECK_EQ(WAIT_IO_COMPLETION, result);
     }
   }
-
-  return false;
 }
 
 // static

webrtc/call/rampup_tests.cc

@@ -44,7 +44,7 @@ RampUpTester::RampUpTester(size_t num_video_streams,
                            bool red,
                            bool report_perf_stats)
     : EndToEndTest(test::CallTest::kLongTimeoutMs),
-      event_(false, false),
+      stop_event_(false, false),
       clock_(Clock::GetRealTimeClock()),
       num_video_streams_(num_video_streams),
       num_audio_streams_(num_audio_streams),
@@ -72,7 +72,6 @@ RampUpTester::RampUpTester(size_t num_video_streams,
 }
 
 RampUpTester::~RampUpTester() {
-  event_.Set();
 }
 
 Call::Config RampUpTester::GetSenderCallConfig() {
@@ -282,25 +281,25 @@ void RampUpTester::OnCallsCreated(Call* sender_call, Call* receiver_call) {
   sender_call_ = sender_call;
 }
 
-bool RampUpTester::BitrateStatsPollingThread(void* obj) {
-  return static_cast<RampUpTester*>(obj)->PollStats();
+void RampUpTester::BitrateStatsPollingThread(void* obj) {
+  static_cast<RampUpTester*>(obj)->PollStats();
 }
 
-bool RampUpTester::PollStats() {
-  if (sender_call_) {
-    Call::Stats stats = sender_call_->GetStats();
+void RampUpTester::PollStats() {
+  do {
+    if (sender_call_) {
+      Call::Stats stats = sender_call_->GetStats();
 
-    EXPECT_GE(stats.send_bandwidth_bps, start_bitrate_bps_);
-    EXPECT_GE(expected_bitrate_bps_, 0);
-    if (stats.send_bandwidth_bps >= expected_bitrate_bps_ &&
-        (min_run_time_ms_ == -1 ||
-         clock_->TimeInMilliseconds() - test_start_ms_ >= min_run_time_ms_)) {
-      ramp_up_finished_ms_ = clock_->TimeInMilliseconds();
-      observation_complete_.Set();
+      EXPECT_GE(stats.send_bandwidth_bps, start_bitrate_bps_);
+      EXPECT_GE(expected_bitrate_bps_, 0);
+      if (stats.send_bandwidth_bps >= expected_bitrate_bps_ &&
+          (min_run_time_ms_ == -1 ||
+           clock_->TimeInMilliseconds() - test_start_ms_ >= min_run_time_ms_)) {
+        ramp_up_finished_ms_ = clock_->TimeInMilliseconds();
+        observation_complete_.Set();
+      }
     }
-  }
-
-  return !event_.Wait(kPollIntervalMs);
+  } while (!stop_event_.Wait(kPollIntervalMs));
 }
 
 void RampUpTester::ReportResult(const std::string& measurement,
@@ -380,6 +379,7 @@ void RampUpTester::PerformTest() {
   poller_thread_.Start();
   EXPECT_TRUE(Wait()) << "Timed out while waiting for ramp-up to complete.";
   TriggerTestDone();
+  stop_event_.Set();
   poller_thread_.Stop();
 }
 
@@ -415,22 +415,22 @@ RampUpDownUpTester::RampUpDownUpTester(size_t num_video_streams,
 
 RampUpDownUpTester::~RampUpDownUpTester() {}
 
-bool RampUpDownUpTester::PollStats() {
-  if (send_stream_) {
-    webrtc::VideoSendStream::Stats stats = send_stream_->GetStats();
-    int transmit_bitrate_bps = 0;
-    for (auto it : stats.substreams) {
-      transmit_bitrate_bps += it.second.total_bitrate_bps;
+void RampUpDownUpTester::PollStats() {
+  do {
+    if (send_stream_) {
+      webrtc::VideoSendStream::Stats stats = send_stream_->GetStats();
+      int transmit_bitrate_bps = 0;
+      for (auto it : stats.substreams) {
+        transmit_bitrate_bps += it.second.total_bitrate_bps;
+      }
+      EvolveTestState(transmit_bitrate_bps, stats.suspended);
+    } else if (num_audio_streams_ > 0 && sender_call_ != nullptr) {
+      // An audio send stream doesn't have bitrate stats, so the call send BW is
+      // currently used instead.
+      int transmit_bitrate_bps = sender_call_->GetStats().send_bandwidth_bps;
+      EvolveTestState(transmit_bitrate_bps, false);
     }
-    EvolveTestState(transmit_bitrate_bps, stats.suspended);
-  } else if (num_audio_streams_ > 0 && sender_call_ != nullptr) {
-    // An audio send stream doesn't have bitrate stats, so the call send BW is
-    // currently used instead.
-    int transmit_bitrate_bps = sender_call_->GetStats().send_bandwidth_bps;
-    EvolveTestState(transmit_bitrate_bps, false);
-  }
-
-  return !event_.Wait(kPollIntervalMs);
+  } while (!stop_event_.Wait(kPollIntervalMs));
 }
 
 Call::Config RampUpDownUpTester::GetReceiverCallConfig() {

webrtc/call/rampup_tests.h

@@ -49,7 +49,7 @@ class RampUpTester : public test::EndToEndTest {
   void PerformTest() override;
 
  protected:
-  virtual bool PollStats();
+  virtual void PollStats();
 
   void AccumulateStats(const VideoSendStream::StreamStats& stream,
                        size_t* total_packets_sent,
@@ -63,7 +63,7 @@ class RampUpTester : public test::EndToEndTest {
   void TriggerTestDone();
 
   webrtc::RtcEventLogNullImpl event_log_;
-  rtc::Event event_;
+  rtc::Event stop_event_;
   Clock* const clock_;
   FakeNetworkPipe::Config forward_transport_config_;
   const size_t num_video_streams_;
@@ -95,7 +95,7 @@ class RampUpTester : public test::EndToEndTest {
       std::vector<FlexfecReceiveStream::Config>* receive_configs) override;
   void OnCallsCreated(Call* sender_call, Call* receiver_call) override;
 
-  static bool BitrateStatsPollingThread(void* obj);
+  static void BitrateStatsPollingThread(void* obj);
 
   const int start_bitrate_bps_;
   const int64_t min_run_time_ms_;
@@ -125,7 +125,7 @@ class RampUpDownUpTester : public RampUpTester {
   ~RampUpDownUpTester() override;
 
  protected:
-  bool PollStats() override;
+  void PollStats() override;
 
  private:
   enum TestStates {

webrtc/common_video/include/incoming_video_stream.h

@@ -31,14 +31,10 @@ class IncomingVideoStream : public rtc::VideoSinkInterface<VideoFrame> {
   ~IncomingVideoStream() override;
 
  protected:
-  static bool IncomingVideoStreamThreadFun(void* obj);
-  bool IncomingVideoStreamProcess();
+  static void IncomingVideoStreamThreadFun(void* obj);
+  void IncomingVideoStreamProcess();
 
  private:
-  enum { kEventStartupTimeMs = 10 };
-  enum { kEventMaxWaitTimeMs = 100 };
-  enum { kFrameRatePeriodMs = 1000 };
-
   void OnFrame(const VideoFrame& video_frame) override;
 
   rtc::ThreadChecker main_thread_checker_;

webrtc/common_video/incoming_video_stream.cc

@@ -16,13 +16,18 @@
 #include "webrtc/system_wrappers/include/event_wrapper.h"
 
 namespace webrtc {
+namespace {
+const int kEventStartupTimeMs = 10;
+const int kEventMaxWaitTimeMs = 100;
+}  // namespace
 
 IncomingVideoStream::IncomingVideoStream(
     int32_t delay_ms,
     rtc::VideoSinkInterface<VideoFrame>* callback)
     : incoming_render_thread_(&IncomingVideoStreamThreadFun,
                               this,
-                              "IncomingVideoStreamThread"),
+                              "IncomingVideoStreamThread",
+                              rtc::kRealtimePriority),
       deliver_buffer_event_(EventTimerWrapper::Create()),
       external_callback_(callback),
       render_buffers_(new VideoRenderFrames(delay_ms)) {
@@ -32,7 +37,6 @@ IncomingVideoStream::IncomingVideoStream(
 
   deliver_buffer_event_->StartTimer(false, kEventStartupTimeMs);
   incoming_render_thread_.Start();
-  incoming_render_thread_.SetPriority(rtc::kRealtimePriority);
 }
 
 IncomingVideoStream::~IncomingVideoStream() {
@@ -57,39 +61,44 @@ void IncomingVideoStream::OnFrame(const VideoFrame& video_frame) {
   }
 }
 
-bool IncomingVideoStream::IncomingVideoStreamThreadFun(void* obj) {
-  return static_cast<IncomingVideoStream*>(obj)->IncomingVideoStreamProcess();
+// static
+void IncomingVideoStream::IncomingVideoStreamThreadFun(void* obj) {
+  static_cast<IncomingVideoStream*>(obj)->IncomingVideoStreamProcess();
 }
 
-bool IncomingVideoStream::IncomingVideoStreamProcess() {
+void IncomingVideoStream::IncomingVideoStreamProcess() {
   RTC_DCHECK_RUN_ON(&render_thread_checker_);
 
-  if (kEventError != deliver_buffer_event_->Wait(kEventMaxWaitTimeMs)) {
-    // Get a new frame to render and the time for the frame after this one.
-    rtc::Optional<VideoFrame> frame_to_render;
-    uint32_t wait_time;
-    {
-      rtc::CritScope cs(&buffer_critsect_);
-      if (!render_buffers_.get()) {
-        // Terminating
-        return false;
+  while (true) {
+    if (kEventError != deliver_buffer_event_->Wait(kEventMaxWaitTimeMs)) {
+      // Get a new frame to render and the time for the frame after this one.
+      rtc::Optional<VideoFrame> frame_to_render;
+      uint32_t wait_time;
+      {
+        rtc::CritScope cs(&buffer_critsect_);
+        if (!render_buffers_.get()) {
+          // Terminating
+          return;
+        }
+
+        frame_to_render = render_buffers_->FrameToRender();
+        wait_time = render_buffers_->TimeToNextFrameRelease();
       }
-      frame_to_render = render_buffers_->FrameToRender();
-      wait_time = render_buffers_->TimeToNextFrameRelease();
-    }
 
-    // Set timer for next frame to render.
-    if (wait_time > kEventMaxWaitTimeMs) {
-      wait_time = kEventMaxWaitTimeMs;
-    }
+      // Set timer for next frame to render.
+      if (wait_time > kEventMaxWaitTimeMs) {
+        wait_time = kEventMaxWaitTimeMs;
+      }
 
-    deliver_buffer_event_->StartTimer(false, wait_time);
+      deliver_buffer_event_->StartTimer(false, wait_time);
 
-    if (frame_to_render) {
-      external_callback_->OnFrame(*frame_to_render);
+      if (frame_to_render) {
+        external_callback_->OnFrame(*frame_to_render);
+      }
+    } else {
+      RTC_NOTREACHED();
     }
   }
-  return true;
 }
 
 }  // namespace webrtc

webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc

@@ -307,10 +307,9 @@ void RtcEventLogHelperThread::ProcessEvents() {
   }
 }
 
-bool RtcEventLogHelperThread::ThreadOutputFunction(void* obj) {
+void RtcEventLogHelperThread::ThreadOutputFunction(void* obj) {
   RtcEventLogHelperThread* helper = static_cast<RtcEventLogHelperThread*>(obj);
   helper->ProcessEvents();
-  return false;
 }
 
 }  // namespace webrtc

webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.h

@@ -81,7 +81,7 @@ class RtcEventLogHelperThread final {
   void SignalNewEvent();
 
  private:
-  static bool ThreadOutputFunction(void* obj);
+  static void ThreadOutputFunction(void* obj);
 
   bool AppendEventToString(rtclog::Event* event);
   bool LogToMemory();

webrtc/modules/video_coding/frame_buffer2_unittest.cc

@@ -197,7 +197,7 @@ class TestFrameBuffer2 : public ::testing::Test {
     ASSERT_FALSE(frames_[index]);
   }
 
-  static bool ExtractLoop(void* obj) {
+  static void ExtractLoop(void* obj) {
     TestFrameBuffer2* tfb = static_cast<TestFrameBuffer2*>(obj);
     while (true) {
       tfb->trigger_extract_event_.Wait(rtc::Event::kForever);
@@ -205,7 +205,7 @@ class TestFrameBuffer2 : public ::testing::Test {
         rtc::CritScope lock(&tfb->crit_);
         tfb->crit_acquired_event_.Set();
         if (tfb->tear_down_)
-          return false;
+          return;
 
         std::unique_ptr<FrameObject> frame;
         FrameBuffer::ReturnReason res =

webrtc/system_wrappers/source/critical_section_unittest.cc

@@ -68,10 +68,9 @@ public:
   }
 };
 
-bool LockUnlockThenStopRunFunction(void* obj) {
+void LockUnlockThenStopRunFunction(void* obj) {
   ProtectedCount* the_count = static_cast<ProtectedCount*>(obj);
   the_count->Increment();
-  return false;
 }
 
 TEST_F(CritSectTest, ThreadWakesOnce) NO_THREAD_SAFETY_ANALYSIS {

webrtc/video/end_to_end_tests.cc

@@ -2142,45 +2142,45 @@ TEST_P(EndToEndTest, RembWithSendSideBwe) {
       sender_call_ = sender_call;
     }
 
-    static bool BitrateStatsPollingThread(void* obj) {
-      return static_cast<BweObserver*>(obj)->PollStats();
+    static void BitrateStatsPollingThread(void* obj) {
+      static_cast<BweObserver*>(obj)->PollStats();
     }
 
-    bool PollStats() {
-      if (sender_call_) {
-        Call::Stats stats = sender_call_->GetStats();
-        switch (state_) {
-          case kWaitForFirstRampUp:
-            if (stats.send_bandwidth_bps >= remb_bitrate_bps_) {
-              state_ = kWaitForRemb;
-              remb_bitrate_bps_ /= 2;
-              rtp_rtcp_->SetREMBData(
-                  remb_bitrate_bps_,
-                  std::vector<uint32_t>(&sender_ssrc_, &sender_ssrc_ + 1));
-              rtp_rtcp_->SendRTCP(kRtcpRr);
-            }
-            break;
+    void PollStats() {
+      do {
+        if (sender_call_) {
+          Call::Stats stats = sender_call_->GetStats();
+          switch (state_) {
+            case kWaitForFirstRampUp:
+              if (stats.send_bandwidth_bps >= remb_bitrate_bps_) {
+                state_ = kWaitForRemb;
+                remb_bitrate_bps_ /= 2;
+                rtp_rtcp_->SetREMBData(
+                    remb_bitrate_bps_,
+                    std::vector<uint32_t>(&sender_ssrc_, &sender_ssrc_ + 1));
+                rtp_rtcp_->SendRTCP(kRtcpRr);
+              }
+              break;
 
-          case kWaitForRemb:
-            if (stats.send_bandwidth_bps == remb_bitrate_bps_) {
-              state_ = kWaitForSecondRampUp;
-              remb_bitrate_bps_ *= 2;
-              rtp_rtcp_->SetREMBData(
-                  remb_bitrate_bps_,
-                  std::vector<uint32_t>(&sender_ssrc_, &sender_ssrc_ + 1));
-              rtp_rtcp_->SendRTCP(kRtcpRr);
-            }
-            break;
+            case kWaitForRemb:
+              if (stats.send_bandwidth_bps == remb_bitrate_bps_) {
+                state_ = kWaitForSecondRampUp;
+                remb_bitrate_bps_ *= 2;
+                rtp_rtcp_->SetREMBData(
+                    remb_bitrate_bps_,
+                    std::vector<uint32_t>(&sender_ssrc_, &sender_ssrc_ + 1));
+                rtp_rtcp_->SendRTCP(kRtcpRr);
+              }
+              break;
 
-          case kWaitForSecondRampUp:
-            if (stats.send_bandwidth_bps == remb_bitrate_bps_) {
-              observation_complete_.Set();
-            }
-            break;
+            case kWaitForSecondRampUp:
+              if (stats.send_bandwidth_bps == remb_bitrate_bps_) {
+                observation_complete_.Set();
+              }
+              break;
+          }
         }
-      }
-
-      return !stop_event_.Wait(1000);
+      } while (!stop_event_.Wait(1000));
     }
 
     void PerformTest() override {

webrtc/video/video_quality_test.cc

@@ -555,37 +555,34 @@ class VideoAnalyzer : public PacketReceiver,
     comparison_available_event_.Set();
   }
 
-  static bool PollStatsThread(void* obj) {
-    return static_cast<VideoAnalyzer*>(obj)->PollStats();
+  static void PollStatsThread(void* obj) {
+    static_cast<VideoAnalyzer*>(obj)->PollStats();
   }
 
-  bool PollStats() {
-    if (done_.Wait(kSendStatsPollingIntervalMs))
-      return false;
+  void PollStats() {
+    while (!done_.Wait(kSendStatsPollingIntervalMs)) {
+      rtc::CritScope crit(&comparison_lock_);
 
-    rtc::CritScope crit(&comparison_lock_);
+      VideoSendStream::Stats send_stats = send_stream_->GetStats();
+      // It's not certain that we yet have estimates for any of these stats.
+      // Check that they are positive before mixing them in.
+      if (send_stats.encode_frame_rate > 0)
+        encode_frame_rate_.AddSample(send_stats.encode_frame_rate);
+      if (send_stats.avg_encode_time_ms > 0)
+        encode_time_ms_.AddSample(send_stats.avg_encode_time_ms);
+      if (send_stats.encode_usage_percent > 0)
+        encode_usage_percent_.AddSample(send_stats.encode_usage_percent);
+      if (send_stats.media_bitrate_bps > 0)
+        media_bitrate_bps_.AddSample(send_stats.media_bitrate_bps);
 
-    VideoSendStream::Stats send_stats = send_stream_->GetStats();
-    // It's not certain that we yet have estimates for any of these stats. Check
-    // that they are positive before mixing them in.
-    if (send_stats.encode_frame_rate > 0)
-      encode_frame_rate_.AddSample(send_stats.encode_frame_rate);
-    if (send_stats.avg_encode_time_ms > 0)
-      encode_time_ms_.AddSample(send_stats.avg_encode_time_ms);
-    if (send_stats.encode_usage_percent > 0)
-      encode_usage_percent_.AddSample(send_stats.encode_usage_percent);
-    if (send_stats.media_bitrate_bps > 0)
-      media_bitrate_bps_.AddSample(send_stats.media_bitrate_bps);
-
-    if (receive_stream_ != nullptr) {
-      VideoReceiveStream::Stats receive_stats = receive_stream_->GetStats();
-      if (receive_stats.decode_ms > 0)
-        decode_time_ms_.AddSample(receive_stats.decode_ms);
-      if (receive_stats.max_decode_ms > 0)
-        decode_time_max_ms_.AddSample(receive_stats.max_decode_ms);
+      if (receive_stream_ != nullptr) {
+        VideoReceiveStream::Stats receive_stats = receive_stream_->GetStats();
+        if (receive_stats.decode_ms > 0)
+          decode_time_ms_.AddSample(receive_stats.decode_ms);
+        if (receive_stats.max_decode_ms > 0)
+          decode_time_max_ms_.AddSample(receive_stats.max_decode_ms);
+      }
     }
-
-    return true;
   }
 
   static bool FrameComparisonThread(void* obj) {

webrtc/voice_engine/channel_proxy.cc

@@ -155,8 +155,12 @@ bool ChannelProxy::SendTelephoneEventOutband(int event, int duration_ms) {
 }
 
 void ChannelProxy::SetBitrate(int bitrate_bps, int64_t probing_interval_ms) {
-  RTC_DCHECK(worker_thread_checker_.CalledOnValidThread() ||
-             module_process_thread_checker_.CalledOnValidThread());
+  // This method can be called on the worker thread, module process thread
+  // or on a TaskQueue via VideoSendStreamImpl::OnEncoderConfigurationChanged.
+  // TODO(solenberg): Figure out a good way to check this or enforce calling
+  // rules.
+  // RTC_DCHECK(worker_thread_checker_.CalledOnValidThread() ||
+  //            module_process_thread_checker_.CalledOnValidThread());
   channel()->SetBitRate(bitrate_bps, probing_interval_ms);
 }