Title: [268102] branches/safari-610-branch
Revision
268102
Author
alanc...@apple.com
Date
2020-10-06 17:23:27 -0700 (Tue, 06 Oct 2020)

Log Message

Cherry-pick r267941. rdar://problem/70024420

    Make sure MediaRecorder does not call fetchData until the last fetchData is completed
    https://bugs.webkit.org/show_bug.cgi?id=217276

    Reviewed by Darin Adler.

    Source/WebCore:

    When fetchData is called while an existing fetchData is inflight, enqueue the callback in a deque.
    When the inflight fetchData completes, call the enqueued callbacks in order with a null blob.

    Add ASSERT in MediaRecorderPrivateWriter to make sure we do not call requestMediaDataWhenReadyOnQueue too many times.

    Covered by updated http/wpt/mediarecorder/MediaRecorder-dataavailable.html.

    * Modules/mediarecorder/MediaRecorder.cpp:
    (WebCore::MediaRecorder::stopRecording):
    (WebCore::MediaRecorder::requestData):
    Do not enable the timer if MediaRecorder is not active as a small optimization.
    (WebCore::MediaRecorder::fetchData):
    * Modules/mediarecorder/MediaRecorder.h:
    * platform/mediarecorder/MediaRecorderPrivateMock.cpp:
    (WebCore::MediaRecorderPrivateMock::fetchData):
    * platform/mediarecorder/MediaRecorderPrivateMock.h:
    * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
    (WebCore::MediaRecorderPrivateWriter::flushCompressedSampleBuffers):

    LayoutTests:

    Add test to cover patch
    Update some test expectations according bot results.

    * http/wpt/mediarecorder/MediaRecorder-dataavailable-expected.txt:
    * http/wpt/mediarecorder/MediaRecorder-dataavailable.html:
    * platform/mac/TestExpectations:

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

Modified Paths

Diff

Modified: branches/safari-610-branch/LayoutTests/ChangeLog (268101 => 268102)


--- branches/safari-610-branch/LayoutTests/ChangeLog	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/LayoutTests/ChangeLog	2020-10-07 00:23:27 UTC (rev 268102)
@@ -1,5 +1,60 @@
 2020-10-06  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r267941. rdar://problem/70024420
+
+    Make sure MediaRecorder does not call fetchData until the last fetchData is completed
+    https://bugs.webkit.org/show_bug.cgi?id=217276
+    
+    Reviewed by Darin Adler.
+    
+    Source/WebCore:
+    
+    When fetchData is called while an existing fetchData is inflight, enqueue the callback in a deque.
+    When the inflight fetchData completes, call the enqueued callbacks in order with a null blob.
+    
+    Add ASSERT in MediaRecorderPrivateWriter to make sure we do not call requestMediaDataWhenReadyOnQueue too many times.
+    
+    Covered by updated http/wpt/mediarecorder/MediaRecorder-dataavailable.html.
+    
+    * Modules/mediarecorder/MediaRecorder.cpp:
+    (WebCore::MediaRecorder::stopRecording):
+    (WebCore::MediaRecorder::requestData):
+    Do not enable the timer if MediaRecorder is not active as a small optimization.
+    (WebCore::MediaRecorder::fetchData):
+    * Modules/mediarecorder/MediaRecorder.h:
+    * platform/mediarecorder/MediaRecorderPrivateMock.cpp:
+    (WebCore::MediaRecorderPrivateMock::fetchData):
+    * platform/mediarecorder/MediaRecorderPrivateMock.h:
+    * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
+    (WebCore::MediaRecorderPrivateWriter::flushCompressedSampleBuffers):
+    
+    LayoutTests:
+    
+    Add test to cover patch
+    Update some test expectations according bot results.
+    
+    * http/wpt/mediarecorder/MediaRecorder-dataavailable-expected.txt:
+    * http/wpt/mediarecorder/MediaRecorder-dataavailable.html:
+    * platform/mac/TestExpectations:
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267941 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-10-04  Youenn Fablet  <you...@apple.com>
+
+            Make sure MediaRecorder does not call fetchData until the last fetchData is completed
+            https://bugs.webkit.org/show_bug.cgi?id=217276
+
+            Reviewed by Darin Adler.
+
+            Add test to cover patch
+            Update some test expectations according bot results.
+
+            * http/wpt/mediarecorder/MediaRecorder-dataavailable-expected.txt:
+            * http/wpt/mediarecorder/MediaRecorder-dataavailable.html:
+            * platform/mac/TestExpectations:
+
+2020-10-06  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r267836. rdar://problem/70024631
 
     http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html is flaky

Modified: branches/safari-610-branch/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable.html (268101 => 268102)


