From 430951a0d4412666245a73eeec4e646fe45b3467 Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
Date: Tue, 19 May 2020 23:27:29 +0200
Subject: [PATCH] Update call expectations in ReceiveStatisticsProxy, add
 thread checks

Bug: chromium:1084619
Change-Id: If9042d44ad99eacd431ee2a5e84486cfaf282d7e
Tbr: stefan@webrtc.org
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175658
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31330}
---
 modules/video_coding/frame_buffer2.cc | 19 ++++++++++++++++---
 modules/video_coding/frame_buffer2.h  |  4 ++++
 video/receive_statistics_proxy2.cc    |  3 ++-
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc
index 944f97bf87..64d3699e01 100644
--- a/modules/video_coding/frame_buffer2.cc
+++ b/modules/video_coding/frame_buffer2.cc
@@ -63,19 +63,25 @@ FrameBuffer::FrameBuffer(Clock* clock,
       last_log_non_decoded_ms_(-kLogNonDecodedIntervalMs),
       add_rtt_to_playout_delay_(
           webrtc::field_trial::IsEnabled("WebRTC-AddRttToPlayoutDelay")),
-      rtt_mult_settings_(RttMultExperiment::GetRttMultValue()) {}
+      rtt_mult_settings_(RttMultExperiment::GetRttMultValue()) {
+  callback_checker_.Detach();
+}
 
-FrameBuffer::~FrameBuffer() {}
+FrameBuffer::~FrameBuffer() {
+  RTC_DCHECK_RUN_ON(&construction_checker_);
+}
 
 void FrameBuffer::NextFrame(
     int64_t max_wait_time_ms,
     bool keyframe_required,
     rtc::TaskQueue* callback_queue,
     std::function<void(std::unique_ptr<EncodedFrame>, ReturnReason)> handler) {
-  RTC_DCHECK_RUN_ON(callback_queue);
+  RTC_DCHECK_RUN_ON(&callback_checker_);
+  RTC_DCHECK(callback_queue->IsCurrent());
   TRACE_EVENT0("webrtc", "FrameBuffer::NextFrame");
   int64_t latest_return_time_ms =
       clock_->TimeInMilliseconds() + max_wait_time_ms;
+
   rtc::CritScope lock(&crit_);
   if (stopped_) {
     return;
@@ -93,6 +99,7 @@ void FrameBuffer::StartWaitForNextFrameOnQueue() {
   int64_t wait_ms = FindNextFrame(clock_->TimeInMilliseconds());
   callback_task_ = RepeatingTaskHandle::DelayedStart(
       callback_queue_->Get(), TimeDelta::Millis(wait_ms), [this] {
+        RTC_DCHECK_RUN_ON(&callback_checker_);
         // If this task has not been cancelled, we did not get any new frames
         // while waiting. Continue with frame delivery.
         rtc::CritScope lock(&crit_);
@@ -211,6 +218,7 @@ int64_t FrameBuffer::FindNextFrame(int64_t now_ms) {
 }
 
 EncodedFrame* FrameBuffer::GetNextFrame() {
+  RTC_DCHECK_RUN_ON(&callback_checker_);
   int64_t now_ms = clock_->TimeInMilliseconds();
   // TODO(ilnik): remove |frames_out| use frames_to_decode_ directly.
   std::vector<EncodedFrame*> frames_out;
@@ -334,7 +342,10 @@ void FrameBuffer::Start() {
 void FrameBuffer::Stop() {
   TRACE_EVENT0("webrtc", "FrameBuffer::Stop");
   rtc::CritScope lock(&crit_);
+  if (stopped_)
+    return;
   stopped_ = true;
+
   CancelCallback();
 }
 
@@ -366,9 +377,11 @@ bool FrameBuffer::ValidReferences(const EncodedFrame& frame) const {
 }
 
 void FrameBuffer::CancelCallback() {
+  // Called from the callback queue or from within Stop().
   frame_handler_ = {};
   callback_task_.Stop();
   callback_queue_ = nullptr;
+  callback_checker_.Detach();
 }
 
 bool FrameBuffer::IsCompleteSuperFrame(const EncodedFrame& frame) {
diff --git a/modules/video_coding/frame_buffer2.h b/modules/video_coding/frame_buffer2.h
index 51f3820d31..d824ddf4d0 100644
--- a/modules/video_coding/frame_buffer2.h
+++ b/modules/video_coding/frame_buffer2.h
@@ -28,6 +28,7 @@
 #include "rtc_base/event.h"
 #include "rtc_base/experiments/rtt_mult_experiment.h"
 #include "rtc_base/numerics/sequence_number_util.h"
+#include "rtc_base/synchronization/sequence_checker.h"
 #include "rtc_base/task_queue.h"
 #include "rtc_base/task_utils/repeating_task.h"
 #include "rtc_base/thread_annotations.h"
@@ -159,6 +160,9 @@ class FrameBuffer {
   EncodedFrame* CombineAndDeleteFrames(
       const std::vector<EncodedFrame*>& frames) const;
 
+  SequenceChecker construction_checker_;
+  SequenceChecker callback_checker_;
+
   // Stores only undecoded frames.
   FrameMap frames_ RTC_GUARDED_BY(crit_);
   DecodedFramesHistory decoded_frames_history_ RTC_GUARDED_BY(crit_);
diff --git a/video/receive_statistics_proxy2.cc b/video/receive_statistics_proxy2.cc
index 0ba4d5d44b..15d08c4d26 100644
--- a/video/receive_statistics_proxy2.cc
+++ b/video/receive_statistics_proxy2.cc
@@ -1002,7 +1002,8 @@ void ReceiveStatisticsProxy::OnCompleteFrame(bool is_keyframe,
 }
 
 void ReceiveStatisticsProxy::OnDroppedFrames(uint32_t frames_dropped) {
-  RTC_DCHECK_RUN_ON(&decode_queue_);
+  // Can be called on either the decode queue or the worker thread
+  // See FrameBuffer2 for more details.
   worker_thread_->PostTask(ToQueuedTask(task_safety_, [frames_dropped, this]() {
     RTC_DCHECK_RUN_ON(&main_thread_);
     stats_.frames_dropped += frames_dropped;