[webkit-changes] [246093] trunk

Tue, 04 Jun 2019 18:04:44 -0700

Title: [246093] trunk
Revision
246093
Author
[email protected]
Date
2019-06-04 18:03:04 -0700 (Tue, 04 Jun 2019)

Log Message

getUserMedia requests should be processed sequentially in UIProcess
https://bugs.webkit.org/show_bug.cgi?id=198430
<rdar://problem/51311420>

Reviewed by Eric Carlson.

Source/WebKit:

Before the patch, we process all incoming gum/gdm requests in parallel.
We now queueu them and process them one at a time.
This allows to take into consideration state changes triggered by one request for the next one.
In particular, if a user grants a request, this might grant the next one as well.

To implement that, we keep a reference of the current request to process.
We queue other requests happening whenever another request comes.
When the request is processed, we look at the next one in the queue.
To ensure we do not stop processing the queue for no good reason, some refactoring is done:
- queue processing happens when sending back IPC response to WebProcess.
- denyRequest/grantRequest are consistently called in the manager proxy.

* UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
(WebKit::UserMediaPermissionRequestManagerProxy::invalidatePendingRequests):
Invalidate pregranted requests as well.
(WebKit::UserMediaPermissionRequestManagerProxy::denyRequest):
Renamed from userMediaAccessWasDenied to denyRequest.
This method is now consistently used whenever the manager proxy wants to deny the request.
It does the IPC to the WebProcess and triggers processing of the next request.
(WebKit::UserMediaPermissionRequestManagerProxy::grantRequest):
Renamed from userMediaAccessWasGranted to grantRequest.
(WebKit::UserMediaPermissionRequestManagerProxy::finishGrantingRequest):
This method is now consistently used whenever the manager proxy wants to deny the request.
It does the IPC to the WebProcess and triggers processing of the next request.
(WebKit::UserMediaPermissionRequestManagerProxy::rejectionTimerFired):
We now keep a queue of request instead of request IDs to make the deny code path more consistent.
(WebKit::UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame):
(WebKit::UserMediaPermissionRequestManagerProxy::processNextUserMediaRequestIfNeeded):
(WebKit::UserMediaPermissionRequestManagerProxy::startProcessingUserMediaPermissionRequest):
(WebKit::UserMediaPermissionRequestManagerProxy::processUserMediaPermissionRequest):
To make sure we do not process a different request, we keep a pointer to the request and compare it with the current media request.
(WebKit::UserMediaPermissionRequestManagerProxy::processUserMediaPermissionInvalidRequest):
(WebKit::UserMediaPermissionRequestManagerProxy::processUserMediaPermissionValidRequest):
(WebKit::UserMediaPermissionRequestManagerProxy::viewIsBecomingVisible):
* UIProcess/UserMediaPermissionRequestManagerProxy.h:
(WebKit::UserMediaPermissionRequestManagerProxy::denyRequest):
* UIProcess/UserMediaPermissionRequestProxy.cpp:
(WebKit::setDeviceAsFirst):
(WebKit::UserMediaPermissionRequestProxy::allow):
(WebKit::UserMediaPermissionRequestProxy::deny):
* UIProcess/UserMediaPermissionRequestProxy.h:

Tools:

* TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm:
(-[GetUserMediaRepromptUIDelegate _webView:requestMediaCaptureAuthorization:decisionHandler:]):
(TestWebKitAPI::TEST):
* TestWebKitAPI/Tests/WebKit/getUserMedia.html:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (246092 => 246093)


--- trunk/Source/WebKit/ChangeLog	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Source/WebKit/ChangeLog	2019-06-05 01:03:04 UTC (rev 246093)
@@ -1,3 +1,53 @@
+2019-06-04  Youenn Fablet  <[email protected]>
+
+        getUserMedia requests should be processed sequentially in UIProcess
+        https://bugs.webkit.org/show_bug.cgi?id=198430
+        <rdar://problem/51311420>
+
+        Reviewed by Eric Carlson.
+
+        Before the patch, we process all incoming gum/gdm requests in parallel.
+        We now queueu them and process them one at a time.
+        This allows to take into consideration state changes triggered by one request for the next one.
+        In particular, if a user grants a request, this might grant the next one as well.
+
+        To implement that, we keep a reference of the current request to process.
+        We queue other requests happening whenever another request comes.
+        When the request is processed, we look at the next one in the queue.
+        To ensure we do not stop processing the queue for no good reason, some refactoring is done:
+        - queue processing happens when sending back IPC response to WebProcess.
+        - denyRequest/grantRequest are consistently called in the manager proxy.
+
+        * UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
+        (WebKit::UserMediaPermissionRequestManagerProxy::invalidatePendingRequests):
+        Invalidate pregranted requests as well.
+        (WebKit::UserMediaPermissionRequestManagerProxy::denyRequest):
+        Renamed from userMediaAccessWasDenied to denyRequest.
+        This method is now consistently used whenever the manager proxy wants to deny the request.
+        It does the IPC to the WebProcess and triggers processing of the next request.
+        (WebKit::UserMediaPermissionRequestManagerProxy::grantRequest):
+        Renamed from userMediaAccessWasGranted to grantRequest.
+        (WebKit::UserMediaPermissionRequestManagerProxy::finishGrantingRequest):
+        This method is now consistently used whenever the manager proxy wants to deny the request.
+        It does the IPC to the WebProcess and triggers processing of the next request.
+        (WebKit::UserMediaPermissionRequestManagerProxy::rejectionTimerFired):
+        We now keep a queue of request instead of request IDs to make the deny code path more consistent.
+        (WebKit::UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame):
+        (WebKit::UserMediaPermissionRequestManagerProxy::processNextUserMediaRequestIfNeeded):
+        (WebKit::UserMediaPermissionRequestManagerProxy::startProcessingUserMediaPermissionRequest):
+        (WebKit::UserMediaPermissionRequestManagerProxy::processUserMediaPermissionRequest):
+        To make sure we do not process a different request, we keep a pointer to the request and compare it with the current media request.
+        (WebKit::UserMediaPermissionRequestManagerProxy::processUserMediaPermissionInvalidRequest):
+        (WebKit::UserMediaPermissionRequestManagerProxy::processUserMediaPermissionValidRequest):
+        (WebKit::UserMediaPermissionRequestManagerProxy::viewIsBecomingVisible):
+        * UIProcess/UserMediaPermissionRequestManagerProxy.h:
+        (WebKit::UserMediaPermissionRequestManagerProxy::denyRequest):
+        * UIProcess/UserMediaPermissionRequestProxy.cpp:
+        (WebKit::setDeviceAsFirst):
+        (WebKit::UserMediaPermissionRequestProxy::allow):
+        (WebKit::UserMediaPermissionRequestProxy::deny):
+        * UIProcess/UserMediaPermissionRequestProxy.h:
+
 2019-06-04  Commit Queue  <[email protected]>
 
         Unreviewed, rolling out r246086.