--- branches/safari-610-branch/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable.html	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable.html	2020-10-07 00:23:27 UTC (rev 268102)
@@ -114,6 +114,31 @@
         }, 1000);
     }, 'MediaRecorder will fire a dataavailable event with a blob data for a video-audio stream when stop() is called');
 
+    promise_test(async t => {
+        const stream = await navigator.mediaDevices.getUserMedia({ audio : true });
+        const recorder = new MediaRecorder(stream);
+
+        let count = 0;
+        const dataPromise = new Promise(resolve => recorder._ondataavailable_ = () => {
+            if (++count === 5)
+                resolve();
+        });
+        const startPromise = new Promise(resolve => recorder._onstart_ = resolve);
+        recorder.start();
+        await startPromise;
+
+        recorder.requestData();
+        recorder.requestData();
+        recorder.requestData();
+
+        await new Promise(resolve => setTimeout(resolve, 0));
+
+        recorder.requestData();
+        recorder.stop();
+
+        return dataPromise;
+    }, 'Make sure to handle well the case of fetching data quickly and stopping the recorder while fetching data is ongoing');
+
 </script>
 </body>
 </html>

Modified: branches/safari-610-branch/LayoutTests/platform/mac/TestExpectations (268101 => 268102)


--- branches/safari-610-branch/LayoutTests/platform/mac/TestExpectations	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/LayoutTests/platform/mac/TestExpectations	2020-10-07 00:23:27 UTC (rev 268102)
@@ -1665,6 +1665,9 @@
 [ Catalina+ ] imported/w3c/web-platform-tests/mediacapture-record [ Pass Failure ]
 [ Catalina+ ] fast/history/page-cache-media-recorder.html [ Pass Failure ]
 
+[ Catalina+ ] imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-no-sink.https.html [ Pass Failure ]
+[ Catalina+ ] imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-peerconnection.https.html [ Pass Failure ]
+
 webkit.org/b/200128 imported/w3c/web-platform-tests/html/semantics/embedded-content/the-video-element/video_timeupdate_on_seek.html [ Timeout Pass ]
 
 # rdar://55405851 ([ macOS ] Layout tests webgpu/*-triangle-strip.html are flaky failures. (201827))

Modified: branches/safari-610-branch/Source/WebCore/ChangeLog (268101 => 268102)


--- branches/safari-610-branch/Source/WebCore/ChangeLog	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/Source/WebCore/ChangeLog	2020-10-07 00:23:27 UTC (rev 268102)
@@ -1,5 +1,72 @@
 2020-10-06  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r267941. rdar://problem/70024420
+
+    Make sure MediaRecorder does not call fetchData until the last fetchData is completed
+    https://bugs.webkit.org/show_bug.cgi?id=217276
+    
+    Reviewed by Darin Adler.
+    
+    Source/WebCore:
+    
+    When fetchData is called while an existing fetchData is inflight, enqueue the callback in a deque.
+    When the inflight fetchData completes, call the enqueued callbacks in order with a null blob.
+    
+    Add ASSERT in MediaRecorderPrivateWriter to make sure we do not call requestMediaDataWhenReadyOnQueue too many times.
+    
+    Covered by updated http/wpt/mediarecorder/MediaRecorder-dataavailable.html.
+    
+    * Modules/mediarecorder/MediaRecorder.cpp:
+    (WebCore::MediaRecorder::stopRecording):
+    (WebCore::MediaRecorder::requestData):
+    Do not enable the timer if MediaRecorder is not active as a small optimization.
+    (WebCore::MediaRecorder::fetchData):
+    * Modules/mediarecorder/MediaRecorder.h:
+    * platform/mediarecorder/MediaRecorderPrivateMock.cpp:
+    (WebCore::MediaRecorderPrivateMock::fetchData):
+    * platform/mediarecorder/MediaRecorderPrivateMock.h:
+    * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
+    (WebCore::MediaRecorderPrivateWriter::flushCompressedSampleBuffers):
+    
+    LayoutTests:
+    
+    Add test to cover patch
+    Update some test expectations according bot results.
+    
+    * http/wpt/mediarecorder/MediaRecorder-dataavailable-expected.txt:
+    * http/wpt/mediarecorder/MediaRecorder-dataavailable.html:
+    * platform/mac/TestExpectations:
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267941 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-10-04  Youenn Fablet  <you...@apple.com>
+
+            Make sure MediaRecorder does not call fetchData until the last fetchData is completed
+            https://bugs.webkit.org/show_bug.cgi?id=217276
+
+            Reviewed by Darin Adler.
+
+            When fetchData is called while an existing fetchData is inflight, enqueue the callback in a deque.
+            When the inflight fetchData completes, call the enqueued callbacks in order with a null blob.
+
+            Add ASSERT in MediaRecorderPrivateWriter to make sure we do not call requestMediaDataWhenReadyOnQueue too many times.
+
+            Covered by updated http/wpt/mediarecorder/MediaRecorder-dataavailable.html.
+
+            * Modules/mediarecorder/MediaRecorder.cpp:
+            (WebCore::MediaRecorder::stopRecording):
+            (WebCore::MediaRecorder::requestData):
+            Do not enable the timer if MediaRecorder is not active as a small optimization.
+            (WebCore::MediaRecorder::fetchData):
+            * Modules/mediarecorder/MediaRecorder.h:
+            * platform/mediarecorder/MediaRecorderPrivateMock.cpp:
+            (WebCore::MediaRecorderPrivateMock::fetchData):
+            * platform/mediarecorder/MediaRecorderPrivateMock.h:
+            * platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
+            (WebCore::MediaRecorderPrivateWriter::flushCompressedSampleBuffers):
+
+2020-10-06  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r267833. rdar://problem/70024626
 
     MediaRecorder should support MediaRecorderOptions.mimeType

