Stopping a cloned MediaStream video track should not stop any other video track
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jul 2019 17:27:53 +0000 (17:27 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Jul 2019 17:27:53 +0000 (17:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199635

Reviewed by Eric Carlson.

Source/WebCore:

In case a track is requesting its source to end, the
RealtimeVideoSource should request its own source to end and not stop it directly.

Also, if a track is removing itself as an observer to a RealtimeVideoSource, we should
stop the underlying source only if this one does not have any other observer.
Covered by updated test.

* platform/mediastream/RealtimeMediaSource.cpp:
(WebCore::RealtimeMediaSource::removeObserver):
* platform/mediastream/RealtimeMediaSource.h:
* platform/mediastream/RealtimeVideoSource.cpp:
(WebCore::RealtimeVideoSource::requestToEnd):
(WebCore::RealtimeVideoSource::stopBeingObserved):
* platform/mediastream/RealtimeVideoSource.h:

LayoutTests:

* fast/mediastream/mediastreamtrack-video-clone-expected.txt:
* fast/mediastream/mediastreamtrack-video-clone.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/mediastream/mediastreamtrack-video-clone-expected.txt
LayoutTests/fast/mediastream/mediastreamtrack-video-clone.html
Source/WebCore/ChangeLog
Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp
Source/WebCore/platform/mediastream/RealtimeMediaSource.h
Source/WebCore/platform/mediastream/RealtimeVideoSource.cpp
Source/WebCore/platform/mediastream/RealtimeVideoSource.h

index cf36feb..e5aa9e1 100644 (file)
@@ -1,3 +1,13 @@
+2019-07-12  Youenn Fablet  <youenn@apple.com>
+
+        Stopping a cloned MediaStream video track should not stop any other video track
+        https://bugs.webkit.org/show_bug.cgi?id=199635
+
+        Reviewed by Eric Carlson.
+
+        * fast/mediastream/mediastreamtrack-video-clone-expected.txt:
+        * fast/mediastream/mediastreamtrack-video-clone.html:
+
 2019-07-12  Timothy Hatcher  <timothy@apple.com>
 
         Drop DarkModeCSSEnabled as an experimental feature and always enable it.
index c36c281..eefda58 100644 (file)
@@ -3,6 +3,9 @@
 PASS Setup for width test 
 PASS Setup for height test 
 PASS Setup for width+height test 
+PASS Stopping a track should not stop its clone 
+PASS Stopping a cloned track should not stop the original track 
+PASS Collecting a cloned track should not stop the original track 
 PASS Check cloned track settings after applying width constraints 
 PASS Check cloned track settings after applying width constraint to original track 
 PASS Check cloned track settings after applying height constraints 
index efdbf86..a1f54de 100644 (file)
@@ -3,6 +3,7 @@
 <head>
     <meta charset="utf-8">
     <title>Clone a video track.</title>
+    <script src="../../resources/gc.js"></script>
     <script src="../../resources/testharness.js"></script>
     <script src="../../resources/testharnessreport.js"></script>
 </head>
         }, "Check cloned track settings after applying width+height constraints to original track");
     }, "Setup for width+height test");
 
+    promise_test(async (t) => {
+        const stream = await navigator.mediaDevices.getUserMedia({ video: { width: 100, height: 100 } });
+        const streamClone = stream.clone();
+
+        video1.srcObject = streamClone;
+        stream.getVideoTracks()[0].stop();
+
+        await video1.play();
+        assert_equals(video1.videoWidth, 100);
+    }, "Stopping a track should not stop its clone");
+
+    promise_test(async (t) => {
+        const stream = await navigator.mediaDevices.getUserMedia({ video: { width: 100, height: 100 } });
+        const streamClone = stream.clone();
+
+        video1.srcObject = stream;
+        streamClone.getVideoTracks()[0].stop();
+
+        await video1.play();
+        assert_equals(video1.videoWidth, 100);
+    }, "Stopping a cloned track should not stop the original track");
+
+    promise_test(async (t) => {
+        const stream = await navigator.mediaDevices.getUserMedia({ video: { width: 100, height: 100 } });
+        stream.clone().getVideoTracks()[0].stop();
+        gc();
+
+        video1.srcObject = stream;
+
+        await video1.play();
+        assert_equals(video1.videoWidth, 100);
+    }, "Collecting a cloned track should not stop the original track");
     </script>
 </body>
 </html>