Modified: trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp (246092 => 246093)


--- trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp	2019-06-05 01:03:04 UTC (rev 246093)
@@ -91,10 +91,19 @@
 
 void UserMediaPermissionRequestManagerProxy::invalidatePendingRequests()
 {
-    for (auto& request : m_pendingUserMediaRequests.values())
+    if (m_currentUserMediaRequest) {
+        m_currentUserMediaRequest->invalidate();
+        m_currentUserMediaRequest = nullptr;
+    }
+
+    auto pendingUserMediaRequests = WTFMove(m_pendingUserMediaRequests);
+    for (auto& request : pendingUserMediaRequests)
         request->invalidate();
-    m_pendingUserMediaRequests.clear();
 
+    auto pregrantedRequests = WTFMove(m_pregrantedRequests);
+    for (auto& request : pregrantedRequests)
+        request->invalidate();
+
     m_pendingDeviceRequests.clear();
 }
 
@@ -158,74 +167,72 @@
 }
 #endif
 
-void UserMediaPermissionRequestManagerProxy::userMediaAccessWasDenied(uint64_t userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason reason)
+void UserMediaPermissionRequestManagerProxy::denyRequest(UserMediaPermissionRequestProxy& request, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason reason, const String& invalidConstraint)
 {
     if (!m_page.hasRunningProcess())
         return;
 
-    ALWAYS_LOG(LOGIDENTIFIER, userMediaID, ", reason: ", reason);
+    ALWAYS_LOG(LOGIDENTIFIER, request.userMediaID(), ", reason: ", reason);
 
-    auto request = m_pendingUserMediaRequests.take(userMediaID);
-    if (!request)
-        return;
-
     if (reason == UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied)
-        m_deniedRequests.append(DeniedRequest { request->mainFrameID(), request->userMediaDocumentSecurityOrigin(), request->topLevelDocumentSecurityOrigin(), request->requiresAudioCapture(), request->requiresVideoCapture(), request->requiresDisplayCapture() });
+        m_deniedRequests.append(DeniedRequest { request.mainFrameID(), request.userMediaDocumentSecurityOrigin(), request.topLevelDocumentSecurityOrigin(), request.requiresAudioCapture(), request.requiresVideoCapture(), request.requiresDisplayCapture() });
 
-    denyRequest(userMediaID, reason, emptyString());
-}
-
-void UserMediaPermissionRequestManagerProxy::denyRequest(uint64_t userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason reason, const String& invalidConstraint)
-{
-    ASSERT(m_page.hasRunningProcess());
-
-    ALWAYS_LOG(LOGIDENTIFIER, userMediaID, ", reason: ", reason);
-
 #if ENABLE(MEDIA_STREAM)
-    m_page.process().send(Messages::WebPage::UserMediaAccessWasDenied(userMediaID, toWebCore(reason), invalidConstraint), m_page.pageID());
+    m_page.process().send(Messages::WebPage::UserMediaAccessWasDenied(request.userMediaID(), toWebCore(reason), invalidConstraint), m_page.pageID());
 #else
     UNUSED_PARAM(reason);
     UNUSED_PARAM(invalidConstraint);
 #endif
+
+    processNextUserMediaRequestIfNeeded();
 }
 
