Attachment 371347 Details for Bug 198430 – Patch for landing

[patch] Patch for landing

bug-198430-20190604155741.patch (text/plain), 38.77 KB, created by youenn fablet on 2019-06-04 15:57:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-06-04 15:57:42 PDT

Size: 38.77 KB

patch

obsolete

>Subversion Revision: 246034
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 3638a0682ed018fabaabb5c75b40302d57e2d0e6..cb77783c589c92180067caff079ad530a50a9667 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,53 @@
>+2019-06-03  Youenn Fablet  <youenn@apple.com>
>+
>+        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-03  Darin Adler  <darin@apple.com>
> 
>         Finish cleanup of String::number for floating point
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>index 1f260291cc7a175dbcf17b3c13928c6d663e0195..699b8ab0c956b3d4fa165b4c8e0389940a959246 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>@@ -91,9 +91,18 @@ UserMediaPermissionRequestManagerProxy::~UserMediaPermissionRequestManagerProxy(
> 
> 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();
>+
>+    auto pregrantedRequests = WTFMove(m_pregrantedRequests);
>+    for (auto& request : pregrantedRequests)
>         request->invalidate();
>-    m_pendingUserMediaRequests.clear();
> 
>     m_pendingDeviceRequests.clear();
> }
>@@ -158,74 +167,72 @@ static uint64_t toWebCore(UserMediaPermissionRequestProxy::UserMediaAccessDenial
> }
> #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);
>-
>-    auto request = m_pendingUserMediaRequests.take(userMediaID);
>-    if (!request)
>-        return;
>+    ALWAYS_LOG(LOGIDENTIFIER, request.userMediaID(), ", reason: ", reason);
> 
>     if (reason == UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied)
>-        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);
>+        m_deniedRequests.append(DeniedRequest { request.mainFrameID(), request.userMediaDocumentSecurityOrigin(), request.topLevelDocumentSecurityOrigin(), request.requiresAudioCapture(), request.requiresVideoCapture(), request.requiresDisplayCapture() });
> 
> #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 @@ bool UserMediaPermissionRequestManagerProxy::wasRequestDenied(uint64_t mainFrame
>     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,72 +335,92 @@ void UserMediaPermissionRequestManagerProxy::requestUserMediaPermissionForFrame(
> #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())
>-        return;
>+    startProcessingUserMediaPermissionRequest(WTFMove(request));
>+}
> 
>-    ALWAYS_LOG(logSiteIdentifier, userMediaID);
>+void UserMediaPermissionRequestManagerProxy::processNextUserMediaRequestIfNeeded()
>+{
>+    if (m_pendingUserMediaRequests.isEmpty()) {
>+        m_currentUserMediaRequest = nullptr;
>+        return;
>+    }
>+    startProcessingUserMediaPermissionRequest(m_pendingUserMediaRequests.takeFirst());
>+}
> 
>-    auto request = m_pendingUserMediaRequests.add(userMediaID, UserMediaPermissionRequestProxy::create(*this, userMediaID, m_page.mainFrame()->frameID(), frameID, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin), { }, { }, WTFMove(userRequest))).iterator->value.copyRef();
>+void UserMediaPermissionRequestManagerProxy::startProcessingUserMediaPermissionRequest(Ref<UserMediaPermissionRequestProxy>&& request)
>+{
>+    m_currentUserMediaRequest = WTFMove(request);
> 
>-    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;
> 
>             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;
> 
>             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);
>@@ -424,75 +432,74 @@ void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionRequest(R
> }
> 
> #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 = getRequestAction(request);
>-    ALWAYS_LOG(LOGIDENTIFIER, request->userMediaID(), ", action: ", action);
>+    auto action = getRequestAction(*m_currentUserMediaRequest);
>+    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;
>     }
> 
>     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 +670,9 @@ void UserMediaPermissionRequestManagerProxy::captureStateChanged(MediaProducer::
> 
> 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()
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>index da3ea273fbb860ab15918cf1de1e415c07a829aa..cc32c31d7d2b57a1bb88623a4f541786560ba263 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>@@ -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 @@ public:
>     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 @@ private:
> #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 @@ private:
> 
>     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);
> #endif
> 
>     void watchdogTimerFired();
> 
>-    HashMap<uint64_t, RefPtr<UserMediaPermissionRequestProxy>> m_pendingUserMediaRequests;
>+    void processNextUserMediaRequestIfNeeded();
>+    void startProcessingUserMediaPermissionRequest(Ref<UserMediaPermissionRequestProxy>&&);
>+
>+    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;
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.cpp b/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.cpp
>index 563bc8fc92fa7d3f63135e3ef7c31080620e7883..4f028406ba4a77516470647671f3a95088aff4ab 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.cpp
>@@ -43,56 +43,37 @@ UserMediaPermissionRequestProxy::UserMediaPermissionRequestProxy(UserMediaPermis
> {
> }
> 
>-void UserMediaPermissionRequestProxy::allow(const String& audioDeviceUID, const String& videoDeviceUID)
>-{
>-    ASSERT(m_manager);
>-    if (!m_manager)
>-        return;
>-
> #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 != notFound)
>-            audioDevice = m_eligibleAudioDevices[index];
>-
>-        ASSERT(audioDevice.enabled());
>-    }
>-
>-    CaptureDevice videoDevice;
>-    if (!videoDeviceUID.isEmpty()) {
>-        size_t index = m_eligibleVideoDevices.findMatching([&](const auto& device) {
>-            return device.persistentId() == videoDeviceUID;
>-        });
>-        ASSERT(index != notFound);
>+static inline void setDeviceAsFirst(Vector<CaptureDevice>& devices, const String& deviceID)
>+{
>+    size_t index = devices.findMatching([&deviceID](const auto& device) {
>+        return device.persistentId() == deviceID;
>+    });
>+    ASSERT(index != notFound);
> 
>-        if (index != notFound)
>-            videoDevice = m_eligibleVideoDevices[index];
>+    if (index) {
>+        auto device = devices[index];
>+        ASSERT(device.enabled());
> 
>-        ASSERT(videoDevice.enabled());
>+        devices.remove(index);
>+        devices.insert(0, WTFMove(device));
>     }
>+}
>+#endif
> 
>-    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();
>-}
>-
>-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();
>+    allow();
> }
> 
> void UserMediaPermissionRequestProxy::allow()
>@@ -101,10 +82,7 @@ void UserMediaPermissionRequestProxy::allow()
>     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 @@ void UserMediaPermissionRequestProxy::deny(UserMediaAccessDenialReason reason)
>     if (!m_manager)
>         return;
> 
>-    m_manager->userMediaAccessWasDenied(m_userMediaID, reason);
>+    m_manager->denyRequest(*this, reason);
>     invalidate();
> }
> 
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h b/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h
>index f0a1420b614a42706827f86616454ee4f0aeafbc..544f6116edb662a8fa9631bc3af8b70dedc5827f 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestProxy.h
>@@ -41,7 +41,6 @@ public:
>     }
> 
>     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 };
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index d7a6e3d87fe647715fd02cee1610117e84ab1712..75f054bf9c4678608f0126b40c7b132644934c5e 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-06-03  Youenn Fablet  <youenn@apple.com>
>+
>+        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-03  Darin Adler  <darin@apple.com>
> 
>         Finish cleanup of String::number for floating point
>diff --git a/Tools/TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm b/Tools/TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm
>index 3dab79f370f1bdf78cd8a07096c9efbaf00483e4..a3e8f52c54e7e824a2181e85dc45a111d8363850 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKit/GetUserMediaReprompt.mm
>@@ -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 @@ @end
> @implementation GetUserMediaRepromptUIDelegate
> - (void)_webView:(WKWebView *)webView requestMediaCaptureAuthorization: (_WKCaptureDevices)devices decisionHandler:(void (^)(BOOL))decisionHandler
> {
>+    numberOfPrompts++;
>     wasPrompted = true;
>     decisionHandler(YES);
> }
>@@ -117,6 +119,31 @@ TEST(WebKit2, GetUserMediaReprompt)
>     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)
>diff --git a/Tools/TestWebKitAPI/Tests/WebKit/getUserMedia.html b/Tools/TestWebKitAPI/Tests/WebKit/getUserMedia.html
>index 4bc074b3c508afa097561a1e52693190652b58c4..82f245bab020f550097280f1469e4ca558b2b99c 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKit/getUserMedia.html
>+++ b/Tools/TestWebKitAPI/Tests/WebKit/getUserMedia.html
>@@ -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>
>

Actions: View | Formatted Diff | Diff

Attachments on bug 198430: 371077 | 371082 | 371090 | 371209 | 371342 | 371347 | 371348