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
2022-02-08 23:37:56 +00:00 · 2022-02-08 23:37:56 +00:00 · 17426d80c0
--- 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<RTCIceServer>&& iceServers,
+  mozilla::ipc::IPCResult RecvCreateIceCtx(const string& name);
+  mozilla::ipc::IPCResult RecvSetIceConfig(
+      nsTArray<RTCIceServer>&& iceServers,
      const RTCIceTransportPolicy& icePolicy);
  mozilla::ipc::IPCResult RecvSetProxyConfig(
      const net::WebrtcProxyConfig& aProxyConfig);
--- 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);
--- 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<dom::RTCIceServer>& aIceServers,
+  void CreateIceCtx(const std::string& aName) override;
+
+  nsresult SetIceConfig(const nsTArray<dom::RTCIceServer>& 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<NrIceResolver> mDNSResolver;
  std::map<std::string, Transport> mTransports;
  bool mObfuscateHostAddresses = false;
+  bool mTurnDisabled = false;
  uint32_t mMinDtlsVersion = 0;
  uint32_t mMaxDtlsVersion = 0;
+  bool mForceNoHost = false;
+  Maybe<NrIceCtx::NatSimulatorConfig> mNatConfig;

  std::set<std::string> 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<NrIceCtx::NatSimulatorConfig> GetNatConfig() {
  return Nothing();
 }

-nsresult MediaTransportHandlerSTS::CreateIceCtx(
-    const std::string& aName, const nsTArray<dom::RTCIceServer>& aIceServers,
-    dom::RTCIceTransportPolicy aIcePolicy) {
-  // We rely on getting an error when this happens, so do it up front.
-  std::vector<NrIceStunServer> stunServers;
-  std::vector<NrIceTurnServer> 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<MediaTransportHandlerSTS>(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<MediaTransportHandlerSTS>(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<dom::RTCIceServer>& aIceServers,
+    dom::RTCIceTransportPolicy aIcePolicy) {
+  // We rely on getting an error when this happens, so do it up front.
+  std::vector<NrIceStunServer> stunServers;
+  std::vector<NrIceTurnServer> turnServers;
+  nsresult rv = ConvertIceServers(aIceServers, &stunServers, &turnServers);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  mInitPromise->Then(
+      mStsThread, __func__,
+      [=, self = RefPtr<MediaTransportHandlerSTS>(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;
 }

--- 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<dom::RTCIceServer>& aIceServers,
      std::vector<NrIceStunServer>* aStunServers,
@ -63,8 +64,9 @@ class MediaTransportHandler {
  virtual void EnterPrivateMode() = 0;
  virtual void ExitPrivateMode() = 0;

-  virtual nsresult CreateIceCtx(const std::string& aName,
-                                const nsTArray<dom::RTCIceServer>& aIceServers,
+  virtual void CreateIceCtx(const std::string& aName) = 0;
+
+  virtual nsresult SetIceConfig(const nsTArray<dom::RTCIceServer>& aIceServers,
                                dom::RTCIceTransportPolicy aIcePolicy) = 0;

  // We will probably be able to move the proxy lookup stuff into
--- 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<dom::RTCIceServer>& aIceServers,
-    dom::RTCIceTransportPolicy aIcePolicy) {
+void MediaTransportHandlerIPC::CreateIceCtx(const std::string& aName) {
  CSFLogDebug(LOGTAG, "MediaTransportHandlerIPC::CreateIceCtx start");
+
+  mInitPromise->Then(
+      mCallbackThread, __func__,
+      [=, self = RefPtr<MediaTransportHandlerIPC>(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<dom::RTCIceServer>& 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<MediaTransportHandlerIPC>(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__);
          }
        }
--- 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<dom::RTCIceServer>& aIceServers,
+  void CreateIceCtx(const std::string& aName) override;
+
+  nsresult SetIceConfig(const nsTArray<dom::RTCIceServer>& aIceServers,
                        dom::RTCIceTransportPolicy aIcePolicy) override;

  // We will probably be able to move the proxy lookup stuff into
--- 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<RTCIceServer>&& iceServers,
+    const string& name) {
+  mImpl->mHandler->CreateIceCtx(name);
+  return ipc::IPCResult::Ok();
+}
+
+mozilla::ipc::IPCResult MediaTransportParent::RecvSetIceConfig(
+    nsTArray<RTCIceServer>&& 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();
 }
--- 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"
--- 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();
--- 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<dom::RTCIceServer>& aIceServers,
+  void CreateIceCtx(const std::string& aName) override {}
+
+  nsresult SetIceConfig(const nsTArray<dom::RTCIceServer>& aIceServers,
                        dom::RTCIceTransportPolicy aIcePolicy) override {
    return NS_OK;
  }