-void UserMediaPermissionRequestManagerProxy::userMediaAccessWasGranted(uint64_t userMediaID, CaptureDevice&& audioDevice, CaptureDevice&& videoDevice)
+void UserMediaPermissionRequestManagerProxy::grantRequest(UserMediaPermissionRequestProxy& request)
 {
-    ASSERT(audioDevice || videoDevice);
-
     if (!m_page.hasRunningProcess())
         return;
 
 #if ENABLE(MEDIA_STREAM)
-    auto logSiteIdentifier = LOGIDENTIFIER;
-    ALWAYS_LOG(logSiteIdentifier, userMediaID, ", video: ", videoDevice ? videoDevice.label() : "", ", audio: ", audioDevice ? audioDevice.label() : " ");
+    ALWAYS_LOG(LOGIDENTIFIER, request.userMediaID(), ", video: ", request.videoDevice().label(), ", audio: ", request.audioDevice().label());
 
-    auto request = m_pendingUserMediaRequests.take(userMediaID);
-    if (!request)
-        return;
-
-    auto& userMediaDocumentSecurityOrigin = request->userMediaDocumentSecurityOrigin();
-    auto& topLevelDocumentSecurityOrigin = request->topLevelDocumentSecurityOrigin();
-    m_page.websiteDataStore().deviceIdHashSaltStorage().deviceIdHashSaltForOrigin(userMediaDocumentSecurityOrigin, topLevelDocumentSecurityOrigin, [this, weakThis = makeWeakPtr(*this), request = request.releaseNonNull(), logSiteIdentifier] (String&& deviceIDHashSalt) mutable {
+    auto& userMediaDocumentSecurityOrigin = request.userMediaDocumentSecurityOrigin();
+    auto& topLevelDocumentSecurityOrigin = request.topLevelDocumentSecurityOrigin();
+    m_page.websiteDataStore().deviceIdHashSaltStorage().deviceIdHashSaltForOrigin(userMediaDocumentSecurityOrigin, topLevelDocumentSecurityOrigin, [this, weakThis = makeWeakPtr(*this), request = makeRef(request)](String&&) mutable {
         if (!weakThis)
             return;
-        if (!grantAccess(request))
-            return;
-
-        ALWAYS_LOG(logSiteIdentifier, deviceIDHashSalt);
-        m_grantedRequests.append(WTFMove(request));
-        if (m_hasFilteredDeviceList)
-            captureDevicesChanged();
-        m_hasFilteredDeviceList = false;
+        finishGrantingRequest(request);
     });
 #else
-    UNUSED_PARAM(userMediaID);
-    UNUSED_PARAM(audioDevice);
-    UNUSED_PARAM(videoDevice);
+    UNUSED_PARAM(request);
 #endif
 }
 
 #if ENABLE(MEDIA_STREAM)
+void UserMediaPermissionRequestManagerProxy::finishGrantingRequest(UserMediaPermissionRequestProxy& request)
+{
+    ALWAYS_LOG(LOGIDENTIFIER, request.userMediaID());
+    if (!UserMediaProcessManager::singleton().willCreateMediaStream(*this, request.hasAudioDevice(), request.hasVideoDevice())) {
+        denyRequest(request, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::OtherFailure, "Unable to extend sandbox.");
+        return;
+    }
+
+    if (request.requestType() == MediaStreamRequest::Type::UserMedia)
+        m_grantedRequests.append(makeRef(request));
+
+    if (m_hasFilteredDeviceList)
+        captureDevicesChanged();
+    m_hasFilteredDeviceList = false;
+
+    ++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());
+
+    processNextUserMediaRequestIfNeeded();
+}
+
 void UserMediaPermissionRequestManagerProxy::resetAccess(uint64_t frameID)
 {
     ALWAYS_LOG(LOGIDENTIFIER, frameID);
@@ -287,30 +294,11 @@
     return false;
 }
 
-bool UserMediaPermissionRequestManagerProxy::grantAccess(const UserMediaPermissionRequestProxy& request)
-{
-    ALWAYS_LOG(LOGIDENTIFIER, request.userMediaID());
-    if (!UserMediaProcessManager::singleton().willCreateMediaStream(*this, request.hasAudioDevice(), request.hasVideoDevice())) {
-        denyRequest(request.userMediaID(), UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::OtherFailure, "Unable to extend sandbox.");
-        return false;
-    }
-
-    ++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
 
 void UserMediaPermissionRequestManagerProxy::rejectionTimerFired()
 {
-    uint64_t userMediaID = m_pendingRejections[0];
-    m_pendingRejections.remove(0);
-
-    denyRequest(userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied, emptyString());
+    denyRequest(m_pendingRejections.takeFirst(), UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied, emptyString());
     if (!m_pendingRejections.isEmpty())
         scheduleNextRejection();
 }
@@ -347,50 +335,80 @@
 #if ENABLE(MEDIA_STREAM)
     auto logSiteIdentifier = LOGIDENTIFIER;
 
+    if (!m_page.hasRunningProcess())
+        return;
+
+    ALWAYS_LOG(logSiteIdentifier, userMediaID);
+
+    auto request = UserMediaPermissionRequestProxy::create(*this, userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), { }, { }, WTFMove(userRequest));
+    if (m_currentUserMediaRequest) {
+        m_pendingUserMediaRequests.append(WTFMove(request));
+        return;
+    }
+
     if (!UserMediaProcessManager::singleton().captureEnabled()) {
         ALWAYS_LOG(logSiteIdentifier, "capture disabled");
-        m_pendingRejections.append(userMediaID);
+        m_pendingRejections.append(WTFMove(request));
         scheduleNextRejection();
         return;
     }
 
-    if (!m_page.hasRunningProcess())
+    startProcessingUserMediaPermissionRequest(WTFMove(request));
+#else
+    UNUSED_PARAM(userMediaID);
+    UNUSED_PARAM(frameID);
+    UNUSED_PARAM(userMediaDocumentOrigin);
+    UNUSED_PARAM(topLevelDocumentOrigin);
+    UNUSED_PARAM(userRequest);
+#endif
+}
+
+void UserMediaPermissionRequestManagerProxy::processNextUserMediaRequestIfNeeded()
+{
+#if ENABLE(MEDIA_STREAM)
+    if (m_pendingUserMediaRequests.isEmpty()) {
+        m_currentUserMediaRequest = nullptr;
         return;
+    }
+    startProcessingUserMediaPermissionRequest(m_pendingUserMediaRequests.takeFirst());
+#endif
+}
 
