track.onmute isn't called for a remote MediaStreamTrack when its counter part track...
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Sep 2018 22:22:15 +0000 (22:22 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Sep 2018 22:22:15 +0000 (22:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176281
<rdar://problem/44525674>

Reviewed by Eric Carlson.

Source/WebCore:

Listen to libwebrtc remove track callbacks.
Implement handling as per https://w3c.github.io/webrtc-pc/#process-remote-track-removal.
This triggers a mute event on the track.

Test: webrtc/remove-track.html

* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::LibWebRTCMediaEndpoint::removeRemoteTrack):
(WebCore::LibWebRTCMediaEndpoint::OnRemoveTrack):
* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
* Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
(WebCore::LibWebRTCPeerConnectionBackend::shouldOfferAllowToReceive const):
Drive by fix: Plan B code path does not mandate having an rtc backend for each sender.

LayoutTests:

* webrtc/remove-track-expected.txt: Added.
* webrtc/remove-track.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/webrtc/remove-track-expected.txt [new file with mode: 0644]
LayoutTests/webrtc/remove-track.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp

index 084e58f..69aedb9 100644 (file)
@@ -1,3 +1,14 @@
+2018-09-17  Youenn Fablet  <youenn@apple.com>
+
+        track.onmute isn't called for a remote MediaStreamTrack when its counter part track is removed from the peer connection
+        https://bugs.webkit.org/show_bug.cgi?id=176281
+        <rdar://problem/44525674>
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/remove-track-expected.txt: Added.
+        * webrtc/remove-track.html: Added.
+
 2018-09-17  Dawei Fenton  <realdawei@apple.com>
 
         Fixed typo in TestExpectations file.
diff --git a/LayoutTests/webrtc/remove-track-expected.txt b/LayoutTests/webrtc/remove-track-expected.txt
new file mode 100644 (file)
index 0000000..de6c3c2
--- /dev/null
@@ -0,0 +1,5 @@
+
+PASS Setup audio video exchange 
+PASS Remove video track 
+PASS Remove audio track 
+
diff --git a/LayoutTests/webrtc/remove-track.html b/LayoutTests/webrtc/remove-track.html
new file mode 100644 (file)
index 0000000..827f522
--- /dev/null
@@ -0,0 +1,72 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Testing basic video exchange from offerer to receiver</title>
+        <script src="../resources/testharness.js"></script>
+        <script src="../resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <script src ="routines.js"></script>
+        <script>
+let firstConnection, secondConnection;
+let stream;
+let remoteAudioTrack, remoteVideoTrack;
+promise_test(async (test) => {
+    if (window.testRunner)
+        testRunner.setUserMediaPermission(true);
+
+    stream = await navigator.mediaDevices.getUserMedia({ video: true, audio: true });
+    await new Promise((resolve, reject) => {
+        createConnections((connection) => {
+            firstConnection = connection;
+            firstConnection.addTrack(stream.getVideoTracks()[0], stream);
+            firstConnection.addTrack(stream.getAudioTracks()[0], stream);
+        }, (connection) => {
+            secondConnection = connection;
+            secondConnection.ontrack = (trackEvent) => {
+                if (!remoteVideoTrack) {
+                    remoteVideoTrack = trackEvent.track;
+                    return;
+                }
+                remoteAudioTrack = trackEvent.track;
+                resolve();
+            };
+        });
+        setTimeout(() => reject("Test timed out"), 5000);
+    });
+}, "Setup audio video exchange");
+
+async function renegotiate()
+{
+    let d = await firstConnection.createOffer();
+    await firstConnection.setLocalDescription(d);
+    await secondConnection.setRemoteDescription(firstConnection.localDescription);
+    d = await secondConnection.createAnswer();
+    await secondConnection.setLocalDescription(d);
+}
+
+promise_test((test) => {
+    const promise = new Promise((resolve, reject) => {
+        remoteVideoTrack.onmute = resolve;
+        setTimeout(() => reject("Test timed out"), 5000);
+    });
+     
+    firstConnection.removeTrack(firstConnection.getSenders()[0]);
+    renegotiate();
+    return promise;
+}, "Remove video track");
+
+promise_test((test) => {
+    const promise = new Promise((resolve, reject) => {
+        remoteAudioTrack.onmute = resolve;
+        setTimeout(() => reject("Test timed out"), 5000);
+    });
+     
+    firstConnection.removeTrack(firstConnection.getSenders()[1]);
+    renegotiate();
+    return promise;
+}, "Remove audio track");
+        </script>
+    </body>
+</html>
index 5549b4b..3c9cd82 100644 (file)
@@ -1,3 +1,25 @@
+2018-09-17  Youenn Fablet  <youenn@apple.com>
+
+        track.onmute isn't called for a remote MediaStreamTrack when its counter part track is removed from the peer connection
+        https://bugs.webkit.org/show_bug.cgi?id=176281
+        <rdar://problem/44525674>
+
+        Reviewed by Eric Carlson.
+
+        Listen to libwebrtc remove track callbacks.
+        Implement handling as per https://w3c.github.io/webrtc-pc/#process-remote-track-removal.
+        This triggers a mute event on the track.
+
+        Test: webrtc/remove-track.html
+
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+        (WebCore::LibWebRTCMediaEndpoint::removeRemoteTrack):
+        (WebCore::LibWebRTCMediaEndpoint::OnRemoveTrack):
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:
+        * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
+        (WebCore::LibWebRTCPeerConnectionBackend::shouldOfferAllowToReceive const):
+        Drive by fix: Plan B code path does not mandate having an rtc backend for each sender.
+
 2018-09-17  Simon Fraser  <simon.fraser@apple.com>
 
         Add more Fullscreen logging
index 8535d2b..b25b564 100644 (file)
@@ -445,6 +445,22 @@ void LibWebRTCMediaEndpoint::newTransceiver(rtc::scoped_refptr<webrtc::RtpTransc
     fireTrackEvent(makeRef(newTransceiver.receiver()), newTransceiver.receiver().track(), rtcReceiver->streams(), makeRef(newTransceiver));
 }
 
+void LibWebRTCMediaEndpoint::removeRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>&& receiver)
+{
+    // FIXME: Support plan B code path.
+    if (!RuntimeEnabledFeatures::sharedFeatures().webRTCUnifiedPlanEnabled())
+        return;
+
+    auto* transceiver = m_peerConnectionBackend.existingTransceiver([&receiver](auto& transceiverBackend) {
+        auto* rtcTransceiver = transceiverBackend.rtcTransceiver();
+        return rtcTransceiver && receiver.get() == rtcTransceiver->receiver().get();
+    });
+    if (!transceiver)
+        return;
+
+    transceiver->receiver().track().source().setMuted(true);
+}
+
 template<typename T>
 std::optional<LibWebRTCMediaEndpoint::Backends> LibWebRTCMediaEndpoint::createTransceiverBackends(T&& trackOrKind, const RTCRtpTransceiverInit& init, LibWebRTCRtpSenderBackend::Source&& source)
 {
@@ -546,6 +562,14 @@ void LibWebRTCMediaEndpoint::OnTrack(rtc::scoped_refptr<webrtc::RtpTransceiverIn
     });
 }
 
