Attachment 371424 Details for Bug 198548 – Patch for landing

[patch] Patch for landing

bug-198548-20190605112741.patch (text/plain), 13.73 KB, created by youenn fablet on 2019-06-05 11:27:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-06-05 11:27:41 PDT

Size: 13.73 KB

patch

obsolete

>Subversion Revision: 246034
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index cb77783c589c92180067caff079ad530a50a9667..667f8d71f26e57044e3c73c6c6491c321b0562d6 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-06-04  Youenn Fablet  <youenn@apple.com>
>+
>+        Use an enumeration in UserMediaPermissionRequestManagerProxy::getUserMediaPermissionInfo callback
>+        https://bugs.webkit.org/show_bug.cgi?id=198548
>+
>+        Reviewed by Eric Carlson.
>+
>+        Move from an Optional<bool> to an enum having three values to ease readability.
>+
>+        * UIProcess/UserMediaPermissionCheckProxy.cpp:
>+        (WebKit::UserMediaPermissionCheckProxy::setUserMediaAccessInfo):
>+        (WebKit::UserMediaPermissionCheckProxy::complete):
>+        * UIProcess/UserMediaPermissionCheckProxy.h:
>+        (WebKit::UserMediaPermissionCheckProxy::invalidate):
>+        * UIProcess/UserMediaPermissionRequestManagerProxy.cpp:
>+        (WebKit::UserMediaPermissionRequestManagerProxy::captureDevicesChanged):
>+        (WebKit::UserMediaPermissionRequestManagerProxy::startProcessingUserMediaPermissionRequest):
>+        (WebKit::UserMediaPermissionRequestManagerProxy::getUserMediaPermissionInfo):
>+        (WebKit::UserMediaPermissionRequestManagerProxy::enumerateMediaDevicesForFrame):
>+        * UIProcess/UserMediaPermissionRequestManagerProxy.h:
>+
> 2019-06-03  Youenn Fablet  <youenn@apple.com>
> 
>         getUserMedia requests should be processed sequentially in UIProcess
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.cpp b/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.cpp
>index 304fa21c24a6668b315e03b94b1ddbf9d9ef72d7..ab7924029e0ef3a3bbab74cbac00c297fa96a3ca 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.cpp
>@@ -49,16 +49,16 @@ UserMediaPermissionCheckProxy::~UserMediaPermissionCheckProxy()
> void UserMediaPermissionCheckProxy::setUserMediaAccessInfo(bool allowed)
> {
>     ASSERT(m_completionHandler);
>-    complete(allowed);
>+    complete(allowed ? PermissionInfo::Granted : PermissionInfo::Unknown);
> }
> 
>-void UserMediaPermissionCheckProxy::complete(Optional<bool> allowed)
>+void UserMediaPermissionCheckProxy::complete(PermissionInfo info)
> {
>     if (!m_completionHandler)
>         return;
> 
>     auto completionHandler = WTFMove(m_completionHandler);
>-    completionHandler(allowed);
>+    completionHandler(info);
> }
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.h b/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.h
>index 9e00bb9374bb56d92064440a6ac74a6a7cfeab1b..dc340981714c72793c0105d783391b7fa945918f 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.h
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionCheckProxy.h
>@@ -38,8 +38,8 @@ namespace WebKit {
> 
> class UserMediaPermissionCheckProxy : public API::ObjectImpl<API::Object::Type::UserMediaPermissionCheck> {
> public:
>-
>-    using CompletionHandler = WTF::CompletionHandler<void(Optional<bool> allowed)>;
>+    enum class PermissionInfo { Error, Unknown, Granted };
>+    using CompletionHandler = WTF::CompletionHandler<void(PermissionInfo)>;
> 
>     static Ref<UserMediaPermissionCheckProxy> create(uint64_t frameID, CompletionHandler&& handler, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin)
>     {
>@@ -48,7 +48,7 @@ public:
> 
>     void deny() { setUserMediaAccessInfo(false); }
>     void setUserMediaAccessInfo(bool);
>-    void invalidate() { complete({ }); }
>+    void invalidate() { complete(PermissionInfo::Error); }
> 
>     uint64_t frameID() const { return m_frameID; }
>     WebCore::SecurityOrigin& userMediaDocumentSecurityOrigin() { return m_userMediaDocumentSecurityOrigin.get(); }
>@@ -58,7 +58,7 @@ private:
>     UserMediaPermissionCheckProxy(uint64_t frameID, CompletionHandler&&, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin);
>     ~UserMediaPermissionCheckProxy();
> 
>-    void complete(Optional<bool> allowed);
>+    void complete(PermissionInfo);
>     
>     uint64_t m_frameID;
>     CompletionHandler m_completionHandler;
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>index b1cf20cb787036510665f3995d16054c30d750f4..4711fccff41ff13dcbacebc120f34bb777d933ed 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>@@ -121,11 +121,18 @@ void UserMediaPermissionRequestManagerProxy::captureDevicesChanged()
>     if (!m_page.hasRunningProcess() || !m_page.mainFrame())
>         return;
> 
>-    auto handler = [this](Optional<bool> originHasPersistentAccess) mutable {
>-        if (!originHasPersistentAccess || !m_page.hasRunningProcess())
>+    auto handler = [this](PermissionInfo permissionInfo) mutable {
>+        switch (permissionInfo) {
>+        case PermissionInfo::Error:
>             return;
>-
>-        if (m_grantedRequests.isEmpty() && !*originHasPersistentAccess)
>+        case PermissionInfo::Unknown:
>+            if (m_grantedRequests.isEmpty())
>+                return;
>+            break;
>+        case PermissionInfo::Granted:
>+            break;
>+        }
>+        if (!m_page.hasRunningProcess())
>             return;
> 
>         m_page.process().send(Messages::WebPage::CaptureDevicesChanged(), m_page.pageID());
>@@ -381,26 +388,27 @@ void UserMediaPermissionRequestManagerProxy::startProcessingUserMediaPermissionR
> 
>     auto& userMediaDocumentSecurityOrigin = m_currentUserMediaRequest->userMediaDocumentSecurityOrigin();
>     auto& topLevelDocumentSecurityOrigin = m_currentUserMediaRequest->topLevelDocumentSecurityOrigin();
>-    getUserMediaPermissionInfo(m_currentUserMediaRequest->frameID(), userMediaDocumentSecurityOrigin, topLevelDocumentSecurityOrigin, [this, request = m_currentUserMediaRequest](Optional<bool> hasPersistentAccess) mutable {
>+    getUserMediaPermissionInfo(m_currentUserMediaRequest->frameID(), userMediaDocumentSecurityOrigin, topLevelDocumentSecurityOrigin, [this, request = m_currentUserMediaRequest](auto permissionInfo) mutable {
>         if (!request->isPending())
>             return;
> 
>-        if (!hasPersistentAccess) {
>+        switch (permissionInfo) {
>+        case PermissionInfo::Error:
>             denyRequest(*m_currentUserMediaRequest, UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::OtherFailure);
>             return;
>+        case PermissionInfo::Unknown:
>+            break;
>+        case PermissionInfo::Granted:
>+            m_currentUserMediaRequest->setHasPersistentAccess();
>+            break;
>         }
>-
>-        ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", persistent access: ", *hasPersistentAccess);
>-        processUserMediaPermissionRequest(*hasPersistentAccess);
>+        processUserMediaPermissionRequest();
>     });
> }
> 
>-void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionRequest(bool hasPersistentAccess)
>+void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionRequest()
> {
>-    ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID());
>-
>-    if (hasPersistentAccess)
>-        m_currentUserMediaRequest->setHasPersistentAccess();
>+    ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", persistent access: ", m_currentUserMediaRequest->hasPersistentAccess());
> 
>     auto& userMediaDocumentSecurityOrigin = m_currentUserMediaRequest->userMediaDocumentSecurityOrigin();
>     auto& topLevelDocumentSecurityOrigin = m_currentUserMediaRequest->topLevelDocumentSecurityOrigin();
>@@ -506,7 +514,7 @@ void UserMediaPermissionRequestManagerProxy::processUserMediaPermissionValidRequ
>     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)
>+void UserMediaPermissionRequestManagerProxy::getUserMediaPermissionInfo(uint64_t frameID, Ref<SecurityOrigin>&& userMediaDocumentOrigin, Ref<SecurityOrigin>&& topLevelDocumentOrigin, CompletionHandler<void(PermissionInfo)>&& handler)
> {
>     auto* webFrame = m_page.process().webFrame(frameID);
>     if (!webFrame || !SecurityOrigin::createFromString(m_page.pageLoadState().activeURL())->isSameSchemeHostPort(topLevelDocumentOrigin.get())) {
>@@ -520,12 +528,10 @@ void UserMediaPermissionRequestManagerProxy::getUserMediaPermissionInfo(uint64_t
>     auto requestID = generateRequestID();
>     m_pendingDeviceRequests.add(requestID);
> 
>-    auto request = UserMediaPermissionCheckProxy::create(frameID, [this, weakThis = makeWeakPtr(*this), requestID, handler = WTFMove(handler)](Optional<bool> allowed) mutable {
>-        if (!weakThis || !m_pendingDeviceRequests.remove(requestID) || !allowed) {
>-            handler({ });
>-            return;
>-        }
>-        handler(*allowed);
>+    auto request = UserMediaPermissionCheckProxy::create(frameID, [this, weakThis = makeWeakPtr(*this), requestID, handler = WTFMove(handler)](auto permissionInfo) mutable {
>+        if (!weakThis || !m_pendingDeviceRequests.remove(requestID))
>+            permissionInfo = PermissionInfo::Error;
>+        handler(permissionInfo);
>     }, WTFMove(userMediaDocumentOrigin), WTFMove(topLevelDocumentOrigin));
> 
>     // FIXME: Remove webFrame, userMediaOrigin and topLevelOrigin from this uiClient API call.
>@@ -596,9 +602,19 @@ void UserMediaPermissionRequestManagerProxy::enumerateMediaDevicesForFrame(uint6
> #if ENABLE(MEDIA_STREAM)
>     ALWAYS_LOG(LOGIDENTIFIER, userMediaID);
> 
>-    auto completionHandler = [this, userMediaID, frameID, userMediaDocumentOrigin = userMediaDocumentOrigin.copyRef(), topLevelDocumentOrigin = topLevelDocumentOrigin.copyRef()](Optional<bool> originHasPersistentAccess) mutable {
>-        if (!originHasPersistentAccess)
>+    auto completionHandler = [this, userMediaID, frameID, userMediaDocumentOrigin = userMediaDocumentOrigin.copyRef(), topLevelDocumentOrigin = topLevelDocumentOrigin.copyRef()](PermissionInfo permissionInfo) mutable {
>+
>+        bool originHasPersistentAccess;
>+        switch (permissionInfo) {
>+        case PermissionInfo::Error:
>             return;
>+        case PermissionInfo::Unknown:
>+            originHasPersistentAccess = false;
>+            break;
>+        case PermissionInfo::Granted:
>+            originHasPersistentAccess = true;
>+            break;
>+        }
> 
>         if (!m_page.hasRunningProcess())
>             return;
>@@ -608,7 +624,7 @@ void UserMediaPermissionRequestManagerProxy::enumerateMediaDevicesForFrame(uint6
> 
>         auto& requestOrigin = userMediaDocumentOrigin.get();
>         auto& topOrigin = topLevelDocumentOrigin.get();
>-        m_page.websiteDataStore().deviceIdHashSaltStorage().deviceIdHashSaltForOrigin(requestOrigin, topOrigin, [this, weakThis = makeWeakPtr(*this), requestID, frameID, userMediaID, userMediaDocumentOrigin = WTFMove(userMediaDocumentOrigin), topLevelDocumentOrigin = WTFMove(topLevelDocumentOrigin), originHasPersistentAccess = *originHasPersistentAccess] (String&& deviceIDHashSalt) {
>+        m_page.websiteDataStore().deviceIdHashSaltStorage().deviceIdHashSaltForOrigin(requestOrigin, topOrigin, [this, weakThis = makeWeakPtr(*this), requestID, frameID, userMediaID, userMediaDocumentOrigin = WTFMove(userMediaDocumentOrigin), topLevelDocumentOrigin = WTFMove(topLevelDocumentOrigin), originHasPersistentAccess] (String&& deviceIDHashSalt) {
>             if (!weakThis || !m_pendingDeviceRequests.remove(requestID))
>                 return;
> 
>diff --git a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>index 1c0e8840ee578c2815126a388800cbf1da0bf7e6..6f13410ca4cc9bfe9e16d0ad627234f69782d0ac 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.h
>@@ -100,7 +100,8 @@ private:
>     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);
> 
>-    void getUserMediaPermissionInfo(uint64_t frameID, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin, CompletionHandler<void(Optional<bool>)>&&);
>+    using PermissionInfo = UserMediaPermissionCheckProxy::PermissionInfo;
>+    void getUserMediaPermissionInfo(uint64_t frameID, Ref<WebCore::SecurityOrigin>&& userMediaDocumentOrigin, Ref<WebCore::SecurityOrigin>&& topLevelDocumentOrigin, CompletionHandler<void(PermissionInfo)>&&);
> 
>     RequestAction getRequestAction(const UserMediaPermissionRequestProxy&);
> 
>@@ -108,7 +109,7 @@ private:
> 
>     Vector<WebCore::CaptureDevice> computeFilteredDeviceList(bool revealIdsAndLabels, const String& deviceIDHashSalt);
> 
>-    void processUserMediaPermissionRequest(bool hasPersistentAccess);
>+    void processUserMediaPermissionRequest();
>     void processUserMediaPermissionInvalidRequest(const String& invalidConstraint);
>     void processUserMediaPermissionValidRequest(Vector<WebCore::CaptureDevice>&& audioDevices, Vector<WebCore::CaptureDevice>&& videoDevices, String&& deviceIdentifierHashSalt);
>     void startProcessingUserMediaPermissionRequest(Ref<UserMediaPermissionRequestProxy>&&);

Actions: View | Formatted Diff | Diff

Attachments on bug 198548: 371351 | 371366 | 371370 | 371376 | 371424 | 371501