Restoring behavior where PeerConnection tracks changes to MediaStreams.

If a MediaStream is added to a PeerConnection, and later a track
is added to the MediaStream, a new RtpSender will now be created for
that track, and it will appear in subsequent offers.
Similarly, removed tracks will remove RtpSenders.

BUG=webrtc:5265

Review URL: https://codereview.webrtc.org/1507973003

Cr-Commit-Position: refs/heads/master@{#11040}

6 files changed, 261 insertions, 59 deletions

diff --git a/talk/app/webrtc/mediastreamobserver.cc b/talk/app/webrtc/mediastreamobserver.cc
index a8561647b2..2650b9a6f7 100644
--- a/talk/app/webrtc/mediastreamobserver.cc
+++ b/talk/app/webrtc/mediastreamobserver.cc
@@ -27,4 +27,75 @@
 
 #include "talk/app/webrtc/mediastreamobserver.h"
 
-// This file is currently stubbed so that Chromium's build files can be updated.
+#include <algorithm>
+
+namespace webrtc {
+
+MediaStreamObserver::MediaStreamObserver(MediaStreamInterface* stream)
+    : stream_(stream),
+      cached_audio_tracks_(stream->GetAudioTracks()),
+      cached_video_tracks_(stream->GetVideoTracks()) {
+  stream_->RegisterObserver(this);
+}
+
+MediaStreamObserver::~MediaStreamObserver() {
+  stream_->UnregisterObserver(this);
+}
+
+void MediaStreamObserver::OnChanged() {
+  AudioTrackVector new_audio_tracks = stream_->GetAudioTracks();
+  VideoTrackVector new_video_tracks = stream_->GetVideoTracks();
+
+  // Find removed audio tracks.
+  for (const auto& cached_track : cached_audio_tracks_) {
+    auto it = std::find_if(
+        new_audio_tracks.begin(), new_audio_tracks.end(),
+        [cached_track](const AudioTrackVector::value_type& new_track) {
+          return new_track->id().compare(cached_track->id()) == 0;
+        });
+    if (it == new_audio_tracks.end()) {
+      SignalAudioTrackRemoved(cached_track.get(), stream_);
+    }
+  }
+
+  // Find added audio tracks.
+  for (const auto& new_track : new_audio_tracks) {
+    auto it = std::find_if(
+        cached_audio_tracks_.begin(), cached_audio_tracks_.end(),
+        [new_track](const AudioTrackVector::value_type& cached_track) {
+          return new_track->id().compare(cached_track->id()) == 0;
+        });
+    if (it == cached_audio_tracks_.end()) {
+      SignalAudioTrackAdded(new_track.get(), stream_);
+    }
+  }
+
+  // Find removed video tracks.
+  for (const auto& cached_track : cached_video_tracks_) {
+    auto it = std::find_if(
+        new_video_tracks.begin(), new_video_tracks.end(),
+        [cached_track](const VideoTrackVector::value_type& new_track) {
+          return new_track->id().compare(cached_track->id()) == 0;
+        });
+    if (it == new_video_tracks.end()) {
+      SignalVideoTrackRemoved(cached_track.get(), stream_);
+    }
+  }
+
+  // Find added video tracks.
+  for (const auto& new_track : new_video_tracks) {
+    auto it = std::find_if(
+        cached_video_tracks_.begin(), cached_video_tracks_.end(),
+        [new_track](const VideoTrackVector::value_type& cached_track) {
+          return new_track->id().compare(cached_track->id()) == 0;
+        });
+    if (it == cached_video_tracks_.end()) {
+      SignalVideoTrackAdded(new_track.get(), stream_);
+    }
+  }
+
+  cached_audio_tracks_ = new_audio_tracks;
+  cached_video_tracks_ = new_video_tracks;
+}
+
+}  // namespace webrtc
diff --git a/talk/app/webrtc/mediastreamobserver.h b/talk/app/webrtc/mediastreamobserver.h
index d6128f3973..1dd6c4c118 100644
--- a/talk/app/webrtc/mediastreamobserver.h
+++ b/talk/app/webrtc/mediastreamobserver.h
@@ -25,9 +25,41 @@
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-// This file is currently stubbed so that Chromium's build files can be updated.
-
 #ifndef TALK_APP_WEBRTC_MEDIASTREAMOBSERVER_H_
 #define TALK_APP_WEBRTC_MEDIASTREAMOBSERVER_H_
 
+#include "talk/app/webrtc/mediastreaminterface.h"
+#include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/base/sigslot.h"
+
+namespace webrtc {
+
+// Helper class which will listen for changes to a stream and emit the
+// corresponding signals.
+class MediaStreamObserver : public ObserverInterface {
+ public:
+  explicit MediaStreamObserver(MediaStreamInterface* stream);
+  ~MediaStreamObserver();
+
+  const MediaStreamInterface* stream() const { return stream_; }
+
+  void OnChanged() override;
+
+  sigslot::signal2<AudioTrackInterface*, MediaStreamInterface*>
+      SignalAudioTrackAdded;
+  sigslot::signal2<AudioTrackInterface*, MediaStreamInterface*>
+      SignalAudioTrackRemoved;
+  sigslot::signal2<VideoTrackInterface*, MediaStreamInterface*>
+      SignalVideoTrackAdded;
+  sigslot::signal2<VideoTrackInterface*, MediaStreamInterface*>
+      SignalVideoTrackRemoved;
+
+ private:
+  rtc::scoped_refptr<MediaStreamInterface> stream_;
+  AudioTrackVector cached_audio_tracks_;
+  VideoTrackVector cached_video_tracks_;
+};
+
+}  // namespace webrtc
+
 #endif  // TALK_APP_WEBRTC_MEDIASTREAMOBSERVER_H_
diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index 6cca166b1c..3a38248b7b 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -27,6 +27,7 @@
 
 #include "talk/app/webrtc/peerconnection.h"
 
+#include <algorithm>
 #include <vector>
 #include <cctype>  // for isdigit
 
@@ -36,6 +37,7 @@
 #include "talk/app/webrtc/jsepsessiondescription.h"
 #include "talk/app/webrtc/mediaconstraintsinterface.h"
 #include "talk/app/webrtc/mediastream.h"
+#include "talk/app/webrtc/mediastreamobserver.h"
 #include "talk/app/webrtc/mediastreamproxy.h"
 #include "talk/app/webrtc/mediastreamtrackproxy.h"
 #include "talk/app/webrtc/remoteaudiosource.h"
@@ -740,48 +742,22 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) {
   }
 
   local_streams_->AddStream(local_stream);
