Changeset 249651 in webkit

Timestamp:

Sep 9, 2019 10:52:15 AM (5 years ago)

Author:

Ryan Haddad

Message:

Unreviewed, rolling out r249574.

Caused fast/mediastream/media-stream-track-source-failure.html
to become flaky.

Reverted changeset:

"Remove MediaStreamPrivate::scheduleDeferredTask"
​https://bugs.webkit.org/show_bug.cgi?id=200975
​https://trac.webkit.org/changeset/249574

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-finished-add.https-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/mediastream/MediaStream.cpp (modified) (2 diffs)
• Source/WebCore/Modules/mediastream/UserMediaRequest.cpp (modified) (6 diffs)
• Source/WebCore/Modules/mediastream/UserMediaRequest.h (modified) (2 diffs)
• Source/WebCore/page/MediaProducer.h (modified) (2 diffs)
• Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp (modified) (4 diffs)
• Source/WebCore/platform/mediastream/MediaStreamPrivate.h (modified) (3 diffs)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2019-09-09  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r249574.
+
+        Caused fast/mediastream/media-stream-track-source-failure.html
+        to become flaky.
+
+        Reverted changeset:
+
+        "Remove MediaStreamPrivate::scheduleDeferredTask"
+        https://bugs.webkit.org/show_bug.cgi?id=200975
+        https://trac.webkit.org/changeset/249574
+
 2019-09-09  Rob Buis  <rbuis@igalia.com>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-finished-add.https-expected.txt

@@ -6 +6 @@
 
 
-PASS Tests 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
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-09-09  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r249574.
+
+        Caused fast/mediastream/media-stream-track-source-failure.html
+        to become flaky.
+
+        Reverted changeset:
+
+        "Remove MediaStreamPrivate::scheduleDeferredTask"
+        https://bugs.webkit.org/show_bug.cgi?id=200975
+        https://trac.webkit.org/changeset/249574
+
 2019-09-09  Rob Buis  <rbuis@igalia.com>
 

trunk/Source/WebCore/Modules/mediastream/MediaStream.cpp

@@ -102 +102 @@
     ALWAYS_LOG(LOGIDENTIFIER);
 
+    setIsActive(m_private->active());
+    m_private->addObserver(*this);
+
     for (auto& trackPrivate : m_private->tracks()) {
         auto track = MediaStreamTrack::create(document, *trackPrivate);
@@ -107 +110 @@
         m_trackSet.add(track->id(), WTFMove(track));
     }
-
-    setIsActive(m_private->active());
-    m_private->addObserver(*this);
     suspendIfNeeded();
 }

trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp

@@ -216 +216 @@
 }
 
-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)
 {
@@ -231 +221 @@
 
     auto callback = [this, protector = makePendingActivity(*this), completionHandler = WTFMove(completionHandler)](RefPtr<MediaStreamPrivate>&& privateStream) mutable {
-        auto scopeExit = makeScopeExit([completionHandler = WTFMove(completionHandler)]() mutable {
+        auto scopeExit = makeScopeExit([&] {
             completionHandler();
         });
-        if (isContextStopped())
+        if (!m_scriptExecutionContext)
             return;
 
@@ -242 +232 @@
             return;
         }
-
-        auto& document = downcast<Document>(*m_scriptExecutionContext);
-        privateStream->monitorOrientation(document.orientationNotifier());
-
-        auto stream = MediaStream::create(document, privateStream.releaseNonNull());
-        stream->startProducingData();
-
-        if (!isMediaStreamCorrectlyStarted(stream)) {
+        privateStream->monitorOrientation(downcast<Document>(m_scriptExecutionContext)->orientationNotifier());
+
+        auto stream = MediaStream::create(*downcast<Document>(m_scriptExecutionContext), privateStream.releaseNonNull());
+        if (stream->getTracks().isEmpty()) {
             deny(MediaAccessDenialReason::HardwareError);
             return;
         }
 
-        ASSERT(document.isCapturing());
-        stream->document()->setHasCaptureMediaStreamTrack();
-        m_promise.resolve(WTFMove(stream));
+        scopeExit.release();
+        m_pendingActivationMediaStream = makeUnique<PendingActivationMediaStream>(WTFMove(protector), *this, WTFMove(stream), WTFMove(completionHandler));
     };
 
@@ -326 +311 @@
 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()))
@@ -344 +334 @@
 {
     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;
 }
 
@@ -362 +396 @@
     }
     m_promise.reject(NotReadableError, makeString("Failed starting capture of a "_s, typeDescription, " track"_s));
