Deflake PeerConnectionSimulcastMediaFlowTests due to unstopped sources.

Only in testing environments are the task queues shut down while sources
still have media flowing. It's still not clear why heap-use-after-free
happens, since it should be enough to close the PC, but it is clear that
the crash is happening due to frames flowing while the test is shutting
down, which is not something happening outside of testing.

In an attempt to deflake, this CL makes sure to manually stop the
test-only sources before closing the peer connection.

Bug: webrtc:15018
Change-Id: I48ee131a8994c9c4caee1bb4875580d255b97da1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299944
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39752}

pc/peer_connection_simulcast_unittest.cc

@@ -176,11 +176,7 @@ const webrtc::RTCOutboundRtpStreamStats* FindOutboundRtpByRid(
 namespace webrtc {
 
 constexpr TimeDelta kDefaultTimeout = TimeDelta::Seconds(5);
-
-// Only used by tests disabled under ASAN, avoids unsued variable compile error.
-#if !defined(ADDRESS_SANITIZER)
 constexpr TimeDelta kLongTimeoutForRampingUp = TimeDelta::Seconds(30);
-#endif  // !defined(ADDRESS_SANITIZER)
 
 class PeerConnectionSimulcastTests : public ::testing::Test {
  public:
@@ -1173,12 +1169,6 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
   EXPECT_THAT(*outbound_rtps[0]->scalability_mode, StrEq("L1T1"));
 }
 
-// TODO(https://crbug.com/webrtc/15018): Investigate heap-use-after free during
-// shutdown of the test that is flakily happening on bots. It's not only
-// happening on ASAN, but it is rare enough on non-ASAN that we don't have to
-// disable everywhere.
-#if !defined(ADDRESS_SANITIZER)
-
 TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
        SendingThreeEncodings_VP8_Simulcast) {
   rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
@@ -1757,6 +1747,4 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
   EXPECT_THAT(*outbound_rtps[2]->scalability_mode, StrEq("L1T3"));
 }
 
-#endif  // !defined(ADDRESS_SANITIZER)
-
 }  // namespace webrtc

pc/test/fake_periodic_video_track_source.h

@@ -29,6 +29,7 @@ class FakePeriodicVideoTrackSource : public VideoTrackSource {
 
   ~FakePeriodicVideoTrackSource() = default;
 
+  FakePeriodicVideoSource& fake_periodic_source() { return source_; }
   const FakePeriodicVideoSource& fake_periodic_source() const {
     return source_;
   }

pc/test/peer_connection_test_wrapper.cc

@@ -39,7 +39,6 @@
 #include "p2p/base/fake_port_allocator.h"
 #include "p2p/base/port_allocator.h"
 #include "pc/test/fake_periodic_video_source.h"
-#include "pc/test/fake_periodic_video_track_source.h"
 #include "pc/test/fake_rtc_certificate_generator.h"
 #include "pc/test/mock_peer_connection_observers.h"
 #include "rtc_base/gunit.h"
@@ -129,6 +128,9 @@ PeerConnectionTestWrapper::PeerConnectionTestWrapper(
 
 PeerConnectionTestWrapper::~PeerConnectionTestWrapper() {
   RTC_DCHECK_RUN_ON(&pc_thread_checker_);
+  // To avoid flaky bot failures, make sure fake sources are stopped prior to
+  // closing the peer connections. See https://crbug.com/webrtc/15018.
+  StopFakeVideoSources();
   // Either network_thread or worker_thread might be active at this point.
   // Relying on ~PeerConnection to properly wait for them doesn't work,
   // as a vptr race might occur (before we enter the destruction body).
@@ -392,6 +394,7 @@ PeerConnectionTestWrapper::GetUserMedia(
 
     auto source = rtc::make_ref_counted<webrtc::FakePeriodicVideoTrackSource>(
         config, /* remote */ false);
+    fake_video_sources_.push_back(source);
 
     std::string videotrack_label = stream_id + kVideoTrackLabelBase;
     rtc::scoped_refptr<webrtc::VideoTrackInterface> video_track(
@@ -401,3 +404,10 @@ PeerConnectionTestWrapper::GetUserMedia(
   }
   return stream;
 }
+
+void PeerConnectionTestWrapper::StopFakeVideoSources() {
+  for (const auto& fake_video_source : fake_video_sources_) {
+    fake_video_source->fake_periodic_source().Stop();
+  }
+  fake_video_sources_.clear();
+}

pc/test/peer_connection_test_wrapper.h

@@ -29,6 +29,7 @@
 #include "api/video/resolution.h"
 #include "pc/test/fake_audio_capture_module.h"
 #include "pc/test/fake_periodic_video_source.h"
+#include "pc/test/fake_periodic_video_track_source.h"
 #include "pc/test/fake_video_track_renderer.h"
 #include "rtc_base/third_party/sigslot/sigslot.h"
 #include "rtc_base/thread.h"
@@ -115,6 +116,7 @@ class PeerConnectionTestWrapper
       webrtc::Resolution resolution = {
           .width = webrtc::FakePeriodicVideoSource::kDefaultWidth,
           .height = webrtc::FakePeriodicVideoSource::kDefaultHeight});
+  void StopFakeVideoSources();
 
  private:
   void SetLocalDescription(webrtc::SdpType type, const std::string& sdp);
@@ -136,6 +138,8 @@ class PeerConnectionTestWrapper
   std::unique_ptr<webrtc::FakeVideoTrackRenderer> renderer_;
   int num_get_user_media_calls_ = 0;
   bool pending_negotiation_;
+  std::vector<rtc::scoped_refptr<webrtc::FakePeriodicVideoTrackSource>>
+      fake_video_sources_;
 };
 
 #endif  // PC_TEST_PEER_CONNECTION_TEST_WRAPPER_H_