From 17426d80c0ecb156eb626608b224b9250d87ac2d Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 8 Feb 2022 23:37:56 +0000 Subject: [PATCH] Bug 1253706: Create and use MediaTransportHandler::SetIceConfig. r=mjf Used to be built into CreateIceCtx, but needed to be stand-alone so it could be called subsequently. Necessitated adding some members so pref-based config state could be saved for later use. Differential Revision: https://phabricator.services.mozilla.com/D135365 --- dom/media/webrtc/MediaTransportParent.h | 5 +- dom/media/webrtc/PMediaTransport.ipdl | 5 +- .../webrtc/jsapi/MediaTransportHandler.cpp | 101 ++++++++++-------- .../webrtc/jsapi/MediaTransportHandler.h | 6 +- .../webrtc/jsapi/MediaTransportHandlerIPC.cpp | 26 +++-- .../webrtc/jsapi/MediaTransportHandlerIPC.h | 5 +- .../webrtc/jsapi/MediaTransportParent.cpp | 12 ++- dom/media/webrtc/jsapi/PeerConnectionImpl.cpp | 10 +- dom/media/webrtc/jsapi/PeerConnectionImpl.h | 5 + .../gtest/mediapipeline_unittest.cpp | 5 +- 10 files changed, 115 insertions(+), 65 deletions(-) diff --git a/dom/media/webrtc/MediaTransportParent.h b/dom/media/webrtc/MediaTransportParent.h index c30f7b405e3d..6283b67ec64f 100644 --- a/dom/media/webrtc/MediaTransportParent.h +++ b/dom/media/webrtc/MediaTransportParent.h @@ -21,8 +21,9 @@ class MediaTransportParent : public dom::PMediaTransportParent { mozilla::ipc::IPCResult RecvClearIceLog(); mozilla::ipc::IPCResult RecvEnterPrivateMode(); mozilla::ipc::IPCResult RecvExitPrivateMode(); - mozilla::ipc::IPCResult RecvCreateIceCtx( - const string& name, nsTArray&& iceServers, + mozilla::ipc::IPCResult RecvCreateIceCtx(const string& name); + mozilla::ipc::IPCResult RecvSetIceConfig( + nsTArray&& iceServers, const RTCIceTransportPolicy& icePolicy); mozilla::ipc::IPCResult RecvSetProxyConfig( const net::WebrtcProxyConfig& aProxyConfig); diff --git a/dom/media/webrtc/PMediaTransport.ipdl b/dom/media/webrtc/PMediaTransport.ipdl index 26e657c978ca..3c896741688f 100644 --- a/dom/media/webrtc/PMediaTransport.ipdl +++ b/dom/media/webrtc/PMediaTransport.ipdl @@ -40,8 +40,9 @@ parent: async EnterPrivateMode(); async ExitPrivateMode(); - async CreateIceCtx(string name, - RTCIceServer[] iceServers, + async CreateIceCtx(string name); + + async SetIceConfig(RTCIceServer[] iceServers, RTCIceTransportPolicy icePolicy); async SetProxyConfig(WebrtcProxyConfig proxyConfig); diff --git a/dom/media/webrtc/jsapi/MediaTransportHandler.cpp b/dom/media/webrtc/jsapi/MediaTransportHandler.cpp index fabcfaf77082..a107d11908ac 100644 --- a/dom/media/webrtc/jsapi/MediaTransportHandler.cpp +++ b/dom/media/webrtc/jsapi/MediaTransportHandler.cpp @@ -78,8 +78,9 @@ class MediaTransportHandlerSTS : public MediaTransportHandler, void EnterPrivateMode() override; void ExitPrivateMode() override; - nsresult CreateIceCtx(const std::string& aName, - const nsTArray& aIceServers, + void CreateIceCtx(const std::string& aName) override; + + nsresult SetIceConfig(const nsTArray& aIceServers, dom::RTCIceTransportPolicy aIcePolicy) override; // We will probably be able to move the proxy lookup stuff into @@ -179,8 +180,11 @@ class MediaTransportHandlerSTS : public MediaTransportHandler, RefPtr mDNSResolver; std::map mTransports; bool mObfuscateHostAddresses = false; + bool mTurnDisabled = false; uint32_t mMinDtlsVersion = 0; uint32_t mMaxDtlsVersion = 0; + bool mForceNoHost = false; + Maybe mNatConfig; std::set mSignaledAddresses; @@ -286,11 +290,7 @@ static NrIceCtx::Policy toNrIcePolicy(dom::RTCIceTransportPolicy aPolicy) { case dom::RTCIceTransportPolicy::Relay: return NrIceCtx::ICE_POLICY_RELAY; case dom::RTCIceTransportPolicy::All: - if (Preferences::GetBool("media.peerconnection.ice.no_host", false)) { - return NrIceCtx::ICE_POLICY_NO_HOST; - } else { - return NrIceCtx::ICE_POLICY_ALL; - } + return NrIceCtx::ICE_POLICY_ALL; default: MOZ_CRASH(); } @@ -531,17 +531,7 @@ static Maybe GetNatConfig() { return Nothing(); } -nsresult MediaTransportHandlerSTS::CreateIceCtx( - const std::string& aName, const nsTArray& aIceServers, - dom::RTCIceTransportPolicy aIcePolicy) { - // We rely on getting an error when this happens, so do it up front. - std::vector stunServers; - std::vector turnServers; - nsresult rv = ConvertIceServers(aIceServers, &stunServers, &turnServers); - if (NS_FAILED(rv)) { - return rv; - } - +void MediaTransportHandlerSTS::CreateIceCtx(const std::string& aName) { mInitPromise = InvokeAsync( GetMainThreadSerialEventTarget(), __func__, [=, self = RefPtr(this)]() { @@ -575,7 +565,7 @@ nsresult MediaTransportHandlerSTS::CreateIceCtx( } // Give us a way to globally turn off TURN support - bool turnDisabled = + mTurnDisabled = Preferences::GetBool("media.peerconnection.turn.disable", false); // We are reading these here, because when we setup the DTLS transport // we are on the wrong thread to read prefs @@ -583,10 +573,9 @@ nsresult MediaTransportHandlerSTS::CreateIceCtx( Preferences::GetUint("media.peerconnection.dtls.version.min"); mMaxDtlsVersion = Preferences::GetUint("media.peerconnection.dtls.version.max"); - - NrIceCtx::Config config; - config.mPolicy = toNrIcePolicy(aIcePolicy); - config.mNatSimulatorConfig = GetNatConfig(); + mForceNoHost = + Preferences::GetBool("media.peerconnection.ice.no_host", false); + mNatConfig = GetNatConfig(); MOZ_RELEASE_ASSERT(STSShutdownHandler::Instance()); STSShutdownHandler::Instance()->Register(this); @@ -594,7 +583,7 @@ nsresult MediaTransportHandlerSTS::CreateIceCtx( return InvokeAsync( mStsThread, __func__, [=, self = RefPtr(this)]() { - mIceCtx = NrIceCtx::Create(aName, config); + mIceCtx = NrIceCtx::Create(aName); if (!mIceCtx) { return InitPromise::CreateAndReject("NrIceCtx::Create failed", __func__); @@ -605,27 +594,8 @@ nsresult MediaTransportHandlerSTS::CreateIceCtx( mIceCtx->SignalConnectionStateChange.connect( this, &MediaTransportHandlerSTS::OnConnectionStateChange); - nsresult rv; - - if (NS_FAILED(rv = mIceCtx->SetStunServers(stunServers))) { - CSFLogError(LOGTAG, "%s: Failed to set stun servers", - __FUNCTION__); - return InitPromise::CreateAndReject( - "Failed to set stun servers", __func__); - } - if (!turnDisabled) { - if (NS_FAILED(rv = mIceCtx->SetTurnServers(turnServers))) { - CSFLogError(LOGTAG, "%s: Failed to set turn servers", - __FUNCTION__); - return InitPromise::CreateAndReject( - "Failed to set turn servers", __func__); - } - } else if (!turnServers.empty()) { - CSFLogError(LOGTAG, "%s: Setting turn servers disabled", - __FUNCTION__); - } - mDNSResolver = new NrIceResolver; + nsresult rv; if (NS_FAILED(rv = mDNSResolver->Init())) { CSFLogError(LOGTAG, "%s: Failed to initialize dns resolver", __FUNCTION__); @@ -644,6 +614,49 @@ nsresult MediaTransportHandlerSTS::CreateIceCtx( return InitPromise::CreateAndResolve(true, __func__); }); }); +} + +nsresult MediaTransportHandlerSTS::SetIceConfig( + const nsTArray& aIceServers, + dom::RTCIceTransportPolicy aIcePolicy) { + // We rely on getting an error when this happens, so do it up front. + std::vector stunServers; + std::vector turnServers; + nsresult rv = ConvertIceServers(aIceServers, &stunServers, &turnServers); + if (NS_FAILED(rv)) { + return rv; + } + + mInitPromise->Then( + mStsThread, __func__, + [=, self = RefPtr(this)]() { + NrIceCtx::Config config; + config.mPolicy = toNrIcePolicy(aIcePolicy); + if (config.mPolicy == NrIceCtx::ICE_POLICY_ALL && mForceNoHost) { + config.mPolicy = NrIceCtx::ICE_POLICY_NO_HOST; + } + config.mNatSimulatorConfig = mNatConfig; + + nsresult rv; + + if (NS_FAILED(rv = mIceCtx->SetStunServers(stunServers))) { + CSFLogError(LOGTAG, "%s: Failed to set stun servers", __FUNCTION__); + return; + } + if (!mTurnDisabled) { + if (NS_FAILED(rv = mIceCtx->SetTurnServers(turnServers))) { + CSFLogError(LOGTAG, "%s: Failed to set turn servers", __FUNCTION__); + return; + } + } else if (!turnServers.empty()) { + CSFLogError(LOGTAG, "%s: Setting turn servers disabled", + __FUNCTION__); + } + if (NS_FAILED(rv = mIceCtx->SetIceConfig(config))) { + CSFLogError(LOGTAG, "%s: Failed to set config", __FUNCTION__); + } + }); + return NS_OK; } diff --git a/dom/media/webrtc/jsapi/MediaTransportHandler.h b/dom/media/webrtc/jsapi/MediaTransportHandler.h index 77d4c3647029..97379433ee19 100644 --- a/dom/media/webrtc/jsapi/MediaTransportHandler.h +++ b/dom/media/webrtc/jsapi/MediaTransportHandler.h @@ -48,6 +48,7 @@ class MediaTransportHandler { explicit MediaTransportHandler(nsISerialEventTarget* aCallbackThread) : mCallbackThread(aCallbackThread) {} + // Exposed so we can synchronously validate ICE servers from PeerConnection static nsresult ConvertIceServers( const nsTArray& aIceServers, std::vector* aStunServers, @@ -63,8 +64,9 @@ class MediaTransportHandler { virtual void EnterPrivateMode() = 0; virtual void ExitPrivateMode() = 0; - virtual nsresult CreateIceCtx(const std::string& aName, - const nsTArray& aIceServers, + virtual void CreateIceCtx(const std::string& aName) = 0; + + virtual nsresult SetIceConfig(const nsTArray& aIceServers, dom::RTCIceTransportPolicy aIcePolicy) = 0; // We will probably be able to move the proxy lookup stuff into diff --git a/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp b/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp index f3b940ed9187..7f1f7e1521ff 100644 --- a/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp +++ b/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp @@ -120,10 +120,25 @@ void MediaTransportHandlerIPC::ExitPrivateMode() { [](const nsCString& aError) {}); } -nsresult MediaTransportHandlerIPC::CreateIceCtx( - const std::string& aName, const nsTArray& aIceServers, - dom::RTCIceTransportPolicy aIcePolicy) { +void MediaTransportHandlerIPC::CreateIceCtx(const std::string& aName) { CSFLogDebug(LOGTAG, "MediaTransportHandlerIPC::CreateIceCtx start"); + + mInitPromise->Then( + mCallbackThread, __func__, + [=, self = RefPtr(this)](bool /*dummy*/) { + if (mChild) { + CSFLogDebug(LOGTAG, "%s starting", __func__); + if (NS_WARN_IF(!mChild->SendCreateIceCtx(aName))) { + CSFLogError(LOGTAG, "%s failed!", __func__); + } + } + }, + [](const nsCString& aError) {}); +} + +nsresult MediaTransportHandlerIPC::SetIceConfig( + const nsTArray& aIceServers, + dom::RTCIceTransportPolicy aIcePolicy) { // Run some validation on this side of the IPC boundary so we can return // errors synchronously. We don't actually use the results. It might make // sense to move this check to PeerConnection and have this API take the @@ -141,9 +156,8 @@ nsresult MediaTransportHandlerIPC::CreateIceCtx( [=, iceServers = aIceServers.Clone(), self = RefPtr(this)](bool /*dummy*/) { if (mChild) { - CSFLogDebug(LOGTAG, "%s starting", __func__); - if (!mChild->SendCreateIceCtx(aName, std::move(iceServers), - aIcePolicy)) { + if (NS_WARN_IF(!mChild->SendSetIceConfig(std::move(iceServers), + aIcePolicy))) { CSFLogError(LOGTAG, "%s failed!", __func__); } } diff --git a/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.h b/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.h index 60f38d682c68..2eedf39f78e3 100644 --- a/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.h +++ b/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.h @@ -22,8 +22,9 @@ class MediaTransportHandlerIPC : public MediaTransportHandler { void EnterPrivateMode() override; void ExitPrivateMode() override; - nsresult CreateIceCtx(const std::string& aName, - const nsTArray& aIceServers, + void CreateIceCtx(const std::string& aName) override; + + nsresult SetIceConfig(const nsTArray& aIceServers, dom::RTCIceTransportPolicy aIcePolicy) override; // We will probably be able to move the proxy lookup stuff into diff --git a/dom/media/webrtc/jsapi/MediaTransportParent.cpp b/dom/media/webrtc/jsapi/MediaTransportParent.cpp index 5ac9ca359420..a1fa65b8af9c 100644 --- a/dom/media/webrtc/jsapi/MediaTransportParent.cpp +++ b/dom/media/webrtc/jsapi/MediaTransportParent.cpp @@ -124,12 +124,18 @@ mozilla::ipc::IPCResult MediaTransportParent::RecvExitPrivateMode() { } mozilla::ipc::IPCResult MediaTransportParent::RecvCreateIceCtx( - const string& name, nsTArray&& iceServers, + const string& name) { + mImpl->mHandler->CreateIceCtx(name); + return ipc::IPCResult::Ok(); +} + +mozilla::ipc::IPCResult MediaTransportParent::RecvSetIceConfig( + nsTArray&& iceServers, const RTCIceTransportPolicy& icePolicy) { - nsresult rv = mImpl->mHandler->CreateIceCtx(name, iceServers, icePolicy); + nsresult rv = mImpl->mHandler->SetIceConfig(iceServers, icePolicy); if (NS_FAILED(rv)) { return ipc::IPCResult::Fail(WrapNotNull(this), __func__, - "MediaTransportHandler::Init failed"); + "MediaTransportHandler::SetIceConfig failed"); } return ipc::IPCResult::Ok(); } diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp b/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp index b60af0d5b5ae..d37dcc10c813 100644 --- a/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp +++ b/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp @@ -453,8 +453,9 @@ nsresult PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver, res = PeerConnectionCtx::InitializeGlobal(mThread); NS_ENSURE_SUCCESS(res, res); - res = mTransportHandler->CreateIceCtx("PC:" + GetName(), - aConfiguration.mIceServers, + mTransportHandler->CreateIceCtx("PC:" + GetName()); + + res = mTransportHandler->SetIceConfig(aConfiguration.mIceServers, aConfiguration.mIceTransportPolicy); if (NS_FAILED(res)) { CSFLogError(LOGTAG, "%s: Failed to init mtransport", __FUNCTION__); @@ -1997,6 +1998,11 @@ PeerConnectionImpl::Close() { return NS_OK; } +nsresult PeerConnectionImpl::SetConfiguration(const RTCConfiguration& aConfig) { + return mTransportHandler->SetIceConfig(aConfig.mIceServers, + aConfig.mIceTransportPolicy); +} + bool PeerConnectionImpl::PluginCrash(uint32_t aPluginID, const nsAString& aPluginName) { // fire an event to the DOM window if this is "ours" diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.h b/dom/media/webrtc/jsapi/PeerConnectionImpl.h index af2ed82164c3..b7ea32650ea2 100644 --- a/dom/media/webrtc/jsapi/PeerConnectionImpl.h +++ b/dom/media/webrtc/jsapi/PeerConnectionImpl.h @@ -385,6 +385,11 @@ class PeerConnectionImpl final MOZ_CAN_RUN_SCRIPT_BOUNDARY bool PluginCrash(uint32_t aPluginID, const nsAString& aPluginName); + NS_IMETHODIMP_TO_ERRORRESULT(SetConfiguration, ErrorResult& rv, + const RTCConfiguration& aConfig) { + rv = SetConfiguration(aConfig); + } + void RecordEndOfCallTelemetry(); nsresult InitializeDataChannel(); diff --git a/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp b/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp index 17b1c2b10f73..20698ee0dd84 100644 --- a/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp +++ b/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp @@ -191,8 +191,9 @@ class LoopbackTransport : public MediaTransportHandler { void EnterPrivateMode() override {} void ExitPrivateMode() override {} - nsresult CreateIceCtx(const std::string& aName, - const nsTArray& aIceServers, + void CreateIceCtx(const std::string& aName) override {} + + nsresult SetIceConfig(const nsTArray& aIceServers, dom::RTCIceTransportPolicy aIcePolicy) override { return NS_OK; }