Diff
Modified: trunk/LayoutTests/ChangeLog (267940 => 267941)
--- trunk/LayoutTests/ChangeLog 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/LayoutTests/ChangeLog 2020-10-04 08:16:33 UTC (rev 267941)
@@ -1,3 +1,17 @@
+2020-10-04 Youenn Fablet <[email protected]>
+
+ 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-03 Oriol Brufau <[email protected]>
[css-lists] Implement list-style-type: <string>
Modified: trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable-expected.txt (267940 => 267941)
--- trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable-expected.txt 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable-expected.txt 2020-10-04 08:16:33 UTC (rev 267941)
@@ -2,4 +2,5 @@
PASS MediaRecorder will fire a dataavailable event with a blob data for a video-only stream when stop() is called
PASS MediaRecorder will fire a dataavailable event with a blob data for a audio-only stream when stop() is called
PASS MediaRecorder will fire a dataavailable event with a blob data for a video-audio stream when stop() is called
+PASS Make sure to handle well the case of fetching data quickly and stopping the recorder while fetching data is ongoing
Modified: trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable.html (267940 => 267941)
--- trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable.html 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/LayoutTests/http/wpt/mediarecorder/MediaRecorder-dataavailable.html 2020-10-04 08:16:33 UTC (rev 267941)
@@ -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: trunk/LayoutTests/platform/mac/TestExpectations (267940 => 267941)
--- trunk/LayoutTests/platform/mac/TestExpectations 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2020-10-04 08:16:33 UTC (rev 267941)
@@ -1655,6 +1655,9 @@
[ Mojave ] imported/w3c/web-platform-tests/mediacapture-record [ Skip ]
[ Mojave ] fast/history/page-cache-media-recorder.html [ Skip ]
+[ 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: trunk/Source/WebCore/ChangeLog (267940 => 267941)
--- trunk/Source/WebCore/ChangeLog 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/Source/WebCore/ChangeLog 2020-10-04 08:16:33 UTC (rev 267941)
@@ -1,3 +1,29 @@
+2020-10-04 Youenn Fablet <[email protected]>
+
+ 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-03 Oriol Brufau <[email protected]>
[css-lists] Implement list-style-type: <string>
Modified: trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp (267940 => 267941)
--- trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp 2020-10-04 08:16:33 UTC (rev 267941)
@@ -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: trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.h (267940 => 267941)
--- trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.h 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/Source/WebCore/Modules/mediarecorder/MediaRecorder.h 2020-10-04 08:16:33 UTC (rev 267941)
@@ -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: trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp (267940 => 267941)
--- trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.cpp 2020-10-04 08:16:33 UTC (rev 267941)
@@ -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: trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h (267940 => 267941)
--- trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/Source/WebCore/platform/mediarecorder/MediaRecorderPrivateMock.h 2020-10-04 08:16:33 UTC (rev 267941)
@@ -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: trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm (267940 => 267941)
--- trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm 2020-10-04 01:58:51 UTC (rev 267940)
+++ trunk/Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm 2020-10-04 08:16:33 UTC (rev 267941)
@@ -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) {