+    m_pendingActivationMediaStream = nullptr;
 }
 

trunk/Source/WebCore/Modules/mediastream/UserMediaRequest.h

@@ -80 +80 @@
     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;
@@ -86 +102 @@
 
     DOMPromiseDeferred<IDLInterface<MediaStream>> m_promise;
+    std::unique_ptr<PendingActivationMediaStream> m_pendingActivationMediaStream;
     MediaStreamRequest m_request;
 };

trunk/Source/WebCore/page/MediaProducer.h

@@ -59 +59 @@
         VideoCaptureMask = HasActiveVideoCaptureDevice | HasMutedVideoCaptureDevice | HasInterruptedVideoCaptureDevice,
         DisplayCaptureMask = HasActiveDisplayCaptureDevice | HasMutedDisplayCaptureDevice | HasInterruptedDisplayCaptureDevice,
-        ActiveCaptureMask = HasActiveAudioCaptureDevice | HasActiveVideoCaptureDevice | HasActiveDisplayCaptureDevice,
         MutedCaptureMask =  HasMutedAudioCaptureDevice | HasMutedVideoCaptureDevice | HasMutedDisplayCaptureDevice,
         MediaCaptureMask = AudioCaptureMask | VideoCaptureMask | DisplayCaptureMask,
@@ -65 +64 @@
     typedef unsigned MediaStateFlags;
 
-    static bool isCapturing(MediaStateFlags state) { return (state & ActiveCaptureMask) || (state & MutedCaptureMask); }
+    static bool isCapturing(MediaStateFlags state) { return (state & HasActiveAudioCaptureDevice) || (state & HasActiveVideoCaptureDevice) || (state & HasMutedAudioCaptureDevice) || (state & HasMutedVideoCaptureDevice); }
 
     virtual MediaStateFlags mediaState() const = 0;

trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp

@@ -283 +283 @@
 
     ALWAYS_LOG(LOGIDENTIFIER, track.logIdentifier(), " ", track.muted());
-    characteristicsChanged();
+    scheduleDeferredTask([this] {
+        characteristicsChanged();
+    });
 }
 
@@ -300 +302 @@
     updateActiveVideoTrack();
 
-    characteristicsChanged();
+    scheduleDeferredTask([this] {
+        characteristicsChanged();
+    });
 }
 
@@ -310 +314 @@
 
     ALWAYS_LOG(LOGIDENTIFIER, track.logIdentifier());
-    characteristicsChanged();
+    scheduleDeferredTask([this] {
+        characteristicsChanged();
+    });
 }
 
@@ -320 +326 @@
 
     ALWAYS_LOG(LOGIDENTIFIER, track.logIdentifier());
-    updateActiveState(NotifyClientOption::Notify);
-    characteristicsChanged();
+    scheduleDeferredTask([this] {
+        updateActiveState(NotifyClientOption::Notify);
+        characteristicsChanged();
+    });
+}
+
+void MediaStreamPrivate::scheduleDeferredTask(Function<void ()>&& function)
+{
+    ASSERT(function);
+    callOnMainThread([weakThis = makeWeakPtr(*this), function = WTFMove(function)] {
+        if (!weakThis)
+            return;
+
+        function();
+    });
 }
 

trunk/Source/WebCore/platform/mediastream/MediaStreamPrivate.h

@@ -48 +48 @@
 #include <wtf/UUID.h>
 #include <wtf/Vector.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
@@ -57 +58 @@
     : public MediaStreamTrackPrivate::Observer
     , public RefCounted<MediaStreamPrivate>
+    , public CanMakeWeakPtr<MediaStreamPrivate>
 #if !RELEASE_LOG_DISABLED
     , private LoggerHelper
@@ -127 +129 @@
     void updateActiveVideoTrack();
 
+    void scheduleDeferredTask(Function<void ()>&&);
     void forEachObserver(const WTF::Function<void(Observer&)>&) const;