-    ALWAYS_LOG(logSiteIdentifier, userMediaID);
+#if ENABLE(MEDIA_STREAM)
+void UserMediaPermissionRequestManagerProxy::startProcessingUserMediaPermissionRequest(Ref<UserMediaPermissionRequestProxy>&& request)
+{
+    m_currentUserMediaRequest = WTFMove(request);
 
-    auto request = m_pendingUserMediaRequests.add(userMediaID, UserMediaPermissionRequestProxy::create(*this, userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), { }, { }, WTFMove(userRequest))).iterator->value.copyRef();
-
-    auto& userMediaOrigin = request->userMediaDocumentSecurityOrigin();
-    auto& topLevelOrigin = request->topLevelDocumentSecurityOrigin();
-    getUserMediaPermissionInfo(frameID, userMediaOrigin, topLevelOrigin, [this, request = request.releaseNonNull(), logSiteIdentifier](Optional<bool> hasPersistentAccess) mutable {
+    auto& userMediaDocumentSecurityOrigin = m_currentUserMediaRequest->userMediaDocumentSecurityOrigin();
+    auto& topLevelDocumentSecurityOrigin = m_currentUserMediaRequest->topLevelDocumentSecurityOrigin();
+    getUserMediaPermissionInfo(m_currentUserMediaRequest->frameID(), userMediaDocumentSecurityOrigin, topLevelDocumentSecurityOrigin, [this, request = m_currentUserMediaRequest](Optional<bool> hasPersistentAccess) mutable {
         if (!request->isPending())
             return;
 
         if (!hasPersistentAccess) {
-            request->deny(UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::OtherFailure);
+            denyRequest(*m_currentUserMediaRequest, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::OtherFailure);
             return;
         }
 
-        ALWAYS_LOG(logSiteIdentifier, request->userMediaID(), ", persistent access: ", *hasPersistentAccess);
-        processUserMediaPermissionRequest(WTFMove(request), *hasPersistentAccess);
+        ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", persistent access: ", *hasPersistentAccess);
+        processUserMediaPermissionRequest(*hasPersistentAccess);
     });
 }
 
-void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionRequest(Ref<UserMediaPermissionRequestProxy>&& request, bool hasPersistentAccess)
+void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionRequest(bool hasPersistentAccess)
 {
-    ALWAYS_LOG(LOGIDENTIFIER, request->userMediaID());
+    ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID());
 
     if (hasPersistentAccess)
-        request->setHasPersistentAccess();
+        m_currentUserMediaRequest->setHasPersistentAccess();
 
