MediaPlayerPrivateMediaStreamAVFObjC should unobserve the tracks from its audio and...
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 May 2020 16:05:54 +0000 (16:05 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 May 2020 16:05:54 +0000 (16:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=211444
<rdar://problem/62886221>

Reviewed by Eric Carlson.

Source/WebCore:

Test: fast/mediastream/MediaStream-removeTrack-while-playing.html

* platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaStreamAVFObjC::~MediaPlayerPrivateMediaStreamAVFObjC):
We keep maps of audio and video tracks we are observing.
Use these two maps to properly unobserve all tracks at destruction time.
While this is not strictly needed since we are using weak pointers, this helps keeping the code healthy.
* platform/mediastream/MediaStreamTrackPrivate.cpp:
(WebCore::MediaStreamTrackPrivate::forEachObserver):
Add a debug ASSERT so that we ensure add/remove observers is done properly.

LayoutTests:

* fast/mediastream/MediaStream-removeTrack-while-playing-expected.txt: Added.
* fast/mediastream/MediaStream-removeTrack-while-playing.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/mediastream/MediaStream-removeTrack-while-playing-expected.txt [new file with mode: 0644]
LayoutTests/fast/mediastream/MediaStream-removeTrack-while-playing.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm
Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp

index 2a716f3..3cf48e6 100644 (file)
@@ -1,3 +1,14 @@
+2020-05-05  Youenn Fablet  <youenn@apple.com>
+
+        MediaPlayerPrivateMediaStreamAVFObjC should unobserve the tracks from its audio and video track sets
+        https://bugs.webkit.org/show_bug.cgi?id=211444
+        <rdar://problem/62886221>
+
+        Reviewed by Eric Carlson.
+
+        * fast/mediastream/MediaStream-removeTrack-while-playing-expected.txt: Added.
+        * fast/mediastream/MediaStream-removeTrack-while-playing.html: Added.
+
 2020-05-05  Alicia Boya GarcĂ­a  <aboya@igalia.com>
 
         [GStreamer] Video loops when ran in rr record --chaos
diff --git a/LayoutTests/fast/mediastream/MediaStream-removeTrack-while-playing-expected.txt b/LayoutTests/fast/mediastream/MediaStream-removeTrack-while-playing-expected.txt
new file mode 100644 (file)
index 0000000..25a5e3d
--- /dev/null
@@ -0,0 +1,4 @@
+
+
+PASS removeTrack while playing 
+
diff --git a/LayoutTests/fast/mediastream/MediaStream-removeTrack-while-playing.html b/LayoutTests/fast/mediastream/MediaStream-removeTrack-while-playing.html
new file mode 100644 (file)
index 0000000..2e510bc
--- /dev/null
@@ -0,0 +1,30 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Testing addTrack/removeTrack while playing</title>
+        <script src="../../resources/testharness.js"></script>
+        <script src="../../resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <video id="video" autoplay playsInline></video>
+        <script>
+promise_test(async (test) => {
+    const localStream = await navigator.mediaDevices.getUserMedia({audio: true, video: true});
+    const audioTrack = localStream.getAudioTracks()[0];
+    const videoTrack = localStream.getVideoTracks()[0];
+
+    video.srcObject = localStream;
+    await video.play();
+    localStream.onremovetrack = () => { video.srcObject = null; };
+
+    if (window.internals) {
+        internals.removeMediaStreamTrack(localStream, videoTrack);
+        internals.removeMediaStreamTrack(localStream, audioTrack);
+    }
+    audioTrack.stop();
+    videoTrack.stop();
+}, "removeTrack while playing");
+        </script>
+    </body>
+</html>
index 305624d..2c9e7bf 100644 (file)
@@ -1,3 +1,22 @@
+2020-05-05  Youenn Fablet  <youenn@apple.com>
+
+        MediaPlayerPrivateMediaStreamAVFObjC should unobserve the tracks from its audio and video track sets
+        https://bugs.webkit.org/show_bug.cgi?id=211444
+        <rdar://problem/62886221>
+
+        Reviewed by Eric Carlson.
+
+        Test: fast/mediastream/MediaStream-removeTrack-while-playing.html
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:
+        (WebCore::MediaPlayerPrivateMediaStreamAVFObjC::~MediaPlayerPrivateMediaStreamAVFObjC):
+        We keep maps of audio and video tracks we are observing.
+        Use these two maps to properly unobserve all tracks at destruction time.
+        While this is not strictly needed since we are using weak pointers, this helps keeping the code healthy.
+        * platform/mediastream/MediaStreamTrackPrivate.cpp:
+        (WebCore::MediaStreamTrackPrivate::forEachObserver):
+        Add a debug ASSERT so that we ensure add/remove observers is done properly.
+
 2020-05-05  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, reverting r261130.
index bb5efa6..3685dba 100644 (file)
@@ -158,8 +158,11 @@ MediaPlayerPrivateMediaStreamAVFObjC::~MediaPlayerPrivateMediaStreamAVFObjC()
     if (m_mediaStreamPrivate) {
         m_mediaStreamPrivate->removeObserver(*this);
 
-        for (auto& track : m_mediaStreamPrivate->tracks())
-            track->removeObserver(*this);
+        for (auto& track : m_audioTrackMap.values())
+            track->streamTrack().removeObserver(*this);
+
+        for (auto& track : m_videoTrackMap.values())
+            track->streamTrack().removeObserver(*this);
     }
 
     [m_boundsChangeListener invalidate];
index d05b787..f363f94 100644 (file)
@@ -81,6 +81,7 @@ MediaStreamTrackPrivate::~MediaStreamTrackPrivate()
 void MediaStreamTrackPrivate::forEachObserver(const Function<void(Observer&)>& apply)
 {
     ASSERT(isMainThread());
+    ASSERT(!m_observers.hasNullReferences());
     auto protectedThis = makeRef(*this);
     m_observers.forEach(apply);
 }