+  MediaStreamObserver* observer = new MediaStreamObserver(local_stream);
+  observer->SignalAudioTrackAdded.connect(this,
+                                          &PeerConnection::OnAudioTrackAdded);
+  observer->SignalAudioTrackRemoved.connect(
+      this, &PeerConnection::OnAudioTrackRemoved);
+  observer->SignalVideoTrackAdded.connect(this,
+                                          &PeerConnection::OnVideoTrackAdded);
+  observer->SignalVideoTrackRemoved.connect(
+      this, &PeerConnection::OnVideoTrackRemoved);
+  stream_observers_.push_back(rtc::scoped_ptr<MediaStreamObserver>(observer));
 
   for (const auto& track : local_stream->GetAudioTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      // Normal case; we've never seen this track before.
-      AudioRtpSender* new_sender = new AudioRtpSender(
-          track.get(), local_stream->label(), session_.get(), stats_.get());
-      senders_.push_back(new_sender);
-      // If the sender has already been configured in SDP, we call SetSsrc,
-      // which will connect the sender to the underlying transport. This can
-      // occur if a local session description that contains the ID of the sender
-      // is set before AddStream is called. It can also occur if the local
-      // session description is not changed and RemoveStream is called, and
-      // later AddStream is called again with the same stream.
-      const TrackInfo* track_info = FindTrackInfo(
-          local_audio_tracks_, local_stream->label(), track->id());
-      if (track_info) {
-        new_sender->SetSsrc(track_info->ssrc);
-      }
-    } else {
-      // We already have a sender for this track, so just change the stream_id
-      // so that it's correct in the next call to CreateOffer.
-      (*sender)->set_stream_id(local_stream->label());
-    }
+    OnAudioTrackAdded(track.get(), local_stream);
   }
   for (const auto& track : local_stream->GetVideoTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      // Normal case; we've never seen this track before.
-      VideoRtpSender* new_sender = new VideoRtpSender(
-          track.get(), local_stream->label(), session_.get());
-      senders_.push_back(new_sender);
-      const TrackInfo* track_info = FindTrackInfo(
-          local_video_tracks_, local_stream->label(), track->id());
-      if (track_info) {
-        new_sender->SetSsrc(track_info->ssrc);
-      }
-    } else {
-      // We already have a sender for this track, so just change the stream_id
-      // so that it's correct in the next call to CreateOffer.
-      (*sender)->set_stream_id(local_stream->label());
-    }
+    OnVideoTrackAdded(track.get(), local_stream);
   }
 
   stats_->AddStream(local_stream);
