Bug 1422215 - Do offer/answer validation sooner. r=drno, a=jcristau
authorByron Campen [:bwc] <docfaraday@gmail.com>
Tue, 05 Dec 2017 16:01:23 -0600
changeset 442807 1cc170b6689afc4fea4518128c59c72dcd88a122
parent 442806 f36855025582c0c6c88b41d3b207c462769ad839
child 442808 82930571e04c5a49d0d27c16c857f858fa254e73
push id8343
push userryanvm@gmail.com
push dateThu, 07 Dec 2017 13:36:26 +0000
treeherdermozilla-beta@1cc170b6689a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrno, jcristau
bugs1422215
milestone58.0
Bug 1422215 - Do offer/answer validation sooner. r=drno, a=jcristau
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -1193,16 +1193,29 @@ JsepSessionImpl::SetLocalDescription(Jse
   UniquePtr<Sdp> parsed;
   nsresult rv = ParseSdp(sdp, &parsed);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Check that content hasn't done anything unsupported with the SDP
   rv = ValidateLocalDescription(*parsed);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  switch (type) {
+    case kJsepSdpOffer:
+      rv = ValidateOffer(*parsed);
+      break;
+    case kJsepSdpAnswer:
+    case kJsepSdpPranswer:
+      rv = ValidateAnswer(*mPendingRemoteDescription, *parsed);
+      break;
+    case kJsepSdpRollback:
+      MOZ_CRASH(); // Handled above
+  }
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // Create transport objects.
   mOldTransports = mTransports; // Save in case we need to rollback
   mTransports.clear();
   for (size_t t = 0; t < parsed->GetMediaSectionCount(); ++t) {
     mTransports.push_back(RefPtr<JsepTransport>(new JsepTransport));
     InitTransport(parsed->GetMediaSection(t), mTransports[t].get());
   }
 
@@ -1232,22 +1245,18 @@ JsepSessionImpl::SetLocalDescriptionOffe
 
 nsresult
 JsepSessionImpl::SetLocalDescriptionAnswer(JsepSdpType type,
                                            UniquePtr<Sdp> answer)
 {
   MOZ_ASSERT(mState == kJsepStateHaveRemoteOffer);
   mPendingLocalDescription = Move(answer);
 
-  nsresult rv = ValidateAnswer(*mPendingRemoteDescription,
-                               *mPendingLocalDescription);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = HandleNegotiatedSession(mPendingLocalDescription,
-                               mPendingRemoteDescription);
+  nsresult rv = HandleNegotiatedSession(mPendingLocalDescription,
+                                        mPendingRemoteDescription);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mCurrentRemoteDescription = Move(mPendingRemoteDescription);
   mCurrentLocalDescription = Move(mPendingLocalDescription);
   mWasOffererLastTime = mIsOfferer;
 
   SetState(kJsepStateStable);
   return NS_OK;
@@ -1303,16 +1312,29 @@ JsepSessionImpl::SetRemoteDescription(Js
   // Parse.
   UniquePtr<Sdp> parsed;
   nsresult rv = ParseSdp(sdp, &parsed);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = ValidateRemoteDescription(*parsed);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  switch (type) {
+    case kJsepSdpOffer:
+      rv = ValidateOffer(*parsed);
+      break;
+    case kJsepSdpAnswer:
+    case kJsepSdpPranswer:
+      rv = ValidateAnswer(*mPendingLocalDescription, *parsed);
+      break;
+    case kJsepSdpRollback:
+      MOZ_CRASH(); // Handled above
+  }
+  NS_ENSURE_SUCCESS(rv, rv);
+
   bool iceLite =
       parsed->GetAttributeList().HasAttribute(SdpAttribute::kIceLiteAttribute);
 
   // check for mismatch ufrag/pwd indicating ice restart
   // can't just check the first one because it might be disabled
   bool iceRestarting = false;
   if (mCurrentRemoteDescription.get()) {
     for (size_t i = 0;
@@ -1778,22 +1800,19 @@ JsepSessionImpl::ParseSdp(const std::str
   return NS_OK;
 }
 
 nsresult
 JsepSessionImpl::SetRemoteDescriptionOffer(UniquePtr<Sdp> offer)
 {
   MOZ_ASSERT(mState == kJsepStateStable);
 
-  nsresult rv = ValidateOffer(*offer);
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // TODO(bug 1095780): Note that we create remote tracks even when
   // They contain only codecs we can't negotiate or other craziness.
-  rv = SetRemoteTracksFromDescription(offer.get());
+  nsresult rv = SetRemoteTracksFromDescription(offer.get());
   NS_ENSURE_SUCCESS(rv, rv);
 
   mPendingRemoteDescription = Move(offer);
 
   SetState(kJsepStateHaveRemoteOffer);
   return NS_OK;
 }
 
@@ -1801,23 +1820,19 @@ nsresult
 JsepSessionImpl::SetRemoteDescriptionAnswer(JsepSdpType type,
                                             UniquePtr<Sdp> answer)
 {
   MOZ_ASSERT(mState == kJsepStateHaveLocalOffer ||
              mState == kJsepStateHaveRemotePranswer);
 
   mPendingRemoteDescription = Move(answer);
 
-  nsresult rv = ValidateAnswer(*mPendingLocalDescription,
-                               *mPendingRemoteDescription);
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // TODO(bug 1095780): Note that this creates remote tracks even if
   // we offered sendonly and other side offered sendrecv or recvonly.
-  rv = SetRemoteTracksFromDescription(mPendingRemoteDescription.get());
+  nsresult rv = SetRemoteTracksFromDescription(mPendingRemoteDescription.get());
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = HandleNegotiatedSession(mPendingLocalDescription,
                                mPendingRemoteDescription);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mCurrentRemoteDescription = Move(mPendingRemoteDescription);
   mCurrentLocalDescription = Move(mPendingLocalDescription);