Bug 1208371 - Break PCImpl::SetRemoteDescription into smaller pieces. r?mt,bwc draft
authorAndreas Pehrson <pehrsons@gmail.com>
Fri, 22 Jan 2016 14:38:50 +0800
changeset 342109 d82b0d2f9bec7f922a48fd7634d3816ead72f471
parent 342108 78d4cebecf0d9195b7d10b2142f409a42ae337f1
child 342110 cb01ba3ab18e534eb09d75415c117b1b8961caed
push id13352
push userpehrsons@gmail.com
push dateFri, 18 Mar 2016 13:49:47 +0000
reviewersmt, bwc
bugs1208371
milestone47.0a1
Bug 1208371 - Break PCImpl::SetRemoteDescription into smaller pieces. r?mt,bwc MozReview-Commit-ID: KzhcdLv4uen
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -1731,16 +1731,162 @@ static void DeferredSetRemote(const std:
     if (!PeerConnectionCtx::GetInstance()->isReady()) {
       MOZ_CRASH("Why is DeferredSetRemote being executed when the "
                 "PeerConnectionCtx isn't ready?");
     }
     wrapper.impl()->SetRemoteDescription(aAction, aSdp.c_str());
   }
 }
 
+nsresult
+PeerConnectionImpl::CreateNewRemoteTracks(RefPtr<PeerConnectionObserver>& aPco)
+{
+  JSErrorResult jrv;
+
+  std::vector<RefPtr<JsepTrack>> newTracks =
+    mJsepSession->GetRemoteTracksAdded();
+
+  // Group new tracks by stream id
+  std::map<std::string, std::vector<RefPtr<JsepTrack>>> tracksByStreamId;
+  for (auto i = newTracks.begin(); i != newTracks.end(); ++i) {
+    RefPtr<JsepTrack> track = *i;
+
+    if (track->GetMediaType() == mozilla::SdpMediaSection::kApplication) {
+      // Ignore datachannel
+      continue;
+    }
+
+    tracksByStreamId[track->GetStreamId()].push_back(track);
+  }
+
+  for (auto i = tracksByStreamId.begin(); i != tracksByStreamId.end(); ++i) {
+    std::string streamId = i->first;
+    std::vector<RefPtr<JsepTrack>>& tracks = i->second;
+
+    RefPtr<RemoteSourceStreamInfo> info =
+      mMedia->GetRemoteStreamById(streamId);
+    if (!info) {
+      nsresult nrv = CreateRemoteSourceStreamInfo(&info, streamId);
+      if (NS_FAILED(nrv)) {
+        aPco->OnSetRemoteDescriptionError(
+            kInternalError,
+            ObString("CreateRemoteSourceStreamInfo failed"),
+            jrv);
+        return nrv;
+      }
+
+      nrv = mMedia->AddRemoteStream(info);
+      if (NS_FAILED(nrv)) {
+        aPco->OnSetRemoteDescriptionError(
+            kInternalError,
+            ObString("AddRemoteStream failed"),
+            jrv);
+        return nrv;
+      }
+
+      CSFLogDebug(logTag, "Added remote stream %s", info->GetId().c_str());
+
+#if !defined(MOZILLA_EXTERNAL_LINKAGE)
+      info->GetMediaStream()->AssignId(NS_ConvertUTF8toUTF16(streamId.c_str()));
+#else
+      info->GetMediaStream()->AssignId((streamId));
+#endif
+    }
+
+    size_t numNewAudioTracks = 0;
+    size_t numNewVideoTracks = 0;
+    size_t numPreexistingTrackIds = 0;
+
+    for (auto j = tracks.begin(); j != tracks.end(); ++j) {
+      RefPtr<JsepTrack> track = *j;
+      if (!info->HasTrack(track->GetTrackId())) {
+        if (track->GetMediaType() == SdpMediaSection::kAudio) {
+          ++numNewAudioTracks;
+        } else if (track->GetMediaType() == SdpMediaSection::kVideo) {
+          ++numNewVideoTracks;
+        } else {
+          MOZ_ASSERT(false);
+          continue;
+        }
+        info->AddTrack(track->GetTrackId());
+#if !defined(MOZILLA_EXTERNAL_LINKAGE)
+        RefPtr<MediaStreamTrackSource> source =
+          new BasicUnstoppableTrackSource(MediaSourceEnum::Other);
+        if (track->GetMediaType() == SdpMediaSection::kAudio) {
+          info->GetMediaStream()->CreateOwnDOMTrack(
+            info->GetNumericTrackId(track->GetTrackId()),
+            MediaSegment::AUDIO, nsString(), source);
+        } else {
+          info->GetMediaStream()->CreateOwnDOMTrack(
+            info->GetNumericTrackId(track->GetTrackId()),
+            MediaSegment::VIDEO, nsString(), source);
+        }
+#endif
+        CSFLogDebug(logTag, "Added remote track %s/%s",
+                    info->GetId().c_str(), track->GetTrackId().c_str());
+      } else {
+        ++numPreexistingTrackIds;
+      }
+    }
+
+    // Now that the streams are all set up, notify about track availability.
+#if !defined(MOZILLA_EXTERNAL_LINKAGE)
+    TracksAvailableCallback* tracksAvailableCallback =
+      new TracksAvailableCallback(numNewAudioTracks,
+                                  numNewVideoTracks,
+                                  mHandle,
+                                  aPco);
+    info->GetMediaStream()->OnTracksAvailable(tracksAvailableCallback);
+#else
+    if (!numPreexistingTrackIds) {
+      aPco->OnAddStream(*info->GetMediaStream(), jrv);
+    }
+#endif
+  }
+  return NS_OK;
+}
+
+void
+PeerConnectionImpl::RemoveOldRemoteTracks(RefPtr<PeerConnectionObserver>& aPco)
+{
+  JSErrorResult jrv;
+
+  std::vector<RefPtr<JsepTrack>> removedTracks =
+    mJsepSession->GetRemoteTracksRemoved();
+
+  for (auto i = removedTracks.begin(); i != removedTracks.end(); ++i) {
+    const std::string& streamId = (*i)->GetStreamId();
+    const std::string& trackId = (*i)->GetTrackId();
+
+    RefPtr<RemoteSourceStreamInfo> info = mMedia->GetRemoteStreamById(streamId);
+    if (!info) {
+      MOZ_ASSERT(false, "A stream/track was removed that wasn't in PCMedia. "
+                        "This is a bug.");
+      continue;
+    }
+
+    mMedia->RemoveRemoteTrack(streamId, trackId);
+
+    DOMMediaStream* stream = info->GetMediaStream();
+    nsTArray<RefPtr<MediaStreamTrack>> tracks;
+    stream->GetTracks(tracks);
+    for (auto& track : tracks) {
+      if (PeerConnectionImpl::GetTrackId(*track) == trackId) {
+        aPco->OnRemoveTrack(*track, jrv);
+        break;
+      }
+    }
+
+    // We might be holding the last ref, but that's ok.
+    if (!info->GetTrackCount()) {
+      aPco->OnRemoveStream(*stream, jrv);
+    }
+  }
+}
+
 NS_IMETHODIMP
 PeerConnectionImpl::SetRemoteDescription(int32_t action, const char* aSDP)
 {
   PC_AUTO_ENTER_API_CALL(true);
 
   if (!aSDP) {
     CSFLogError(logTag, "%s - aSDP is NULL", __FUNCTION__);
     return NS_ERROR_FAILURE;
@@ -1809,147 +1955,23 @@ PeerConnectionImpl::SetRemoteDescription
         error = kInternalError;
     }
 
     std::string errorString = mJsepSession->GetLastError();
     CSFLogError(logTag, "%s: pc = %s, error = %s",
                 __FUNCTION__, mHandle.c_str(), errorString.c_str());
     pco->OnSetRemoteDescriptionError(error, ObString(errorString.c_str()), jrv);
   } else {
-    std::vector<RefPtr<JsepTrack>> newTracks =
-      mJsepSession->GetRemoteTracksAdded();
-
-    // Group new tracks by stream id
-    std::map<std::string, std::vector<RefPtr<JsepTrack>>> tracksByStreamId;
-    for (auto i = newTracks.begin(); i != newTracks.end(); ++i) {
-      RefPtr<JsepTrack> track = *i;
-
-      if (track->GetMediaType() == mozilla::SdpMediaSection::kApplication) {
-        // Ignore datachannel
-        continue;
-      }
-
-      tracksByStreamId[track->GetStreamId()].push_back(track);
+    nrv = CreateNewRemoteTracks(pco);
+    if (NS_FAILED(nrv)) {
+      // aPco was already notified, just return early.
+      return NS_OK;
     }
 
-    for (auto i = tracksByStreamId.begin(); i != tracksByStreamId.end(); ++i) {
-      std::string streamId = i->first;
-      std::vector<RefPtr<JsepTrack>>& tracks = i->second;
-
-      RefPtr<RemoteSourceStreamInfo> info =
-        mMedia->GetRemoteStreamById(streamId);
-      if (!info) {
-        nsresult nrv = CreateRemoteSourceStreamInfo(&info, streamId);
-        if (NS_FAILED(nrv)) {
-          pco->OnSetRemoteDescriptionError(
-              kInternalError,
-              ObString("CreateRemoteSourceStreamInfo failed"),
-              jrv);
-          return NS_OK;
-        }
-
-        nrv = mMedia->AddRemoteStream(info);
-        if (NS_FAILED(nrv)) {
-          pco->OnSetRemoteDescriptionError(
-              kInternalError,
-              ObString("AddRemoteStream failed"),
-              jrv);
-          return NS_OK;
-        }
-        CSFLogDebug(logTag, "Added remote stream %s", info->GetId().c_str());
-
-#if !defined(MOZILLA_EXTERNAL_LINKAGE)
-        info->GetMediaStream()->AssignId(NS_ConvertUTF8toUTF16(streamId.c_str()));
-#else
-        info->GetMediaStream()->AssignId((streamId));
-#endif
-      }
-
-      size_t numNewAudioTracks = 0;
-      size_t numNewVideoTracks = 0;
-      size_t numPreexistingTrackIds = 0;
-
-      for (auto j = tracks.begin(); j != tracks.end(); ++j) {
-        RefPtr<JsepTrack> track = *j;
-        if (!info->HasTrack(track->GetTrackId())) {
-          if (track->GetMediaType() == SdpMediaSection::kAudio) {
-            ++numNewAudioTracks;
-          } else if (track->GetMediaType() == SdpMediaSection::kVideo) {
-            ++numNewVideoTracks;
-          } else {
-            MOZ_ASSERT(false);
-            continue;
-          }
-          info->AddTrack(track->GetTrackId());
-#if !defined(MOZILLA_EXTERNAL_LINKAGE)
-          RefPtr<MediaStreamTrackSource> source =
-            new BasicUnstoppableTrackSource(MediaSourceEnum::Other);
-          if (track->GetMediaType() == SdpMediaSection::kAudio) {
-            info->GetMediaStream()->CreateOwnDOMTrack(
-              info->GetNumericTrackId(track->GetTrackId()),
-              MediaSegment::AUDIO, nsString(), source);
-          } else {
-            info->GetMediaStream()->CreateOwnDOMTrack(
-              info->GetNumericTrackId(track->GetTrackId()),
-              MediaSegment::VIDEO, nsString(), source);
-          }
-#endif
-          CSFLogDebug(logTag, "Added remote track %s/%s",
-                      info->GetId().c_str(), track->GetTrackId().c_str());
-        } else {
-          ++numPreexistingTrackIds;
-        }
-      }
-
-      // Now that the streams are all set up, notify about track availability.
-#if !defined(MOZILLA_EXTERNAL_LINKAGE)
-      TracksAvailableCallback* tracksAvailableCallback =
-        new TracksAvailableCallback(numNewAudioTracks,
-                                    numNewVideoTracks,
-                                    mHandle,
-                                    pco);
-      info->GetMediaStream()->OnTracksAvailable(tracksAvailableCallback);
-#else
-      if (!numPreexistingTrackIds) {
-        pco->OnAddStream(*info->GetMediaStream(), jrv);
-      }
-#endif
-    }
-
-    std::vector<RefPtr<JsepTrack>> removedTracks =
-      mJsepSession->GetRemoteTracksRemoved();
-
-    for (auto i = removedTracks.begin(); i != removedTracks.end(); ++i) {
-      const std::string& streamId = (*i)->GetStreamId();
-      const std::string& trackId = (*i)->GetTrackId();
-
-      RefPtr<RemoteSourceStreamInfo> info = mMedia->GetRemoteStreamById(streamId);
-      if (!info) {
-        MOZ_ASSERT(false, "A stream/track was removed that wasn't in PCMedia. "
-                          "This is a bug.");
-        continue;
-      }
-
-      mMedia->RemoveRemoteTrack(streamId, trackId);
-
-      DOMMediaStream* stream = info->GetMediaStream();
-      nsTArray<RefPtr<MediaStreamTrack>> tracks;
-      stream->GetTracks(tracks);
-      for (auto& track : tracks) {
-        if (PeerConnectionImpl::GetTrackId(*track) == trackId) {
-          pco->OnRemoveTrack(*track, jrv);
-          break;
-        }
-      }
-
-      // We might be holding the last ref, but that's ok.
-      if (!info->GetTrackCount()) {
-        pco->OnRemoveStream(*stream, jrv);
-      }
-    }
+    RemoveOldRemoteTracks(pco);
 
     pco->OnSetRemoteDescriptionSuccess(jrv);
 #if !defined(MOZILLA_EXTERNAL_LINKAGE)
     startCallTelem();
 #endif
   }
 
   UpdateSignalingState(sdpType == mozilla::kJsepSdpRollback);
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h
@@ -386,16 +386,20 @@ public:
 
   NS_IMETHODIMP SetLocalDescription (int32_t aAction, const char* aSDP);
 
   void SetLocalDescription (int32_t aAction, const nsAString& aSDP, ErrorResult &rv)
   {
     rv = SetLocalDescription(aAction, NS_ConvertUTF16toUTF8(aSDP).get());
   }
 
+  nsresult CreateNewRemoteTracks(RefPtr<PeerConnectionObserver>& aPco);
+
+  void RemoveOldRemoteTracks(RefPtr<PeerConnectionObserver>& aPco);
+
   NS_IMETHODIMP SetRemoteDescription (int32_t aAction, const char* aSDP);
 
   void SetRemoteDescription (int32_t aAction, const nsAString& aSDP, ErrorResult &rv)
   {
     rv = SetRemoteDescription(aAction, NS_ConvertUTF16toUTF8(aSDP).get());
   }
 
   NS_IMETHODIMP_TO_ERRORRESULT(GetStats, ErrorResult &rv,