Attachment 371351 Details for Bug 198548 – Patch

[patch] Patch

bug-198548-20190604163739.patch (text/plain), 12.73 KB, created by youenn fablet on 2019-06-04 16:37:40 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-06-04 16:37:40 PDT

Size: 12.73 KB

patch

obsolete

>Subversion Revision: 246034
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index cb77783c589c92180067caff079ad530a50a9667..794a125153ca32fd09c0b78ce06273c5b0df2a2f 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 NOBODY (OOPS!).
>+
>+        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..17bef6ea75a6d327d515e789ffd9bbfda38f6146 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::PersistentAccess : PermissionInfo::NoAccess);
> }
> 
>-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..99fa269ee3f9239fe8092484fcb053e6cafdce0f 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, NoAccess, PersistentAccess };
>+    using CompletionHandler = 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..17f257d4ddb4f3fdcdc7fca630251066d93fe490 100644
>--- a/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserMediaPermissionRequestManagerProxy.cpp
>@@ -121,11 +121,20 @@ 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::NoAccess:
>+            if (m_grantedRequests.isEmpty())
>+                return;
>+            break;
>+        case PermissionInfo::PersistentAccess:
>+            if (!m_page.hasRunningProcess())
>+                return;
>+            break;
>+        }
>+        if (!m_page.hasRunningProcess())
>             return;
> 
>         m_page.process().send(Messages::WebPage::CaptureDevicesChanged(), m_page.pageID());
>@@ -381,17 +390,23 @@ 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::NoAccess:
>+            ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", no persistent access");
>+            processUserMediaPermissionRequest(false);
>+            break;
>+        case PermissionInfo::PersistentAccess:
>+            ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", persistent access");
>+            processUserMediaPermissionRequest(true);
>+            break;
>         }
>-
>-        ALWAYS_LOG(LOGIDENTIFIER, m_currentUserMediaRequest->userMediaID(), ", persistent access: ", *hasPersistentAccess);
>-        processUserMediaPermissionRequest(*hasPersistentAccess);
>     });
> }
> 
>@@ -506,7 +521,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 +535,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 +609,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::NoAccess:
>+            originHasPersistentAccess = false;
>+            break;
>+        case PermissionInfo::PersistentAccess:
>+            originHasPersistentAccess = true;
>+            break;
>+        }
> 
>         if (!m_page.hasRunningProcess())
>             return;
>@@ -608,7 +631,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..49a76d552a4491c761090ea974ab73270c8f3934 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&);
>

Actions: View | Formatted Diff | Diff

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