From 16038abb90a3d93f22b2275db4adeb6af4e3ee71 Mon Sep 17 00:00:00 2001
From: Markus Handell <handellm@webrtc.org>
Date: Thu, 28 May 2020 08:37:30 +0200
Subject: [PATCH] FrameForwarder: remove lock recursions.

This change removes lock recursions and adds thread annotations.

Bug: webrtc:11567
Change-Id: I5416cfc8e482bd966eec87c3790abbebc37a84d8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176224
Reviewed-by: Magnus Flodman <mflodman@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31403}
---
 test/frame_forwarder.cc                | 10 ++++++++++
 test/frame_forwarder.h                 | 18 +++++++++++++-----
 video/video_stream_encoder_unittest.cc |  4 ++--
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/test/frame_forwarder.cc b/test/frame_forwarder.cc
index d1a2ddb1c2..d8ec4b5060 100644
--- a/test/frame_forwarder.cc
+++ b/test/frame_forwarder.cc
@@ -26,6 +26,12 @@ void FrameForwarder::IncomingCapturedFrame(const VideoFrame& video_frame) {
 void FrameForwarder::AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
                                      const rtc::VideoSinkWants& wants) {
   rtc::CritScope lock(&crit_);
+  AddOrUpdateSinkLocked(sink, wants);
+}
+
+void FrameForwarder::AddOrUpdateSinkLocked(
+    rtc::VideoSinkInterface<VideoFrame>* sink,
+    const rtc::VideoSinkWants& wants) {
   RTC_DCHECK(!sink_ || sink_ == sink);
   sink_ = sink;
   sink_wants_ = wants;
@@ -42,6 +48,10 @@ rtc::VideoSinkWants FrameForwarder::sink_wants() const {
   return sink_wants_;
 }
 
+rtc::VideoSinkWants FrameForwarder::sink_wants_locked() const {
+  return sink_wants_;
+}
+
 bool FrameForwarder::has_sinks() const {
   rtc::CritScope lock(&crit_);
   return sink_ != nullptr;
diff --git a/test/frame_forwarder.h b/test/frame_forwarder.h
index cf29f5f074..d391160fab 100644
--- a/test/frame_forwarder.h
+++ b/test/frame_forwarder.h
@@ -26,14 +26,22 @@ class FrameForwarder : public rtc::VideoSourceInterface<VideoFrame> {
   FrameForwarder();
   ~FrameForwarder() override;
   // Forwards |video_frame| to the registered |sink_|.
-  virtual void IncomingCapturedFrame(const VideoFrame& video_frame);
-  rtc::VideoSinkWants sink_wants() const;
-  bool has_sinks() const;
+  virtual void IncomingCapturedFrame(const VideoFrame& video_frame)
+      RTC_LOCKS_EXCLUDED(crit_);
+  rtc::VideoSinkWants sink_wants() const RTC_LOCKS_EXCLUDED(crit_);
+  bool has_sinks() const RTC_LOCKS_EXCLUDED(crit_);
 
  protected:
+  rtc::VideoSinkWants sink_wants_locked() const
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
   void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
-                       const rtc::VideoSinkWants& wants) override;
-  void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) override;
+                       const rtc::VideoSinkWants& wants)
+      RTC_LOCKS_EXCLUDED(crit_) override;
+  void AddOrUpdateSinkLocked(rtc::VideoSinkInterface<VideoFrame>* sink,
+                             const rtc::VideoSinkWants& wants)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
+  void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink)
+      RTC_LOCKS_EXCLUDED(crit_) override;
 
   rtc::CriticalSection crit_;
   rtc::VideoSinkInterface<VideoFrame>* sink_ RTC_GUARDED_BY(crit_);
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 69a966bff7..005118dbba 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -554,9 +554,9 @@ class AdaptingFrameForwarder : public test::FrameForwarder {
   void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
                        const rtc::VideoSinkWants& wants) override {
     rtc::CritScope cs(&crit_);
-    last_wants_ = sink_wants();
+    last_wants_ = sink_wants_locked();
     adapter_.OnSinkWants(wants);
-    test::FrameForwarder::AddOrUpdateSink(sink, wants);
+    test::FrameForwarder::AddOrUpdateSinkLocked(sink, wants);
   }
   cricket::VideoAdapter adapter_;
   bool adaptation_enabled_ RTC_GUARDED_BY(crit_);