From e61d4c83ef636569901a42909710b817c2b87ae3 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Thu, 16 Sep 2021 08:59:11 +0000 Subject: [PATCH] Return proxied object in OnTransceiver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes it possible to invoke methods on the transceiver object from any thread. Also makes a few of the mock observer objects thread-safe, to allow testing when the main thread is not the signaling thread. Bug: webrtc:13183 Change-Id: Ic97efef71a21c3075700a028103061032f8d2bcc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232120 Commit-Queue: Harald Alvestrand Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/main@{#35010} --- pc/BUILD.gn | 1 + pc/sdp_offer_answer.cc | 4 +- pc/sdp_offer_answer_unittest.cc | 140 +++++++++++++++++++++++ pc/test/mock_peer_connection_observers.h | 44 +++++-- 4 files changed, 179 insertions(+), 10 deletions(-) create mode 100644 pc/sdp_offer_answer_unittest.cc diff --git a/pc/BUILD.gn b/pc/BUILD.gn index d5d11f6156..7a7b18fc06 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -1053,6 +1053,7 @@ if (rtc_include_tests && !build_with_chromium) { "rtp_sender_receiver_unittest.cc", "rtp_transceiver_unittest.cc", "sctp_utils_unittest.cc", + "sdp_offer_answer_unittest.cc", "sdp_serializer_unittest.cc", "stats_collector_unittest.cc", "test/fake_audio_capture_module_unittest.cc", diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index b8a43a9cb2..b40370acc0 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -1725,7 +1725,9 @@ RTCError SdpOfferAnswerHandler::ApplyRemoteDescription( RTC_LOG(LS_INFO) << "Processing the addition of a remote track for MID=" << content->name << "."; - now_receiving_transceivers.push_back(transceiver); + // Since the transceiver is passed to the user in an + // OnTrack event, we must use the proxied transceiver. + now_receiving_transceivers.push_back(transceiver_ext); } } // 2.2.8.1.9: If direction is "sendonly" or "inactive", and transceiver's diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc new file mode 100644 index 0000000000..df343cce3a --- /dev/null +++ b/pc/sdp_offer_answer_unittest.cc @@ -0,0 +1,140 @@ +/* + * Copyright 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include + +#include +#include +#include +#include + +#include "absl/types/optional.h" +#include "api/audio/audio_mixer.h" +#include "api/audio_codecs/audio_decoder_factory.h" +#include "api/audio_codecs/audio_encoder_factory.h" +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" +#include "api/create_peerconnection_factory.h" +#include "api/jsep.h" +#include "api/media_stream_interface.h" +#include "api/media_types.h" +#include "api/peer_connection_interface.h" +#include "api/rtc_error.h" +#include "api/rtp_parameters.h" +#include "api/rtp_receiver_interface.h" +#include "api/rtp_sender_interface.h" +#include "api/rtp_transceiver_interface.h" +#include "api/scoped_refptr.h" +#include "api/set_remote_description_observer_interface.h" +#include "api/uma_metrics.h" +#include "api/video_codecs/builtin_video_decoder_factory.h" +#include "api/video_codecs/builtin_video_encoder_factory.h" +#include "api/video_codecs/video_decoder_factory.h" +#include "api/video_codecs/video_encoder_factory.h" +#include "media/base/stream_params.h" +#include "modules/audio_device/include/audio_device.h" +#include "modules/audio_processing/include/audio_processing.h" +#include "p2p/base/port_allocator.h" +#include "pc/media_session.h" +#include "pc/peer_connection_wrapper.h" +#include "pc/sdp_utils.h" +#include "pc/session_description.h" +#include "pc/test/fake_audio_capture_module.h" +#include "pc/test/mock_peer_connection_observers.h" +#include "rtc_base/checks.h" +#include "rtc_base/gunit.h" +#include "rtc_base/ref_counted_object.h" +#include "rtc_base/rtc_certificate_generator.h" +#include "rtc_base/thread.h" +#include "system_wrappers/include/metrics.h" +#include "test/gmock.h" +#include "test/gtest.h" + +// This file contains unit tests that relate to the behavior of the +// SdpOfferAnswer module. +// Tests are writen as integration tests with PeerConnection, since the +// behaviors are still linked so closely that it is hard to test them in +// isolation. + +namespace webrtc { + +using RTCConfiguration = PeerConnectionInterface::RTCConfiguration; + +namespace { + +std::unique_ptr CreateAndStartThread() { + auto thread = rtc::Thread::Create(); + thread->Start(); + return thread; +} + +} // namespace + +class SdpOfferAnswerTest : public ::testing::Test { + public: + SdpOfferAnswerTest() + // Note: We use a PeerConnectionFactory with a distinct + // signaling thread, so that thread handling can be tested. + : signaling_thread_(CreateAndStartThread()), + pc_factory_( + CreatePeerConnectionFactory(nullptr, + nullptr, + signaling_thread_.get(), + FakeAudioCaptureModule::Create(), + CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory(), + CreateBuiltinVideoEncoderFactory(), + CreateBuiltinVideoDecoderFactory(), + nullptr /* audio_mixer */, + nullptr /* audio_processing */)) { + webrtc::metrics::Reset(); + } + + std::unique_ptr CreatePeerConnection() { + RTCConfiguration config; + config.sdp_semantics = SdpSemantics::kUnifiedPlan; + return CreatePeerConnection(config); + } + + std::unique_ptr CreatePeerConnection( + const RTCConfiguration& config) { + auto observer = std::make_unique(); + auto pc = pc_factory_->CreatePeerConnection(config, nullptr, nullptr, + observer.get()); + EXPECT_TRUE(pc.get()); + observer->SetPeerConnectionInterface(pc.get()); + return std::make_unique(pc_factory_, pc, + std::move(observer)); + } + + protected: + std::unique_ptr signaling_thread_; + rtc::scoped_refptr pc_factory_; + + private: +}; + +TEST_F(SdpOfferAnswerTest, OnTrackReturnsProxiedObject) { + auto caller = CreatePeerConnection(); + auto callee = CreatePeerConnection(); + + auto audio_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO); + + ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get())); + // Verify that caller->observer->OnTrack() has been called with a + // proxied transceiver object. + ASSERT_EQ(callee->observer()->on_track_transceivers_.size(), 1u); + auto transceiver = callee->observer()->on_track_transceivers_[0]; + // Since the signaling thread is not the current thread, + // this will DCHECK if the transceiver is not proxied. + transceiver->stopped(); +} + +} // namespace webrtc diff --git a/pc/test/mock_peer_connection_observers.h b/pc/test/mock_peer_connection_observers.h index 8054e08333..2698d956af 100644 --- a/pc/test/mock_peer_connection_observers.h +++ b/pc/test/mock_peer_connection_observers.h @@ -259,25 +259,38 @@ class MockCreateSessionDescriptionObserver error_("MockCreateSessionDescriptionObserver not called") {} virtual ~MockCreateSessionDescriptionObserver() {} void OnSuccess(SessionDescriptionInterface* desc) override { + MutexLock lock(&mutex_); called_ = true; error_ = ""; desc_.reset(desc); } void OnFailure(webrtc::RTCError error) override { + MutexLock lock(&mutex_); called_ = true; error_ = error.message(); } - bool called() const { return called_; } - bool result() const { return error_.empty(); } - const std::string& error() const { return error_; } + bool called() const { + MutexLock lock(&mutex_); + return called_; + } + bool result() const { + MutexLock lock(&mutex_); + return error_.empty(); + } + const std::string& error() const { + MutexLock lock(&mutex_); + return error_; + } std::unique_ptr MoveDescription() { + MutexLock lock(&mutex_); return std::move(desc_); } private: - bool called_; - std::string error_; - std::unique_ptr desc_; + mutable Mutex mutex_; + bool called_ RTC_GUARDED_BY(mutex_); + std::string error_ RTC_GUARDED_BY(mutex_); + std::unique_ptr desc_ RTC_GUARDED_BY(mutex_); }; class MockSetSessionDescriptionObserver @@ -292,19 +305,32 @@ class MockSetSessionDescriptionObserver error_("MockSetSessionDescriptionObserver not called") {} ~MockSetSessionDescriptionObserver() override {} void OnSuccess() override { + MutexLock lock(&mutex_); + called_ = true; error_ = ""; } void OnFailure(webrtc::RTCError error) override { + MutexLock lock(&mutex_); called_ = true; error_ = error.message(); } - bool called() const { return called_; } - bool result() const { return error_.empty(); } - const std::string& error() const { return error_; } + bool called() const { + MutexLock lock(&mutex_); + return called_; + } + bool result() const { + MutexLock lock(&mutex_); + return error_.empty(); + } + const std::string& error() const { + MutexLock lock(&mutex_); + return error_; + } private: + mutable Mutex mutex_; bool called_; std::string error_; };