Modified: branches/safari-610-branch/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp (268101 => 268102)


--- branches/safari-610-branch/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp	2020-10-07 00:23:27 UTC (rev 268102)
@@ -201,18 +201,14 @@
         return Exception { InvalidStateError, "The MediaRecorder's state cannot be inactive"_s };
 
     stopRecordingInternal();
-    auto& privateRecorder = *m_private;
-    privateRecorder.fetchData([this, pendingActivity = makePendingActivity(*this), privateRecorder = WTFMove(m_private)](auto&& buffer, auto& mimeType) {
-        queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this, buffer = WTFMove(buffer), mimeType]() mutable {
-            if (!m_isActive)
-                return;
-            dispatchEvent(BlobEvent::create(eventNames().dataavailableEvent, Event::CanBubble::No, Event::IsCancelable::No, buffer ? Blob::create(scriptExecutionContext(), buffer.releaseNonNull(), mimeType) : Blob::create(scriptExecutionContext())));
-
-            if (!m_isActive)
-                return;
-            dispatchEvent(Event::create(eventNames().stopEvent, Event::CanBubble::No, Event::IsCancelable::No));
-        });
-    });
+    fetchData([this](auto&& buffer, auto& mimeType) {
+        if (!m_isActive)
+            return;
+        dispatchEvent(BlobEvent::create(eventNames().dataavailableEvent, Event::CanBubble::No, Event::IsCancelable::No, buffer ? Blob::create(scriptExecutionContext(), buffer.releaseNonNull(), mimeType) : Blob::create(scriptExecutionContext())));
+        if (!m_isActive)
+            return;
+        dispatchEvent(Event::create(eventNames().stopEvent, Event::CanBubble::No, Event::IsCancelable::No));
+    }, TakePrivateRecorder::Yes);
     return { };
 }
 
@@ -223,18 +219,45 @@
 
     if (m_timeSliceTimer.isActive())
         m_timeSliceTimer.stop();
-    m_private->fetchData([this, pendingActivity = makePendingActivity(*this)](auto&& buffer, auto& mimeType) {
-        queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this, buffer = WTFMove(buffer), mimeType]() mutable {
-            if (!m_isActive)
-                return;
 
-            dispatchEvent(BlobEvent::create(eventNames().dataavailableEvent, Event::CanBubble::No, Event::IsCancelable::No, buffer ? Blob::create(scriptExecutionContext(), buffer.releaseNonNull(), mimeType) : Blob::create(scriptExecutionContext())));
+    fetchData([this](auto&& buffer, auto& mimeType) {
+        if (!m_isActive)
+            return;
 
-            if (m_timeSlice)
-                m_timeSliceTimer.startOneShot(Seconds::fromMilliseconds(*m_timeSlice));
+        dispatchEvent(BlobEvent::create(eventNames().dataavailableEvent, Event::CanBubble::No, Event::IsCancelable::No, buffer ? Blob::create(scriptExecutionContext(), buffer.releaseNonNull(), mimeType) : Blob::create(scriptExecutionContext())));
+
+        if (m_isActive && m_timeSlice)
+            m_timeSliceTimer.startOneShot(Seconds::fromMilliseconds(*m_timeSlice));
+    }, TakePrivateRecorder::No);
+    return { };
+}
+
+void MediaRecorder::fetchData(FetchDataCallback&& callback, TakePrivateRecorder takeRecorder)
+{
+    auto& privateRecorder = *m_private;
+
+    std::unique_ptr<MediaRecorderPrivate> takenPrivateRecorder;
+    if (takeRecorder == TakePrivateRecorder::Yes)
+        takenPrivateRecorder = WTFMove(m_private);
+
+    auto fetchDataCallback = [this, privateRecorder = WTFMove(takenPrivateRecorder), callback = WTFMove(callback)](auto&& buffer, auto& mimeType) mutable {
+        queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [buffer = WTFMove(buffer), mimeType, callback = WTFMove(callback)]() mutable {
+            callback(WTFMove(buffer), mimeType);
         });
+    };
+
+    if (m_isFetchingData) {
+        m_pendingFetchDataTasks.append(WTFMove(fetchDataCallback));
+        return;
+    }
+
+    m_isFetchingData = true;
+    privateRecorder.fetchData([this, pendingActivity = makePendingActivity(*this), callback = WTFMove(fetchDataCallback)](auto&& buffer, auto& mimeType) mutable {
+        m_isFetchingData = false;
+        callback(WTFMove(buffer), mimeType);
+        for (auto& task : std::exchange(m_pendingFetchDataTasks, { }))
+            task({ }, mimeType);
     });
-    return { };
 }
 
 void MediaRecorder::stopRecordingInternal()