-    auto& userMediaDocumentSecurityOrigin = request->userMediaDocumentSecurityOrigin();
-    auto& topLevelDocumentSecurityOrigin = request->topLevelDocumentSecurityOrigin();
-    m_page.websiteDataStore().deviceIdHashSaltStorage().deviceIdHashSaltForOrigin(userMediaDocumentSecurityOrigin, topLevelDocumentSecurityOrigin, [this, request = WTFMove(request)] (String&& deviceIDHashSalt) mutable {
+    auto& userMediaDocumentSecurityOrigin = m_currentUserMediaRequest->userMediaDocumentSecurityOrigin();
+    auto& topLevelDocumentSecurityOrigin = m_currentUserMediaRequest->topLevelDocumentSecurityOrigin();
+    m_page.websiteDataStore().deviceIdHashSaltStorage().deviceIdHashSaltForOrigin(userMediaDocumentSecurityOrigin, topLevelDocumentSecurityOrigin, [this, request = m_currentUserMediaRequest] (String&& deviceIDHashSalt) mutable {
         if (!request->isPending())
             return;
 
-        RealtimeMediaSourceCenter::InvalidConstraintsHandler invalidHandler = [this, request = request.copyRef()](const String& invalidConstraint) {
+        RealtimeMediaSourceCenter::InvalidConstraintsHandler invalidHandler = [this, request](const String& invalidConstraint) {
             if (!request->isPending())
                 return;
 
@@ -397,10 +415,10 @@
             if (!m_page.hasRunningProcess())
                 return;
 
-            processUserMediaPermissionInvalidRequest(request.get(), invalidConstraint);
+            processUserMediaPermissionInvalidRequest(invalidConstraint);
         };
 
-        auto validHandler = [this, request = request.copyRef()](Vector<CaptureDevice>&& audioDevices, Vector<CaptureDevice>&& videoDevices, String&& deviceIdentifierHashSalt) mutable {
+        auto validHandler = [this, request](Vector<CaptureDevice>&& audioDevices, Vector<CaptureDevice>&& videoDevices, String&& deviceIdentifierHashSalt) mutable {
             if (!request->isPending())
                 return;
 
@@ -407,58 +425,52 @@
             if (!m_page.hasRunningProcess() || !m_page.mainFrame())
                 return;
 
-            processUserMediaPermissionValidRequest(WTFMove(request), WTFMove(audioDevices), WTFMove(videoDevices), WTFMove(deviceIdentifierHashSalt));
+            processUserMediaPermissionValidRequest(WTFMove(audioDevices), WTFMove(videoDevices), WTFMove(deviceIdentifierHashSalt));
         };
 
         syncWithWebCorePrefs();
 
-        RealtimeMediaSourceCenter::singleton().validateRequestConstraints(WTFMove(validHandler), WTFMove(invalidHandler), request->userRequest(), WTFMove(deviceIDHashSalt));
+        RealtimeMediaSourceCenter::singleton().validateRequestConstraints(WTFMove(validHandler), WTFMove(invalidHandler), m_currentUserMediaRequest->userRequest(), WTFMove(deviceIDHashSalt));
     });
-#else
-    UNUSED_PARAM(userMediaID);
-    UNUSED_PARAM(frameID);
-    UNUSED_PARAM(userMediaDocumentOrigin);
-    UNUSED_PARAM(topLevelDocumentOrigin);
-    UNUSED_PARAM(userRequest);
+}
 #endif
-}
 
 #if ENABLE(MEDIA_STREAM)
-void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionInvalidRequest(const UserMediaPermissionRequestProxy& request, const String& invalidConstraint)
+void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionInvalidRequest(const String& invalidConstraint)
 {
-    ALWAYS_LOG(LOGIDENTIFIER, request.userMediaID());
-    bool filterConstraint = !request.hasPersistentAccess() && !wasGrantedVideoOrAudioAccess(request.frameID(), request.userMediaDocumentSecurityOrigin(), request.topLevelDocumentSecurityOrigin());
+    ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID());
+    bool filterConstraint = !m_currentUserMediaRequest->hasPersistentAccess() && !wasGrantedVideoOrAudioAccess(m_currentUserMediaRequest->frameID(), m_currentUserMediaRequest->userMediaDocumentSecurityOrigin(), m_currentUserMediaRequest->topLevelDocumentSecurityOrigin());
 
-    denyRequest(request.userMediaID(), UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::InvalidConstraint, filterConstraint ? String { } : invalidConstraint);
+    denyRequest(*m_currentUserMediaRequest, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::InvalidConstraint, filterConstraint ? String { } : invalidConstraint);
 }
 
-void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionValidRequest(Ref<UserMediaPermissionRequestProxy>&& request, Vector<CaptureDevice>&& audioDevices, Vector<CaptureDevice>&& videoDevices, String&& deviceIdentifierHashSalt)
+void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionValidRequest(Vector<CaptureDevice>&& audioDevices, Vector<CaptureDevice>&& videoDevices, String&& deviceIdentifierHashSalt)
 {
-    ALWAYS_LOG(LOGIDENTIFIER, request->userMediaID(), ", video: ", videoDevices.size(), " audio: ", audioDevices.size());
+    ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", video: ", videoDevices.size(), " audio: ", audioDevices.size());
     if (videoDevices.isEmpty() && audioDevices.isEmpty()) {
-        denyRequest(request->userMediaID(), UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::NoConstraints, emptyString());
+        denyRequest(*m_currentUserMediaRequest, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::NoConstraints, emptyString());
         return;
     }
 
-    request->setDeviceIdentifierHashSalt(WTFMove(deviceIdentifierHashSalt));
-    request->setEligibleVideoDeviceUIDs(WTFMove(videoDevices));
-    request->setEligibleAudioDeviceUIDs(WTFMove(audioDevices));
+    m_currentUserMediaRequest->setDeviceIdentifierHashSalt(WTFMove(deviceIdentifierHashSalt));
+    m_currentUserMediaRequest->setEligibleVideoDeviceUIDs(WTFMove(videoDevices));
+    m_currentUserMediaRequest->setEligibleAudioDeviceUIDs(WTFMove(audioDevices));
 
-    auto action = ""
-    ALWAYS_LOG(LOGIDENTIFIER, request->userMediaID(), ", action: ", action);
+    auto action = ""
+    ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", action: ", action);
 
     if (action == RequestAction::Deny) {
-        denyRequest(request->userMediaID(), UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied, emptyString());
+        denyRequest(*m_currentUserMediaRequest, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied, emptyString());
         return;
     }
 
     if (action == RequestAction::Grant) {
-        ASSERT(request->requestType() != MediaStreamRequest::Type::DisplayMedia);
+        ASSERT(m_currentUserMediaRequest->requestType() != MediaStreamRequest::Type::DisplayMedia);
 
         if (m_page.isViewVisible())
-            grantAccess(request);
+            grantRequest(*m_currentUserMediaRequest);
         else
-            m_pregrantedRequests.append(WTFMove(request));
+            m_pregrantedRequests.append(m_currentUserMediaRequest.releaseNonNull());
 
         return;
     }
@@ -465,34 +477,33 @@
 
     if (m_page.isControlledByAutomation()) {
         if (WebAutomationSession* automationSession = m_page.process().processPool().automationSession()) {
-            ALWAYS_LOG(LOGIDENTIFIER, request->userMediaID(), ", page controlled by automation");
+            ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", page controlled by automation");
             if (automationSession->shouldAllowGetUserMediaForPage(m_page))
-                request->allow();
+                grantRequest(*m_currentUserMediaRequest);
             else
-                userMediaAccessWasDenied(request->userMediaID(), UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied);
-
+                denyRequest(*m_currentUserMediaRequest, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied);
             return;
         }
     }
 
     if (m_page.preferences().mockCaptureDevicesEnabled() && !m_page.preferences().mockCaptureDevicesPromptEnabled()) {
-        ALWAYS_LOG(LOGIDENTIFIER, request->userMediaID(), ", mock devices don't require prompt");
-        request->allow();
+        ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", mock devices don't require prompt");
+        grantRequest(*m_currentUserMediaRequest);
         return;
     }
 
     // If page navigated, there is no need to call the page client for authorization.