+void LibWebRTCMediaEndpoint::OnRemoveTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface> receiver)
+{
+    callOnMainThread([protectedThis = makeRef(*this), receiver = WTFMove(receiver)]() mutable {
+        if (protectedThis->isStopped())
+            return;
+        protectedThis->removeRemoteTrack(WTFMove(receiver));
+    });
+}
 
 std::unique_ptr<RTCDataChannelHandler> LibWebRTCMediaEndpoint::createDataChannel(const String& label, const RTCDataChannelInit& options)
 {
index 5806a83..177e7aa 100644 (file)
@@ -118,6 +118,7 @@ private:
     void OnDataChannel(rtc::scoped_refptr<webrtc::DataChannelInterface>) final;
     void OnAddTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&) final;
     void OnTrack(rtc::scoped_refptr<webrtc::RtpTransceiverInterface>) final;
+    void OnRemoveTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>) final;
 
     void OnRenegotiationNeeded() final;
     void OnIceConnectionChange(webrtc::PeerConnectionInterface::IceConnectionState) final;
@@ -135,6 +136,7 @@ private:
     void addRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>&&, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&);
     void removeRemoteStream(webrtc::MediaStreamInterface&);
     void newTransceiver(rtc::scoped_refptr<webrtc::RtpTransceiverInterface>&&);
+    void removeRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiverInterface>&&);
 
     void fireTrackEvent(Ref<RTCRtpReceiver>&&, Ref<MediaStreamTrack>&&, const std::vector<rtc::scoped_refptr<webrtc::MediaStreamInterface>>&, RefPtr<RTCRtpTransceiver>&&);
 
index c21d26b..3c4f99a 100644 (file)
@@ -486,8 +486,8 @@ bool LibWebRTCPeerConnectionBackend::shouldOfferAllowToReceive(const char* kind)
         if (transceiver->direction() != RTCRtpTransceiverDirection::Sendrecv)
             continue;
 
-        auto& backend = static_cast<LibWebRTCRtpSenderBackend&>(*transceiver->sender().backend());
-        if (!backend.rtcSender())
+        auto* backend = static_cast<LibWebRTCRtpSenderBackend*>(transceiver->sender().backend());
+        if (backend && !backend->rtcSender())
             return true;
     }
     return false;