Bug 1142105 - Part 3: Extract more SDP-related functionality out of JsepSessionImpl, and some readability improvements. r=mt
authorByron Campen [:bwc] <docfaraday@gmail.com>
Wed, 29 Jul 2015 13:10:24 -0500
changeset 287732 b1b3c99e1756a1e284609adef6e36470a893501f
parent 287731 2c18076d35765948328b80267bbd9d0a23e54381
child 287733 b9f133b1d7390f68a6afcf8c24aadd9c7b3bf7d1
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmt
bugs1142105
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1142105 - Part 3: Extract more SDP-related functionality out of JsepSessionImpl, and some readability improvements. r=mt
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.h
media/webrtc/signaling/src/sdp/SdpHelper.cpp
media/webrtc/signaling/src/sdp/SdpHelper.h
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -284,55 +284,55 @@ JsepSessionImpl::GetRemoteTracksAdded() 
 
 std::vector<RefPtr<JsepTrack>>
 JsepSessionImpl::GetRemoteTracksRemoved() const
 {
   return GetTracks(mRemoteTracksRemoved);
 }
 
 nsresult
-JsepSessionImpl::AddOfferMSections(const JsepOfferOptions& options, Sdp* sdp)
+JsepSessionImpl::SetupOfferMSections(const JsepOfferOptions& options, Sdp* sdp)
 {
   // First audio, then video, then datachannel, for interop
   // TODO(bug 1121756): We need to group these by stream-id, _then_ by media
   // type, according to the spec. However, this is not going to interop with
   // older versions of Firefox if a video-only stream is added before an
   // audio-only stream.
   // We should probably wait until 38 is ESR before trying to do this.
-  nsresult rv = AddOfferMSectionsByType(
+  nsresult rv = SetupOfferMSectionsByType(
       SdpMediaSection::kAudio, options.mOfferToReceiveAudio, sdp);
 
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = AddOfferMSectionsByType(
+  rv = SetupOfferMSectionsByType(
       SdpMediaSection::kVideo, options.mOfferToReceiveVideo, sdp);
 
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!(options.mDontOfferDataChannel.isSome() &&
         *options.mDontOfferDataChannel)) {
-    rv = AddOfferMSectionsByType(
+    rv = SetupOfferMSectionsByType(
         SdpMediaSection::kApplication, Maybe<size_t>(), sdp);
 
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   if (!sdp->GetMediaSectionCount()) {
     JSEP_SET_ERROR("Cannot create an offer with no local tracks, "
                    "no offerToReceiveAudio/Video, and no DataChannel.");
     return NS_ERROR_INVALID_ARG;
   }
 
   return NS_OK;
 }
 
 nsresult
-JsepSessionImpl::AddOfferMSectionsByType(SdpMediaSection::MediaType mediatype,
-                                         Maybe<size_t> offerToReceiveMaybe,
-                                         Sdp* sdp)
+JsepSessionImpl::SetupOfferMSectionsByType(SdpMediaSection::MediaType mediatype,
+                                           Maybe<size_t> offerToReceiveMaybe,
+                                           Sdp* sdp)
 {
   // Convert the Maybe into a size_t*, since that is more readable, especially
   // when using it as an in/out param.
   size_t offerToReceiveCount;
   size_t* offerToReceiveCountPtr = nullptr;
 
   if (offerToReceiveMaybe) {
     offerToReceiveCount = *offerToReceiveMaybe;
@@ -462,17 +462,17 @@ JsepSessionImpl::SetRecvAsNeededOrDisabl
       }
     } else if (msection.IsSending()) {
       msection.SetReceiving(true);
       continue;
     }
 
     if (!msection.IsSending()) {
       // Unused m-section, and no reason to offer to recv on it
-      DisableMsection(sdp, &msection);
+      mSdpHelper.DisableMsection(sdp, &msection);
     }
   }
 
   return NS_OK;
 }
 
 nsresult
 JsepSessionImpl::AddRecvonlyMsections(SdpMediaSection::MediaType mediatype,
@@ -486,40 +486,38 @@ JsepSessionImpl::AddRecvonlyMsections(Sd
     NS_ENSURE_SUCCESS(rv, rv);
   }
   return NS_OK;
 }
 
 // This function creates a skeleton SDP based on the old descriptions
 // (ie; all m-sections are inactive).
 nsresult
-JsepSessionImpl::CreateReoffer(const Sdp& oldLocalSdp,
-                               const Sdp& oldAnswer,
-                               Sdp* newSdp)
+JsepSessionImpl::AddReofferMsections(const Sdp& oldLocalSdp,
+                                     const Sdp& oldAnswer,
+                                     Sdp* newSdp)
 {
   nsresult rv;
 
   for (size_t i = 0; i < oldLocalSdp.GetMediaSectionCount(); ++i) {
     // We do not set the direction in this function (or disable when previously
-    // disabled), that happens in |AddOfferMSectionsByType|
+    // disabled), that happens in |SetupOfferMSectionsByType|
     rv = CreateOfferMSection(oldLocalSdp.GetMediaSection(i).GetMediaType(),
                              SdpDirectionAttribute::kInactive,
                              newSdp);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = mSdpHelper.CopyStickyParams(oldAnswer.GetMediaSection(i),
                                      &newSdp->GetMediaSection(i));
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
-// TODO(1142105): Do we teach SdpHelper about BundlePolicy, or are
-// we getting too much JSEP in there at that point?
 void
 JsepSessionImpl::SetupBundle(Sdp* sdp) const
 {
   std::vector<std::string> mids;
   std::set<SdpMediaSection::MediaType> observedTypes;
 
   // This has the effect of changing the bundle level if the first m-section
   // goes from disabled to enabled. This is kinda inefficient.
@@ -603,51 +601,52 @@ JsepSessionImpl::CreateOffer(const JsepO
 {
   mLastError.clear();
 
   if (mState != kJsepStateStable) {
     JSEP_SET_ERROR("Cannot create offer in state " << GetStateStr(mState));
     return NS_ERROR_UNEXPECTED;
   }
 
+  // Undo track assignments from a previous call to CreateOffer
+  // (ie; if the track has not been negotiated yet, it doesn't necessarily need
+  // to stay in the same m-section that it was in)
+  for (auto i = mLocalTracks.begin(); i != mLocalTracks.end(); ++i) {
+    if (!i->mNegotiated) {
+      i->mAssignedMLine.reset();
+    }
+  }
+
   UniquePtr<Sdp> sdp;
 
   // Make the basic SDP that is common to offer/answer.
   nsresult rv = CreateGenericSDP(&sdp);
   NS_ENSURE_SUCCESS(rv, rv);
-  ++mSessionVersion;
 
   if (mCurrentLocalDescription) {
-    rv = CreateReoffer(*mCurrentLocalDescription, *GetAnswer(), sdp.get());
+    rv = AddReofferMsections(*mCurrentLocalDescription,
+                             *GetAnswer(),
+                             sdp.get());
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  // Get rid of all m-line assignments that have not been negotiated
-  if (NS_SUCCEEDED(rv)) {
-    for (auto i = mLocalTracks.begin(); i != mLocalTracks.end(); ++i) {
-      if (!i->mNegotiated) {
-        i->mAssignedMLine.reset();
-      }
-    }
-  }
-
-  // Now add all the m-lines that we are attempting to negotiate.
-  rv = AddOfferMSections(options, sdp.get());
+  // Ensure that we have all the m-sections we need, and disable extras
+  rv = SetupOfferMSections(options, sdp.get());
   NS_ENSURE_SUCCESS(rv, rv);
 
   SetupBundle(sdp.get());
 
-  if (GetAnswer()) {
-    // We have an old answer from a previous negotiation
-    rv = SetupTransportParams(*GetAnswer(), *sdp, sdp.get());
+  if (mCurrentLocalDescription) {
+    rv = CopyPreviousTransportParams(*GetAnswer(), *sdp, sdp.get());
     NS_ENSURE_SUCCESS(rv,rv);
   }
 
   *offer = sdp->ToString();
   mGeneratedLocalDescription = Move(sdp);
+  ++mSessionVersion;
 
   return NS_OK;
 }
 
 std::string
 JsepSessionImpl::GetLocalDescription() const
 {
   std::ostringstream os;
@@ -769,46 +768,20 @@ JsepSessionImpl::GetCommonCodecs(const S
 
   return matchingCodecs;
 }
 
 void
 JsepSessionImpl::AddCommonExtmaps(const SdpMediaSection& remoteMsection,
                                   SdpMediaSection* msection)
 {
-  if (!remoteMsection.GetAttributeList().HasAttribute(
-        SdpAttribute::kExtmapAttribute)) {
-    return;
-  }
-
   auto* ourExtensions = GetRtpExtensions(remoteMsection.GetMediaType());
 
-  if (!ourExtensions) {
-    return;
-  }
-
-  UniquePtr<SdpExtmapAttributeList> ourExtmap(new SdpExtmapAttributeList);
-  auto& theirExtmap = remoteMsection.GetAttributeList().GetExtmap().mExtmaps;
-  for (auto i = theirExtmap.begin(); i != theirExtmap.end(); ++i) {
-    for (auto j = ourExtensions->begin(); j != ourExtensions->end(); ++j) {
-      if (i->extensionname == j->extensionname) {
-        ourExtmap->mExtmaps.push_back(*i);
-
-        // RFC 5285 says that ids >= 4096 can be used by the offerer to
-        // force the answerer to pick, otherwise the value in the offer is
-        // used.
-        if (ourExtmap->mExtmaps.back().entry >= 4096) {
-          ourExtmap->mExtmaps.back().entry = j->entry;
-        }
-      }
-    }
-  }
-
-  if (!ourExtmap->mExtmaps.empty()) {
-    msection->GetAttributeList().SetAttribute(ourExtmap.release());
+  if (ourExtensions) {
+    mSdpHelper.AddCommonExtmaps(remoteMsection, *ourExtensions, msection);
   }
 }
 
 nsresult
 JsepSessionImpl::CreateAnswer(const JsepAnswerOptions& options,
                               std::string* answer)
 {
   mLastError.clear();
@@ -831,18 +804,16 @@ JsepSessionImpl::CreateAnswer(const Jsep
   UniquePtr<Sdp> sdp;
 
   // Make the basic SDP that is common to offer/answer.
   nsresult rv = CreateGenericSDP(&sdp);
   NS_ENSURE_SUCCESS(rv, rv);
 
   const Sdp& offer = *mPendingRemoteDescription;
 
-  std::vector<SdpGroupAttributeList::Group> bundleGroups;
-
   // Copy the bundle groups into our answer
   UniquePtr<SdpGroupAttributeList> groupAttr(new SdpGroupAttributeList);
   mSdpHelper.GetBundleGroups(offer, &groupAttr->mGroups);
   sdp->GetAttributeList().SetAttribute(groupAttr.release());
 
   // Disable send for local tracks if the offer no longer allows it
   // (i.e., the m-section is recvonly, inactive or disabled)
   for (auto i = mLocalTracks.begin(); i != mLocalTracks.end(); ++i) {
@@ -864,19 +835,18 @@ JsepSessionImpl::CreateAnswer(const Jsep
   size_t numMsections = offer.GetMediaSectionCount();
 
   for (size_t i = 0; i < numMsections; ++i) {
     const SdpMediaSection& remoteMsection = offer.GetMediaSection(i);
     rv = CreateAnswerMSection(options, i, remoteMsection, sdp.get());
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  if (GetAnswer()) {
-    // We have an old answer from a previous negotiation
-    rv = SetupTransportParams(*GetAnswer(), *sdp, sdp.get());
+  if (mCurrentLocalDescription) {
+    rv = CopyPreviousTransportParams(*GetAnswer(), *sdp, sdp.get());
     NS_ENSURE_SUCCESS(rv,rv);
   }
 
   *answer = sdp->ToString();
   mGeneratedLocalDescription = Move(sdp);
 
   return NS_OK;
 }
@@ -948,17 +918,17 @@ JsepSessionImpl::CreateAnswerMSection(co
                            remoteMsection.GetProtocol(),
                            sdp::kIPv4,
                            "0.0.0.0");
 
   nsresult rv = mSdpHelper.CopyStickyParams(remoteMsection, &msection);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (mSdpHelper.MsectionIsDisabled(remoteMsection)) {
-    DisableMsection(sdp, &msection);
+    mSdpHelper.DisableMsection(sdp, &msection);
     return NS_OK;
   }
 
   SdpSetupAttribute::Role role;
   rv = DetermineAnswererSetupRole(remoteMsection, &role);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = AddTransportAttributes(&msection, role);
@@ -990,23 +960,23 @@ JsepSessionImpl::CreateAnswerMSection(co
       break;
     }
   }
 
   // Add extmap attributes.
   AddCommonExtmaps(remoteMsection, &msection);
 
   if (!msection.IsReceiving() && !msection.IsSending()) {
-    DisableMsection(sdp, &msection);
+    mSdpHelper.DisableMsection(sdp, &msection);
     return NS_OK;
   }
 
   if (msection.GetFormats().empty()) {
     // Could not negotiate anything. Disable m-section.
-    DisableMsection(sdp, &msection);
+    mSdpHelper.DisableMsection(sdp, &msection);
   }
 
   return NS_OK;
 }
 
 nsresult
 JsepSessionImpl::SetRecvonlySsrc(SdpMediaSection* msection)
 {
@@ -1153,17 +1123,17 @@ JsepSessionImpl::SetLocalDescription(Jse
   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);
 
   // Create transport objects.
-  mOldTransports = mTransports;
+  mOldTransports = mTransports; // Save in case we need to rollback
   for (size_t t = 0; t < parsed->GetMediaSectionCount(); ++t) {
     if (t >= mTransports.size()) {
       mTransports.push_back(RefPtr<JsepTransport>(new JsepTransport));
     }
 
     UpdateTransport(parsed->GetMediaSection(t), mTransports[t].get());
   }
 
@@ -1380,21 +1350,19 @@ JsepSessionImpl::HandleNegotiatedSession
   rv = SetUniquePayloadTypes();
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Ouch, this probably needs some dirty bit instead of just clearing
   // stuff for renegotiation.
   mNegotiatedTrackPairs = trackPairs;
 
   // Mark all assigned local tracks as negotiated
-  if (NS_SUCCEEDED(rv)) {
-    for (auto i = mLocalTracks.begin(); i != mLocalTracks.end(); ++i) {
-      if (i->mAssignedMLine.isSome()) {
-        i->mNegotiated = true;
-      }
+  for (auto i = mLocalTracks.begin(); i != mLocalTracks.end(); ++i) {
+    if (i->mAssignedMLine.isSome()) {
+      i->mNegotiated = true;
     }
   }
 
   mGeneratedLocalDescription.reset();
   return NS_OK;
 }
 
 nsresult
@@ -1674,19 +1642,19 @@ JsepSessionImpl::AddTransportAttributes(
       new SdpStringAttribute(SdpAttribute::kIcePwdAttribute, mIcePwd));
 
   msection->GetAttributeList().SetAttribute(new SdpSetupAttribute(dtlsRole));
 
   return NS_OK;
 }
 
 nsresult
-JsepSessionImpl::SetupTransportParams(const Sdp& oldAnswer,
-                                      const Sdp& newOffer,
-                                      Sdp* newLocal)
+JsepSessionImpl::CopyPreviousTransportParams(const Sdp& oldAnswer,
+                                             const Sdp& newOffer,
+                                             Sdp* newLocal)
 {
   for (size_t i = 0; i < oldAnswer.GetMediaSectionCount(); ++i) {
     if (!mSdpHelper.MsectionIsDisabled(newLocal->GetMediaSection(i)) &&
         mSdpHelper.AreOldTransportParamsValid(oldAnswer, newOffer, i)) {
       // If newLocal is an offer, this will be the number of components we used
       // last time, and if it is an answer, this will be the number of
       // components we've decided we're using now.
       size_t numComponents = mTransports[i]->mComponents;
@@ -2363,110 +2331,59 @@ JsepSessionImpl::EndOfLocalCandidates(co
     JSEP_SET_ERROR("Cannot add ICE candidate in state " << GetStateStr(mState));
     return NS_ERROR_UNEXPECTED;
   }
 
   if (level >= sdp->GetMediaSectionCount()) {
     return NS_OK;
   }
 
-  SdpHelper::BundledMids bundledMids;
-  nsresult rv = GetNegotiatedBundledMids(&bundledMids);
-  if (NS_FAILED(rv)) {
-    MOZ_ASSERT(false);
-    mLastError += " (This should have been caught sooner!)";
-    return NS_ERROR_FAILURE;
-  }
-
   std::string defaultRtcpCandidateAddrCopy(defaultRtcpCandidateAddr);
   if (mState == kJsepStateStable && mTransports[level]->mComponents == 1) {
     // We know we're doing rtcp-mux by now. Don't create an rtcp attr.
     defaultRtcpCandidateAddrCopy = "";
     defaultRtcpCandidatePort = 0;
   }
 
-  SdpMediaSection& msection = sdp->GetMediaSection(level);
-
-  // TODO(bug 1142105): Factor some of this out into a helper class
+  // If offer/answer isn't done, it is too early to tell whether these defaults
+  // need to be applied to other m-sections.
+  SdpHelper::BundledMids bundledMids;
   if (mState == kJsepStateStable) {
-    // offer/answer is done. Do we actually incorporate these defaults?
-    if (msection.GetAttributeList().HasAttribute(SdpAttribute::kMidAttribute)) {
-      std::string mid(msection.GetAttributeList().GetMid());
-      if (bundledMids.count(mid)) {
-        const SdpMediaSection* masterBundleMsection(bundledMids[mid]);
-        if (msection.GetLevel() != masterBundleMsection->GetLevel()) {
-          // Slave bundle m-section. Skip.
-          return NS_OK;
-        }
-
-        // Master bundle m-section. Set defaultCandidateAddr and
-        // defaultCandidatePort on all bundled m-sections.
-        for (auto i = bundledMids.begin(); i != bundledMids.end(); ++i) {
-          if (i->second != masterBundleMsection) {
-            continue;
-          }
-          SdpMediaSection* bundledMsection =
-            mSdpHelper.FindMsectionByMid(*sdp, i->first);
-          if (!bundledMsection) {
-            MOZ_ASSERT(false);
-            continue;
-          }
-          mSdpHelper.SetDefaultAddresses(defaultCandidateAddr,
-                                         defaultCandidatePort,
-                                         defaultRtcpCandidateAddrCopy,
-                                         defaultRtcpCandidatePort,
-                                         bundledMsection);
-        }
-      }
+    nsresult rv = GetNegotiatedBundledMids(&bundledMids);
+    if (NS_FAILED(rv)) {
+      MOZ_ASSERT(false);
+      mLastError += " (This should have been caught sooner!)";
+      return NS_ERROR_FAILURE;
     }
   }
 
-  mSdpHelper.SetDefaultAddresses(defaultCandidateAddr,
-                                 defaultCandidatePort,
-                                 defaultRtcpCandidateAddrCopy,
-                                 defaultRtcpCandidatePort,
-                                 &msection);
-
-  // TODO(bug 1095793): Will this have an mid someday?
-
-  SdpAttributeList& attrs = msection.GetAttributeList();
-  attrs.SetAttribute(
-      new SdpFlagAttribute(SdpAttribute::kEndOfCandidatesAttribute));
-  if (!mIsOfferer) {
-    attrs.RemoveAttribute(SdpAttribute::kIceOptionsAttribute);
-  }
+  mSdpHelper.SetDefaultAddresses(
+      defaultCandidateAddr,
+      defaultCandidatePort,
+      defaultRtcpCandidateAddrCopy,
+      defaultRtcpCandidatePort,
+      sdp,
+      level,
+      bundledMids);
 
   return NS_OK;
 }
 
 nsresult
 JsepSessionImpl::GetNegotiatedBundledMids(SdpHelper::BundledMids* bundledMids)
 {
   const Sdp* answerSdp = GetAnswer();
 
   if (!answerSdp) {
     return NS_OK;
   }
 
   return mSdpHelper.GetBundledMids(*answerSdp, bundledMids);
 }
 
-void
-JsepSessionImpl::DisableMsection(Sdp* sdp, SdpMediaSection* msection) const
-{
-  mSdpHelper.DisableMsection(sdp, msection);
-  msection->ClearCodecs();
-
-  // We need to have something here to fit the grammar
-  // TODO(bcampen@mozilla.com): What's the accepted way of doing this? Just
-  // add the codecs we do support? Does it matter? Most endpoints just pick
-  // something that the other end supported.
-  msection->AddCodec("111", "NULL", 0, 0);
-}
-
 nsresult
 JsepSessionImpl::EnableOfferMsection(SdpMediaSection* msection)
 {
   // We assert here because adding rtcp-mux to a non-disabled m-section that
   // did not already have rtcp-mux can cause problems.
   MOZ_ASSERT(mSdpHelper.MsectionIsDisabled(*msection));
 
   msection->SetPort(9);
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
@@ -212,40 +212,40 @@ private:
   nsresult CreateReceivingTrack(size_t mline,
                                 const Sdp& sdp,
                                 const SdpMediaSection& msection,
                                 RefPtr<JsepTrack>* track);
   nsresult HandleNegotiatedSession(const UniquePtr<Sdp>& local,
                                    const UniquePtr<Sdp>& remote);
   nsresult AddTransportAttributes(SdpMediaSection* msection,
                                   SdpSetupAttribute::Role dtlsRole);
-  nsresult SetupTransportParams(const Sdp& oldAnswer,
-                                const Sdp& newOffer,
-                                Sdp* newLocal);
-  nsresult AddOfferMSections(const JsepOfferOptions& options, Sdp* sdp);
+  nsresult CopyPreviousTransportParams(const Sdp& oldAnswer,
+                                       const Sdp& newOffer,
+                                       Sdp* newLocal);
+  nsresult SetupOfferMSections(const JsepOfferOptions& options, Sdp* sdp);
   // Non-const so it can assign m-line index to tracks
-  nsresult AddOfferMSectionsByType(SdpMediaSection::MediaType type,
-                                   Maybe<size_t> offerToReceive,
-                                   Sdp* sdp);
+  nsresult SetupOfferMSectionsByType(SdpMediaSection::MediaType type,
+                                     Maybe<size_t> offerToReceive,
+                                     Sdp* sdp);
   nsresult BindLocalTracks(SdpMediaSection::MediaType mediatype,
                            Sdp* sdp);
   nsresult BindTrackToMsection(JsepSendingTrack* track,
                                SdpMediaSection* msection);
   nsresult EnsureRecvForRemoteTracks(SdpMediaSection::MediaType mediatype,
                                      Sdp* sdp,
                                      size_t* offerToReceive);
   nsresult SetRecvAsNeededOrDisable(SdpMediaSection::MediaType mediatype,
                                     Sdp* sdp,
                                     size_t* offerToRecv);
   nsresult AddRecvonlyMsections(SdpMediaSection::MediaType mediatype,
                                 size_t count,
                                 Sdp* sdp);
-  nsresult CreateReoffer(const Sdp& oldLocalSdp,
-                         const Sdp& oldAnswer,
-                         Sdp* newSdp);
+  nsresult AddReofferMsections(const Sdp& oldLocalSdp,
+                               const Sdp& oldAnswer,
+                               Sdp* newSdp);
   void SetupBundle(Sdp* sdp) const;
   nsresult GetRemoteIds(const Sdp& sdp,
                         const SdpMediaSection& msection,
                         std::string* streamId,
                         std::string* trackId);
   nsresult CreateOfferMSection(SdpMediaSection::MediaType type,
                                SdpDirectionAttribute::Direction direction,
                                Sdp* sdp);
@@ -275,17 +275,16 @@ private:
                        JsepTransport* transport);
 
   nsresult FinalizeTransport(const SdpAttributeList& remote,
                              const SdpAttributeList& answer,
                              const RefPtr<JsepTransport>& transport);
 
   nsresult GetNegotiatedBundledMids(SdpHelper::BundledMids* bundledMids);
 
-  void DisableMsection(Sdp* sdp, SdpMediaSection* msection) const;
   nsresult EnableOfferMsection(SdpMediaSection* msection);
 
   nsresult SetUniquePayloadTypes();
   nsresult GetAllPayloadTypes(const JsepTrackNegotiatedDetails& trackDetails,
                               std::vector<uint8_t>* payloadTypesOut);
   const Sdp* GetAnswer() const;
 
   std::vector<JsepSendingTrack> mLocalTracks;
--- a/media/webrtc/signaling/src/sdp/SdpHelper.cpp
+++ b/media/webrtc/signaling/src/sdp/SdpHelper.cpp
@@ -133,16 +133,21 @@ SdpHelper::DisableMsection(Sdp* sdp, Sdp
 
   // Clear out attributes.
   msection->GetAttributeList().Clear();
 
   auto* direction =
     new SdpDirectionAttribute(SdpDirectionAttribute::kInactive);
   msection->GetAttributeList().SetAttribute(direction);
   msection->SetPort(0);
+
+  msection->ClearCodecs();
+
+  // We need to have something here to fit the grammar, this seems safe.
+  msection->AddCodec("0", "PCMU", 8000, 1);
 }
 
 void
 SdpHelper::GetBundleGroups(
     const Sdp& sdp,
     std::vector<SdpGroupAttributeList::Group>* bundleGroups) const
 {
   if (sdp.GetAttributeList().HasAttribute(SdpAttribute::kGroupAttribute)) {
@@ -266,16 +271,71 @@ SdpHelper::AddCandidateToSdp(Sdp* sdp,
   return NS_OK;
 }
 
 void
 SdpHelper::SetDefaultAddresses(const std::string& defaultCandidateAddr,
                                uint16_t defaultCandidatePort,
                                const std::string& defaultRtcpCandidateAddr,
                                uint16_t defaultRtcpCandidatePort,
+                               Sdp* sdp,
+                               uint16_t level,
+                               BundledMids bundledMids)
+{
+  SdpMediaSection& msection = sdp->GetMediaSection(level);
+
+  if (msection.GetAttributeList().HasAttribute(SdpAttribute::kMidAttribute)) {
+    std::string mid(msection.GetAttributeList().GetMid());
+    if (bundledMids.count(mid)) {
+      const SdpMediaSection* masterBundleMsection(bundledMids[mid]);
+      if (msection.GetLevel() != masterBundleMsection->GetLevel()) {
+        // Slave bundle m-section. Skip.
+        return;
+      }
+
+      // Master bundle m-section. Set defaultCandidateAddr and
+      // defaultCandidatePort on all bundled m-sections.
+      for (auto i = bundledMids.begin(); i != bundledMids.end(); ++i) {
+        if (i->second != masterBundleMsection) {
+          continue;
+        }
+        SdpMediaSection* bundledMsection = FindMsectionByMid(*sdp, i->first);
+        if (!bundledMsection) {
+          MOZ_ASSERT(false);
+          continue;
+        }
+        SetDefaultAddresses(defaultCandidateAddr,
+                            defaultCandidatePort,
+                            defaultRtcpCandidateAddr,
+                            defaultRtcpCandidatePort,
+                            bundledMsection);
+      }
+    }
+  }
+
+  SetDefaultAddresses(defaultCandidateAddr,
+                      defaultCandidatePort,
+                      defaultRtcpCandidateAddr,
+                      defaultRtcpCandidatePort,
+                      &msection);
+
+  // TODO(bug 1095793): Will this have an mid someday?
+
+  SdpAttributeList& attrs = msection.GetAttributeList();
+  attrs.SetAttribute(
+      new SdpFlagAttribute(SdpAttribute::kEndOfCandidatesAttribute));
+  // Remove trickle-ice option
+  attrs.RemoveAttribute(SdpAttribute::kIceOptionsAttribute);
+}
+
+void
+SdpHelper::SetDefaultAddresses(const std::string& defaultCandidateAddr,
+                               uint16_t defaultCandidatePort,
+                               const std::string& defaultRtcpCandidateAddr,
+                               uint16_t defaultRtcpCandidatePort,
                                SdpMediaSection* msection)
 {
   msection->GetConnection().SetAddress(defaultCandidateAddr);
   msection->SetPort(defaultCandidatePort);
 
   if (!defaultRtcpCandidateAddr.empty()) {
     sdp::AddrType ipVersion = sdp::kIPv4;
     if (defaultRtcpCandidateAddr.find(':') != std::string::npos) {
@@ -599,11 +659,44 @@ SdpHelper::GetPtAsInt(const std::string&
   size_t length = static_cast<size_t>(end - ptString.c_str());
   if ((pt > UINT16_MAX) || (length != ptString.size())) {
     return false;
   }
   *ptOutparam = pt;
   return true;
 }
 
+void
+SdpHelper::AddCommonExtmaps(
+    const SdpMediaSection& remoteMsection,
+    const std::vector<SdpExtmapAttributeList::Extmap>& localExtensions,
+    SdpMediaSection* localMsection)
+{
+  if (!remoteMsection.GetAttributeList().HasAttribute(
+        SdpAttribute::kExtmapAttribute)) {
+    return;
+  }
+
+  UniquePtr<SdpExtmapAttributeList> localExtmap(new SdpExtmapAttributeList);
+  auto& theirExtmap = remoteMsection.GetAttributeList().GetExtmap().mExtmaps;
+  for (auto i = theirExtmap.begin(); i != theirExtmap.end(); ++i) {
+    for (auto j = localExtensions.begin(); j != localExtensions.end(); ++j) {
+      if (i->extensionname == j->extensionname) {
+        localExtmap->mExtmaps.push_back(*i);
+
+        // RFC 5285 says that ids >= 4096 can be used by the offerer to
+        // force the answerer to pick, otherwise the value in the offer is
+        // used.
+        if (localExtmap->mExtmaps.back().entry >= 4096) {
+          localExtmap->mExtmaps.back().entry = j->entry;
+        }
+      }
+    }
+  }
+
+  if (!localExtmap->mExtmaps.empty()) {
+    localMsection->GetAttributeList().SetAttribute(localExtmap.release());
+  }
+}
+
 } // namespace mozilla
 
 
--- a/media/webrtc/signaling/src/sdp/SdpHelper.h
+++ b/media/webrtc/signaling/src/sdp/SdpHelper.h
@@ -61,16 +61,23 @@ class SdpHelper {
     nsresult AddCandidateToSdp(Sdp* sdp,
                                const std::string& candidate,
                                const std::string& mid,
                                uint16_t level);
     void SetDefaultAddresses(const std::string& defaultCandidateAddr,
                              uint16_t defaultCandidatePort,
                              const std::string& defaultRtcpCandidateAddr,
                              uint16_t defaultRtcpCandidatePort,
+                             Sdp* sdp,
+                             uint16_t level,
+                             BundledMids bundledMids);
+    void SetDefaultAddresses(const std::string& defaultCandidateAddr,
+                             uint16_t defaultCandidatePort,
+                             const std::string& defaultRtcpCandidateAddr,
+                             uint16_t defaultRtcpCandidatePort,
                              SdpMediaSection* msection);
     void SetupMsidSemantic(const std::vector<std::string>& msids,
                            Sdp* sdp) const;
 
     std::string GetCNAME(const SdpMediaSection& msection) const;
     void SetSsrcs(const std::vector<uint32_t>& ssrcs,
                   const std::string& cname,
                   SdpMediaSection* msection) const;
@@ -87,16 +94,21 @@ class SdpHelper {
     SdpMediaSection::Protocol GetProtocolForMediaType(
         SdpMediaSection::MediaType type);
     void appendSdpParseErrors(
           const std::vector<std::pair<size_t, std::string> >& aErrors,
           std::string* aErrorString);
 
     static bool GetPtAsInt(const std::string& ptString, uint16_t* ptOutparam);
 
+    void AddCommonExtmaps(
+        const SdpMediaSection& remoteMsection,
+        const std::vector<SdpExtmapAttributeList::Extmap>& localExtensions,
+        SdpMediaSection* localMsection);
+
   private:
     std::string& mLastError;
 
     DISALLOW_COPY_ASSIGN(SdpHelper);
 };
 } // namespace mozilla
 
 #endif // _SDPHELPER_H_