-    auto* webFrame = m_page.process().webFrame(request->frameID());
+    auto* webFrame = m_page.process().webFrame(m_currentUserMediaRequest->frameID());
 
-    if (!webFrame || !SecurityOrigin::createFromString(m_page.pageLoadState().activeURL())->isSameSchemeHostPort(request->topLevelDocumentSecurityOrigin())) {
-        denyRequest(request->userMediaID(), UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::NoConstraints, emptyString());
+    if (!webFrame || !SecurityOrigin::createFromString(m_page.pageLoadState().activeURL())->isSameSchemeHostPort(m_currentUserMediaRequest->topLevelDocumentSecurityOrigin())) {
+        denyRequest(*m_currentUserMediaRequest, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::NoConstraints, emptyString());
         return;
     }
 
     // FIXME: Remove webFrame, userMediaOrigin and topLevelOrigin from this uiClient API call.
-    auto userMediaOrigin = API::SecurityOrigin::create(request->userMediaDocumentSecurityOrigin());
-    auto topLevelOrigin = API::SecurityOrigin::create(request->topLevelDocumentSecurityOrigin());
-    m_page.uiClient().decidePolicyForUserMediaPermissionRequest(m_page, *webFrame, WTFMove(userMediaOrigin), WTFMove(topLevelOrigin), request);
+    auto userMediaOrigin = API::SecurityOrigin::create(m_currentUserMediaRequest->userMediaDocumentSecurityOrigin());
+    auto topLevelOrigin = API::SecurityOrigin::create(m_currentUserMediaRequest->topLevelDocumentSecurityOrigin());
+    m_page.uiClient().decidePolicyForUserMediaPermissionRequest(m_page, *webFrame, WTFMove(userMediaOrigin), WTFMove(topLevelOrigin), *m_currentUserMediaRequest);
 }
 
 void UserMediaPermissionRequestManagerProxy::getUserMediaPermissionInfo(uint64_t frameID, Ref<SecurityOrigin>&& userMediaDocumentOrigin, Ref<SecurityOrigin>&& topLevelDocumentOrigin, CompletionHandler<void(Optional<bool>)>&& handler)
@@ -663,9 +674,9 @@
 
 void UserMediaPermissionRequestManagerProxy::viewIsBecomingVisible()
 {
-    for (auto& request : m_pregrantedRequests)
-        request->allow();
-    m_pregrantedRequests.clear();
+    auto pregrantedRequests = WTFMove(m_pregrantedRequests);
+    for (auto& request : pregrantedRequests)
+        grantRequest(request);
 }
 
 void UserMediaPermissionRequestManagerProxy::watchdogTimerFired()

Modified: trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h (246092 => 246093)


--- trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h	2019-06-05 01:03:04 UTC (rev 246093)
@@ -24,6 +24,7 @@
 #include <WebCore/MediaProducer.h>
 #include <WebCore/SecurityOrigin.h>
 #include <wtf/CompletionHandler.h>
+#include <wtf/Deque.h>
 #include <wtf/HashMap.h>
 #include <wtf/LoggerHelper.h>
 #include <wtf/RunLoop.h>
@@ -64,8 +65,8 @@
     void resetAccess(uint64_t mainFrameID);
     void viewIsBecomingVisible();
 
-    void userMediaAccessWasGranted(uint64_t, WebCore::CaptureDevice&& audioDevice, WebCore::CaptureDevice&& videoDevice);
-    void userMediaAccessWasDenied(uint64_t, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason);
+    void grantRequest(UserMediaPermissionRequestProxy&);
+    void denyRequest(UserMediaPermissionRequestProxy&, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason, const String& invalidConstraint = { });
 
     void enumerateMediaDevicesForFrame(uint64_t userMediaID, uint64_t frameID, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin);
 
@@ -93,9 +94,8 @@
 #endif
 
     Ref<UserMediaPermissionRequestProxy> createPermissionRequest(uint64_t userMediaID, uint64_t mainFrameID, uint64_t frameID, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin, Vector<WebCore::CaptureDevice>&& audioDevices, Vector<WebCore::CaptureDevice>&& videoDevices, WebCore::MediaStreamRequest&&);
-    void denyRequest(uint64_t userMediaID, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason, const String& invalidConstraint);
 #if ENABLE(MEDIA_STREAM)
-    bool grantAccess(const UserMediaPermissionRequestProxy&);
+    void finishGrantingRequest(UserMediaPermissionRequestProxy&);
 
     const UserMediaPermissionRequestProxy* searchForGrantedRequest(uint64_t frameID, const WebCore::SecurityOrigin& userMediaDocumentOrigin, const WebCore::SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo) const;
     bool wasRequestDenied(uint64_t mainFrameID, const WebCore::SecurityOrigin& userMediaDocumentOrigin, const WebCore::SecurityOrigin& topLevelDocumentOrigin, bool needsAudio, bool needsVideo, bool needsScreenCapture);
