Bug 1253706: Move initial configuration setting into a setConfiguration call. r=mjf,emilio

Having a single codepath for this should make things easier to maintain.

Differential Revision: https://phabricator.services.mozilla.com/D135366

dom/media/PeerConnection.jsm

@@ -528,10 +528,6 @@ class RTCPeerConnection {
       );
     }
 
-    this._validateConfig(rtcConfig);
-    this._config = Object.assign({}, rtcConfig);
-    this._applyPrefsToConfig(rtcConfig);
-
     this._documentPrincipal = Cu.getWebIDLCallerPrincipal();
 
     if (_globalPCList._networkdown) {
@@ -571,12 +567,9 @@ class RTCPeerConnection {
     // Add a reference to the PeerConnection to global list (before init).
     _globalPCList.addPC(this);
 
-    this._impl.initialize(
-      observer,
-      this._win,
-      rtcConfig,
-      Services.tm.currentThread
-    );
+    this._impl.initialize(observer, this._win, Services.tm.currentThread);
+
+    this.setConfiguration(rtcConfig);
 
     this._certificateReady = this._initCertificate(certificate);
     this._initIdp();
@@ -603,12 +596,14 @@ class RTCPeerConnection {
     this._checkClosed();
     this._validateConfig(rtcConfig);
     this._checkIfIceRestartRequired(rtcConfig);
-    this._config = Object.assign({}, rtcConfig);
 
     // Allow prefs to tweak these settings before passing to c++, but hide all
     // of that from JS.
-    this._applyPrefsToConfig(rtcConfig);
-    this._impl.setConfiguration(rtcConfig);
+    const configWithPrefTweaks = Object.assign({}, rtcConfig);
+    this._applyPrefsToConfig(configWithPrefTweaks);
+    this._impl.setConfiguration(configWithPrefTweaks);
+
+    this._config = Object.assign({}, rtcConfig);
   }
 
   async _initCertificate(certificate) {

dom/media/webrtc/jsapi/PeerConnectionImpl.cpp

@@ -335,6 +335,8 @@ PeerConnectionImpl::PeerConnectionImpl(const GlobalObject* aGlobal)
       Preferences::GetBool("media.peerconnection.ice.force_ice_tcp", false);
   memset(mMaxReceiving, 0, sizeof(mMaxReceiving));
   memset(mMaxSending, 0, sizeof(mMaxSending));
+  mJsConfiguration.mCertificatesProvided = false;
+  mJsConfiguration.mPeerIdentityProvided = false;
 }
 
 PeerConnectionImpl::~PeerConnectionImpl() {
@@ -379,7 +381,6 @@ PeerConnectionImpl::~PeerConnectionImpl() {
 
 nsresult PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver,
                                         nsGlobalWindowInner* aWindow,
-                                        const RTCConfiguration& aConfiguration,
                                         nsISupports* aThread) {
   nsresult res;
 
@@ -391,9 +392,6 @@ nsresult PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver,
   }
   CheckThread();
 
-  // Store the configuration for about:webrtc
-  StoreConfigurationForAboutWebrtc(aConfiguration);
-
   mPCObserver = &aObserver;
 
   // Find the STS thread
@@ -455,13 +453,6 @@ nsresult PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver,
 
   mTransportHandler->CreateIceCtx("PC:" + GetName());
 
-  res = mTransportHandler->SetIceConfig(aConfiguration.mIceServers,
-                                        aConfiguration.mIceTransportPolicy);
-  if (NS_FAILED(res)) {
-    CSFLogError(LOGTAG, "%s: Failed to init mtransport", __FUNCTION__);
-    return NS_ERROR_FAILURE;
-  }
-
   mJsepSession =
       MakeUnique<JsepSessionImpl>(mName, MakeUnique<PCUuidGenerator>());
   mJsepSession->SetRtxIsAllowed(mRtxIsAllowed);
@@ -473,29 +464,6 @@ nsresult PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver,
     return res;
   }
 
-  JsepBundlePolicy bundlePolicy;
-  switch (aConfiguration.mBundlePolicy) {
-    case dom::RTCBundlePolicy::Balanced:
-      bundlePolicy = kBundleBalanced;
-      break;
-    case dom::RTCBundlePolicy::Max_compat:
-      bundlePolicy = kBundleMaxCompat;
-      break;
-    case dom::RTCBundlePolicy::Max_bundle:
-      bundlePolicy = kBundleMaxBundle;
-      break;
-    default:
-      MOZ_CRASH();
-  }
-
-  res = mJsepSession->SetBundlePolicy(bundlePolicy);
-  if (NS_FAILED(res)) {
-    CSFLogError(LOGTAG, "%s: Couldn't set bundle policy, res=%u, error=%s",
-                __FUNCTION__, static_cast<unsigned>(res),
-                mJsepSession->GetLastError().c_str());
-    return res;
-  }
-
   mMedia = new PeerConnectionMedia(this);
 
   // Initialize the media object.
@@ -513,22 +481,16 @@ nsresult PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver,
 
 void PeerConnectionImpl::Initialize(PeerConnectionObserver& aObserver,
                                     nsGlobalWindowInner& aWindow,
-                                    const RTCConfiguration& aConfiguration,
                                     nsISupports* aThread, ErrorResult& rv) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aThread);
   mThread = do_QueryInterface(aThread);
 
-  nsresult res = Initialize(aObserver, &aWindow, aConfiguration, aThread);
+  nsresult res = Initialize(aObserver, &aWindow, aThread);
   if (NS_FAILED(res)) {
     rv.Throw(res);
     return;
   }
-
-  if (!aConfiguration.mPeerIdentity.IsEmpty()) {
-    mPeerIdentity = new PeerIdentity(aConfiguration.mPeerIdentity);
-    mPrivacyRequested = Some(true);
-  }
 }
 
 void PeerConnectionImpl::SetCertificate(
@@ -1998,9 +1960,45 @@ PeerConnectionImpl::Close() {
   return NS_OK;
 }
 
-nsresult PeerConnectionImpl::SetConfiguration(const RTCConfiguration& aConfig) {
-  return mTransportHandler->SetIceConfig(aConfig.mIceServers,
-                                         aConfig.mIceTransportPolicy);
+nsresult PeerConnectionImpl::SetConfiguration(
+    const RTCConfiguration& aConfiguration) {
+  nsresult rv = mTransportHandler->SetIceConfig(
+      aConfiguration.mIceServers, aConfiguration.mIceTransportPolicy);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  JsepBundlePolicy bundlePolicy;
+  switch (aConfiguration.mBundlePolicy) {
+    case dom::RTCBundlePolicy::Balanced:
+      bundlePolicy = kBundleBalanced;
+      break;
+    case dom::RTCBundlePolicy::Max_compat:
+      bundlePolicy = kBundleMaxCompat;
+      break;
+    case dom::RTCBundlePolicy::Max_bundle:
+      bundlePolicy = kBundleMaxBundle;
+      break;
+    default:
+      MOZ_CRASH();
+  }
+
+  rv = mJsepSession->SetBundlePolicy(bundlePolicy);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    CSFLogError(LOGTAG, "%s: Couldn't set bundle policy, res=%u, error=%s",
+                __FUNCTION__, static_cast<unsigned>(rv),
+                mJsepSession->GetLastError().c_str());
+    return rv;
+  }
+
+  if (!aConfiguration.mPeerIdentity.IsEmpty()) {
+    mPeerIdentity = new PeerIdentity(aConfiguration.mPeerIdentity);
+    mPrivacyRequested = Some(true);
+  }
+
+  // Store the configuration for about:webrtc
+  StoreConfigurationForAboutWebrtc(aConfiguration);
+  return NS_OK;
 }
 
 bool PeerConnectionImpl::PluginCrash(uint32_t aPluginID,
@@ -3151,6 +3149,7 @@ void PeerConnectionImpl::StoreConfigurationForAboutWebrtc(
   // configured, at least until setConfiguration is implemented
   // see https://bugzilla.mozilla.org/show_bug.cgi?id=1253706
   // @TODO bug 1739451 call this from setConfiguration
+  mJsConfiguration.mIceServers.Clear();
   for (const auto& server : aConfig.mIceServers) {
     RTCIceServerInternal internal;
     internal.mCredentialProvided = server.mCredential.WasPassed();
@@ -3171,11 +3170,14 @@ void PeerConnectionImpl::StoreConfigurationForAboutWebrtc(
       mozalloc_handle_oom(0);
     }
   }
+  mJsConfiguration.mSdpSemantics.Reset();
   if (aConfig.mSdpSemantics.WasPassed()) {
     mJsConfiguration.mSdpSemantics.Construct(aConfig.mSdpSemantics.Value());
   }
 
+  mJsConfiguration.mIceTransportPolicy.Reset();
   mJsConfiguration.mIceTransportPolicy.Construct(aConfig.mIceTransportPolicy);
+  mJsConfiguration.mBundlePolicy.Reset();
   mJsConfiguration.mBundlePolicy.Construct(aConfig.mBundlePolicy);
   mJsConfiguration.mPeerIdentityProvided = !aConfig.mPeerIdentity.IsEmpty();
   mJsConfiguration.mCertificatesProvided = !aConfig.mCertificates.Length();

dom/media/webrtc/jsapi/PeerConnectionImpl.h

@@ -219,14 +219,11 @@ class PeerConnectionImpl final
   }
 
   nsresult Initialize(PeerConnectionObserver& aObserver,
-                      nsGlobalWindowInner* aWindow,
-                      const RTCConfiguration& aConfiguration,
-                      nsISupports* aThread);
+                      nsGlobalWindowInner* aWindow, nsISupports* aThread);
 
   // Initialize PeerConnection from an RTCConfiguration object (JS entrypoint)
   void Initialize(PeerConnectionObserver& aObserver,
-                  nsGlobalWindowInner& aWindow,
-                  const RTCConfiguration& aConfiguration, nsISupports* aThread,
+                  nsGlobalWindowInner& aWindow, nsISupports* aThread,
                   ErrorResult& rv);
 
   void SetCertificate(mozilla::dom::RTCCertificate& aCertificate);
@@ -386,8 +383,8 @@ class PeerConnectionImpl final
                                                const nsAString& aPluginName);
 
   NS_IMETHODIMP_TO_ERRORRESULT(SetConfiguration, ErrorResult& rv,
-                               const RTCConfiguration& aConfig) {
-    rv = SetConfiguration(aConfig);
+                               const RTCConfiguration& aConfiguration) {
+    rv = SetConfiguration(aConfiguration);
   }
 
   void RecordEndOfCallTelemetry();

dom/media/webrtc/jsep/JsepSessionImpl.cpp

@@ -130,6 +130,11 @@ nsresult JsepSessionImpl::AddTransceiver(RefPtr<JsepTransceiver> transceiver) {
 
 nsresult JsepSessionImpl::SetBundlePolicy(JsepBundlePolicy policy) {
   mLastError.clear();
+
+  if (mBundlePolicy == policy) {
+    return NS_OK;
+  }
+
   if (mCurrentLocalDescription) {
     JSEP_SET_ERROR(
         "Changing the bundle policy is only supported before the "

dom/webidl/PeerConnectionImpl.webidl

@@ -24,7 +24,6 @@ interface PeerConnectionImpl  {
   /* Must be called first. Observer events dispatched on the thread provided */
   [Throws]
   void initialize(PeerConnectionObserver observer, Window window,
-                  RTCConfiguration iceServers,
                   nsISupports thread);
 
   /* JSEP calls */