Diff
Modified: trunk/LayoutTests/ChangeLog (249717 => 249718)
--- trunk/LayoutTests/ChangeLog 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/LayoutTests/ChangeLog 2019-09-10 16:06:51 UTC (rev 249718)
@@ -1,3 +1,15 @@
+2019-09-10 Youenn Fablet <you...@apple.com>
+
+ Remove MediaStreamPrivate::scheduleDeferredTask
+ https://bugs.webkit.org/show_bug.cgi?id=200975
+ <rdar://problem/55113418>
+
+ Reviewed by Eric Carlson.
+
+ * fast/mediastream/media-stream-track-source-failure.html:
+ page mediaState may be updated synchronously.
+ Get it just before failing a capture track to verify that the state is being updated after the track is stopped.
+
2019-09-10 Chris Dumez <cdu...@apple.com>
Unreviewed, unskip / rebaseline a few service worker tests
Modified: trunk/LayoutTests/fast/mediastream/media-stream-track-source-failure.html (249717 => 249718)
--- trunk/LayoutTests/fast/mediastream/media-stream-track-source-failure.html 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/LayoutTests/fast/mediastream/media-stream-track-source-failure.html 2019-09-10 16:06:51 UTC (rev 249718)
@@ -22,7 +22,7 @@
}
if (numberOfTries <= 0) {
- reject('Page state did not change in time.');
+ reject('Page state did not change in time, original state is ' + originalState);
return;
}
@@ -33,10 +33,10 @@
{
promise_test((test) => {
return new Promise((resolve, reject) => {
-
+ const mediaState = internals.pageMediaState();
track._onended_ = () => {
new Promise((innerResolve, innerReject) => {
- waitForPageStateChange(10, internals.pageMediaState(), innerResolve, innerReject)
+ waitForPageStateChange(10, mediaState, innerResolve, innerReject)
}).then((pageMediaState) => {
assert_equals(video.videoTracks.length, 1, "Unexpected video track count");
assert_equals(video.audioTracks.length, 1, "Unexpected audio track count");
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (249717 => 249718)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2019-09-10 16:06:51 UTC (rev 249718)
@@ -1,3 +1,12 @@
+2019-09-10 Youenn Fablet <you...@apple.com>
+
+ Remove MediaStreamPrivate::scheduleDeferredTask
+ https://bugs.webkit.org/show_bug.cgi?id=200975
+
+ Reviewed by Eric Carlson.
+
+ * web-platform-tests/mediacapture-streams/MediaStream-finished-add.https-expected.txt:
+
2019-09-10 Chris Dumez <cdu...@apple.com>
Unreviewed, unskip / rebaseline a few service worker tests
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-finished-add.https-expected.txt (249717 => 249718)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-finished-add.https-expected.txt 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-finished-add.https-expected.txt 2019-09-10 16:06:51 UTC (rev 249718)
@@ -5,5 +5,5 @@
This test checks that adding a track to an inactive MediaStream is allowed.
-FAIL Tests that adding a track to an inactive MediaStream is allowed assert_false: audio stream is inactive after stopping its only audio track expected false got true
+PASS Tests that adding a track to an inactive MediaStream is allowed
Modified: trunk/Source/WebCore/ChangeLog (249717 => 249718)
--- trunk/Source/WebCore/ChangeLog 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/Source/WebCore/ChangeLog 2019-09-10 16:06:51 UTC (rev 249718)
@@ -1,5 +1,41 @@
2019-09-10 Youenn Fablet <you...@apple.com>
+ Remove MediaStreamPrivate::scheduleDeferredTask
+ https://bugs.webkit.org/show_bug.cgi?id=200975
+
+ Reviewed by Eric Carlson.
+
+ All calls to scheduleDeferredTask are done on the main thread.
+ This was initially done to trigger less reconfiguration.
+ But this makes the implementation significantly more complex.
+
+ For instance, we have to wait for the document to update its media state
+ and send it to UIProcess before calling the allow completion handler.
+
+ Covered by existing tests.
+
+ * Modules/mediastream/MediaStream.cpp:
+ (WebCore::MediaStream::MediaStream):
+ Make sure to update the document media state once the tracks have been added, similarly to the other constructor.
+ This ensures the document media state is computed with the new MediaStreamTrack.
+ * Modules/mediastream/UserMediaRequest.cpp:
+ (WebCore::isMediaStreamCorrectlyStarted):
+ (WebCore::UserMediaRequest::allow):
+ (WebCore::UserMediaRequest::stop):
+ (WebCore::UserMediaRequest::mediaStreamDidFail):
+ * Modules/mediastream/UserMediaRequest.h:
+ * page/MediaProducer.h:
+ (WebCore::MediaProducer::isCapturing):
+ Make sure to include getDisplayMedia as part of capture check.
+ * platform/mediastream/MediaStreamPrivate.cpp:
+ (WebCore::MediaStreamPrivate::trackMutedChanged):
+ (WebCore::MediaStreamPrivate::trackEnabledChanged):
+ (WebCore::MediaStreamPrivate::trackStarted):
+ (WebCore::MediaStreamPrivate::trackEnded):
+ * platform/mediastream/MediaStreamPrivate.h:
+
+2019-09-10 Youenn Fablet <you...@apple.com>
+
Audio sometimes fail to capture in WebRTC
https://bugs.webkit.org/show_bug.cgi?id=180748
<rdar://problem/36032346>
Modified: trunk/Source/WebCore/Modules/mediastream/MediaStream.cpp (249717 => 249718)
--- trunk/Source/WebCore/Modules/mediastream/MediaStream.cpp 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/Source/WebCore/Modules/mediastream/MediaStream.cpp 2019-09-10 16:06:51 UTC (rev 249718)
@@ -101,14 +101,14 @@
{
ALWAYS_LOG(LOGIDENTIFIER);
- setIsActive(m_private->active());
- m_private->addObserver(*this);
-
for (auto& trackPrivate : m_private->tracks()) {
auto track = MediaStreamTrack::create(document, *trackPrivate);
track->addObserver(*this);
m_trackSet.add(track->id(), WTFMove(track));
}
+
+ setIsActive(m_private->active());
+ m_private->addObserver(*this);
suspendIfNeeded();
}
Modified: trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp (249717 => 249718)
--- trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp 2019-09-10 16:06:51 UTC (rev 249718)
@@ -215,15 +215,25 @@
controller->requestUserMediaAccess(*this);
}
+static inline bool isMediaStreamCorrectlyStarted(const MediaStream& stream)
+{
+ if (stream.getTracks().isEmpty())
+ return false;
+
+ return WTF::allOf(stream.getTracks(), [](auto& track) {
+ return !track->source().captureDidFail();
+ });
+}
+
void UserMediaRequest::allow(CaptureDevice&& audioDevice, CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt, CompletionHandler<void()>&& completionHandler)
{
RELEASE_LOG(MediaStream, "UserMediaRequest::allow %s %s", audioDevice ? audioDevice.persistentId().utf8().data() : "", videoDevice ? videoDevice.persistentId().utf8().data() : "");
auto callback = [this, protector = makePendingActivity(*this), completionHandler = WTFMove(completionHandler)](RefPtr<MediaStreamPrivate>&& privateStream) mutable {
- auto scopeExit = makeScopeExit([&] {
+ auto scopeExit = makeScopeExit([completionHandler = WTFMove(completionHandler)]() mutable {
completionHandler();
});
- if (!m_scriptExecutionContext)
+ if (isContextStopped())
return;
if (!privateStream) {
@@ -231,16 +241,21 @@
deny(MediaAccessDenialReason::HardwareError);
return;
}
- privateStream->monitorOrientation(downcast<Document>(m_scriptExecutionContext)->orientationNotifier());
- auto stream = MediaStream::create(*downcast<Document>(m_scriptExecutionContext), privateStream.releaseNonNull());
- if (stream->getTracks().isEmpty()) {
+ auto& document = downcast<Document>(*m_scriptExecutionContext);
+ privateStream->monitorOrientation(document.orientationNotifier());
+
+ auto stream = MediaStream::create(document, privateStream.releaseNonNull());
+ stream->startProducingData();
+
+ if (!isMediaStreamCorrectlyStarted(stream)) {
deny(MediaAccessDenialReason::HardwareError);
return;
}
- scopeExit.release();
- m_pendingActivationMediaStream = makeUnique<PendingActivationMediaStream>(WTFMove(protector), *this, WTFMove(stream), WTFMove(completionHandler));
+ ASSERT(document.isCapturing());
+ stream->document()->setHasCaptureMediaStreamTrack();
+ m_promise.resolve(WTFMove(stream));
};
auto& document = downcast<Document>(*scriptExecutionContext());
@@ -310,11 +325,6 @@
void UserMediaRequest::stop()
{
- // Protecting 'this' since nulling m_pendingActivationMediaStream might destroy it.
- Ref<UserMediaRequest> protectedThis(*this);
-
- m_pendingActivationMediaStream = nullptr;
-
auto& document = downcast<Document>(*m_scriptExecutionContext);
if (auto* controller = UserMediaController::from(document.page()))
controller->cancelUserMediaAccessRequest(*this);
@@ -335,50 +345,6 @@
return downcast<Document>(m_scriptExecutionContext);
}
-UserMediaRequest::PendingActivationMediaStream::PendingActivationMediaStream(Ref<PendingActivity<UserMediaRequest>>&& protectingUserMediaRequest, UserMediaRequest& userMediaRequest, Ref<MediaStream>&& stream, CompletionHandler<void()>&& completionHandler)
- : m_protectingUserMediaRequest(WTFMove(protectingUserMediaRequest))
- , m_userMediaRequest(userMediaRequest)
- , m_mediaStream(WTFMove(stream))
- , m_completionHandler(WTFMove(completionHandler))
-{
- m_mediaStream->privateStream().addObserver(*this);
- m_mediaStream->startProducingData();
-}
-
-UserMediaRequest::PendingActivationMediaStream::~PendingActivationMediaStream()
-{
- m_mediaStream->privateStream().removeObserver(*this);
- m_completionHandler();
- if (auto* document = m_mediaStream->document())
- document->updateIsPlayingMedia();
-}
-
-void UserMediaRequest::PendingActivationMediaStream::characteristicsChanged()
-{
- if (!m_userMediaRequest.m_pendingActivationMediaStream)
- return;
-
- for (auto& track : m_mediaStream->privateStream().tracks()) {
- if (track->source().captureDidFail()) {
- m_userMediaRequest.mediaStreamDidFail(track->source().type());
- return;
- }
- }
-
- if (m_mediaStream->privateStream().hasVideo() || m_mediaStream->privateStream().hasAudio()) {
- m_userMediaRequest.mediaStreamIsReady(WTFMove(m_mediaStream));
- return;
- }
-}
-
-void UserMediaRequest::mediaStreamIsReady(Ref<MediaStream>&& stream)
-{
- RELEASE_LOG(MediaStream, "UserMediaRequest::mediaStreamIsReady");
- stream->document()->setHasCaptureMediaStreamTrack();
- m_promise.resolve(WTFMove(stream));
- m_pendingActivationMediaStream = nullptr;
-}
-
void UserMediaRequest::mediaStreamDidFail(RealtimeMediaSource::Type type)
{
RELEASE_LOG(MediaStream, "UserMediaRequest::mediaStreamDidFail");
@@ -395,7 +361,6 @@
break;
}
m_promise.reject(NotReadableError, makeString("Failed starting capture of a "_s, typeDescription, " track"_s));
- m_pendingActivationMediaStream = nullptr;
}
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.h (249717 => 249718)
--- trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.h 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.h 2019-09-10 16:06:51 UTC (rev 249718)
@@ -79,29 +79,12 @@
const char* activeDOMObjectName() const final;
bool canSuspendForDocumentSuspension() const final;
- void mediaStreamIsReady(Ref<MediaStream>&&);
void mediaStreamDidFail(RealtimeMediaSource::Type);
- class PendingActivationMediaStream : private MediaStreamPrivate::Observer {
- WTF_MAKE_FAST_ALLOCATED;
- public:
- PendingActivationMediaStream(Ref<PendingActivity<UserMediaRequest>>&&, UserMediaRequest&, Ref<MediaStream>&&, CompletionHandler<void()>&&);
- ~PendingActivationMediaStream();
-
- private:
- void characteristicsChanged() final;
-
- Ref<PendingActivity<UserMediaRequest>> m_protectingUserMediaRequest;
- UserMediaRequest& m_userMediaRequest;
- Ref<MediaStream> m_mediaStream;
- CompletionHandler<void()> m_completionHandler;
- };
-
Vector<String> m_videoDeviceUIDs;
Vector<String> m_audioDeviceUIDs;
DOMPromiseDeferred<IDLInterface<MediaStream>> m_promise;
- std::unique_ptr<PendingActivationMediaStream> m_pendingActivationMediaStream;
MediaStreamRequest m_request;
};
Modified: trunk/Source/WebCore/page/MediaProducer.h (249717 => 249718)
--- trunk/Source/WebCore/page/MediaProducer.h 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/Source/WebCore/page/MediaProducer.h 2019-09-10 16:06:51 UTC (rev 249718)
@@ -58,12 +58,13 @@
AudioCaptureMask = HasActiveAudioCaptureDevice | HasMutedAudioCaptureDevice | HasInterruptedAudioCaptureDevice,
VideoCaptureMask = HasActiveVideoCaptureDevice | HasMutedVideoCaptureDevice | HasInterruptedVideoCaptureDevice,
DisplayCaptureMask = HasActiveDisplayCaptureDevice | HasMutedDisplayCaptureDevice | HasInterruptedDisplayCaptureDevice,
+ ActiveCaptureMask = HasActiveAudioCaptureDevice | HasActiveVideoCaptureDevice | HasActiveDisplayCaptureDevice,
MutedCaptureMask = HasMutedAudioCaptureDevice | HasMutedVideoCaptureDevice | HasMutedDisplayCaptureDevice,
MediaCaptureMask = AudioCaptureMask | VideoCaptureMask | DisplayCaptureMask,
};
typedef unsigned MediaStateFlags;
- static bool isCapturing(MediaStateFlags state) { return (state & HasActiveAudioCaptureDevice) || (state & HasActiveVideoCaptureDevice) || (state & HasMutedAudioCaptureDevice) || (state & HasMutedVideoCaptureDevice); }
+ static bool isCapturing(MediaStateFlags state) { return (state & ActiveCaptureMask) || (state & MutedCaptureMask); }
virtual MediaStateFlags mediaState() const = 0;
Modified: trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp (249717 => 249718)
--- trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp 2019-09-10 16:06:51 UTC (rev 249718)
@@ -282,9 +282,7 @@
#endif
ALWAYS_LOG(LOGIDENTIFIER, track.logIdentifier(), " ", track.muted());
- scheduleDeferredTask([this] {
- characteristicsChanged();
- });
+ characteristicsChanged();
}
void MediaStreamPrivate::trackSettingsChanged(MediaStreamTrackPrivate&)
@@ -301,9 +299,7 @@
ALWAYS_LOG(LOGIDENTIFIER, track.logIdentifier(), " ", track.enabled());
updateActiveVideoTrack();
- scheduleDeferredTask([this] {
- characteristicsChanged();
- });
+ characteristicsChanged();
}
void MediaStreamPrivate::trackStarted(MediaStreamTrackPrivate& track)
@@ -313,9 +309,7 @@
#endif
ALWAYS_LOG(LOGIDENTIFIER, track.logIdentifier());
- scheduleDeferredTask([this] {
- characteristicsChanged();
- });
+ characteristicsChanged();
}
void MediaStreamPrivate::trackEnded(MediaStreamTrackPrivate& track)
@@ -325,23 +319,10 @@
#endif
ALWAYS_LOG(LOGIDENTIFIER, track.logIdentifier());
- scheduleDeferredTask([this] {
- updateActiveState(NotifyClientOption::Notify);
- characteristicsChanged();
- });
+ updateActiveState(NotifyClientOption::Notify);
+ characteristicsChanged();
}
-void MediaStreamPrivate::scheduleDeferredTask(Function<void ()>&& function)
-{
- ASSERT(function);
- callOnMainThread([weakThis = makeWeakPtr(*this), function = WTFMove(function)] {
- if (!weakThis)
- return;
-
- function();
- });
-}
-
void MediaStreamPrivate::monitorOrientation(OrientationNotifier& notifier)
{
for (auto& track : m_trackSet.values()) {
Modified: trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.h (249717 => 249718)
--- trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.h 2019-09-10 16:06:50 UTC (rev 249717)
+++ trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.h 2019-09-10 16:06:51 UTC (rev 249718)
@@ -47,7 +47,6 @@
#include <wtf/RefPtr.h>
#include <wtf/UUID.h>
#include <wtf/Vector.h>
-#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -57,7 +56,6 @@
class MediaStreamPrivate final
: public MediaStreamTrackPrivate::Observer
, public RefCounted<MediaStreamPrivate>
- , public CanMakeWeakPtr<MediaStreamPrivate>
#if !RELEASE_LOG_DISABLED
, private LoggerHelper
#endif
@@ -128,7 +126,6 @@
void characteristicsChanged();
void updateActiveVideoTrack();
- void scheduleDeferredTask(Function<void ()>&&);
void forEachObserver(const WTF::Function<void(Observer&)>&) const;
#if !RELEASE_LOG_DISABLED