@@ -108,20 +108,24 @@
 
     Vector<WebCore::CaptureDevice> computeFilteredDeviceList(bool revealIdsAndLabels, const String& deviceIDHashSalt);
 
-    void processUserMediaPermissionRequest(Ref<UserMediaPermissionRequestProxy>&&, bool hasPersistentAccess);
-    void processUserMediaPermissionInvalidRequest(const UserMediaPermissionRequestProxy&, const String& invalidConstraint);
-    void processUserMediaPermissionValidRequest(Ref<UserMediaPermissionRequestProxy>&&, Vector<WebCore::CaptureDevice>&& audioDevices, Vector<WebCore::CaptureDevice>&& videoDevices, String&& deviceIdentifierHashSalt);
+    void processUserMediaPermissionRequest(bool hasPersistentAccess);
+    void processUserMediaPermissionInvalidRequest(const String& invalidConstraint);
+    void processUserMediaPermissionValidRequest(Vector<WebCore::CaptureDevice>&& audioDevices, Vector<WebCore::CaptureDevice>&& videoDevices, String&& deviceIdentifierHashSalt);
+    void startProcessingUserMediaPermissionRequest(Ref<UserMediaPermissionRequestProxy>&&);
 #endif
 
     void watchdogTimerFired();
 
-    HashMap<uint64_t, RefPtr<UserMediaPermissionRequestProxy>> m_pendingUserMediaRequests;
+    void processNextUserMediaRequestIfNeeded();
+
+    RefPtr<UserMediaPermissionRequestProxy> m_currentUserMediaRequest;
+    Deque<Ref<UserMediaPermissionRequestProxy>> m_pendingUserMediaRequests;
     HashSet<uint64_t> m_pendingDeviceRequests;
 
     WebPageProxy& m_page;
 
     RunLoop::Timer<UserMediaPermissionRequestManagerProxy> m_rejectionTimer;
-    Vector<uint64_t> m_pendingRejections;
+    Deque<Ref<UserMediaPermissionRequestProxy>> m_pendingRejections;
 
     Vector<Ref<UserMediaPermissionRequestProxy>> m_pregrantedRequests;
     Vector<Ref<UserMediaPermissionRequestProxy>> m_grantedRequests;

Modified: trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.cpp (246092 => 246093)


--- trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.cpp	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.cpp	2019-06-05 01:03:04 UTC (rev 246093)
@@ -43,58 +43,39 @@
 {
 }
 
-void UserMediaPermissionRequestProxy::allow(const String& audioDeviceUID, const String& videoDeviceUID)
+#if ENABLE(MEDIA_STREAM)
+static inline void setDeviceAsFirst(Vector<CaptureDevice>& devices, const String& deviceID)
 {
-    ASSERT(m_manager);
-    if (!m_manager)
-        return;
+    size_t index = devices.findMatching([&deviceID](const auto& device) {
+        return device.persistentId() == deviceID;
+    });
+    ASSERT(index != notFound);
 
-#if ENABLE(MEDIA_STREAM)
-    CaptureDevice audioDevice;
-    if (!audioDeviceUID.isEmpty()) {
-        size_t index = m_eligibleAudioDevices.findMatching([&](const auto& device) {
-            return device.persistentId() == audioDeviceUID;
-        });
-        ASSERT(index != notFound);
+    if (index) {
+        auto device = devices[index];
+        ASSERT(device.enabled());
 
-        if (index != notFound)
-            audioDevice = m_eligibleAudioDevices[index];
-
-        ASSERT(audioDevice.enabled());
+        devices.remove(index);
+        devices.insert(0, WTFMove(device));
     }
+}
+#endif
 
-    CaptureDevice videoDevice;
-    if (!videoDeviceUID.isEmpty()) {
-        size_t index = m_eligibleVideoDevices.findMatching([&](const auto& device) {
-            return device.persistentId() == videoDeviceUID;
-        });
-        ASSERT(index != notFound);
-
-        if (index != notFound)
-            videoDevice = m_eligibleVideoDevices[index];
-
-        ASSERT(videoDevice.enabled());
-    }
-
-    m_manager->userMediaAccessWasGranted(m_userMediaID, WTFMove(audioDevice), WTFMove(videoDevice));
+void UserMediaPermissionRequestProxy::allow(const String& audioDeviceUID, const String& videoDeviceUID)
+{
+#if ENABLE(MEDIA_STREAM)
+    if (!audioDeviceUID.isEmpty())
+        setDeviceAsFirst(m_eligibleAudioDevices, audioDeviceUID);
+    if (!videoDeviceUID.isEmpty())
+        setDeviceAsFirst(m_eligibleVideoDevices, videoDeviceUID);
 #else
     UNUSED_PARAM(audioDeviceUID);
     UNUSED_PARAM(videoDeviceUID);
 #endif
 
-    invalidate();
+    allow();
 }
 
-void UserMediaPermissionRequestProxy::allow(WebCore::CaptureDevice&& audioDevice, WebCore::CaptureDevice&& videoDevice)
-{
-    ASSERT(m_manager);
-    if (!m_manager)
-        return;
-
-    m_manager->userMediaAccessWasGranted(m_userMediaID, WTFMove(audioDevice), WTFMove(videoDevice));
-    invalidate();
-}
-
 void UserMediaPermissionRequestProxy::allow()
 {
     ASSERT(m_manager);
@@ -101,10 +82,7 @@
     if (!m_manager)
         return;
 
-    auto audioDevice = !m_eligibleAudioDevices.isEmpty() ? m_eligibleAudioDevices[0] : CaptureDevice();
-    auto videoDevice = !m_eligibleVideoDevices.isEmpty() ? m_eligibleVideoDevices[0] : CaptureDevice();
-
-    m_manager->userMediaAccessWasGranted(m_userMediaID, WTFMove(audioDevice), WTFMove(videoDevice));
+    m_manager->grantRequest(*this);
     invalidate();
 }
 
@@ -113,7 +91,7 @@
     if (!m_manager)
         return;
 
-    m_manager->userMediaAccessWasDenied(m_userMediaID, reason);
+    m_manager->denyRequest(*this, reason);
     invalidate();
 }