@@ -789,32 +765,24 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) {
   return true;
 }
 
-// TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around
-// indefinitely, when we have unified plan SDP.
 void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) {
   TRACE_EVENT0("webrtc", "PeerConnection::RemoveStream");
   for (const auto& track : local_stream->GetAudioTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
-                      << " doesn't exist.";
-      continue;
-    }
-    (*sender)->Stop();
-    senders_.erase(sender);
+    OnAudioTrackRemoved(track.get(), local_stream);
   }
   for (const auto& track : local_stream->GetVideoTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
-                      << " doesn't exist.";
-      continue;
-    }
-    (*sender)->Stop();
-    senders_.erase(sender);
+    OnVideoTrackRemoved(track.get(), local_stream);
   }
 
   local_streams_->RemoveStream(local_stream);
+  stream_observers_.erase(
+      std::remove_if(
+          stream_observers_.begin(), stream_observers_.end(),
+          [local_stream](const rtc::scoped_ptr<MediaStreamObserver>& observer) {
+            return observer->stream()->label().compare(local_stream->label()) ==
+                   0;
+          }),
+      stream_observers_.end());
 
   if (IsClosed()) {
     return;
@@ -1432,6 +1400,80 @@ void PeerConnection::ChangeSignalingState(
   observer_->OnStateChange(PeerConnectionObserver::kSignalingState);
 }
 
+void PeerConnection::OnAudioTrackAdded(AudioTrackInterface* track,
+                                       MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender != senders_.end()) {
+    // We already have a sender for this track, so just change the stream_id
+    // so that it's correct in the next call to CreateOffer.
+    (*sender)->set_stream_id(stream->label());
+    return;
+  }
+
+  // Normal case; we've never seen this track before.
+  AudioRtpSender* new_sender =
+      new AudioRtpSender(track, stream->label(), session_.get(), stats_.get());
+  senders_.push_back(new_sender);
+  // If the sender has already been configured in SDP, we call SetSsrc,
+  // which will connect the sender to the underlying transport. This can
+  // occur if a local session description that contains the ID of the sender
+  // is set before AddStream is called. It can also occur if the local
+  // session description is not changed and RemoveStream is called, and
+  // later AddStream is called again with the same stream.
+  const TrackInfo* track_info =
+      FindTrackInfo(local_audio_tracks_, stream->label(), track->id());
+  if (track_info) {
+    new_sender->SetSsrc(track_info->ssrc);
+  }
+}
+
+// TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around
+// indefinitely, when we have unified plan SDP.
+void PeerConnection::OnAudioTrackRemoved(AudioTrackInterface* track,
+                                         MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender == senders_.end()) {
+    LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
+                    << " doesn't exist.";
+    return;
+  }
+  (*sender)->Stop();
+  senders_.erase(sender);
+}
+
+void PeerConnection::OnVideoTrackAdded(VideoTrackInterface* track,
+                                       MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender != senders_.end()) {
+    // We already have a sender for this track, so just change the stream_id
+    // so that it's correct in the next call to CreateOffer.
+    (*sender)->set_stream_id(stream->label());
+    return;
+  }
+
+  // Normal case; we've never seen this track before.
+  VideoRtpSender* new_sender =
+      new VideoRtpSender(track, stream->label(), session_.get());
+  senders_.push_back(new_sender);
+  const TrackInfo* track_info =
+      FindTrackInfo(local_video_tracks_, stream->label(), track->id());
+  if (track_info) {
+    new_sender->SetSsrc(track_info->ssrc);
+  }
+}
+
+void PeerConnection::OnVideoTrackRemoved(VideoTrackInterface* track,
+                                         MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender == senders_.end()) {
+    LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
+                    << " doesn't exist.";
+    return;
+  }
+  (*sender)->Stop();
+  senders_.erase(sender);
+}
+
 void PeerConnection::PostSetSessionDescriptionFailure(
     SetSessionDescriptionObserver* observer,
     const std::string& error) {
diff --git a/talk/app/webrtc/peerconnection.h b/talk/app/webrtc/peerconnection.h
index 9cb3635690..f6c4fc3e9b 100644
--- a/talk/app/webrtc/peerconnection.h
+++ b/talk/app/webrtc/peerconnection.h
@@ -42,6 +42,7 @@
 
 namespace webrtc {
 
+class MediaStreamObserver;
 class RemoteMediaStreamFactory;
 
 typedef std::vector<PortAllocatorFactoryInterface::StunConfiguration>
@@ -201,6 +202,16 @@ class PeerConnection : public PeerConnectionInterface,
   void OnSessionStateChange(WebRtcSession* session, WebRtcSession::State state);
   void ChangeSignalingState(SignalingState signaling_state);
 
+  // Signals from MediaStreamObserver.
+  void OnAudioTrackAdded(AudioTrackInterface* track,
+                         MediaStreamInterface* stream);
+  void OnAudioTrackRemoved(AudioTrackInterface* track,
+                           MediaStreamInterface* stream);
+  void OnVideoTrackAdded(VideoTrackInterface* track,
+                         MediaStreamInterface* stream);
+  void OnVideoTrackRemoved(VideoTrackInterface* track,
+                           MediaStreamInterface* stream);
+
   rtc::Thread* signaling_thread() const {
     return factory_->signaling_thread();
   }
@@ -364,6 +375,8 @@ class PeerConnection : public PeerConnectionInterface,
   // Streams created as a result of SetRemoteDescription.
   rtc::scoped_refptr<StreamCollection> remote_streams_;
 
+  std::vector<rtc::scoped_ptr<MediaStreamObserver>> stream_observers_;
+
   // These lists store track info seen in local/remote descriptions.
   TrackInfos remote_audio_tracks_;
   TrackInfos remote_video_tracks_;
diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc
index edf75931a4..5e239313d6 100644
--- a/talk/app/webrtc/peerconnectioninterface_unittest.cc
+++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc
@@ -1156,6 +1156,48 @@ TEST_F(PeerConnectionInterfaceTest, SsrcInOfferAnswer) {
   EXPECT_NE(audio_ssrc, video_ssrc);
 }
 
+// Test that it's possible to call AddTrack on a MediaStream after adding
+// the stream to a PeerConnection.
+// TODO(deadbeef): Remove this test once this behavior is no longer supported.
+TEST_F(PeerConnectionInterfaceTest, AddTrackAfterAddStream) {
+  CreatePeerConnection();
+  // Create audio stream and add to PeerConnection.
+  AddVoiceStream(kStreamLabel1);
+  MediaStreamInterface* stream = pc_->local_streams()->at(0);
+
+  // Add video track to the audio-only stream.
+  scoped_refptr<VideoTrackInterface> video_track(
+      pc_factory_->CreateVideoTrack("video_label", nullptr));
+  stream->AddTrack(video_track.get());
+
+  scoped_ptr<SessionDescriptionInterface> offer;
+  ASSERT_TRUE(DoCreateOffer(offer.use(), nullptr));
+
+  const cricket::MediaContentDescription* video_desc =
+      cricket::GetFirstVideoContentDescription(offer->description());
+  EXPECT_TRUE(video_desc != nullptr);
+}
+
+// Test that it's possible to call RemoveTrack on a MediaStream after adding
+// the stream to a PeerConnection.
+// TODO(deadbeef): Remove this test once this behavior is no longer supported.
+TEST_F(PeerConnectionInterfaceTest, RemoveTrackAfterAddStream) {
+  CreatePeerConnection();
+  // Create audio/video stream and add to PeerConnection.
+  AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label");
+  MediaStreamInterface* stream = pc_->local_streams()->at(0);
+
+  // Remove the video track.
+  stream->RemoveTrack(stream->GetVideoTracks()[0]);
+
+  scoped_ptr<SessionDescriptionInterface> offer;
+  ASSERT_TRUE(DoCreateOffer(offer.use(), nullptr));
+
+  const cricket::MediaContentDescription* video_desc =
+      cricket::GetFirstVideoContentDescription(offer->description());
+  EXPECT_TRUE(video_desc == nullptr);
+}
+
 // Test that we can specify a certain track that we want statistics about.
 TEST_F(PeerConnectionInterfaceTest, GetStatsForSpecificTrack) {
   InitiateCall();
diff --git a/talk/libjingle.gyp b/talk/libjingle.gyp
index e8035fe7d1..e3480ca7a6 100755
--- a/talk/libjingle.gyp
+++ b/talk/libjingle.gyp
@@ -746,6 +746,8 @@
         'app/webrtc/mediastream.cc',
         'app/webrtc/mediastream.h',
         'app/webrtc/mediastreaminterface.h',
+        'app/webrtc/mediastreamobserver.cc',
+        'app/webrtc/mediastreamobserver.h',
         'app/webrtc/mediastreamprovider.h',
         'app/webrtc/mediastreamproxy.h',
         'app/webrtc/mediastreamtrack.h',