Modified: branches/safari-610-branch/Source/WebCore/Modules/mediarecorder/MediaRecorder.h (268101 => 268102)


--- branches/safari-610-branch/Source/WebCore/Modules/mediarecorder/MediaRecorder.h	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/Source/WebCore/Modules/mediarecorder/MediaRecorder.h	2020-10-07 00:23:27 UTC (rev 268102)
@@ -94,9 +94,12 @@
     bool virtualHasPendingActivity() const final;
     
     void stopRecordingInternal();
-
     void dispatchError(Exception&&);
 
+    enum class TakePrivateRecorder { No, Yes };
+    using FetchDataCallback = Function<void(RefPtr<SharedBuffer>&&, const String& mimeType)>;
+    void fetchData(FetchDataCallback&&, TakePrivateRecorder);
+
     // MediaStream::Observer
     void didAddTrack(MediaStreamTrackPrivate&) final { handleTrackChange(); }
     void didRemoveTrack(MediaStreamTrackPrivate&) final { handleTrackChange(); }
@@ -120,6 +123,8 @@
     Timer m_timeSliceTimer;
     
     bool m_isActive { true };
+    bool m_isFetchingData { false };
+    Deque<FetchDataCallback> m_pendingFetchDataTasks;
 };
 
 } // namespace WebCore

Modified: branches/safari-610-branch/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp (268101 => 268102)


--- branches/safari-610-branch/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp	2020-10-07 00:23:27 UTC (rev 268102)
@@ -30,6 +30,7 @@
 
 #include "MediaStreamTrackPrivate.h"
 #include "SharedBuffer.h"
+#include "Timer.h"
 
 namespace WebCore {
 
@@ -91,7 +92,11 @@
         m_buffer.clear();
         buffer = SharedBuffer::create(WTFMove(value));
     }
-    completionHandler(WTFMove(buffer), mimeType());
+
+    // Delay calling the completion handler a bit to mimick real writer behavior.
+    m_delayCompletingTimer.doTask([completionHandler = WTFMove(completionHandler), buffer = WTFMove(buffer), mimeType = mimeType()]() mutable {
+        completionHandler(WTFMove(buffer), mimeType);
+    }, 50_ms);
 }
 
 const String& MediaRecorderPrivateMock::mimeType() const

Modified: branches/safari-610-branch/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h (268101 => 268102)


--- branches/safari-610-branch/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h	2020-10-07 00:23:27 UTC (rev 268102)
@@ -32,6 +32,7 @@
 
 namespace WebCore {
 
+class DeferrableTaskTimer;
 class MediaStreamTrackPrivate;
 
 class WEBCORE_EXPORT MediaRecorderPrivateMock final
@@ -55,6 +56,7 @@
     unsigned m_counter { 0 };
     String m_audioTrackID;
     String m_videoTrackID;
+    DeferrableTaskTimer m_delayCompletingTimer;
 };
 
 } // namespace WebCore

Modified: branches/safari-610-branch/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm (268101 => 268102)


--- branches/safari-610-branch/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm	2020-10-07 00:23:23 UTC (rev 268101)
+++ branches/safari-610-branch/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm	2020-10-07 00:23:27 UTC (rev 268102)
@@ -356,6 +356,7 @@
         return;
     }
 
+    ASSERT(!m_isFlushingSamples);
     m_isFlushingSamples = true;
     auto block = makeBlockPtr([this, weakThis = makeWeakPtr(*this), hasPendingAudioSamples, hasPendingVideoSamples, audioSampleQueue = WTFMove(m_pendingAudioSampleQueue), videoSampleQueue = WTFMove(m_pendingVideoSampleQueue), completionHandler = WTFMove(completionHandler)]() mutable {
         if (!weakThis) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to