replaceTrack triggers negotiationneeded

https://bugs.webkit.org/show_bug.cgi?id=180342
<rdar://problem/35822426>

Reviewed by Eric Carlson.

Source/WebCore:

Covered by updated test.
Fix the case of replacing a track for which data is already flowing.
We should probably do the same for null tracks when fully implementing transceivers.

* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::enqueueReplaceTrackTask):

LayoutTests:

* webrtc/video-replace-track.html:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index ec3dfd9..18b8aaf 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2018-03-15  Youenn Fablet  <youenn@apple.com>
+
+        replaceTrack triggers negotiationneeded
+        https://bugs.webkit.org/show_bug.cgi?id=180342
+        <rdar://problem/35822426>
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/video-replace-track.html:
+
 2018-03-15  Chris Dumez  <cdumez@apple.com>
 
         webkitdirectory-open-panel stumbles over unordered results

diff --git a/LayoutTests/webrtc/video-replace-track.html b/LayoutTests/webrtc/video-replace-track.html
index 71739aa..e52c719 100644 (file)
--- a/LayoutTests/webrtc/video-replace-track.html
+++ b/LayoutTests/webrtc/video-replace-track.html
@@ -101,6 +101,7 @@ promise_test((test) => {
     });
 }, "Switching from front to back camera");
 
+var didReplaceTrack = false;
 promise_test((test) => {
     if (window.testRunner)
         testRunner.setUserMediaPermission(true);
@@ -115,6 +116,9 @@ promise_test((test) => {
         return new Promise((resolve, reject) => {
             createConnections((firstConnection) => {
                 sender = firstConnection.addTrack(frontStream.getVideoTracks()[0], frontStream);
+                firstConnection.addEventListener('negotiationneeded', test.step_func(() => {
+                    assert_false(didReplaceTrack, 'negotiationeeded should not be called after replacing an ongoing track');
+                }));
             }, (secondConnection) => {
                 secondConnection.ontrack = (trackEvent) => {
                     resolve(trackEvent.streams[0]);
@@ -132,6 +136,7 @@ promise_test((test) => {
     }).then((stream) => {
         backStream = stream;
         assert_true(backStream.getVideoTracks()[0].getSettings().height === 240, "backStream should be small");
+        didReplaceTrack = true;
         return sender.replaceTrack(backStream.getVideoTracks()[0]);
     }).then(() => {
         return testBackCameraImage();

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 74f111b..e7d3fc3 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2018-03-15  Youenn Fablet  <youenn@apple.com>
+
+        replaceTrack triggers negotiationneeded
+        https://bugs.webkit.org/show_bug.cgi?id=180342
+        <rdar://problem/35822426>
+
+        Reviewed by Eric Carlson.
+
+        Covered by updated test.
+        Fix the case of replacing a track for which data is already flowing.
+        We should probably do the same for null tracks when fully implementing transceivers.
+
+        * Modules/mediastream/RTCPeerConnection.cpp:
+        (WebCore::RTCPeerConnection::enqueueReplaceTrackTask):
+
 2018-03-15  Keith Rollin  <krollin@apple.com>
 
         Telemetry for stalled webpage loads

diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
index 9f3a174..a5832b7 100644 (file)
--- a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
+++ b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
@@ -559,8 +559,10 @@ void RTCPeerConnection::enqueueReplaceTrackTask(RTCRtpSender& sender, Ref<MediaS
     scriptExecutionContext()->postTask([protectedThis = makeRef(*this), protectedSender = makeRef(sender), promise = WTFMove(promise), withTrack = WTFMove(withTrack)](ScriptExecutionContext&) mutable {
         if (protectedThis->isClosed())
             return;
+        bool hasTrack = protectedSender->track();
         protectedSender->setTrack(WTFMove(withTrack));
-        protectedThis->m_backend->notifyAddedTrack(protectedSender.get());
+        if (!hasTrack)
+            protectedThis->m_backend->notifyAddedTrack(protectedSender.get());
         promise.resolve();
     });
 }