diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index b1beb844e1..09317b828b 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -909,6 +909,10 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { const std::string& label, const DataChannelInit* config) = 0; + // NOTE: For the following 6 methods, it's only safe to dereference the + // SessionDescriptionInterface on signaling_thread() (for example, calling + // ToString). + // Returns the more recently applied description; "pending" if it exists, and // otherwise "current". See below. virtual const SessionDescriptionInterface* local_description() const = 0; @@ -1136,6 +1140,14 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface { // thus the observer object can be safely destroyed. virtual void Close() = 0; + // The thread on which all PeerConnectionObserver callbacks will be invoked, + // as well as callbacks for other classes such as DataChannelObserver. + // + // Also the only thread on which it's safe to use SessionDescriptionInterface + // pointers. + // TODO(deadbeef): Make pure virtual when all subclasses implement it. + virtual rtc::Thread* signaling_thread() const { return nullptr; } + protected: // Dtor protected as objects shouldn't be deleted via this interface. ~PeerConnectionInterface() override = default; diff --git a/api/peer_connection_proxy.h b/api/peer_connection_proxy.h index aa72d7cb10..0cc3b3b8e7 100644 --- a/api/peer_connection_proxy.h +++ b/api/peer_connection_proxy.h @@ -147,6 +147,7 @@ PROXY_METHOD2(bool, PROXY_METHOD1(bool, StartRtcEventLog, std::unique_ptr) PROXY_METHOD0(void, StopRtcEventLog) PROXY_METHOD0(void, Close) +BYPASS_PROXY_CONSTMETHOD0(rtc::Thread*, signaling_thread) END_PROXY_MAP() } // namespace webrtc diff --git a/api/proxy.h b/api/proxy.h index b1ebe31acd..0e5d622eb5 100644 --- a/api/proxy.h +++ b/api/proxy.h @@ -400,11 +400,13 @@ class ConstMethodCall : public rtc::Message, public rtc::MessageHandler { // For use when returning purely const state (set during construction). // Use with caution. This method should only be used when the return value will // always be the same. -#define BYPASS_PROXY_CONSTMETHOD0(r, method) \ - r method() const override { \ - static_assert(!std::is_pointer::value, "Type is a pointer"); \ - static_assert(!std::is_reference::value, "Type is a reference"); \ - return c_->method(); \ +#define BYPASS_PROXY_CONSTMETHOD0(r, method) \ + r method() const override { \ + static_assert( \ + std::is_same::value || !std::is_pointer::value, \ + "Type is a pointer"); \ + static_assert(!std::is_reference::value, "Type is a reference"); \ + return c_->method(); \ } } // namespace webrtc diff --git a/api/test/dummy_peer_connection.h b/api/test/dummy_peer_connection.h index 97a97d0c81..0ca7d3f1b4 100644 --- a/api/test/dummy_peer_connection.h +++ b/api/test/dummy_peer_connection.h @@ -237,7 +237,11 @@ class DummyPeerConnection : public PeerConnectionInterface { void StopRtcEventLog() { FATAL() << "Not implemented"; } - void Close() {} + void Close() override {} + + rtc::Thread* signaling_thread() const override { + return rtc::Thread::Current(); + } }; static_assert( diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 6b07bbe74e..9d04b4c787 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -270,6 +270,7 @@ rtc_library("peerconnection") { "../rtc_base:weak_ptr", "../rtc_base/experiments:field_trial_parser", "../rtc_base/synchronization:mutex", + "../rtc_base/synchronization:sequence_checker", "../rtc_base/system:file_wrapper", "../rtc_base/system:rtc_export", "../rtc_base/third_party/base64", diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 1cb575244f..41cb68c645 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -260,14 +260,15 @@ class PeerConnection : public PeerConnectionInternal, void Close() override; + rtc::Thread* signaling_thread() const final { + return factory_->signaling_thread(); + } + // PeerConnectionInternal implementation. rtc::Thread* network_thread() const final { return factory_->network_thread(); } rtc::Thread* worker_thread() const final { return factory_->worker_thread(); } - rtc::Thread* signaling_thread() const final { - return factory_->signaling_thread(); - } std::string session_id() const override { RTC_DCHECK_RUN_ON(signaling_thread()); diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h index 1a78ed204b..029febab2d 100644 --- a/pc/peer_connection_internal.h +++ b/pc/peer_connection_internal.h @@ -30,7 +30,6 @@ class PeerConnectionInternal : public PeerConnectionInterface { public: virtual rtc::Thread* network_thread() const = 0; virtual rtc::Thread* worker_thread() const = 0; - virtual rtc::Thread* signaling_thread() const = 0; // The SDP session ID as defined by RFC 3264. virtual std::string session_id() const = 0; diff --git a/sdk/android/src/jni/pc/peer_connection.cc b/sdk/android/src/jni/pc/peer_connection.cc index 9cebda3813..1165333644 100644 --- a/sdk/android/src/jni/pc/peer_connection.cc +++ b/sdk/android/src/jni/pc/peer_connection.cc @@ -478,17 +478,39 @@ static jlong JNI_PeerConnection_GetNativePeerConnection( static ScopedJavaLocalRef JNI_PeerConnection_GetLocalDescription( JNIEnv* jni, const JavaParamRef& j_pc) { - const SessionDescriptionInterface* sdp = - ExtractNativePC(jni, j_pc)->local_description(); - return sdp ? NativeToJavaSessionDescription(jni, sdp) : nullptr; + PeerConnectionInterface* pc = ExtractNativePC(jni, j_pc); + // It's only safe to operate on SessionDescriptionInterface on the + // signaling thread, but |jni| may only be used on the current thread, so we + // must do this odd dance. + std::string sdp; + std::string type; + pc->signaling_thread()->Invoke(RTC_FROM_HERE, [pc, &sdp, &type] { + const SessionDescriptionInterface* desc = pc->local_description(); + if (desc) { + RTC_CHECK(desc->ToString(&sdp)) << "got so far: " << sdp; + type = desc->type(); + } + }); + return sdp.empty() ? nullptr : NativeToJavaSessionDescription(jni, sdp, type); } static ScopedJavaLocalRef JNI_PeerConnection_GetRemoteDescription( JNIEnv* jni, const JavaParamRef& j_pc) { - const SessionDescriptionInterface* sdp = - ExtractNativePC(jni, j_pc)->remote_description(); - return sdp ? NativeToJavaSessionDescription(jni, sdp) : nullptr; + PeerConnectionInterface* pc = ExtractNativePC(jni, j_pc); + // It's only safe to operate on SessionDescriptionInterface on the + // signaling thread, but |jni| may only be used on the current thread, so we + // must do this odd dance. + std::string sdp; + std::string type; + pc->signaling_thread()->Invoke(RTC_FROM_HERE, [pc, &sdp, &type] { + const SessionDescriptionInterface* desc = pc->remote_description(); + if (desc) { + RTC_CHECK(desc->ToString(&sdp)) << "got so far: " << sdp; + type = desc->type(); + } + }); + return sdp.empty() ? nullptr : NativeToJavaSessionDescription(jni, sdp, type); } static ScopedJavaLocalRef JNI_PeerConnection_GetCertificate( diff --git a/sdk/android/src/jni/pc/sdp_observer.cc b/sdk/android/src/jni/pc/sdp_observer.cc index fc59d1749a..d1842a3db0 100644 --- a/sdk/android/src/jni/pc/sdp_observer.cc +++ b/sdk/android/src/jni/pc/sdp_observer.cc @@ -31,8 +31,11 @@ CreateSdpObserverJni::~CreateSdpObserverJni() = default; void CreateSdpObserverJni::OnSuccess(SessionDescriptionInterface* desc) { JNIEnv* env = AttachCurrentThreadIfNeeded(); - Java_SdpObserver_onCreateSuccess(env, j_observer_global_, - NativeToJavaSessionDescription(env, desc)); + std::string sdp; + RTC_CHECK(desc->ToString(&sdp)) << "got so far: " << sdp; + Java_SdpObserver_onCreateSuccess( + env, j_observer_global_, + NativeToJavaSessionDescription(env, sdp, desc->type())); // OnSuccess transfers ownership of the description (there's a TODO to make // it use unique_ptr...). delete desc; diff --git a/sdk/android/src/jni/pc/session_description.cc b/sdk/android/src/jni/pc/session_description.cc index 1b335215dc..bbac721e51 100644 --- a/sdk/android/src/jni/pc/session_description.cc +++ b/sdk/android/src/jni/pc/session_description.cc @@ -37,12 +37,10 @@ std::unique_ptr JavaToNativeSessionDescription( ScopedJavaLocalRef NativeToJavaSessionDescription( JNIEnv* jni, - const SessionDescriptionInterface* desc) { - std::string sdp; - RTC_CHECK(desc->ToString(&sdp)) << "got so far: " << sdp; + const std::string& sdp, + const std::string& type) { return Java_SessionDescription_Constructor( - jni, - Java_Type_fromCanonicalForm(jni, NativeToJavaString(jni, desc->type())), + jni, Java_Type_fromCanonicalForm(jni, NativeToJavaString(jni, type)), NativeToJavaString(jni, sdp)); } diff --git a/sdk/android/src/jni/pc/session_description.h b/sdk/android/src/jni/pc/session_description.h index fe308474a7..f0f49cb2ee 100644 --- a/sdk/android/src/jni/pc/session_description.h +++ b/sdk/android/src/jni/pc/session_description.h @@ -13,6 +13,7 @@ #include #include +#include #include "api/jsep.h" #include "sdk/android/native_api/jni/scoped_java_ref.h" @@ -26,7 +27,8 @@ std::unique_ptr JavaToNativeSessionDescription( ScopedJavaLocalRef NativeToJavaSessionDescription( JNIEnv* jni, - const SessionDescriptionInterface* desc); + const std::string& sdp, + const std::string& type); } // namespace jni } // namespace webrtc diff --git a/sdk/objc/api/peerconnection/RTCPeerConnection.mm b/sdk/objc/api/peerconnection/RTCPeerConnection.mm index 42a43a79cd..9288a13745 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnection.mm +++ b/sdk/objc/api/peerconnection/RTCPeerConnection.mm @@ -358,19 +358,27 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack( } - (RTC_OBJC_TYPE(RTCSessionDescription) *)localDescription { - const webrtc::SessionDescriptionInterface *description = - _peerConnection->local_description(); - return description ? - [[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithNativeDescription:description] : - nil; + // It's only safe to operate on SessionDescriptionInterface on the signaling thread. + return _peerConnection->signaling_thread()->Invoke( + RTC_FROM_HERE, [self] { + const webrtc::SessionDescriptionInterface *description = + _peerConnection->local_description(); + return description ? + [[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithNativeDescription:description] : + nil; + }); } - (RTC_OBJC_TYPE(RTCSessionDescription) *)remoteDescription { - const webrtc::SessionDescriptionInterface *description = - _peerConnection->remote_description(); - return description ? - [[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithNativeDescription:description] : - nil; + // It's only safe to operate on SessionDescriptionInterface on the signaling thread. + return _peerConnection->signaling_thread()->Invoke( + RTC_FROM_HERE, [self] { + const webrtc::SessionDescriptionInterface *description = + _peerConnection->remote_description(); + return description ? + [[RTC_OBJC_TYPE(RTCSessionDescription) alloc] initWithNativeDescription:description] : + nil; + }); } - (RTCSignalingState)signalingState {