Return proxied object in OnTransceiver
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 <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35010}
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index d5d11f6..7a7b18f 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -1053,6 +1053,7 @@
"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 b8a43a9..b40370a 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -1725,7 +1725,9 @@
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 0000000..df343cc
--- /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 <stdint.h>
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+#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<rtc::Thread> 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<PeerConnectionWrapper> CreatePeerConnection() {
+ RTCConfiguration config;
+ config.sdp_semantics = SdpSemantics::kUnifiedPlan;
+ return CreatePeerConnection(config);
+ }
+
+ std::unique_ptr<PeerConnectionWrapper> CreatePeerConnection(
+ const RTCConfiguration& config) {
+ auto observer = std::make_unique<MockPeerConnectionObserver>();
+ auto pc = pc_factory_->CreatePeerConnection(config, nullptr, nullptr,
+ observer.get());
+ EXPECT_TRUE(pc.get());
+ observer->SetPeerConnectionInterface(pc.get());
+ return std::make_unique<PeerConnectionWrapper>(pc_factory_, pc,
+ std::move(observer));
+ }
+
+ protected:
+ std::unique_ptr<rtc::Thread> signaling_thread_;
+ rtc::scoped_refptr<PeerConnectionFactoryInterface> 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 8054e08..2698d95 100644
--- a/pc/test/mock_peer_connection_observers.h
+++ b/pc/test/mock_peer_connection_observers.h
@@ -259,25 +259,38 @@
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<SessionDescriptionInterface> MoveDescription() {
+ MutexLock lock(&mutex_);
return std::move(desc_);
}
private:
- bool called_;
- std::string error_;
- std::unique_ptr<SessionDescriptionInterface> desc_;
+ mutable Mutex mutex_;
+ bool called_ RTC_GUARDED_BY(mutex_);
+ std::string error_ RTC_GUARDED_BY(mutex_);
+ std::unique_ptr<SessionDescriptionInterface> desc_ RTC_GUARDED_BY(mutex_);
};
class MockSetSessionDescriptionObserver
@@ -292,19 +305,32 @@
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_;
};
