Attachment 369891 Details for Bug 197851 – Patch

[patch] Patch

bug-197851-20190514142009.patch (text/plain), 32.05 KB, created by youenn fablet on 2019-05-14 14:20:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-05-14 14:20:10 PDT

Size: 32.05 KB

patch

obsolete

>Subversion Revision: 245178
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 71ce07311f9042abfcb751e298c35361f67824ba..1bc1db8fd58617ee0f32a6e60c886f1a00cca808 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-05-14  Youenn Fablet  <youenn@apple.com>
>+
>+        getUserMedia sandbox extensions should not be revoked when a getUserMedia allowed request is being processed
>+        https://bugs.webkit.org/show_bug.cgi?id=197851
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a completion handler to create a new capture stream.
>+        This is used by WK2 layer to acknowledge the pending capture request is completed.
>+        Just after the completion handler, make sure to update the document media state.
>+        This is done to ensure that, should capture is failing, the UIProcess
>+        knows about it and can manage proper sandbox extension revocation.
>+
>+        Test: fast/mediastream/gum-stop-track.html
>+
>+        * Modules/mediastream/UserMediaRequest.cpp:
>+        (WebCore::UserMediaRequest::allow):
>+        (WebCore::UserMediaRequest::PendingActivationMediaStream::PendingActivationMediaStream):
>+        (WebCore::UserMediaRequest::PendingActivationMediaStream::~PendingActivationMediaStream):
>+        * Modules/mediastream/UserMediaRequest.h:
>+        (WebCore::UserMediaRequest::PendingActivationMediaStream::create):
>+        * platform/mock/MockRealtimeMediaSourceCenter.cpp:
>+        (WebCore::MockRealtimeMediaSourceCenter::mockRealtimeMediaSourceCenterEnabled):
>+        * platform/mock/MockRealtimeMediaSourceCenter.h:
>+
> 2019-05-10  Youenn Fablet  <youenn@apple.com>
> 
>         Use the main screen for screen capture
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 3bf4572f28bedf88c29afef19645fa8a2519288f..3ff0d207f34fff1a9a92395c5cc3a69119e15b9d 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,46 @@
>+2019-05-14  Youenn Fablet  <youenn@apple.com>
>+
>+        getUserMedia sandbox extensions should not be revoked when a getUserMedia allowed request is being processed
>+        https://bugs.webkit.org/show_bug.cgi?id=197851
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Before the patch, stopping capture in a document and quickly triggering a new capture
>+        might fail as the UIProcess would grant access and revoke sandbox access based on the fact
>+        the page is no longer capturing.
>+        To fix that issue, keep a state in the UIProcess to not revoke sandbox extensions in case of
>+        capture being started.
>+        Add an IPC message back to tell UIProcess when an allowed capture is finished.
>+        Just after doing that, make sure the document is updating the media state to UIProcess, which will trigger proper sandbox extension handling.
>+
>+        This should also trigger the case of an allowed getUserMedia call that fails to start for some reason.
>+        In that case, the patch will automatically trigger a document media state refresh which will trigger a sandbox revokation.
>+
>+        Covered by added test by adding a debug assertion that will be hit in case we revoke extensions while a document is capturing.
>+
>+        * UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
>+        (WebKit::UserMediaPermissionRequestManagerProxy::~UserMediaPermissionRequestManagerProxy):
>+        (WebKit::UserMediaPermissionRequestManagerProxy::grantAccess):
>+        (WebKit::UserMediaPermissionRequestManagerProxy::captureStateChanged):
>+        * UIProcess/UserMediaPermissionRequestManagerProxy.h:
>+        * UIProcess/UserMediaProcessManager.cpp:
>+        (WebKit::UserMediaProcessManager::willCreateMediaStream):
>+        (WebKit::UserMediaProcessManager::revokeSandboxExtensionsIfNeeded):
>+        * UIProcess/UserMediaProcessManager.h:
>+        * UIProcess/WebPageProxy.h:
>+        (WebKit::WebPageProxy::isCapturingAudio const):
>+        (WebKit::WebPageProxy::isCapturingVideo const):
>+        * WebProcess/MediaStream/UserMediaPermissionRequestManager.cpp:
>+        (WebKit::UserMediaPermissionRequestManager::userMediaAccessWasGranted):
>+        * WebProcess/MediaStream/UserMediaPermissionRequestManager.h:
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::userMediaAccessWasGranted):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/WebPage.messages.in:
>+        * WebProcess/WebProcess.cpp:
>+        (WebKit::checkDocumentsCaptureStateConsistency):
>+        (WebKit::WebProcess::revokeUserMediaDeviceSandboxExtensions):
>+
> 2019-05-10  Youenn Fablet  <youenn@apple.com>
> 
>         Use kDNSServiceFlagsKnownUnique for DNSServiceRegisterRecord only on platforms supporting it
>diff --git a/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp b/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp
>index da2628f27c5727876d11c95a32b57123a40e7b1c..f061bc7eb104eb3e277f01272ce5ed4ed39a0e7e 100644
>--- a/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp
>+++ b/Source/WebCore/Modules/mediastream/UserMediaRequest.cpp
>@@ -47,6 +47,7 @@
> #include "SchemeRegistry.h"
> #include "Settings.h"
> #include "UserMediaController.h"
>+#include <wtf/Scope.h>
> 
> namespace WebCore {
> 
>@@ -214,11 +215,14 @@ void UserMediaRequest::start()
>     controller->requestUserMediaAccess(*this);
> }
> 
>-void UserMediaRequest::allow(CaptureDevice&& audioDevice, CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt)
>+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)](RefPtr<MediaStreamPrivate>&& privateStream) mutable {
>+    auto callback = [this, protector = makePendingActivity(*this), completionHandler = WTFMove(completionHandler)](RefPtr<MediaStreamPrivate>&& privateStream) mutable {
>+        auto scopeExit = makeScopeExit([&] {
>+            completionHandler();
>+        });
>         if (!m_scriptExecutionContext)
>             return;
> 
>@@ -235,7 +239,8 @@ void UserMediaRequest::allow(CaptureDevice&& audioDevice, CaptureDevice&& videoD
>             return;
>         }
> 
>-        m_pendingActivationMediaStream = PendingActivationMediaStream::create(WTFMove(protector), *this, WTFMove(stream));
>+        scopeExit.release();
>+        m_pendingActivationMediaStream = PendingActivationMediaStream::create(WTFMove(protector), *this, WTFMove(stream), WTFMove(completionHandler));
>     };
> 
>     auto& document = downcast<Document>(*scriptExecutionContext());
>@@ -330,10 +335,11 @@ Document* UserMediaRequest::document() const
>     return downcast<Document>(m_scriptExecutionContext);
> }
> 
>-UserMediaRequest::PendingActivationMediaStream::PendingActivationMediaStream(Ref<PendingActivity<UserMediaRequest>>&& protectingUserMediaRequest, UserMediaRequest& userMediaRequest, Ref<MediaStream>&& stream)
>+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();
>@@ -342,6 +348,9 @@ UserMediaRequest::PendingActivationMediaStream::PendingActivationMediaStream(Ref
> UserMediaRequest::PendingActivationMediaStream::~PendingActivationMediaStream()
> {
>     m_mediaStream->privateStream().removeObserver(*this);
>+    m_completionHandler();
>+    if (auto* document = m_mediaStream->document())
>+        document->updateIsPlayingMedia();
> }
> 
> void UserMediaRequest::PendingActivationMediaStream::characteristicsChanged()
>diff --git a/Source/WebCore/Modules/mediastream/UserMediaRequest.h b/Source/WebCore/Modules/mediastream/UserMediaRequest.h
>index 6de7e5c8d7ba413e20368e54027807517b751d95..e93d134170dd31698be2c65006623279a24733a3 100644
>--- a/Source/WebCore/Modules/mediastream/UserMediaRequest.h
>+++ b/Source/WebCore/Modules/mediastream/UserMediaRequest.h
>@@ -40,6 +40,7 @@
> #include "MediaConstraints.h"
> #include "MediaStreamPrivate.h"
> #include "MediaStreamRequest.h"
>+#include <wtf/CompletionHandler.h>
> 
> namespace WebCore {
> 
>@@ -54,7 +55,7 @@ public:
>     void start();
> 
>     WEBCORE_EXPORT void setAllowedMediaDeviceUIDs(const String& audioDeviceUID, const String& videoDeviceUID);
>-    WEBCORE_EXPORT void allow(CaptureDevice&& audioDevice, CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt);
>+    WEBCORE_EXPORT void allow(CaptureDevice&& audioDevice, CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt, CompletionHandler<void()>&&);
> 
>     enum MediaAccessDenialReason { NoConstraints, UserMediaDisabled, NoCaptureDevices, InvalidConstraint, HardwareError, PermissionDenied, InvalidAccess, IllegalConstraint, OtherFailure };
>     WEBCORE_EXPORT void deny(MediaAccessDenialReason, const String& errorMessage = emptyString());
>@@ -83,20 +84,21 @@ private:
> 
>     class PendingActivationMediaStream : public RefCounted<PendingActivationMediaStream>, private MediaStreamPrivate::Observer {
>     public:
>-        static Ref<PendingActivationMediaStream> create(Ref<PendingActivity<UserMediaRequest>>&& protectingUserMediaRequest, UserMediaRequest& userMediaRequest, Ref<MediaStream>&& stream)
>+        static Ref<PendingActivationMediaStream> create(Ref<PendingActivity<UserMediaRequest>>&& protectingUserMediaRequest, UserMediaRequest& userMediaRequest, Ref<MediaStream>&& stream, CompletionHandler<void()>&& completionHandler)
>         {
>-            return adoptRef(*new PendingActivationMediaStream { WTFMove(protectingUserMediaRequest), userMediaRequest, WTFMove(stream) });
>+            return adoptRef(*new PendingActivationMediaStream { WTFMove(protectingUserMediaRequest), userMediaRequest, WTFMove(stream), WTFMove(completionHandler) });
>         }
>         ~PendingActivationMediaStream();
> 
>     private:
>-        PendingActivationMediaStream(Ref<PendingActivity<UserMediaRequest>>&&, UserMediaRequest&, Ref<MediaStream>&&);
>+        PendingActivationMediaStream(Ref<PendingActivity<UserMediaRequest>>&&, UserMediaRequest&, Ref<MediaStream>&&, CompletionHandler<void()>&&);
> 
>         void characteristicsChanged() final;
> 
>         Ref<PendingActivity<UserMediaRequest>> m_protectingUserMediaRequest;
>         UserMediaRequest& m_userMediaRequest;
>         Ref<MediaStream> m_mediaStream;
>+        CompletionHandler<void()> m_completionHandler;
>     };
> 
>     Vector<String> m_videoDeviceUIDs;
>diff --git a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp
>index c4de5bd0999dec5fcf533fa18c34d2d1342359ed..69fad4b415e03fe2fd07af76d475c18e0387ab6d 100644
>--- a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp
>+++ b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp
>@@ -208,6 +208,14 @@ void MockRealtimeMediaSourceCenter::setMockRealtimeMediaSourceCenterEnabled(bool
>         center.unsetDisplayCaptureFactory(mock.displayCaptureFactory());
> }
> 
>+bool MockRealtimeMediaSourceCenter::mockRealtimeMediaSourceCenterEnabled()
>+{
>+    MockRealtimeMediaSourceCenter& mock = singleton();
>+    RealtimeMediaSourceCenter& center = RealtimeMediaSourceCenter::singleton();
>+
>+    return &center.audioCaptureFactory() == &mock.audioCaptureFactory() || &center.videoCaptureFactory() == &mock.videoCaptureFactory() || &center.displayCaptureFactory() == &mock.displayCaptureFactory();
>+}
>+
> static void createCaptureDevice(const MockMediaDevice& device)
> {
>     deviceListForDevice(device).append(MockRealtimeMediaSourceCenter::captureDeviceWithPersistentID(device.type(), device.persistentId).value());
>diff --git a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h
>index deea05377f97582a8e56ed49778fd2e9c826d9b1..6d16fe77a6cebe3b551381b6b9aabb0eec934dd3 100644
>--- a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h
>+++ b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h
>@@ -41,6 +41,7 @@ public:
>     WEBCORE_EXPORT static MockRealtimeMediaSourceCenter& singleton();
> 
>     WEBCORE_EXPORT static void setMockRealtimeMediaSourceCenterEnabled(bool);
>+    WEBCORE_EXPORT static bool mockRealtimeMediaSourceCenterEnabled();
> 
>     WEBCORE_EXPORT static void setDevices(Vector<MockMediaDevice>&&);
>     WEBCORE_EXPORT static void addDevice(const MockMediaDevice&);
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>index 46e0a6aff59c4058e17d13173e52ce5c39665ea7..1f260291cc7a175dbcf17b3c13928c6d663e0195 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>@@ -83,7 +83,7 @@ UserMediaPermissionRequestManagerProxy::UserMediaPermissionRequestManagerProxy(W
> UserMediaPermissionRequestManagerProxy::~UserMediaPermissionRequestManagerProxy()
> {
> #if ENABLE(MEDIA_STREAM)
>-    UserMediaProcessManager::singleton().endedCaptureSession(*this);
>+    UserMediaProcessManager::singleton().revokeSandboxExtensionsIfNeeded(page().process());
>     proxies().remove(this);
> #endif
>     invalidatePendingRequests();
>@@ -295,7 +295,12 @@ bool UserMediaPermissionRequestManagerProxy::grantAccess(const UserMediaPermissi
>         return false;
>     }
> 
>-    m_page.process().send(Messages::WebPage::UserMediaAccessWasGranted(request.userMediaID(), request.audioDevice(), request.videoDevice(), request.deviceIdentifierHashSalt()), m_page.pageID());
>+    ++m_hasPendingCapture;
>+    m_page.process().connection()->sendWithAsyncReply(Messages::WebPage::UserMediaAccessWasGranted { request.userMediaID(), request.audioDevice(), request.videoDevice(), request.deviceIdentifierHashSalt() }, [this, weakThis = makeWeakPtr(this)] {
>+        if (!weakThis)
>+            return;
>+        --m_hasPendingCapture;
>+    }, m_page.pageID());
>     return true;
> }
> #endif
>@@ -632,13 +637,8 @@ void UserMediaPermissionRequestManagerProxy::captureStateChanged(MediaProducer::
>         return;
> 
> #if ENABLE(MEDIA_STREAM)
>-    bool wasCapturingAudio = oldState & MediaProducer::AudioCaptureMask;
>-    bool wasCapturingVideo = oldState & MediaProducer::VideoCaptureMask;
>-    bool isCapturingAudio = newState & MediaProducer::AudioCaptureMask;
>-    bool isCapturingVideo = newState & MediaProducer::VideoCaptureMask;
>-
>-    if ((wasCapturingAudio && !isCapturingAudio) || (wasCapturingVideo && !isCapturingVideo))
>-        UserMediaProcessManager::singleton().endedCaptureSession(*this);
>+    if (!m_hasPendingCapture)
>+        UserMediaProcessManager::singleton().revokeSandboxExtensionsIfNeeded(page().process());
> 
>     if (m_captureState == (newState & activeCaptureMask))
>         return;
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>index d87af419d225e46c2f5bab4c1b75a7260a36f358..da3ea273fbb860ab15918cf1de1e415c07a829aa 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>@@ -144,6 +144,7 @@ private:
>     const void* m_logIdentifier;
> #endif
>     bool m_hasFilteredDeviceList { false };
>+    uint64_t m_hasPendingCapture { 0 };
> };
> 
> String convertEnumerationToString(UserMediaPermissionRequestManagerProxy::RequestAction);
>diff --git a/Source/WebKit/UIProcess/UserMediaProcessManager.cpp b/Source/WebKit/UIProcess/UserMediaProcessManager.cpp
>index 9f83de8d2c8ba56335e82455e21dd5d362439c3a..a02816cb40d420eec8572ea80892ea73ebc38b16 100644
>--- a/Source/WebKit/UIProcess/UserMediaProcessManager.cpp
>+++ b/Source/WebKit/UIProcess/UserMediaProcessManager.cpp
>@@ -72,26 +72,26 @@ bool UserMediaProcessManager::willCreateMediaStream(UserMediaPermissionRequestMa
>     }
>     
> #if ENABLE(SANDBOX_EXTENSIONS) && USE(APPLE_INTERNAL_SDK)
>-    if (!proxy.page().preferences().mockCaptureDevicesEnabled()) {
>-        auto& process = proxy.page().process();
>-        size_t extensionCount = 0;
>-
>-        if (withAudio && !process.hasAudioCaptureExtension())
>-            extensionCount++;
>-        else
>-            withAudio = false;
>-
>-        if (withVideo && !process.hasVideoCaptureExtension())
>-            extensionCount++;
>-        else
>-            withVideo = false;
>-
>-        if (extensionCount) {
>-            SandboxExtension::HandleArray handles;
>-            handles.allocate(extensionCount);
>+    auto& process = proxy.page().process();
>+    size_t extensionCount = 0;
>+
>+    if (withAudio && !process.hasAudioCaptureExtension())
>+        extensionCount++;
>+    else
>+        withAudio = false;
> 
>-            Vector<String> ids;
>-            ids.reserveCapacity(extensionCount);
>+    if (withVideo && !process.hasVideoCaptureExtension())
>+        extensionCount++;
>+    else
>+        withVideo = false;
>+
>+    if (extensionCount) {
>+        SandboxExtension::HandleArray handles;
>+        Vector<String> ids;
>+
>+        if (!proxy.page().preferences().mockCaptureDevicesEnabled()) {
>+            handles.allocate(extensionCount);
>+            ids.reserveInitialCapacity(extensionCount);
> 
>             if (withAudio && SandboxExtension::createHandleForGenericExtension(audioExtensionPath, handles[--extensionCount]))
>                 ids.append(audioExtensionPath);
>@@ -103,16 +103,16 @@ bool UserMediaProcessManager::willCreateMediaStream(UserMediaPermissionRequestMa
>                 WTFLogAlways("Could not create a required sandbox extension, capture will fail!");
>                 return false;
>             }
>+        }
> 
>-            for (const auto& id : ids)
>-                RELEASE_LOG(WebRTC, "UserMediaProcessManager::willCreateMediaStream - granting extension %s", id.utf8().data());
>+        for (const auto& id : ids)
>+            RELEASE_LOG(WebRTC, "UserMediaProcessManager::willCreateMediaStream - granting extension %s", id.utf8().data());
> 
>-            if (withAudio)
>-                process.grantAudioCaptureExtension();
>-            if (withVideo)
>-                process.grantVideoCaptureExtension();
>-            process.send(Messages::WebProcess::GrantUserMediaDeviceSandboxExtensions(MediaDeviceSandboxExtensions(ids, WTFMove(handles))), proxy.page().pageID());
>-        }
>+        if (withAudio)
>+            process.grantAudioCaptureExtension();
>+        if (withVideo)
>+            process.grantVideoCaptureExtension();
>+        process.send(Messages::WebProcess::GrantUserMediaDeviceSandboxExtensions(MediaDeviceSandboxExtensions(ids, WTFMove(handles))), proxy.page().pageID());
>     }
> #else
>     UNUSED_PARAM(proxy);
>@@ -125,20 +125,17 @@ bool UserMediaProcessManager::willCreateMediaStream(UserMediaPermissionRequestMa
>     return true;
> }
> 
>-void UserMediaProcessManager::endedCaptureSession(UserMediaPermissionRequestManagerProxy& proxy)
>+void UserMediaProcessManager::revokeSandboxExtensionsIfNeeded(WebProcessProxy& process)
> {
> #if ENABLE(SANDBOX_EXTENSIONS)
>     bool hasAudioCapture = false;
>     bool hasVideoCapture = false;
> 
>-    auto& process = proxy.page().process();
>-    UserMediaPermissionRequestManagerProxy::forEach([&hasAudioCapture, &hasVideoCapture, &proxy, &process](auto& managerProxy) {
>-        if (&proxy == &managerProxy || &process != &managerProxy.page().process())
>+    UserMediaPermissionRequestManagerProxy::forEach([&hasAudioCapture, &hasVideoCapture, &process](auto& managerProxy) {
>+        if (&process != &managerProxy.page().process())
>             return;
>-        if (managerProxy.page().hasActiveAudioStream())
>-            hasAudioCapture = true;
>-        if (managerProxy.page().hasActiveVideoStream())
>-            hasVideoCapture = true;
>+        hasAudioCapture |= managerProxy.page().isCapturingAudio();
>+        hasVideoCapture |= managerProxy.page().isCapturingVideo();
>     });
> 
>     if (hasAudioCapture && hasVideoCapture)
>@@ -160,7 +157,7 @@ void UserMediaProcessManager::endedCaptureSession(UserMediaPermissionRequestMana
>     for (const auto& id : params)
>         RELEASE_LOG(WebRTC, "UserMediaProcessManager::endedCaptureSession - revoking extension %s", id.utf8().data());
> 
>-    process.send(Messages::WebProcess::RevokeUserMediaDeviceSandboxExtensions(params), proxy.page().pageID());
>+    process.send(Messages::WebProcess::RevokeUserMediaDeviceSandboxExtensions(params), 0);
> #endif
> }
> 
>diff --git a/Source/WebKit/UIProcess/UserMediaProcessManager.h b/Source/WebKit/UIProcess/UserMediaProcessManager.h
>index fe87eaaf75605c64df48754afd083f31565ef09b..ae9e44664facbea7ba767994c46b209daae0074d 100644
>--- a/Source/WebKit/UIProcess/UserMediaProcessManager.h
>+++ b/Source/WebKit/UIProcess/UserMediaProcessManager.h
>@@ -38,7 +38,7 @@ public:
>     bool willCreateMediaStream(UserMediaPermissionRequestManagerProxy&, bool withAudio, bool withVideo);
>     void muteCaptureMediaStreamsExceptIn(WebPageProxy&);
> 
>-    void endedCaptureSession(UserMediaPermissionRequestManagerProxy&);
>+    void revokeSandboxExtensionsIfNeeded(WebProcessProxy&);
> 
>     void setCaptureEnabled(bool);
>     bool captureEnabled() const { return m_captureEnabled; }
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index b239a9c73a5fd18a00ffa821c81a5fc03469e01f..a0f0154ee5072e7a319786a2e4726756eb6facd8 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1295,6 +1295,8 @@ public:
>     bool isPlayingAudio() const { return !!(m_mediaState & WebCore::MediaProducer::IsPlayingAudio); }
>     void isPlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags, uint64_t);
>     void updatePlayingMediaDidChange(WebCore::MediaProducer::MediaStateFlags);
>+    bool isCapturingAudio() const { return m_mediaState & WebCore::MediaProducer::AudioCaptureMask; }
>+    bool isCapturingVideo() const { return m_mediaState & WebCore::MediaProducer::VideoCaptureMask; }
>     bool hasActiveAudioStream() const { return m_mediaState & WebCore::MediaProducer::HasActiveAudioCaptureDevice; }
>     bool hasActiveVideoStream() const { return m_mediaState & WebCore::MediaProducer::HasActiveVideoCaptureDevice; }
>     WebCore::MediaProducer::MediaStateFlags mediaStateFlags() const { return m_mediaState; }
>diff --git a/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.cpp b/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.cpp
>index 6c4db3b681d86cc69e8c9981d6befc20a2df76b2..67dcb19fc856e931759cc2e120e65bb43edc99ce 100644
>--- a/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.cpp
>+++ b/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.cpp
>@@ -137,14 +137,14 @@ void UserMediaPermissionRequestManager::removeMediaRequestFromMaps(UserMediaRequ
>     m_userMediaRequestToIDMap.remove(&request);
> }
> 
>-void UserMediaPermissionRequestManager::userMediaAccessWasGranted(uint64_t requestID, CaptureDevice&& audioDevice, CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt)
>+void UserMediaPermissionRequestManager::userMediaAccessWasGranted(uint64_t requestID, CaptureDevice&& audioDevice, CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt, CompletionHandler<void()>&& completionHandler)
> {
>     auto request = m_idToUserMediaRequestMap.take(requestID);
>     if (!request)
>         return;
>     removeMediaRequestFromMaps(*request);
> 
>-    request->allow(WTFMove(audioDevice), WTFMove(videoDevice), WTFMove(deviceIdentifierHashSalt));
>+    request->allow(WTFMove(audioDevice), WTFMove(videoDevice), WTFMove(deviceIdentifierHashSalt), WTFMove(completionHandler));
> }
> 
> void UserMediaPermissionRequestManager::userMediaAccessWasDenied(uint64_t requestID, WebCore::UserMediaRequest::MediaAccessDenialReason reason, String&& invalidConstraint)
>diff --git a/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.h b/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.h
>index b0c2c35e79d20934cab578bbee0e6b105bd7e57b..d7c5c68344b72738d9497df012ca376bb84188da 100644
>--- a/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.h
>+++ b/Source/WebKit/WebProcess/MediaStream/UserMediaPermissionRequestManager.h
>@@ -42,7 +42,7 @@ public:
> 
>     void startUserMediaRequest(WebCore::UserMediaRequest&);
>     void cancelUserMediaRequest(WebCore::UserMediaRequest&);
>-    void userMediaAccessWasGranted(uint64_t, WebCore::CaptureDevice&& audioDevice, WebCore::CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt);
>+    void userMediaAccessWasGranted(uint64_t, WebCore::CaptureDevice&& audioDevice, WebCore::CaptureDevice&& videoDevice, String&& deviceIdentifierHashSalt, CompletionHandler<void()>&&);
>     void userMediaAccessWasDenied(uint64_t, WebCore::UserMediaRequest::MediaAccessDenialReason, String&&);
> 
>     void enumerateMediaDevices(WebCore::MediaDevicesEnumerationRequest&);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 0a7e33d1b3cc3adde76f1674ac9d6e35a2fc3a30..fa233289208cd00c9e26b0d38687e6db2e6fc78b 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -4148,9 +4148,9 @@ void WebPage::didReceiveNotificationPermissionDecision(uint64_t notificationID,
> 
> #if ENABLE(MEDIA_STREAM)
> 
>-void WebPage::userMediaAccessWasGranted(uint64_t userMediaID, WebCore::CaptureDevice&& audioDevice, WebCore::CaptureDevice&& videoDevice, String&& mediaDeviceIdentifierHashSalt)
>+void WebPage::userMediaAccessWasGranted(uint64_t userMediaID, WebCore::CaptureDevice&& audioDevice, WebCore::CaptureDevice&& videoDevice, String&& mediaDeviceIdentifierHashSalt, CompletionHandler<void()>&& completionHandler)
> {
>-    m_userMediaPermissionRequestManager->userMediaAccessWasGranted(userMediaID, WTFMove(audioDevice), WTFMove(videoDevice), WTFMove(mediaDeviceIdentifierHashSalt));
>+    m_userMediaPermissionRequestManager->userMediaAccessWasGranted(userMediaID, WTFMove(audioDevice), WTFMove(videoDevice), WTFMove(mediaDeviceIdentifierHashSalt), WTFMove(completionHandler));
> }
> 
> void WebPage::userMediaAccessWasDenied(uint64_t userMediaID, uint64_t reason, String&& invalidConstraint)
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 52d90fe3c32cbd3db4dd882a296e7b792cdfa113..515c4948b177f2e966e9ac00ff7efa98a75ebeeb 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -1443,7 +1443,7 @@ private:
>     void didReceiveNotificationPermissionDecision(uint64_t notificationID, bool allowed);
> 
> #if ENABLE(MEDIA_STREAM)
>-    void userMediaAccessWasGranted(uint64_t userMediaID, WebCore::CaptureDevice&& audioDeviceUID, WebCore::CaptureDevice&& videoDeviceUID, String&& mediaDeviceIdentifierHashSalt);
>+    void userMediaAccessWasGranted(uint64_t userMediaID, WebCore::CaptureDevice&& audioDeviceUID, WebCore::CaptureDevice&& videoDeviceUID, String&& mediaDeviceIdentifierHashSalt, CompletionHandler<void()>&&);
>     void userMediaAccessWasDenied(uint64_t userMediaID, uint64_t reason, String&& invalidConstraint);
> 
>     void didCompleteMediaDeviceEnumeration(uint64_t userMediaID, const Vector<WebCore::CaptureDevice>& devices, String&& deviceIdentifierHashSalt, bool originHasPersistentAccess);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>index f2f49d53c4f04f092bfe5c295c104f61eeac76b7..941c60761d85e9bc7d9279f8be6f784fe122ffe1 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>@@ -351,7 +351,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
> 
> #if ENABLE(MEDIA_STREAM)
>     # MediaSteam
>-    UserMediaAccessWasGranted(uint64_t userMediaID, WebCore::CaptureDevice audioDevice, WebCore::CaptureDevice videoDevice, String mediaDeviceIdentifierHashSalt)
>+    UserMediaAccessWasGranted(uint64_t userMediaID, WebCore::CaptureDevice audioDevice, WebCore::CaptureDevice videoDevice, String mediaDeviceIdentifierHashSalt) -> () Async
>     UserMediaAccessWasDenied(uint64_t userMediaID, uint64_t reason, String invalidConstraint)
>     DidCompleteMediaDeviceEnumeration(uint64_t userMediaID, Vector<WebCore::CaptureDevice> devices, String mediaDeviceIdentifierHashSalt, bool hasPersistentAccess)
>     CaptureDevicesChanged()
>diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp
>index 0811834a266940988a4c26468c095f6d25e97949..a55565bd683d6ca81f0fd2ae8ab5d6290c877d55 100644
>--- a/Source/WebKit/WebProcess/WebProcess.cpp
>+++ b/Source/WebKit/WebProcess/WebProcess.cpp
>@@ -1867,11 +1867,30 @@ void WebProcess::grantUserMediaDeviceSandboxExtensions(MediaDeviceSandboxExtensi
>     }
> }
> 
>+static inline void checkDocumentsCaptureStateConsistency(const Vector<String>& extensionIDs)
>+{
>+#if !ASSERT_DISABLED
>+    bool isCapturingAudio = WTF::anyOf(Document::allDocumentsMap().values(), [](auto* document) {
>+        return document->mediaState() & MediaProducer::AudioCaptureMask;
>+    });
>+    bool isCapturingVideo = WTF::anyOf(Document::allDocumentsMap().values(), [](auto* document) {
>+        return document->mediaState() & MediaProducer::VideoCaptureMask;
>+    });
>+
>+    if (isCapturingAudio)
>+        ASSERT(extensionIDs.findMatching([](auto& id) { return id.contains("microphone"); }) == notFound);
>+    if (isCapturingVideo)
>+        ASSERT(extensionIDs.findMatching([](auto& id) { return id.contains("camera"); }) == notFound);
>+#endif
>+}
>+
> void WebProcess::revokeUserMediaDeviceSandboxExtensions(const Vector<String>& extensionIDs)
> {
>+    checkDocumentsCaptureStateConsistency(extensionIDs);
>+
>     for (const auto& extensionID : extensionIDs) {
>         auto extension = m_mediaCaptureSandboxExtensions.take(extensionID);
>-        ASSERT(extension);
>+        ASSERT(extension || MockRealtimeMediaSourceCenter::mockRealtimeMediaSourceCenterEnabled());
>         if (extension) {
>             extension->revoke();
>             RELEASE_LOG(WebRTC, "UserMediaPermissionRequestManager::revokeUserMediaDeviceSandboxExtensions - revoked extension %s", extensionID.utf8().data());
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 62709fae5819d709f9f9f746da3f41694b346171..657183c448add920cb770f52f4758bc98b979b8d 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-05-14  Youenn Fablet  <youenn@apple.com>
>+
>+        getUserMedia sandbox extensions should not be revoked when a getUserMedia allowed request is being processed
>+        https://bugs.webkit.org/show_bug.cgi?id=197851
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/mediastream/gum-stop-track-expected.txt: Added.
>+        * fast/mediastream/gum-stop-track.html: Added.
>+
> 2019-05-10  Youenn Fablet  <youenn@apple.com>
> 
>         A service worker process should app nap when all its clients app nap
>diff --git a/LayoutTests/fast/mediastream/gum-stop-track-expected.txt b/LayoutTests/fast/mediastream/gum-stop-track-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..3f22642a269cf27106155658d2d40dfe40ce014e
>--- /dev/null
>+++ b/LayoutTests/fast/mediastream/gum-stop-track-expected.txt
>@@ -0,0 +1,3 @@
>+
>+PASS Call getUserMedia and track.stop in a loop 
>+
>diff --git a/LayoutTests/fast/mediastream/gum-stop-track.html b/LayoutTests/fast/mediastream/gum-stop-track.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..555baca31eea57d9683d9497422f326d44c322a6
>--- /dev/null
>+++ b/LayoutTests/fast/mediastream/gum-stop-track.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE HTML>
>+<html>
>+    <head>
>+        <script src="../../resources/testharness.js"></script>
>+        <script src="../../resources/testharnessreport.js"></script>
>+    </head>
>+    <body>
>+        <script>
>+promise_test(async (test) => {
>+    if (window.testRunner)
>+        testRunner.setUserMediaPermission(true);
>+
>+    for (var cptr = 0; cptr < 5; ++cptr) {
>+        const stream = await navigator.mediaDevices.getUserMedia({audio:false, video:true});
>+        stream.getTracks().forEach((track) => track.stop());
>+    }
>+}, "Call getUserMedia and track.stop in a loop");
>+        </script>
>+    </body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 197851: 369788 | 369855 | 369891 | 369967