Support onremovetrack for RTCPeerConnection removed tracks
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2018 02:31:30 +0000 (02:31 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2018 02:31:30 +0000 (02:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191299

Reviewed by Eric Carlson.

LayoutTests/imported/w3c:

* web-platform-tests/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https-expected.txt:

Source/WebCore:

When applying SDP, LibWebRTCMediaEndpoint gets notified of a removed track.
In that case, make sure to remove it from its remote stream(s) so as
to notify the application of the changes.
Work around the receiver missing the list of streams by storing in a map
the list of the remote streams for a given remote track.

Covered by rebased test.

* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::LibWebRTCMediaEndpoint::mediaStreamFromRTCStream):
(WebCore::LibWebRTCMediaEndpoint::removeRemoteTrack):
(WebCore::LibWebRTCMediaEndpoint::removeRemoteStream):
(WebCore::LibWebRTCMediaEndpoint::stop):
* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237908 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h

index de09d7e..0699dfd 100644 (file)
@@ -1,3 +1,12 @@
+2018-11-06  Youenn Fablet  <youenn@apple.com>
+
+        Support onremovetrack for RTCPeerConnection removed tracks
+        https://bugs.webkit.org/show_bug.cgi?id=191299
+
+        Reviewed by Eric Carlson.
+
+        * web-platform-tests/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https-expected.txt:
+
 2018-11-06  Javier Fernandez  <jfernandez@igalia.com>
 
         CSS grid elements with justify-content: space-around have extra whitespace, sometimes a lot
index 7f15bab..cd464fd 100644 (file)
@@ -1,6 +1,4 @@
 
-Harness Error (TIMEOUT), message = null
-
 PASS addTrack() with a track and no stream makes ontrack fire with a track and no stream. 
 PASS addTrack() with a track and a stream makes ontrack fire with a track and a stream. 
 PASS ontrack fires before setRemoteDescription resolves. 
@@ -10,9 +8,9 @@ PASS stream.onaddtrack fires before setRemoteDescription resolves.
 PASS addTrack() with a track and two streams makes ontrack fire with a track and two streams. 
 PASS ontrack's receiver matches getReceivers(). 
 PASS removeTrack() does not remove the receiver. 
-TIMEOUT removeTrack() makes stream.onremovetrack fire and the track to be removed from the stream. Test timed out
-NOTRUN stream.onremovetrack fires before setRemoteDescription resolves. 
-NOTRUN removeTrack() makes track.onmute fire and the track to be muted. 
-NOTRUN track.onmute fires before setRemoteDescription resolves. 
-NOTRUN removeTrack() twice is safe. 
+PASS removeTrack() makes stream.onremovetrack fire and the track to be removed from the stream. 
+PASS stream.onremovetrack fires before setRemoteDescription resolves. 
+PASS removeTrack() makes track.onmute fire and the track to be muted. 
+PASS track.onmute fires before setRemoteDescription resolves. 
+PASS removeTrack() twice is safe. 
 
index c21e21c..f57e3e2 100644 (file)
@@ -1,3 +1,25 @@
+2018-11-06  Youenn Fablet  <youenn@apple.com>
+
+        Support onremovetrack for RTCPeerConnection removed tracks
+        https://bugs.webkit.org/show_bug.cgi?id=191299
+
+        Reviewed by Eric Carlson.
+
+        When applying SDP, LibWebRTCMediaEndpoint gets notified of a removed track.
+        In that case, make sure to remove it from its remote stream(s) so as
+        to notify the application of the changes.
+        Work around the receiver missing the list of streams by storing in a map
+        the list of the remote streams for a given remote track.
+
+        Covered by rebased test.
+
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+        (WebCore::LibWebRTCMediaEndpoint::mediaStreamFromRTCStream):
+        (WebCore::LibWebRTCMediaEndpoint::removeRemoteTrack):
+        (WebCore::LibWebRTCMediaEndpoint::removeRemoteStream):
+        (WebCore::LibWebRTCMediaEndpoint::stop):
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
+
 2018-11-06  Timothy Hatcher  <timothy@apple.com>
 
         REGRESSION (r237878): css-dark-mode/supported-color-schemes.html is failing on Mojave
index 1373058..ac4e251 100644 (file)
@@ -355,9 +355,9 @@ void LibWebRTCMediaEndpoint::OnSignalingChange(webrtc::PeerConnectionInterface::
 
 MediaStream& LibWebRTCMediaEndpoint::mediaStreamFromRTCStream(webrtc::MediaStreamInterface& rtcStream)
 {
-    auto mediaStream = m_streams.ensure(&rtcStream, [&rtcStream, this] {
-        auto label = rtcStream.id();
-        return MediaStream::create(*m_peerConnectionBackend.connection().scriptExecutionContext(), MediaStreamPrivate::create({ }, fromStdString(label)));
+    auto label = fromStdString(rtcStream.id());
+    auto mediaStream = m_remoteStreamsById.ensure(label, [label, this]() mutable {
+        return MediaStream::create(*m_peerConnectionBackend.connection().scriptExecutionContext(), MediaStreamPrivate::create({ }, WTFMove(label)));
     });
     return *mediaStream.iterator->value;
 }
@@ -408,6 +408,11 @@ void LibWebRTCMediaEndpoint::fireTrackEvent(Ref<RTCRtpReceiver>&& receiver, Ref<
         streams.append(&mediaStream);
         mediaStream.addTrackFromPlatform(track.get());
     }
+    auto streamIds = WTF::map(streams, [](auto& stream) -> String {
+        return stream->id();
+    });
+    m_remoteStreamsFromRemoteTrack.add(track.ptr(), WTFMove(streamIds));
+
     m_peerConnectionBackend.connection().fireEvent(RTCTrackEvent::create(eventNames().trackEvent,
         Event::CanBubble::No, Event::IsCancelable::No, WTFMove(receiver), WTFMove(track), WTFMove(streams), WTFMove(transceiver)));
 }
@@ -487,7 +492,14 @@ void LibWebRTCMediaEndpoint::removeRemoteTrack(rtc::scoped_refptr<webrtc::RtpRec
     if (!transceiver)
         return;
 
-    transceiver->receiver().track().source().setMuted(true);
+    auto& track = transceiver->receiver().track();
+
+    for (auto& id : m_remoteStreamsFromRemoteTrack.get(&track)) {
+        if (auto stream = m_remoteStreamsById.get(id))
+            stream->privateStream().removeTrack(track.privateTrack(), MediaStreamPrivate::NotifyClientOption::Notify);
+    }
+
+    track.source().setMuted(true);
 }
 
 template<typename T>
@@ -543,7 +555,7 @@ std::unique_ptr<LibWebRTCRtpTransceiverBackend> LibWebRTCMediaEndpoint::transcei
 
 void LibWebRTCMediaEndpoint::removeRemoteStream(webrtc::MediaStreamInterface& rtcStream)
 {
-    bool removed = m_streams.remove(&rtcStream);
+    bool removed = m_remoteStreamsById.remove(fromStdString(rtcStream.id()));
     ASSERT_UNUSED(removed, removed);
 }
 
@@ -626,7 +638,8 @@ void LibWebRTCMediaEndpoint::stop()
 
     m_backend->Close();
     m_backend = nullptr;
-    m_streams.clear();
+    m_remoteStreamsById.clear();
+    m_remoteStreamsFromRemoteTrack.clear();
 }
 
 void LibWebRTCMediaEndpoint::OnRenegotiationNeeded()
index efb5113..c6994af 100644 (file)
@@ -184,7 +184,8 @@ private:
     SetLocalSessionDescriptionObserver<LibWebRTCMediaEndpoint> m_setLocalSessionDescriptionObserver;
     SetRemoteSessionDescriptionObserver<LibWebRTCMediaEndpoint> m_setRemoteSessionDescriptionObserver;
 
-    HashMap<webrtc::MediaStreamInterface*, RefPtr<MediaStream>> m_streams;
+    HashMap<String, RefPtr<MediaStream>> m_remoteStreamsById;
+    HashMap<MediaStreamTrack*, Vector<String>> m_remoteStreamsFromRemoteTrack;
 
     bool m_isInitiator { false };
     Timer m_statsLogTimer;