getUserMedia is resolving before the document knows it is capturing
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Dec 2017 01:39:36 +0000 (01:39 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Dec 2017 01:39:36 +0000 (01:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180699

Patch by Youenn Fablet <youenn@apple.com> on 2017-12-12
Reviewed by Eric Carlson.

Source/WebCore:

Covered by updated test.

Ensure the document state is capturing when getUserMedia promise is resolved by doing the following:
- Promise is resolved when MediaStream is producing data.
- MediaStream asks Document to update its state when MediaStream state is updated.

Introduce PendingActivationMediaStream for waiting for the MediaStream to produce data.

* Modules/mediastream/MediaStream.cpp:
(WebCore::MediaStream::statusDidChange):
* Modules/mediastream/UserMediaRequest.cpp:
(WebCore::UserMediaRequest::allow):
(WebCore::UserMediaRequest::contextDestroyed):
(WebCore::UserMediaRequest::PendingActivationMediaStream::PendingActivationMediaStream):
(WebCore::UserMediaRequest::PendingActivationMediaStream::~PendingActivationMediaStream):
(WebCore::UserMediaRequest::PendingActivationMediaStream::characteristicsChanged):
(WebCore::UserMediaRequest::mediaStreamIsReady):
* Modules/mediastream/UserMediaRequest.h:
(WebCore::UserMediaRequest::PendingActivationMediaStream::create):
* platform/mediastream/RealtimeMediaSourceCenter.h:
* WebCore/WebCore.xcodeproj/project.pbxproj:

LayoutTests:

* webrtc/video.html: Adding a check that document is capturing within getUserMedia promise resolution callback.
Adding this check without the changes to WebCore makes the test flaky, sometimes the promise resolution happens
after document state is updated.

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

LayoutTests/ChangeLog
LayoutTests/webrtc/video.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaStream.cpp
Source/WebCore/Modules/mediastream/UserMediaRequest.cpp
Source/WebCore/Modules/mediastream/UserMediaRequest.h
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/platform/mediastream/RealtimeMediaSourceCenter.h

index 2c58478..77a9ca5 100644 (file)
@@ -1,3 +1,14 @@
+2017-12-12  Youenn Fablet  <youenn@apple.com>
+
+        getUserMedia is resolving before the document knows it is capturing
+        https://bugs.webkit.org/show_bug.cgi?id=180699
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/video.html: Adding a check that document is capturing within getUserMedia promise resolution callback.
+        Adding this check without the changes to WebCore makes the test flaky, sometimes the promise resolution happens
+        after document state is updated.
+
 2017-12-12  John Wilander  <wilander@apple.com>
 
         Storage Access API: Implement frame-specific access in the network storage session layer
index 6d060e6..387e7b4 100644 (file)
@@ -44,6 +44,8 @@ promise_test((test) => {
         testRunner.setUserMediaPermission(true);
 
     return navigator.mediaDevices.getUserMedia({video: {advanced: [{width:{min:1280}}, {height:{min:720} } ]}}).then((stream) => {
+        if (window.internals)
+            assert_true(internals.pageMediaState().includes('HasActiveVideoCaptureDevice'), "Unexpected HasActiveVideoCaptureDevice");
         return new Promise((resolve, reject) => {
             createConnections((firstConnection) => {
                 var track = stream.getVideoTracks()[0];
index 5cd4508..798c920 100644 (file)
@@ -1,3 +1,32 @@
+2017-12-12  Youenn Fablet  <youenn@apple.com>
+
+        getUserMedia is resolving before the document knows it is capturing
+        https://bugs.webkit.org/show_bug.cgi?id=180699
+
+        Reviewed by Eric Carlson.
+
+        Covered by updated test.
+
+        Ensure the document state is capturing when getUserMedia promise is resolved by doing the following:
+        - Promise is resolved when MediaStream is producing data.
+        - MediaStream asks Document to update its state when MediaStream state is updated.
+
+        Introduce PendingActivationMediaStream for waiting for the MediaStream to produce data.
+
+        * Modules/mediastream/MediaStream.cpp:
+        (WebCore::MediaStream::statusDidChange):
+        * Modules/mediastream/UserMediaRequest.cpp:
+        (WebCore::UserMediaRequest::allow):
+        (WebCore::UserMediaRequest::contextDestroyed):
+        (WebCore::UserMediaRequest::PendingActivationMediaStream::PendingActivationMediaStream):
+        (WebCore::UserMediaRequest::PendingActivationMediaStream::~PendingActivationMediaStream):
+        (WebCore::UserMediaRequest::PendingActivationMediaStream::characteristicsChanged):
+        (WebCore::UserMediaRequest::mediaStreamIsReady):
+        * Modules/mediastream/UserMediaRequest.h:
+        (WebCore::UserMediaRequest::PendingActivationMediaStream::create):
+        * platform/mediastream/RealtimeMediaSourceCenter.h:
+        * WebCore/WebCore.xcodeproj/project.pbxproj:
+
 2017-12-12  John Wilander  <wilander@apple.com>
 
         Storage Access API: Implement frame-specific access in the network storage session layer
index e5a06c5..b77809d 100644 (file)
@@ -334,8 +334,10 @@ void MediaStream::statusDidChange()
     m_mediaSession->canProduceAudioChanged();
 
     if (Document* document = this->document()) {
-        if (m_isActive)
-            document->setHasActiveMediaStreamTrack();
+        if (!m_isActive)
+            return;
+        document->setHasActiveMediaStreamTrack();
+        document->updateIsPlayingMedia();
     }
 }
 
index 95ac60f..f527059 100644 (file)
@@ -161,8 +161,7 @@ void UserMediaRequest::allow(CaptureDevice&& audioDevice, CaptureDevice&& videoD
     m_allowedAudioDevice = audioDevice;
     m_allowedVideoDevice = videoDevice;
 
-    RefPtr<UserMediaRequest> protectedThis = this;
-    RealtimeMediaSourceCenter::NewMediaStreamHandler callback = [this, protectedThis = WTFMove(protectedThis)](RefPtr<MediaStreamPrivate>&& privateStream) mutable {
+    auto callback = [this, protectedThis = makeRef(*this)](RefPtr<MediaStreamPrivate>&& privateStream) mutable {
         if (!m_scriptExecutionContext)
             return;
 
@@ -178,9 +177,7 @@ void UserMediaRequest::allow(CaptureDevice&& audioDevice, CaptureDevice&& videoD
             return;
         }
 
-        stream->startProducingData();
-        
-        m_promise.resolve(stream);
+        m_pendingActivationMediaStream = PendingActivationMediaStream::create(WTFMove(protectedThis), WTFMove(stream));
     };
 
     m_audioConstraints.deviceIDHashSalt = deviceIdentifierHashSalt;
@@ -243,6 +240,7 @@ void UserMediaRequest::contextDestroyed()
         m_controller->cancelUserMediaAccessRequest(*this);
         m_controller = nullptr;
     }
+    m_pendingActivationMediaStream = nullptr;
 }
 
 Document* UserMediaRequest::document() const
@@ -250,6 +248,32 @@ Document* UserMediaRequest::document() const
     return downcast<Document>(m_scriptExecutionContext);
 }
 
+UserMediaRequest::PendingActivationMediaStream::PendingActivationMediaStream(Ref<UserMediaRequest>&& request, Ref<MediaStream>&& stream)
+    : m_request(WTFMove(request))
+    , m_mediaStream(WTFMove(stream))
+{
+    m_mediaStream->privateStream().addObserver(*this);
+    m_mediaStream->startProducingData();
+}
+
+UserMediaRequest::PendingActivationMediaStream::~PendingActivationMediaStream()
+{
+    m_mediaStream->privateStream().removeObserver(*this);
+}
+
+void UserMediaRequest::PendingActivationMediaStream::characteristicsChanged()
+{
+    if (m_mediaStream->privateStream().hasVideo() || m_mediaStream->privateStream().hasAudio())
+        m_request->mediaStreamIsReady(m_mediaStream.copyRef());
+}
+
+void UserMediaRequest::mediaStreamIsReady(Ref<MediaStream>&& stream)
+{
+    m_promise.resolve(WTFMove(stream));
+    // We are in an observer iterator loop, we do not want to change the observers within this loop.
+    callOnMainThread([stream = WTFMove(m_pendingActivationMediaStream)] { });
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(MEDIA_STREAM)
index 1be3c13..d64cf11 100644 (file)
@@ -38,6 +38,7 @@
 #include "CaptureDevice.h"
 #include "JSDOMPromiseDeferred.h"
 #include "MediaConstraints.h"
+#include "MediaStreamPrivate.h"
 
 namespace WebCore {
 
@@ -73,7 +74,26 @@ private:
     UserMediaRequest(Document&, UserMediaController&, MediaConstraints&& audioConstraints, MediaConstraints&& videoConstraints, DOMPromiseDeferred<IDLInterface<MediaStream>>&&);
 
     void contextDestroyed() final;
-    
+
+    void mediaStreamIsReady(Ref<MediaStream>&&);
+
+    class PendingActivationMediaStream : public RefCounted<PendingActivationMediaStream>, private MediaStreamPrivate::Observer {
+    public:
+        static Ref<PendingActivationMediaStream> create(Ref<UserMediaRequest>&& request, Ref<MediaStream>&& stream)
+        {
+            return adoptRef(*new PendingActivationMediaStream { WTFMove(request), WTFMove(stream) });
+        }
+        ~PendingActivationMediaStream();
+
+    private:
+        PendingActivationMediaStream(Ref<UserMediaRequest>&&, Ref<MediaStream>&&);
+
+        void characteristicsChanged() final;
+
+        Ref<UserMediaRequest> m_request;
+        Ref<MediaStream> m_mediaStream;
+    };
+
     MediaConstraints m_audioConstraints;
     MediaConstraints m_videoConstraints;
 
@@ -86,6 +106,7 @@ private:
     UserMediaController* m_controller;
     DOMPromiseDeferred<IDLInterface<MediaStream>> m_promise;
     RefPtr<UserMediaRequest> m_protector;
+    RefPtr<PendingActivationMediaStream> m_pendingActivationMediaStream;
 };
 
 } // namespace WebCore
index 833c045..2c32216 100644 (file)
                078E091517D14D1C00420AA1 /* MediaStream.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B4D17CEC32700848E51 /* MediaStream.h */; };
                078E091617D14D1C00420AA1 /* MediaStreamEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B5017CEC32700848E51 /* MediaStreamEvent.h */; };
                078E091717D14D1C00420AA1 /* MediaStreamRegistry.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B5317CEC32700848E51 /* MediaStreamRegistry.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               078E091817D14D1C00420AA1 /* MediaStreamTrack.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B5517CEC32700848E51 /* MediaStreamTrack.h */; };
+               078E091817D14D1C00420AA1 /* MediaStreamTrack.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B5517CEC32700848E51 /* MediaStreamTrack.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E091917D14D1C00420AA1 /* MediaStreamTrackEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B5817CEC32700848E51 /* MediaStreamTrackEvent.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E091E17D14D1C00420AA1 /* RTCDataChannel.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B6417CEC32700848E51 /* RTCDataChannel.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E091F17D14D1C00420AA1 /* RTCDataChannelEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B6717CEC32700848E51 /* RTCDataChannelEvent.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E092E17D14D1C00420AA1 /* UserMediaClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B8D17CEC32700848E51 /* UserMediaClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E092F17D14D1C00420AA1 /* UserMediaController.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B8F17CEC32700848E51 /* UserMediaController.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E093017D14D1C00420AA1 /* UserMediaRequest.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B9117CEC32700848E51 /* UserMediaRequest.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               078E093717D16B2C00420AA1 /* MediaStreamPrivate.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B9D17CF0AD400848E51 /* MediaStreamPrivate.h */; };
+               078E093717D16B2C00420AA1 /* MediaStreamPrivate.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B9D17CF0AD400848E51 /* MediaStreamPrivate.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E093A17D16E1C00420AA1 /* MediaConstraints.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221B9917CF0AD400848E51 /* MediaConstraints.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E093C17D16E1C00420AA1 /* RTCDataChannelHandler.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221BA217CF0AD400848E51 /* RTCDataChannelHandler.h */; settings = {ATTRIBUTES = (Private, ); }; };
                078E093D17D16E1C00420AA1 /* RTCDataChannelHandlerClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 07221BA317CF0AD400848E51 /* RTCDataChannelHandlerClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
                931CBD11161A44E900E4C874 /* ScrollingStateTree.h in Headers */ = {isa = PBXBuildFile; fileRef = 931CBD0B161A44E900E4C874 /* ScrollingStateTree.h */; settings = {ATTRIBUTES = (Private, ); }; };
                931D72F615FE695300C4C07E /* LayoutMilestones.h in Headers */ = {isa = PBXBuildFile; fileRef = 931D72F515FE695300C4C07E /* LayoutMilestones.h */; settings = {ATTRIBUTES = (Private, ); }; };
                932AD70617EFA2C40038F8FF /* MainFrame.h in Headers */ = {isa = PBXBuildFile; fileRef = 932AD70417EFA2C30038F8FF /* MainFrame.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               932CC0B71DFFD158004C0F9F /* MediaTrackConstraints.h in Headers */ = {isa = PBXBuildFile; fileRef = 932CC0B61DFFD158004C0F9F /* MediaTrackConstraints.h */; };
+               932CC0B71DFFD158004C0F9F /* MediaTrackConstraints.h in Headers */ = {isa = PBXBuildFile; fileRef = 932CC0B61DFFD158004C0F9F /* MediaTrackConstraints.h */; settings = {ATTRIBUTES = (Private, ); }; };
                932CC0D51DFFD667004C0F9F /* JSMediaTrackConstraints.h in Headers */ = {isa = PBXBuildFile; fileRef = 932CC0D11DFFD667004C0F9F /* JSMediaTrackConstraints.h */; };
                93309DD7099E64920056E581 /* AppendNodeCommand.h in Headers */ = {isa = PBXBuildFile; fileRef = 93309D88099E64910056E581 /* AppendNodeCommand.h */; };
                93309DD9099E64920056E581 /* ApplyStyleCommand.h in Headers */ = {isa = PBXBuildFile; fileRef = 93309D8A099E64910056E581 /* ApplyStyleCommand.h */; };
                9393E600151A99F200066F06 /* CSSImageSetValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 9393E5FE151A99F200066F06 /* CSSImageSetValue.h */; settings = {ATTRIBUTES = (Private, ); }; };
                939885C408B7E3D100E707C4 /* EventNames.h in Headers */ = {isa = PBXBuildFile; fileRef = 939885C208B7E3D100E707C4 /* EventNames.h */; settings = {ATTRIBUTES = (Private, ); }; };
                939B02EF0EA2DBC400C54570 /* WidthIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = 939B02ED0EA2DBC400C54570 /* WidthIterator.h */; };
-               93A806151E03B51C008A1F26 /* DoubleRange.h in Headers */ = {isa = PBXBuildFile; fileRef = 93A806111E03B51C008A1F26 /* DoubleRange.h */; };
-               93A806171E03B51C008A1F26 /* LongRange.h in Headers */ = {isa = PBXBuildFile; fileRef = 93A806131E03B51C008A1F26 /* LongRange.h */; };
+               93A806151E03B51C008A1F26 /* DoubleRange.h in Headers */ = {isa = PBXBuildFile; fileRef = 93A806111E03B51C008A1F26 /* DoubleRange.h */; settings = {ATTRIBUTES = (Private, ); }; };
+               93A806171E03B51C008A1F26 /* LongRange.h in Headers */ = {isa = PBXBuildFile; fileRef = 93A806131E03B51C008A1F26 /* LongRange.h */; settings = {ATTRIBUTES = (Private, ); }; };
                93A8061E1E03B585008A1F26 /* JSDoubleRange.h in Headers */ = {isa = PBXBuildFile; fileRef = 93A8061A1E03B585008A1F26 /* JSDoubleRange.h */; };
                93A806201E03B585008A1F26 /* JSLongRange.h in Headers */ = {isa = PBXBuildFile; fileRef = 93A8061C1E03B585008A1F26 /* JSLongRange.h */; };
                93B2D8160F9920D2006AE6B2 /* SuddenTermination.h in Headers */ = {isa = PBXBuildFile; fileRef = 93B2D8150F9920D2006AE6B2 /* SuddenTermination.h */; settings = {ATTRIBUTES = (Private, ); }; };
index e65b22e..c822d22 100644 (file)
@@ -62,7 +62,7 @@ public:
     using InvalidConstraintsHandler = WTF::Function<void(const String& invalidConstraint)>;
     virtual void validateRequestConstraints(ValidConstraintsHandler&&, InvalidConstraintsHandler&&, const MediaConstraints& audioConstraints, const MediaConstraints& videoConstraints, String&&);
 
-    using NewMediaStreamHandler = std::function<void(RefPtr<MediaStreamPrivate>&&)>;
+    using NewMediaStreamHandler = WTF::Function<void(RefPtr<MediaStreamPrivate>&&)>;
     virtual void createMediaStream(NewMediaStreamHandler&&, CaptureDevice&& audioDevice, CaptureDevice&& videoDevice, const MediaConstraints* audioConstraints, const MediaConstraints* videoConstraints);
 
     WEBCORE_EXPORT virtual Vector<CaptureDevice> getMediaStreamDevices();