Modified: trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h (246092 => 246093)


--- trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h	2019-06-05 01:03:04 UTC (rev 246093)
@@ -41,7 +41,6 @@
     }
 
     void allow(const String& audioDeviceUID, const String& videoDeviceUID);
-    void allow(WebCore::CaptureDevice&& audioDevice, WebCore::CaptureDevice&& videoDevice);
     void allow();
 
     enum class UserMediaAccessDenialReason { NoConstraints, UserMediaDisabled, NoCaptureDevices, InvalidConstraint, HardwareError, PermissionDenied, OtherFailure };

Modified: trunk/Tools/ChangeLog (246092 => 246093)


--- trunk/Tools/ChangeLog	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Tools/ChangeLog	2019-06-05 01:03:04 UTC (rev 246093)
@@ -1,3 +1,16 @@
+2019-06-04  Youenn Fablet  <[email protected]>
+
+        getUserMedia requests should be processed sequentially in UIProcess
+        https://bugs.webkit.org/show_bug.cgi?id=198430
+        <rdar://problem/51311420>
+
+        Reviewed by Eric Carlson.
+
+        * TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm:
+        (-[GetUserMediaRepromptUIDelegate _webView:requestMediaCaptureAuthorization:decisionHandler:]):
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/Tests/WebKit/getUserMedia.html:
+
 2019-06-04  David Kilzer  <[email protected]>
 
         REGRESSION (r244557): Leak of WKNSString in WTR::runOpenPanel() while running WebKit layout tests

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm (246092 => 246093)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm	2019-06-05 01:03:04 UTC (rev 246093)
@@ -37,6 +37,7 @@
 #import <WebKit/_WKProcessPoolConfiguration.h>
 
 static bool wasPrompted = false;
+static int numberOfPrompts = 0;
 
 @interface GetUserMediaRepromptUIDelegate : NSObject<WKUIDelegate>
 - (void)_webView:(WKWebView *)webView requestMediaCaptureAuthorization: (_WKCaptureDevices)devices decisionHandler:(void (^)(BOOL))decisionHandler;
@@ -46,6 +47,7 @@
 @implementation GetUserMediaRepromptUIDelegate
 - (void)_webView:(WKWebView *)webView requestMediaCaptureAuthorization: (_WKCaptureDevices)devices decisionHandler:(void (^)(BOOL))decisionHandler
 {
+    numberOfPrompts++;
     wasPrompted = true;
     decisionHandler(YES);
 }
@@ -117,6 +119,31 @@
     EXPECT_TRUE([webView haveStream:YES]);
 }
 
+TEST(WebKit2, MultipleGetUserMediaSynchronously)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto processPoolConfig = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    auto preferences = [configuration preferences];
+    preferences._mediaCaptureRequiresSecureConnection = NO;
+    preferences._mediaDevicesEnabled = YES;
+    preferences._mockCaptureDevicesEnabled = YES;
+    auto webView = [[GetUserMediaRepromptTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration.get() processPoolConfiguration:processPoolConfig.get()];
+    auto delegate = adoptNS([[GetUserMediaRepromptUIDelegate alloc] init]);
+    webView.UIDelegate = delegate.get();
+
+    wasPrompted = false;
+    numberOfPrompts = 0;
+    [webView loadTestPageNamed:@"getUserMedia"];
+    TestWebKitAPI::Util::run(&wasPrompted);
+    EXPECT_EQ(numberOfPrompts, 1);
+
+    wasPrompted = false;
+    numberOfPrompts = 0;
+    [webView stringByEvaluatingJavaScript:@"doMultipleGetUserMediaSynchronously()"];
+    TestWebKitAPI::Util::run(&wasPrompted);
+    EXPECT_EQ(numberOfPrompts, 1);
+}
+
 } // namespace TestWebKitAPI
 
 #endif // ENABLE(MEDIA_STREAM)

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit/getUserMedia.html (246092 => 246093)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit/getUserMedia.html	2019-06-05 00:40:35 UTC (rev 246092)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit/getUserMedia.html	2019-06-05 01:03:04 UTC (rev 246093)
@@ -36,6 +36,20 @@
             {
                 return stream !== null;
             }
+
+            function doMultipleGetUserMediaSynchronously()
+            {
+                navigator.mediaDevices.getUserMedia({video: true});
+                navigator.mediaDevices.getUserMedia({video: true});
+                navigator.mediaDevices.getUserMedia({video: true});
+
+                // This one should prompt.
+                navigator.mediaDevices.getUserMedia({audio: true});
+
+                navigator.mediaDevices.getUserMedia({audio: true});
+                navigator.mediaDevices.getUserMedia({audio: true});
+                navigator.mediaDevices.getUserMedia({audio: true, video: true});
+            }
         </script>
     <head>
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to