index c0c8881..dbf92a6 100644 (file)
@@ -1,3 +1,25 @@
+2019-07-12  Youenn Fablet  <youenn@apple.com>
+
+        Stopping a cloned MediaStream video track should not stop any other video track
+        https://bugs.webkit.org/show_bug.cgi?id=199635
+
+        Reviewed by Eric Carlson.
+
+        In case a track is requesting its source to end, the
+        RealtimeVideoSource should request its own source to end and not stop it directly.
+
+        Also, if a track is removing itself as an observer to a RealtimeVideoSource, we should
+        stop the underlying source only if this one does not have any other observer.
+        Covered by updated test.
+
+        * platform/mediastream/RealtimeMediaSource.cpp:
+        (WebCore::RealtimeMediaSource::removeObserver):
+        * platform/mediastream/RealtimeMediaSource.h:
+        * platform/mediastream/RealtimeVideoSource.cpp:
+        (WebCore::RealtimeVideoSource::requestToEnd):
+        (WebCore::RealtimeVideoSource::stopBeingObserved):
+        * platform/mediastream/RealtimeVideoSource.h:
+
 2019-07-12  Timothy Hatcher  <timothy@apple.com>
 
         Drop DarkModeCSSEnabled as an experimental feature and always enable it.
index 6260e37..ceee177 100644 (file)
@@ -69,10 +69,9 @@ void RealtimeMediaSource::addObserver(RealtimeMediaSource::Observer& observer)
 void RealtimeMediaSource::removeObserver(RealtimeMediaSource::Observer& observer)
 {
     auto locker = holdLock(m_observersLock);
-
     m_observers.remove(&observer);
     if (m_observers.isEmpty())
-        stop();
+        stopBeingObserved();
 }
 
 void RealtimeMediaSource::setInterrupted(bool interrupted, bool pageMuted)
index 7ab4ab2..a83e6df 100644 (file)
@@ -109,7 +109,7 @@ public:
     bool isProducingData() const { return m_isProducingData; }
     void start();
     void stop();
-    void requestToEnd(Observer& callingObserver);
+    virtual void requestToEnd(Observer& callingObserver);
 
     bool muted() const { return m_muted; }
     void setMuted(bool);
@@ -233,6 +233,8 @@ private:
     virtual void stopProducingData() { }
     virtual void settingsDidChange(OptionSet<RealtimeMediaSourceSettings::Flag>) { }
 
+    virtual void stopBeingObserved() { stop(); }
+
     virtual void hasEnded() { }
 
 #if !RELEASE_LOG_DISABLED
index ec93705..b135b3c 100644 (file)
@@ -118,6 +118,16 @@ bool RealtimeVideoSource::preventSourceFromStopping()
     return hasObserverPreventingStopping;
 }
 
+void RealtimeVideoSource::requestToEnd(RealtimeMediaSource::Observer&)
+{
+    m_source->requestToEnd(*this);
+}
+
+void RealtimeVideoSource::stopBeingObserved()
+{
+    m_source->requestToEnd(*this);
+}
+
 void RealtimeVideoSource::sourceStopped()
 {
     if (m_source->captureDidFail()) {
index 6f79468..aa37533 100644 (file)
@@ -46,6 +46,8 @@ private:
     bool supportsSizeAndFrameRate(Optional<int> width, Optional<int> height, Optional<double> frameRate) final;
     void setSizeAndFrameRate(Optional<int> width, Optional<int> height, Optional<double> frameRate) final;
     Ref<RealtimeMediaSource> clone() final;
+    void requestToEnd(RealtimeMediaSource::Observer& callingObserver) final;
+    void stopBeingObserved() final;
 
     const RealtimeMediaSourceCapabilities& capabilities() final { return m_source->capabilities(); }
     const RealtimeMediaSourceSettings& settings() final { return m_currentSettings; }