From 5af4144f3eab21bde4a3ffd1ce3969106262129f Mon Sep 17 00:00:00 2001
From: Byron Campen <docfaraday@gmail.com>
Date: Fri, 12 Aug 2022 16:29:07 +0000
Subject: [PATCH] Bug 1783765: Make sure we clear out mUncommittedJsepSession
 when sRD/sLD fails in JS. r=jib,webidl,smaug

Differential Revision: https://phabricator.services.mozilla.com/D154168
---
 dom/media/PeerConnection.jsm                  | 69 +++++++++++--------
 dom/media/webrtc/jsapi/PeerConnectionImpl.cpp |  4 ++
 dom/media/webrtc/jsapi/PeerConnectionImpl.h   |  2 +
 dom/webidl/PeerConnectionImpl.webidl          |  2 +
 4 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/dom/media/PeerConnection.jsm b/dom/media/PeerConnection.jsm
index f984854fc7ec..11fcf1f94b53 100644
--- a/dom/media/PeerConnection.jsm
+++ b/dom/media/PeerConnection.jsm
@@ -1101,12 +1101,18 @@ class RTCPeerConnection {
       } else {
         this._sanityCheckSdp(sdp);
       }
-      await new Promise((resolve, reject) => {
-        this._onSetDescriptionSuccess = resolve;
-        this._onSetDescriptionFailure = reject;
-        this._pc.setLocalDescription(this._actions[type], sdp);
-      });
-      await p;
+
+      try {
+        await new Promise((resolve, reject) => {
+          this._onSetDescriptionSuccess = resolve;
+          this._onSetDescriptionFailure = reject;
+          this._pc.setLocalDescription(this._actions[type], sdp);
+        });
+        await p;
+      } catch (e) {
+        this._pc.onSetDescriptionError();
+        throw e;
+      }
       await this._pc.onSetDescriptionSuccess(type, false);
     });
   }
@@ -1194,37 +1200,44 @@ class RTCPeerConnection {
     }
     this._checkClosed();
     return this._chain(async () => {
-      if (type == "offer" && this.signalingState == "have-local-offer") {
-        await new Promise((resolve, reject) => {
-          this._onSetDescriptionSuccess = resolve;
-          this._onSetDescriptionFailure = reject;
-          this._pc.setLocalDescription(Ci.IPeerConnection.kActionRollback, "");
-        });
-        await this._pc.onSetDescriptionSuccess("rollback", false);
-        this._updateCanTrickle();
-      }
+      try {
+        if (type == "offer" && this.signalingState == "have-local-offer") {
+          await new Promise((resolve, reject) => {
+            this._onSetDescriptionSuccess = resolve;
+            this._onSetDescriptionFailure = reject;
+            this._pc.setLocalDescription(
+              Ci.IPeerConnection.kActionRollback,
+              ""
+            );
+          });
+          await this._pc.onSetDescriptionSuccess("rollback", false);
+          this._updateCanTrickle();
+        }
 
-      if (this._closed) {
-        return;
-      }
+        if (this._closed) {
+          return;
+        }
 
-      const p = this._getPermission();
-
-      const haveSetRemote = (async () => {
         this._sanityCheckSdp(sdp);
-        await new Promise((resolve, reject) => {
+
+        const p = this._getPermission();
+
+        const haveSetRemote = new Promise((resolve, reject) => {
           this._onSetDescriptionSuccess = resolve;
           this._onSetDescriptionFailure = reject;
           this._pc.setRemoteDescription(this._actions[type], sdp);
         });
-      })();
 
-      if (type != "rollback") {
-        // Do setRemoteDescription and identity validation in parallel
-        await this._validateIdentity(sdp);
+        if (type != "rollback") {
+          // Do setRemoteDescription and identity validation in parallel
+          await this._validateIdentity(sdp);
+        }
+        await p;
+        await haveSetRemote;
+      } catch (e) {
+        this._pc.onSetDescriptionError();
+        throw e;
       }
-      await p;
-      await haveSetRemote;
 
       await this._pc.onSetDescriptionSuccess(type, true);
       this._updateCanTrickle();
diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp b/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp
index 0715a9fbd06f..fb3def839b20 100644
--- a/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp
+++ b/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp
@@ -2632,6 +2632,10 @@ void PeerConnectionImpl::DoSetDescriptionSuccessPostProcessing(
       }));
 }
 
+void PeerConnectionImpl::OnSetDescriptionError() {
+  mUncommittedJsepSession = nullptr;
+}
+
 RTCSignalingState PeerConnectionImpl::GetSignalingState() const {
   switch (mJsepSession->GetState()) {
     case kJsepStateStable:
diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.h b/dom/media/webrtc/jsapi/PeerConnectionImpl.h
index 7863c346195b..ed8af2152c4a 100644
--- a/dom/media/webrtc/jsapi/PeerConnectionImpl.h
+++ b/dom/media/webrtc/jsapi/PeerConnectionImpl.h
@@ -464,6 +464,8 @@ class PeerConnectionImpl final
   already_AddRefed<dom::Promise> OnSetDescriptionSuccess(
       dom::RTCSdpType aSdpType, bool aRemote, ErrorResult& aError);
 
+  void OnSetDescriptionError();
+
   bool IsClosed() const;
   // called when DTLS connects; we only need this once
   nsresult OnAlpnNegotiated(bool aPrivacyRequested);
diff --git a/dom/webidl/PeerConnectionImpl.webidl b/dom/webidl/PeerConnectionImpl.webidl
index 07909f0091fb..0649676f597b 100644
--- a/dom/webidl/PeerConnectionImpl.webidl
+++ b/dom/webidl/PeerConnectionImpl.webidl
@@ -89,6 +89,8 @@ interface PeerConnectionImpl  {
   [Throws]
   Promise<void> onSetDescriptionSuccess(RTCSdpType type, boolean remote);
 
+  void onSetDescriptionError();
+
   /* Attributes */
   /* This provides the implementation with the certificate it uses to
    * authenticate itself.  The JS side must set this before calling