Update Java MediaStream when native stream's set of tracks changes.

This will handle the scenario where, for example, the initial
offer/answer only negotiates audio, and video is added later (to the
same stream). Previously, there was absolutely no way to get a handle to
the new track without hacking the SDP. Now, the stream will be updated
after setRemoteDescription finishes.

Bug: webrtc:5677
Change-Id: Iea31bb7744da6b82afdaf44c8f74d721298a9474
Reviewed-on: https://webrtc-review.googlesource.com/6261
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20228}

sdk/android/instrumentationtests/src/org/webrtc/PeerConnectionTest.java

@@ -158,7 +158,7 @@ public class PeerConnectionTest {
       }
 
       if (expectedIceConnectionChanges.isEmpty()) {
-        System.out.println(name + "Got an unexpected ice connection change " + newState);
+        System.out.println(name + "Got an unexpected ICE connection change " + newState);
         return;
       }
 
@@ -167,7 +167,7 @@ public class PeerConnectionTest {
 
     @Override
     public synchronized void onIceConnectionReceivingChange(boolean receiving) {
-      System.out.println(name + "Got an ice connection receiving change " + receiving);
+      System.out.println(name + "Got an ICE connection receiving change " + receiving);
     }
 
     public synchronized void expectIceGatheringChange(IceGatheringState newState) {
@@ -182,6 +182,9 @@ public class PeerConnectionTest {
       if (newState == IceGatheringState.GATHERING) {
         return;
       }
+      if (expectedIceGatheringChanges.isEmpty()) {
+        System.out.println(name + "Got an unexpected ICE gathering change " + newState);
+      }
       assertEquals(expectedIceGatheringChanges.removeFirst(), newState);
     }
 
@@ -192,15 +195,15 @@ public class PeerConnectionTest {
     @Override
     public synchronized void onAddStream(MediaStream stream) {
       assertEquals(expectedAddStreamLabels.removeFirst(), stream.label());
-      assertEquals(1, stream.videoTracks.size());
+      for (AudioTrack track : stream.audioTracks) {
-      assertEquals(1, stream.audioTracks.size());
+        assertEquals("audio", track.kind());
-      assertTrue(stream.videoTracks.get(0).id().endsWith("VideoTrack"));
+      }
-      assertTrue(stream.audioTracks.get(0).id().endsWith("AudioTrack"));
+      for (VideoTrack track : stream.videoTracks) {
-      assertEquals("video", stream.videoTracks.get(0).kind());
+        assertEquals("video", track.kind());
-      assertEquals("audio", stream.audioTracks.get(0).kind());
         VideoRenderer renderer = createVideoRenderer(this);
-      stream.videoTracks.get(0).addRenderer(renderer);
+        track.addRenderer(renderer);
         assertNull(renderers.put(stream, new WeakReference<VideoRenderer>(renderer)));
+      }
       gotRemoteStreams.add(stream);
     }
 
@@ -619,7 +622,7 @@ public class PeerConnectionTest {
     // instead of in addTracksToPC.
     final CameraEnumerator enumerator = new Camera1Enumerator(false /* captureToTexture */);
     final VideoCapturer videoCapturer =
-        enumerator.createCapturer(enumerator.getDeviceNames()[0], null);
+        enumerator.createCapturer(enumerator.getDeviceNames()[0], null /* eventsHandler */);
     final VideoSource videoSource = factory.createVideoSource(videoCapturer);
     videoCapturer.startCapture(640, 480, 30);
 
@@ -1007,7 +1010,7 @@ public class PeerConnectionTest {
     // instead of in addTracksToPC.
     final CameraEnumerator enumerator = new Camera1Enumerator(false /* captureToTexture */);
     final VideoCapturer videoCapturer =
-        enumerator.createCapturer(enumerator.getDeviceNames()[0], null);
+        enumerator.createCapturer(enumerator.getDeviceNames()[0], null /* eventsHandler */);
     final VideoSource videoSource = factory.createVideoSource(videoCapturer);
     videoCapturer.startCapture(640, 480, 30);
 
@@ -1163,6 +1166,133 @@ public class PeerConnectionTest {
     System.gc();
   }
 
+  /**
+   * Test that a Java MediaStream is updated when the native stream is.
+   * <p>
+   * Specifically, test that when remote tracks are indicated as being added or
+   * removed from a MediaStream (via "a=ssrc" or "a=msid" in a remote
+   * description), the existing remote MediaStream object is updated.
+   * <p>
+   * This test starts with just an audio track, adds a video track, then
+   * removes it. It only applies remote offers, which is sufficient to test
+   * this functionality and simplifies the test. This means that no media will
+   * actually be sent/received; we're just testing that the Java MediaStream
+   * object gets updated when the native object changes.
+   */
+  @Test
+  @MediumTest
+  public void testRemoteStreamUpdatedWhenTracksAddedOrRemoved() throws Exception {
+    PeerConnectionFactory.Options options = new PeerConnectionFactory.Options();
+    PeerConnectionFactory factory = new PeerConnectionFactory(options);
+
+    // This test is fine with default PC constraints and no ICE servers.
+    MediaConstraints pcConstraints = new MediaConstraints();
+    LinkedList<PeerConnection.IceServer> iceServers = new LinkedList<PeerConnection.IceServer>();
+
+    // Use OfferToReceiveAudio/Video to ensure every offer has an audio and
+    // video m= section. Simplifies the test because it means we don't have to
+    // actually apply the offer to "offeringPC"; it's just used as an SDP
+    // factory.
+    MediaConstraints offerConstraints = new MediaConstraints();
+    offerConstraints.mandatory.add(
+        new MediaConstraints.KeyValuePair("OfferToReceiveAudio", "true"));
+    offerConstraints.mandatory.add(
+        new MediaConstraints.KeyValuePair("OfferToReceiveVideo", "true"));
+
+    // This PeerConnection will only be used to generate offers.
+    ObserverExpectations offeringExpectations = new ObserverExpectations("offerer");
+    PeerConnection offeringPC =
+        factory.createPeerConnection(iceServers, pcConstraints, offeringExpectations);
+    assertNotNull(offeringPC);
+
+    ObserverExpectations expectations = new ObserverExpectations("PC under test");
+    PeerConnection pcUnderTest =
+        factory.createPeerConnection(iceServers, pcConstraints, expectations);
+    assertNotNull(pcUnderTest);
+
+    // Add offerer media stream with just an audio track.
+    MediaStream localStream = factory.createLocalMediaStream("stream");
+    AudioTrack localAudioTrack =
+        factory.createAudioTrack("audio", factory.createAudioSource(new MediaConstraints()));
+    localStream.addTrack(localAudioTrack);
+    // TODO(deadbeef): Use addTrack once that's available.
+    offeringExpectations.expectRenegotiationNeeded();
+    offeringPC.addStream(localStream);
+    // Create offer.
+    SdpObserverLatch sdpLatch = new SdpObserverLatch();
+    offeringPC.createOffer(sdpLatch, offerConstraints);
+    assertTrue(sdpLatch.await());
+    SessionDescription offerSdp = sdpLatch.getSdp();
+
+    // Apply remote offer to PC under test.
+    sdpLatch = new SdpObserverLatch();
+    expectations.expectSignalingChange(SignalingState.HAVE_REMOTE_OFFER);
+    expectations.expectAddStream("stream");
+    pcUnderTest.setRemoteDescription(sdpLatch, offerSdp);
+    assertTrue(sdpLatch.await());
+    // Sanity check that we get one remote stream with one audio track.
+    MediaStream remoteStream = expectations.gotRemoteStreams.iterator().next();
+    assertEquals(remoteStream.audioTracks.size(), 1);
+    assertEquals(remoteStream.videoTracks.size(), 0);
+
+    // Add a video track...
+    final CameraEnumerator enumerator = new Camera1Enumerator(false /* captureToTexture */);
+    final VideoCapturer videoCapturer =
+        enumerator.createCapturer(enumerator.getDeviceNames()[0], null /* eventsHandler */);
+    final VideoSource videoSource = factory.createVideoSource(videoCapturer);
+    VideoTrack videoTrack = factory.createVideoTrack("video", videoSource);
+    offeringExpectations.expectRenegotiationNeeded();
+    localStream.addTrack(videoTrack);
+    // ... and create an updated offer.
+    sdpLatch = new SdpObserverLatch();
+    offeringPC.createOffer(sdpLatch, offerConstraints);
+    assertTrue(sdpLatch.await());
+    offerSdp = sdpLatch.getSdp();
+
+    // Apply remote offer with new video track to PC under test.
+    sdpLatch = new SdpObserverLatch();
+    pcUnderTest.setRemoteDescription(sdpLatch, offerSdp);
+    assertTrue(sdpLatch.await());
+    // The remote stream should now have a video track.
+    assertEquals(remoteStream.audioTracks.size(), 1);
+    assertEquals(remoteStream.videoTracks.size(), 1);
+
+    // Finally, create another offer with the audio track removed.
+    offeringExpectations.expectRenegotiationNeeded();
+    localStream.removeTrack(localAudioTrack);
+    localAudioTrack.dispose();
+    sdpLatch = new SdpObserverLatch();
+    offeringPC.createOffer(sdpLatch, offerConstraints);
+    assertTrue(sdpLatch.await());
+    offerSdp = sdpLatch.getSdp();
+
+    // Apply remote offer with just a video track to PC under test.
+    sdpLatch = new SdpObserverLatch();
+    pcUnderTest.setRemoteDescription(sdpLatch, offerSdp);
+    assertTrue(sdpLatch.await());
+    // The remote stream should no longer have an audio track.
+    assertEquals(remoteStream.audioTracks.size(), 0);
+    assertEquals(remoteStream.videoTracks.size(), 1);
+
+    // Free the Java-land objects. Video capturer and source aren't owned by
+    // the PeerConnection and need to be disposed separately.
+    // TODO(deadbeef): Should all these events really occur on disposal?
+    // "Gathering complete" is especially odd since gathering never started.
+    // Note that this test isn't meant to test these events, but we must do
+    // this or otherwise it will crash.
+    offeringExpectations.expectIceConnectionChange(IceConnectionState.CLOSED);
+    offeringExpectations.expectSignalingChange(SignalingState.CLOSED);
+    offeringExpectations.expectIceGatheringChange(IceGatheringState.COMPLETE);
+    offeringPC.dispose();
+    expectations.expectIceConnectionChange(IceConnectionState.CLOSED);
+    expectations.expectSignalingChange(SignalingState.CLOSED);
+    expectations.expectIceGatheringChange(IceGatheringState.COMPLETE);
+    pcUnderTest.dispose();
+    videoCapturer.dispose();
+    videoSource.dispose();
+    factory.dispose();
+  }
+
   private static void negotiate(PeerConnection offeringPC,
       ObserverExpectations offeringExpectations, PeerConnection answeringPC,
       ObserverExpectations answeringExpectations) {
@@ -1229,7 +1359,7 @@ public class PeerConnectionTest {
 
     // Call getStats (old implementation) before shutting down PC.
     expectations.expectOldStatsCallback();
-    assertTrue(pc.getStats(expectations, null));
+    assertTrue(pc.getStats(expectations, null /* track */));
     assertTrue(expectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
 
     // Call the new getStats implementation as well.
@@ -1245,7 +1375,7 @@ public class PeerConnectionTest {
     // Call getStats (old implementation) after calling close(). Should still
     // work.
     expectations.expectOldStatsCallback();
-    assertTrue(pc.getStats(expectations, null));
+    assertTrue(pc.getStats(expectations, null /* track */));
     assertTrue(expectations.waitForAllExpectationsToBeSatisfied(TIMEOUT_SECONDS));
 
     System.out.println("FYI stats: ");

sdk/android/src/jni/classreferenceholder.cc

@@ -78,6 +78,7 @@ ClassReferenceHolder::ClassReferenceHolder(JNIEnv* jni) {
   LoadClass(jni, "org/webrtc/MediaCodecVideoEncoder$VideoCodecType");
   LoadClass(jni, "org/webrtc/MediaSource$State");
   LoadClass(jni, "org/webrtc/MediaStream");
+  LoadClass(jni, "org/webrtc/MediaStreamTrack");
   LoadClass(jni, "org/webrtc/MediaStreamTrack$MediaType");
   LoadClass(jni, "org/webrtc/MediaStreamTrack$State");
   LoadClass(jni, "org/webrtc/NetworkMonitor");

sdk/android/src/jni/jni_helpers.cc

@@ -379,6 +379,7 @@ Iterable::Iterator::Iterator(JNIEnv* jni, jobject iterable) : jni_(jni) {
   jclass iterator_class = GetObjectClass(jni, iterator_);
   has_next_id_ = GetMethodID(jni, iterator_class, "hasNext", "()Z");
   next_id_ = GetMethodID(jni, iterator_class, "next", "()Ljava/lang/Object;");
+  remove_id_ = GetMethodID(jni, iterator_class, "remove", "()V");
 
   // Start at the first element in the collection.
   ++(*this);
@@ -414,6 +415,11 @@ Iterable::Iterator& Iterable::Iterator::operator++() {
   return *this;
 }
 
+void Iterable::Iterator::Remove() {
+  jni_->CallVoidMethod(iterator_, remove_id_);
+  CHECK_EXCEPTION(jni_) << "error during CallVoidMethod";
+}
+
 // Provides a way to compare the iterator with itself and with the end iterator.
 // Note: all other comparison results are undefined, just like for C++ input
 // iterators.

sdk/android/src/jni/jni_helpers.h

@@ -184,6 +184,10 @@ class Iterable {
     // Advances the iterator one step.
     Iterator& operator++();
 
+    // Removes the element the iterator is pointing to. Must still advance the
+    // iterator afterwards.
+    void Remove();
+
     // Provides a way to compare the iterator with itself and with the end
     // iterator.
     // Note: all other comparison results are undefined, just like for C++ input
@@ -200,6 +204,7 @@ class Iterable {
     jobject value_ = nullptr;
     jmethodID has_next_id_ = nullptr;
     jmethodID next_id_ = nullptr;
+    jmethodID remove_id_ = nullptr;
     rtc::ThreadChecker thread_checker_;
 
     RTC_DISALLOW_COPY_AND_ASSIGN(Iterator);

sdk/android/src/jni/pc/peerconnectionobserver_jni.cc

@@ -10,8 +10,10 @@
 
 #include "sdk/android/src/jni/pc/peerconnectionobserver_jni.h"
 
+#include <pc/mediastreamobserver.h>
 #include <string>
 
+#include "rtc_base/ptr_util.h"
 #include "sdk/android/src/jni/classreferenceholder.h"
 #include "sdk/android/src/jni/pc/java_native_conversion.h"
 
@@ -31,6 +33,13 @@ PeerConnectionObserverJni::PeerConnectionObserverJni(JNIEnv* jni,
       j_media_stream_class_(jni, FindClass(jni, "org/webrtc/MediaStream")),
       j_media_stream_ctor_(
           GetMethodID(jni, *j_media_stream_class_, "<init>", "(J)V")),
+      j_media_stream_track_class_(
+          jni,
+          FindClass(jni, "org/webrtc/MediaStreamTrack")),
+      j_track_dispose_id_(
+          GetMethodID(jni, *j_media_stream_track_class_, "dispose", "()V")),
+      j_native_track_id_(
+          GetFieldID(jni, *j_media_stream_track_class_, "nativeTrack", "J")),
       j_audio_track_class_(jni, FindClass(jni, "org/webrtc/AudioTrack")),
       j_audio_track_ctor_(
           GetMethodID(jni, *j_audio_track_class_, "<init>", "(J)V")),
@@ -133,47 +142,132 @@ void PeerConnectionObserverJni::OnAddStream(
   jobject j_stream = GetOrCreateJavaStream(stream);
 
   for (const auto& track : stream->GetAudioTracks()) {
-    jstring id = JavaStringFromStdString(jni(), track->id());
+    AddNativeAudioTrackToJavaStream(track, j_stream);
-    // Java AudioTrack holds one reference. Corresponding Release() is in
-    // MediaStreamTrack_free, triggered by AudioTrack.dispose().
-    track->AddRef();
-    jobject j_track =
-        jni()->NewObject(*j_audio_track_class_, j_audio_track_ctor_,
-                         reinterpret_cast<jlong>(track.get()), id);
-    CHECK_EXCEPTION(jni()) << "error during NewObject";
-    jfieldID audio_tracks_id = GetFieldID(
-        jni(), *j_media_stream_class_, "audioTracks", "Ljava/util/LinkedList;");
-    jobject audio_tracks = GetObjectField(jni(), j_stream, audio_tracks_id);
-    jmethodID add = GetMethodID(jni(), GetObjectClass(jni(), audio_tracks),
-                                "add", "(Ljava/lang/Object;)Z");
-    jboolean added = jni()->CallBooleanMethod(audio_tracks, add, j_track);
-    CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
-    RTC_CHECK(added);
   }
-
   for (const auto& track : stream->GetVideoTracks()) {
-    jstring id = JavaStringFromStdString(jni(), track->id());
+    AddNativeVideoTrackToJavaStream(track, j_stream);
-    // Java VideoTrack holds one reference. Corresponding Release() is in
-    // MediaStreamTrack_free, triggered by VideoTrack.dispose().
-    track->AddRef();
-    jobject j_track =
-        jni()->NewObject(*j_video_track_class_, j_video_track_ctor_,
-                         reinterpret_cast<jlong>(track.get()), id);
-    CHECK_EXCEPTION(jni()) << "error during NewObject";
-    jfieldID video_tracks_id = GetFieldID(
-        jni(), *j_media_stream_class_, "videoTracks", "Ljava/util/LinkedList;");
-    jobject video_tracks = GetObjectField(jni(), j_stream, video_tracks_id);
-    jmethodID add = GetMethodID(jni(), GetObjectClass(jni(), video_tracks),
-                                "add", "(Ljava/lang/Object;)Z");
-    jboolean added = jni()->CallBooleanMethod(video_tracks, add, j_track);
-    CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
-    RTC_CHECK(added);
   }
 
   jmethodID m = GetMethodID(jni(), *j_observer_class_, "onAddStream",
                             "(Lorg/webrtc/MediaStream;)V");
   jni()->CallVoidMethod(*j_observer_global_, m, j_stream);
   CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
+
+  // Create an observer to update the Java stream when the native stream's set
+  // of tracks changes.
+  auto observer = rtc::MakeUnique<MediaStreamObserver>(stream);
+  observer->SignalAudioTrackRemoved.connect(
+      this, &PeerConnectionObserverJni::OnAudioTrackRemovedFromStream);
+  observer->SignalVideoTrackRemoved.connect(
+      this, &PeerConnectionObserverJni::OnVideoTrackRemovedFromStream);
+  observer->SignalAudioTrackAdded.connect(
+      this, &PeerConnectionObserverJni::OnAudioTrackAddedToStream);
+  observer->SignalVideoTrackAdded.connect(
+      this, &PeerConnectionObserverJni::OnVideoTrackAddedToStream);
+  stream_observers_.push_back(std::move(observer));
+}
+
+void PeerConnectionObserverJni::AddNativeAudioTrackToJavaStream(
+    rtc::scoped_refptr<AudioTrackInterface> track,
+    jobject j_stream) {
+  jstring id = JavaStringFromStdString(jni(), track->id());
+  // Java AudioTrack holds one reference. Corresponding Release() is in
+  // MediaStreamTrack_free, triggered by AudioTrack.dispose().
+  track->AddRef();
+  jobject j_track = jni()->NewObject(*j_audio_track_class_, j_audio_track_ctor_,
+                                     reinterpret_cast<jlong>(track.get()), id);
+  CHECK_EXCEPTION(jni()) << "error during NewObject";
+
+  // Now add to the audioTracks linked list.
+  jfieldID audio_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "audioTracks", "Ljava/util/LinkedList;");
+  jobject audio_tracks = GetObjectField(jni(), j_stream, audio_tracks_id);
+  jmethodID add = GetMethodID(jni(), GetObjectClass(jni(), audio_tracks), "add",
+                              "(Ljava/lang/Object;)Z");
+  jboolean added = jni()->CallBooleanMethod(audio_tracks, add, j_track);
+  CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
+  RTC_CHECK(added);
+}
+
+void PeerConnectionObserverJni::AddNativeVideoTrackToJavaStream(
+    rtc::scoped_refptr<VideoTrackInterface> track,
+    jobject j_stream) {
+  jstring id = JavaStringFromStdString(jni(), track->id());
+  // Java VideoTrack holds one reference. Corresponding Release() is in
+  // MediaStreamTrack_free, triggered by VideoTrack.dispose().
+  track->AddRef();
+  jobject j_track = jni()->NewObject(*j_video_track_class_, j_video_track_ctor_,
+                                     reinterpret_cast<jlong>(track.get()), id);
+  CHECK_EXCEPTION(jni()) << "error during NewObject";
+
+  // Now add to the videoTracks linked list.
+  jfieldID video_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "videoTracks", "Ljava/util/LinkedList;");
+  jobject video_tracks = GetObjectField(jni(), j_stream, video_tracks_id);
+  jmethodID add = GetMethodID(jni(), GetObjectClass(jni(), video_tracks), "add",
+                              "(Ljava/lang/Object;)Z");
+  jboolean added = jni()->CallBooleanMethod(video_tracks, add, j_track);
+  CHECK_EXCEPTION(jni()) << "error during CallBooleanMethod";
+  RTC_CHECK(added);
+}
+
+void PeerConnectionObserverJni::RemoveAndDisposeNativeTrackFromJavaTrackList(
+    MediaStreamTrackInterface* track,
+    jobject j_tracks) {
+  Iterable iterable_tracks(jni(), j_tracks);
+  for (auto it = iterable_tracks.begin(); it != iterable_tracks.end(); ++it) {
+    MediaStreamTrackInterface* native_track =
+        reinterpret_cast<MediaStreamTrackInterface*>(
+            jni()->GetLongField(*it, j_native_track_id_));
+    CHECK_EXCEPTION(jni()) << "error during GetLongField";
+    if (native_track == track) {
+      jni()->CallVoidMethod(*it, j_track_dispose_id_);
+      it.Remove();
+      return;
+    }
+  }
+  // If we reached this point, we didn't find the track, which means we're
+  // getting a "track removed" callback but the Java stream doesn't have a
+  // corresponding track, which indicates a bug somewhere.
+  RTC_NOTREACHED();
+}
+
+void PeerConnectionObserverJni::OnAudioTrackAddedToStream(
+    AudioTrackInterface* track,
+    MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+  AddNativeAudioTrackToJavaStream(track, j_stream);
+}
+
+void PeerConnectionObserverJni::OnVideoTrackAddedToStream(
+    VideoTrackInterface* track,
+    MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+  AddNativeVideoTrackToJavaStream(track, j_stream);
+}
+
+void PeerConnectionObserverJni::OnAudioTrackRemovedFromStream(
+    AudioTrackInterface* track,
+    MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+  jfieldID audio_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "audioTracks", "Ljava/util/LinkedList;");
+  jobject audio_tracks = GetObjectField(jni(), j_stream, audio_tracks_id);
+  RemoveAndDisposeNativeTrackFromJavaTrackList(track, audio_tracks);
+}
+
+void PeerConnectionObserverJni::OnVideoTrackRemovedFromStream(
+    VideoTrackInterface* track,
+    MediaStreamInterface* stream) {
+  ScopedLocalRefFrame local_ref_frame(jni());
+  jobject j_stream = GetOrCreateJavaStream(stream);
+  jfieldID video_tracks_id = GetFieldID(
+      jni(), *j_media_stream_class_, "videoTracks", "Ljava/util/LinkedList;");
+  jobject video_tracks = GetObjectField(jni(), j_stream, video_tracks_id);
+  RemoveAndDisposeNativeTrackFromJavaTrackList(track, video_tracks);
 }
 
 void PeerConnectionObserverJni::OnRemoveStream(
@@ -187,6 +281,7 @@ void PeerConnectionObserverJni::OnRemoveStream(
                             "(Lorg/webrtc/MediaStream;)V");
   jni()->CallVoidMethod(*j_observer_global_, m, j_stream);
   CHECK_EXCEPTION(jni()) << "error during CallVoidMethod";
+
   // Release the refptr reference so that DisposeRemoteStream can assert
   // it removes the final reference.
   stream = nullptr;
@@ -250,7 +345,18 @@ void PeerConnectionObserverJni::SetConstraints(
 
 void PeerConnectionObserverJni::DisposeRemoteStream(
     const NativeToJavaStreamsMap::iterator& it) {
+  MediaStreamInterface* stream = it->first;
   jobject j_stream = it->second;
+
+  // Remove the observer first, so it doesn't react to events during deletion.
+  stream_observers_.erase(
+      std::remove_if(
+          stream_observers_.begin(), stream_observers_.end(),
+          [stream](const std::unique_ptr<MediaStreamObserver>& observer) {
+            return observer->stream() == stream;
+          }),
+      stream_observers_.end());
+
   remote_streams_.erase(it);
   jni()->CallVoidMethod(
       j_stream, GetMethodID(jni(), *j_media_stream_class_, "dispose", "()V"));
@@ -277,7 +383,6 @@ jobject PeerConnectionObserverJni::GetOrCreateJavaStream(
   if (it != remote_streams_.end()) {
     return it->second;
   }
-
   // Java MediaStream holds one reference. Corresponding Release() is in
   // MediaStream_free, triggered by MediaStream.dispose().
   stream->AddRef();

sdk/android/src/jni/pc/peerconnectionobserver_jni.h

@@ -11,6 +11,7 @@
 #ifndef SDK_ANDROID_SRC_JNI_PC_PEERCONNECTIONOBSERVER_JNI_H_
 #define SDK_ANDROID_SRC_JNI_PC_PEERCONNECTIONOBSERVER_JNI_H_
 
+#include <pc/mediastreamobserver.h>
 #include <map>
 #include <memory>
 #include <vector>
@@ -25,7 +26,8 @@ namespace jni {
 // Adapter between the C++ PeerConnectionObserver interface and the Java
 // PeerConnection.Observer interface.  Wraps an instance of the Java interface
 // and dispatches C++ callbacks to Java.
-class PeerConnectionObserverJni : public PeerConnectionObserver {
+class PeerConnectionObserverJni : public PeerConnectionObserver,
+                                  public sigslot::has_slots<> {
  public:
   PeerConnectionObserverJni(JNIEnv* jni, jobject j_observer);
   virtual ~PeerConnectionObserverJni();
@@ -56,6 +58,10 @@ class PeerConnectionObserverJni : public PeerConnectionObserver {
  private:
   typedef std::map<MediaStreamInterface*, jobject> NativeToJavaStreamsMap;
   typedef std::map<RtpReceiverInterface*, jobject> NativeToJavaRtpReceiverMap;
+  typedef std::map<MediaStreamTrackInterface*, jobject>
+      NativeToJavaMediaTrackMap;
+  typedef std::map<MediaStreamTrackInterface*, RtpReceiverInterface*>
+      NativeMediaStreamTrackToNativeRtpReceiver;
 
   void DisposeRemoteStream(const NativeToJavaStreamsMap::iterator& it);
   void DisposeRtpReceiver(const NativeToJavaRtpReceiverMap::iterator& it);
@@ -70,10 +76,44 @@ class PeerConnectionObserverJni : public PeerConnectionObserver {
       JNIEnv* jni,
       const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams);
 
+  // The three methods below must be called from within a local ref
+  // frame (e.g., using ScopedLocalRefFrame), otherwise they will
+  // leak references.
+  //
+  // Create a Java track object to wrap |track|, and add it to |j_stream|.
+  void AddNativeAudioTrackToJavaStream(
+      rtc::scoped_refptr<AudioTrackInterface> track,
+      jobject j_stream);
+  void AddNativeVideoTrackToJavaStream(
+      rtc::scoped_refptr<VideoTrackInterface> track,
+      jobject j_stream);
+  // Remove and dispose the Java MediaStreamTrack object that wraps |track|,
+  // given |j_tracks| which is a linked list of tracks (either the videoTracks
+  // or audioTracks member of MediaStream).
+  //
+  // DCHECKs if the track isn't found.
+  void RemoveAndDisposeNativeTrackFromJavaTrackList(
+      MediaStreamTrackInterface* track,
+      jobject j_tracks);
+
+  // Callbacks invoked when a native stream changes, and the Java stream needs
+  // to be updated; MediaStreamObserver is used to make this simpler.
+  void OnAudioTrackAddedToStream(AudioTrackInterface* track,
+                                 MediaStreamInterface* stream);
+  void OnVideoTrackAddedToStream(VideoTrackInterface* track,
+                                 MediaStreamInterface* stream);
+  void OnAudioTrackRemovedFromStream(AudioTrackInterface* track,
+                                     MediaStreamInterface* stream);
+  void OnVideoTrackRemovedFromStream(VideoTrackInterface* track,
+                                     MediaStreamInterface* stream);
+
   const ScopedGlobalRef<jobject> j_observer_global_;
   const ScopedGlobalRef<jclass> j_observer_class_;
   const ScopedGlobalRef<jclass> j_media_stream_class_;
   const jmethodID j_media_stream_ctor_;
+  const ScopedGlobalRef<jclass> j_media_stream_track_class_;
+  const jmethodID j_track_dispose_id_;
+  const jfieldID j_native_track_id_;
   const ScopedGlobalRef<jclass> j_audio_track_class_;
   const jmethodID j_audio_track_ctor_;
   const ScopedGlobalRef<jclass> j_video_track_class_;
@@ -82,11 +122,13 @@ class PeerConnectionObserverJni : public PeerConnectionObserver {
   const jmethodID j_data_channel_ctor_;
   const ScopedGlobalRef<jclass> j_rtp_receiver_class_;
   const jmethodID j_rtp_receiver_ctor_;
+
   // C++ -> Java remote streams. The stored jobects are global refs and must be
   // manually deleted upon removal. Use DisposeRemoteStream().
   NativeToJavaStreamsMap remote_streams_;
   NativeToJavaRtpReceiverMap rtp_receivers_;
   std::unique_ptr<MediaConstraintsJni> constraints_;
+  std::vector<std::unique_ptr<webrtc::MediaStreamObserver>> stream_observers_;
 };
 
 }  // namespace jni